diff --git a/prow/cluster/deck_rbac.yaml b/prow/cluster/deck_rbac.yaml index da7097b58119..a8616d329baa 100644 --- a/prow/cluster/deck_rbac.yaml +++ b/prow/cluster/deck_rbac.yaml @@ -19,7 +19,8 @@ rules: - list # Required when deck runs with `--rerun-creates-job=true` # **Warning:** Only use this for non-public deck instances, this allows - # anyone with access to your Deck instance to create new Prowjobs + # anyone with access to your Deck instance to create new Prowjobs. + # Additionally, there is not yet protection against CSRF. Use with caution # - create --- kind: Role diff --git a/prow/cmd/deck/BUILD.bazel b/prow/cmd/deck/BUILD.bazel index 3e0f061cc98d..3d081c273e88 100644 --- a/prow/cmd/deck/BUILD.bazel +++ b/prow/cmd/deck/BUILD.bazel @@ -66,13 +66,17 @@ go_test( "//prow/client/clientset/versioned/fake:go_default_library", "//prow/config:go_default_library", "//prow/flagutil:go_default_library", + "//prow/githuboauth:go_default_library", "//prow/pluginhelp:go_default_library", "//prow/spyglass/lenses/buildlog:go_default_library", "//prow/spyglass/lenses/junit:go_default_library", "//prow/spyglass/lenses/metadata:go_default_library", "//prow/tide:go_default_library", "//prow/tide/history:go_default_library", + "//vendor/github.com/google/go-github/github:go_default_library", + "//vendor/github.com/gorilla/sessions:go_default_library", "//vendor/github.com/sirupsen/logrus:go_default_library", + "//vendor/golang.org/x/oauth2:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library", @@ -105,6 +109,7 @@ go_library( "//prow/errorutil:go_default_library", "//prow/flagutil:go_default_library", "//prow/gcsupload:go_default_library", + "//prow/github:go_default_library", "//prow/githuboauth:go_default_library", "//prow/kube:go_default_library", "//prow/logrusutil:go_default_library", diff --git a/prow/cmd/deck/main.go b/prow/cmd/deck/main.go index 369c37548f91..778086a3d7ee 100644 --- a/prow/cmd/deck/main.go +++ b/prow/cmd/deck/main.go @@ -54,6 +54,7 @@ import ( "k8s.io/test-infra/prow/config" "k8s.io/test-infra/prow/deck/jobs" prowflagutil "k8s.io/test-infra/prow/flagutil" + prowgithub "k8s.io/test-infra/prow/github" "k8s.io/test-infra/prow/githuboauth" "k8s.io/test-infra/prow/kube" "k8s.io/test-infra/prow/logrusutil" @@ -172,6 +173,8 @@ var ( } ) +type authCfgGetter func() *config.RerunAuthConfig + type traceResponseWriter struct { http.ResponseWriter statusCode int @@ -389,12 +392,14 @@ func prodOnlyMain(cfg config.Getter, o options, mux *http.ServeMux) *http.ServeM }, podLogClients, cfg) ja.Start() + cfgGetter := func() *config.RerunAuthConfig { return &cfg().Deck.RerunAuthConfig } + // setup prod only handlers mux.Handle("/data.js", gziphandler.GzipHandler(handleData(ja))) mux.Handle("/prowjobs.js", gziphandler.GzipHandler(handleProwJobs(ja))) mux.Handle("/badge.svg", gziphandler.GzipHandler(handleBadge(ja))) mux.Handle("/log", gziphandler.GzipHandler(handleLog(ja))) - mux.Handle("/rerun", gziphandler.GzipHandler(handleRerun(prowJobClient, o.rerunCreatesJob))) + mux.Handle("/prowjob", gziphandler.GzipHandler(handleProwJob(prowJobClient))) if o.spyglass { @@ -489,6 +494,7 @@ func prodOnlyMain(cfg config.Getter, o options, mux *http.ServeMux) *http.ServeM mux.Handle("/github-login", goa.HandleLogin(oauthClient)) // Handles redirect from GitHub OAuth server. mux.Handle("/github-login/redirect", goa.HandleRedirect(oauthClient, githuboauth.NewGitHubClientGetter())) + mux.Handle("/rerun", gziphandler.GzipHandler(handleRerun(prowJobClient, o.rerunCreatesJob, cfgGetter, goa, githuboauth.NewGitHubClientGetter()))) } // optionally inject http->https redirect handler when behind loadbalancer @@ -1150,9 +1156,39 @@ func handleProwJob(prowJobClient prowv1.ProwJobInterface) http.HandlerFunc { } } -func handleRerun(prowJobClient prowv1.ProwJobInterface, createProwJob bool) http.HandlerFunc { +// canTriggerJob determines whether the given user can trigger any job. +func canTriggerJob(user string, cfg *config.RerunAuthConfig) bool { + if cfg.AllowAnyone { + return true + } + for _, u := range cfg.AuthorizedUsers { + if prowgithub.NormLogin(u) == prowgithub.NormLogin(user) { + return true + } + } + return false +} + +func marshalJob(w http.ResponseWriter, pj prowapi.ProwJob, l *logrus.Entry) { + b, err := yaml.Marshal(&pj) + if err != nil { + http.Error(w, fmt.Sprintf("Error marshaling: %v", err), http.StatusInternalServerError) + l.WithError(err).Error("Error marshaling job.") + return + } + w.Header().Set("Content-Type", "application/x-yaml") + if _, err := w.Write(b); err != nil { + l.WithError(err).Error("Error writing log.") + } +} + +// handleRerun triggers a rerun of the given job if that features is enabled, it receives a +// POST request, and the user has the necessary permissions. Otherwise, it writes the config +// for a new job but does not trigger it. +func handleRerun(prowJobClient prowv1.ProwJobInterface, createProwJob bool, cfg authCfgGetter, goa *githuboauth.Agent, ghc githuboauth.GitHubClientGetter) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { name := r.URL.Query().Get("prowjob") + l := logrus.WithField("prowjob", name) if name == "" { http.Error(w, "request did not provide the 'name' query parameter", http.StatusBadRequest) return @@ -1162,7 +1198,7 @@ func handleRerun(prowJobClient prowv1.ProwJobInterface, createProwJob bool) http http.Error(w, fmt.Sprintf("ProwJob not found: %v", err), http.StatusNotFound) if !kerrors.IsNotFound(err) { // admins only care about errors other than not found - logrus.WithError(err).Warning("ProwJob not found.") + l.WithError(err).Warning("ProwJob not found.") } return } @@ -1172,29 +1208,35 @@ func handleRerun(prowJobClient prowv1.ProwJobInterface, createProwJob bool) http // On Prow instances that require auth even for viewing Deck this is okayish, because the Prowjob UUID // is hard to guess // Ref: https://github.com/kubernetes/test-infra/pull/12827#issuecomment-502850414 - if createProwJob { - if r.Method != http.MethodPost { - http.Error(w, "request must be of type POST", http.StatusMethodNotAllowed) + switch r.Method { + case http.MethodGet: + marshalJob(w, newPJ, l) + case http.MethodPost: + if !createProwJob { + http.Error(w, "Direct rerun feature is not yet enabled", http.StatusMethodNotAllowed) + return + } + login, err := goa.GetLogin(r, ghc) + if err != nil { + http.Error(w, fmt.Sprintf("Error retrieving GitHub login: %v", err), http.StatusInternalServerError) + l.WithError(err).Errorf("Error retrieving GitHub login") + return + } + if !canTriggerJob(login, cfg()) { + http.Redirect(w, r, "/?rerun=error", http.StatusFound) return } if _, err := prowJobClient.Create(&newPJ); err != nil { - logrus.WithError(err).Error("Error creating job") + l.WithError(err).Error("Error creating job") http.Error(w, fmt.Sprintf("Error creating job: %v", err), http.StatusInternalServerError) return } - w.WriteHeader(http.StatusNoContent) + http.Redirect(w, r, "/?rerun=success", http.StatusFound) return - } - b, err := yaml.Marshal(&newPJ) - if err != nil { - http.Error(w, fmt.Sprintf("Error marshaling: %v", err), http.StatusInternalServerError) - logrus.WithError(err).Error("Error marshaling jobs.") + default: + http.Error(w, fmt.Sprintf("bad verb %v", r.Method), http.StatusMethodNotAllowed) return } - w.Header().Set("Content-Type", "application/json") - if _, err := w.Write(b); err != nil { - logrus.WithError(err).Error("Error writing log.") - } } } diff --git a/prow/cmd/deck/main_test.go b/prow/cmd/deck/main_test.go index 2e5e5d5096e7..e9c9471b2911 100644 --- a/prow/cmd/deck/main_test.go +++ b/prow/cmd/deck/main_test.go @@ -23,8 +23,12 @@ import ( "errors" "flag" "fmt" + "github.com/gorilla/sessions" + "github.com/sirupsen/logrus" + "golang.org/x/oauth2" "io" "io/ioutil" + "k8s.io/test-infra/prow/githuboauth" "net/http" "net/http/httptest" "net/url" @@ -33,6 +37,8 @@ import ( "testing" "time" + "github.com/google/go-github/github" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -253,20 +259,80 @@ func TestProwJob(t *testing.T) { } } +type mockGitHubConfigGetter struct { + githubLogin string +} + +func (getter mockGitHubConfigGetter) GetGitHubClient(accessToken string, dryRun bool) githuboauth.GitHubClientWrapper { + return getter +} + +func (getter mockGitHubConfigGetter) GetUser(login string) (*github.User, error) { + return &github.User{Login: &getter.githubLogin}, nil +} + // TestRerun just checks that the result can be unmarshaled properly, has an // updated status, and has equal spec. func TestRerun(t *testing.T) { testCases := []struct { name string - shouldCreateProwjob bool + login string + authorized []string + allowAnyone bool + rerunCreatesJob bool + shouldCreateProwJob bool + httpCode int + httpMethod string }{ { name: "Handler returns ProwJob", - shouldCreateProwjob: false, + login: "authorized", + authorized: []string{"authorized", "alsoauthorized"}, + allowAnyone: false, + rerunCreatesJob: true, + shouldCreateProwJob: true, + httpCode: http.StatusFound, + httpMethod: http.MethodPost, + }, + { + name: "User not authorized to create prow job", + login: "random-dude", + authorized: []string{"authorized", "alsoauthorized"}, + allowAnyone: false, + rerunCreatesJob: true, + shouldCreateProwJob: false, + httpCode: http.StatusFound, + httpMethod: http.MethodPost, + }, + { + name: "RerunCreatesJob set to false, should not create prow job", + login: "authorized", + authorized: []string{"authorized", "alsoauthorized"}, + allowAnyone: true, + rerunCreatesJob: false, + shouldCreateProwJob: false, + httpCode: http.StatusOK, + httpMethod: http.MethodGet, + }, + { + name: "Allow anyone set to true, creates job", + login: "ugh", + authorized: []string{"authorized", "alsoauthorized"}, + allowAnyone: true, + rerunCreatesJob: true, + shouldCreateProwJob: true, + httpCode: http.StatusFound, + httpMethod: http.MethodPost, }, { - name: "Handler creates ProwJob", - shouldCreateProwjob: true, + name: "Direct rerun disabled, post request", + login: "authorized", + authorized: []string{"authorized", "alsoauthorized"}, + allowAnyone: true, + rerunCreatesJob: false, + shouldCreateProwJob: false, + httpCode: http.StatusMethodNotAllowed, + httpMethod: http.MethodPost, }, } @@ -292,18 +358,44 @@ func TestRerun(t *testing.T) { State: prowapi.PendingState, }, }) - handler := handleRerun(fakeProwJobClient.ProwV1().ProwJobs("prowjobs"), tc.shouldCreateProwjob) - req, err := http.NewRequest(http.MethodPost, "/rerun?prowjob=wowsuch", nil) + configGetter := func() *config.RerunAuthConfig { + return &config.RerunAuthConfig{ + AllowAnyone: tc.allowAnyone, + AuthorizedUsers: tc.authorized, + } + } + + req, err := http.NewRequest(tc.httpMethod, "/rerun?prowjob=wowsuch", nil) + req.AddCookie(&http.Cookie{ + Name: "github_login", + Value: tc.login, + Path: "/", + Expires: time.Now().Add(time.Hour * 24 * 30), + Secure: true, + }) + mockCookieStore := sessions.NewCookieStore([]byte("secret-key")) + session, err := sessions.GetRegistry(req).Get(mockCookieStore, "access-token-session") + if err != nil { + t.Fatalf("Error making access token session: %v", err) + } + session.Values["access-token"] = &oauth2.Token{} + if err != nil { t.Fatalf("Error making request: %v", err) } rr := httptest.NewRecorder() + mockConfig := &config.GitHubOAuthConfig{ + CookieStore: mockCookieStore, + } + goa := githuboauth.NewAgent(mockConfig, &logrus.Entry{}) + ghc := mockGitHubConfigGetter{githubLogin: tc.login} + handler := handleRerun(fakeProwJobClient.ProwV1().ProwJobs("prowjobs"), tc.rerunCreatesJob, configGetter, goa, ghc) handler.ServeHTTP(rr, req) + if rr.Code != tc.httpCode { + t.Fatalf("Bad error code: %d", rr.Code) + } - if tc.shouldCreateProwjob { - if rr.Code != http.StatusNoContent { - t.Fatalf("Unexpected http status code: %d, expected 204", rr.Code) - } + if tc.shouldCreateProwJob { pjs, err := fakeProwJobClient.ProwV1().ProwJobs("prowjobs").List(metav1.ListOptions{}) if err != nil { t.Fatalf("failed to list prowjobs: %v", err) @@ -312,10 +404,7 @@ func TestRerun(t *testing.T) { t.Errorf("expected to get two prowjobs, got %d", numPJs) } - } else { - if rr.Code != http.StatusOK { - t.Fatalf("Bad error code: %d", rr.Code) - } + } else if !tc.rerunCreatesJob && tc.httpCode == http.StatusOK { resp := rr.Result() defer resp.Body.Close() body, err := ioutil.ReadAll(resp.Body) diff --git a/prow/cmd/deck/static/prow/prow.ts b/prow/cmd/deck/static/prow/prow.ts index abb9b8cc8c0d..5dcef2e78641 100644 --- a/prow/cmd/deck/static/prow/prow.ts +++ b/prow/cmd/deck/static/prow/prow.ts @@ -650,9 +650,15 @@ function redraw(fz: FuzzySearch): void { max = 2 * 3600; } drawJobHistogram(totalJob, jobHistogram, now - (12 * 3600), now, max); - if (rerunStatus != null) { + if (rerunStatus === "error") { modal.style.display = "block"; - rerunCommand.innerHTML = `Nice try! The direct rerun feature hasn't been implemented yet, so that button does nothing.`; + rerunCommand.innerHTML = "You don't have permission to rerun that job"; + } else if (rerunStatus === "success") { + modal.style.display = "block"; + rerunCommand.innerHTML = "Job successfully triggered. Wait 30 seconds and refresh the page for the job to show up"; + } else if (rerunStatus != null) { + modal.style.display = "block"; + rerunCommand.innerHTML = "Nice try! The direct rerun feature hasn't been implemented yet, so that button does nothing."; } } @@ -661,7 +667,7 @@ function createRerunCell(modal: HTMLElement, rerunElement: HTMLElement, prowjob: const url = `${location.protocol}//${location.host}/rerun?prowjob=${prowjob}`; const c = document.createElement("td"); const i = icon.create("refresh", "Show instructions for rerunning this job"); - const login = getCookieByName("github_login"); + const login = getCookieByName("access-token-session"); i.onclick = () => { modal.style.display = "block"; rerunElement.innerHTML = `kubectl create -f "${url}"`; @@ -671,7 +677,7 @@ function createRerunCell(modal: HTMLElement, rerunElement: HTMLElement, prowjob: copyButton.innerHTML = "file_copy"; rerunElement.appendChild(copyButton); const runButton = document.createElement('a'); - runButton.innerHTML = ""; + runButton.innerHTML = ""; if (login === "") { runButton.href = `/github-login?dest=%2F?rerun=work_in_progress`; } else { diff --git a/prow/githuboauth/githuboauth.go b/prow/githuboauth/githuboauth.go index a8504e82cfc6..6b4c8fd6c80c 100644 --- a/prow/githuboauth/githuboauth.go +++ b/prow/githuboauth/githuboauth.go @@ -149,6 +149,29 @@ func (ga *Agent) HandleLogin(client OAuthClient) http.HandlerFunc { } } +// GetLogin returns the username of the already authenticated GitHub user. +func (ga *Agent) GetLogin(r *http.Request, getter GitHubClientGetter) (string, error) { + session, err := ga.gc.CookieStore.Get(r, tokenSession) + if err != nil { + return "", err + } + val := session.Values[tokenKey] + if val == nil { + return "", fmt.Errorf("Could not find GitHub token") + } + var token = &oauth2.Token{} + token, ok := val.(*oauth2.Token) + if !ok { + return "", fmt.Errorf("Unexpected GitHub token type") + } + ghc := getter.GetGitHubClient(token.AccessToken, false) + userInfo, err := ghc.GetUser("") + if err != nil { + return "", err + } + return *userInfo.Login, nil +} + // HandleLogout handles GitHub logout request from front-end. It invalidates cookie sessions and // redirect back to the front page. func (ga *Agent) HandleLogout(client OAuthClient) http.HandlerFunc { diff --git a/prow/githuboauth/githuboauth_test.go b/prow/githuboauth/githuboauth_test.go index c8612bac1f59..a389b2dc150e 100644 --- a/prow/githuboauth/githuboauth_test.go +++ b/prow/githuboauth/githuboauth_test.go @@ -252,6 +252,28 @@ func (fgc *fakeGetter) GetGitHubClient(accessToken string, dryRun bool) GitHubCl return &fakeGitHubClient{login: fgc.login} } +func TestGetLogin(t *testing.T) { + cookie := sessions.NewCookieStore([]byte("secret-key")) + mockConfig := getMockConfig(cookie) + mockLogger := logrus.WithField("uni-test", "githuboauth") + mockAgent := NewAgent(mockConfig, mockLogger) + mockToken := &oauth2.Token{} + mockRequest := httptest.NewRequest(http.MethodGet, "/someurl", nil) + mockSession, err := sessions.GetRegistry(mockRequest).Get(cookie, "access-token-session") + if err != nil { + t.Fatalf("Error with getting mock session: %v", err) + } + mockSession.Values["access-token"] = mockToken + + login, err := mockAgent.GetLogin(mockRequest, &fakeGetter{"correct-login"}) + if err != nil { + t.Fatalf("Error getting login: %v", err) + } + if login != "correct-login" { + t.Fatalf("Incorrect login: %s", login) + } +} + func TestHandleRedirectWithInvalidState(t *testing.T) { gob.Register(&oauth2.Token{}) cookie := sessions.NewCookieStore([]byte("secret-key"))