diff --git a/prow/cmd/deck/main.go b/prow/cmd/deck/main.go index 2ad4da012260b..8f1f89a1b0ffb 100644 --- a/prow/cmd/deck/main.go +++ b/prow/cmd/deck/main.go @@ -451,13 +451,14 @@ func prodOnlyMain(cfg config.Getter, o options, mux *http.ServeMux) *http.ServeM githubOAuthConfig.InitGitHubOAuthConfig(cookie) goa := githuboauth.NewAgent(&githubOAuthConfig, logrus.WithField("client", "githuboauth")) - oauthClient := &oauth2.Config{ + oauthClient := githuboauth.NewClient(&oauth2.Config{ ClientID: githubOAuthConfig.ClientID, ClientSecret: githubOAuthConfig.ClientSecret, RedirectURL: githubOAuthConfig.RedirectURL, Scopes: githubOAuthConfig.Scopes, Endpoint: github.Endpoint, - } + }, + ) repoSet := make(map[string]bool) for r := range cfg().Presubmits { @@ -1211,6 +1212,5 @@ func handleFavicon(staticFilesLocation string, cfg config.Getter) http.HandlerFu func isValidatedGitOAuthConfig(githubOAuthConfig *config.GitHubOAuthConfig) bool { return githubOAuthConfig.ClientID != "" && githubOAuthConfig.ClientSecret != "" && - githubOAuthConfig.RedirectURL != "" && - githubOAuthConfig.FinalRedirectURL != "" + githubOAuthConfig.RedirectURL != "" } diff --git a/prow/cmd/deck/static/common/common.ts b/prow/cmd/deck/static/common/common.ts index c579f0e1ae010..a68e90c8c136a 100644 --- a/prow/cmd/deck/static/common/common.ts +++ b/prow/cmd/deck/static/common/common.ts @@ -201,3 +201,18 @@ export namespace tidehistory { return link; } } + +export function getCookieByName(name: string): string { + if (!document.cookie) { + return ""; + } + const docCookies = decodeURIComponent(document.cookie).split(";"); + for (const cookie of docCookies) { + const c = cookie.trim(); + const pref = name + "="; + if (c.indexOf(pref) === 0) { + return c.slice(pref.length); + } + } + return ""; +} diff --git a/prow/cmd/deck/static/pr/pr.ts b/prow/cmd/deck/static/pr/pr.ts index 403642c2607c9..b7e229cebbe50 100644 --- a/prow/cmd/deck/static/pr/pr.ts +++ b/prow/cmd/deck/static/pr/pr.ts @@ -4,7 +4,7 @@ import {Context} from '../api/github'; import {Label, PullRequest, UserData} from '../api/pr'; import {Job, JobState} from '../api/prow'; import {Blocker, TideData, TidePool, TideQuery as ITideQuery} from '../api/tide'; -import {tidehistory} from '../common/common'; +import {getCookieByName, tidehistory} from '../common/common'; declare const tideData: TideData; declare const allBuilds: Job[]; @@ -179,24 +179,6 @@ function onLoadQuery(): string { return ""; } -/** - * Gets cookie by its name. - */ -function getCookieByName(name: string): string { - if (!document.cookie) { - return ""; - } - const cookies = decodeURIComponent(document.cookie).split(";"); - for (const cookie of cookies) { - const c = cookie.trim(); - const pref = name + "="; - if (c.indexOf(pref) === 0) { - return c.slice(pref.length); - } - } - return ""; -} - /** * Creates an alert for merge blocking issues on tide. */ @@ -1154,7 +1136,7 @@ function createPRCard(pr: PullRequest, builds: UnifiedContext[] = [], queries: P * Redirect to initiate github login flow. */ function forceGitHubLogin(): void { - window.location.href = window.location.origin + "/github-login"; + window.location.href = window.location.origin + "/github-login?dest=%2Fpr"; } type VagueState = "succeeded" | "failed" | "pending" | "unknown"; diff --git a/prow/cmd/deck/static/prow/prow.ts b/prow/cmd/deck/static/prow/prow.ts index a66f01910e50f..f8ddd932d8d4a 100644 --- a/prow/cmd/deck/static/prow/prow.ts +++ b/prow/cmd/deck/static/prow/prow.ts @@ -1,6 +1,6 @@ import moment from "moment"; import {Job, JobState, JobType} from "../api/prow"; -import {cell, icon} from "../common/common"; +import {cell, getCookieByName, icon} from "../common/common"; import {FuzzySearch} from './fuzzy-search'; import {JobHistogram, JobSample} from './histogram'; @@ -397,6 +397,7 @@ function escapeRegexLiteral(s: string): string { } function redraw(fz: FuzzySearch): void { + const rerunStatus = getParameterByName("rerun"); const modal = document.getElementById('rerun')!; const rerunCommand = document.getElementById('rerun-content')!; window.onclick = (event) => { @@ -648,12 +649,18 @@ function redraw(fz: FuzzySearch): void { max = 2 * 3600; } drawJobHistogram(totalJob, jobHistogram, now - (12 * 3600), now, max); + 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.`; + } + } function createRerunCell(modal: HTMLElement, rerunElement: HTMLElement, prowjob: string): HTMLTableDataCellElement { - const url = `https://${window.location.hostname}/rerun?prowjob=${prowjob}`; + const url = `/rerun?prowjob=${prowjob}`; const c = document.createElement("td"); const i = icon.create("refresh", "Show instructions for rerunning this job"); + const login = getCookieByName("github_login"); i.onclick = () => { modal.style.display = "block"; rerunElement.innerHTML = `kubectl create -f "${url}"`; @@ -662,6 +669,24 @@ function createRerunCell(modal: HTMLElement, rerunElement: HTMLElement, prowjob: copyButton.onclick = () => copyToClipboardWithToast(`kubectl create -f "${url}"`); copyButton.innerHTML = "file_copy"; rerunElement.appendChild(copyButton); + const runButton = document.createElement('a'); + runButton.innerHTML = ""; + if (login === "") { + runButton.href = `/github-login?dest=%2F?rerun=work_in_progress`; + } else { + if (rerunCreatesJob) { + runButton.onclick = () => { + const form = document.createElement('form'); + form.method = 'POST'; + form.action = `${url}`; + c.appendChild(form); + form.submit(); + }; + } else { + runButton.href = `/?rerun=work_in_progress`; + } + } + rerunElement.appendChild(runButton); }; c.appendChild(i); c.classList.add("icon-cell"); diff --git a/prow/config/githuboauth.go b/prow/config/githuboauth.go index d13e00888ef1c..9fc9162148357 100644 --- a/prow/config/githuboauth.go +++ b/prow/config/githuboauth.go @@ -31,11 +31,10 @@ type Cookie struct { // GitHubOAuthConfig is a config for requesting users access tokens from GitHub API. It also has // a Cookie Store that retains user credentials deriving from GitHub API. type GitHubOAuthConfig struct { - ClientID string `json:"client_id"` - ClientSecret string `json:"client_secret"` - RedirectURL string `json:"redirect_url"` - Scopes []string `json:"scopes,omitempty"` - FinalRedirectURL string `json:"final_redirect_url"` + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret"` + RedirectURL string `json:"redirect_url"` + Scopes []string `json:"scopes,omitempty"` CookieStore *sessions.CookieStore `json:"-"` } diff --git a/prow/githuboauth/githuboauth.go b/prow/githuboauth/githuboauth.go index cadee9f557a32..c67241da6c5c4 100644 --- a/prow/githuboauth/githuboauth.go +++ b/prow/githuboauth/githuboauth.go @@ -21,6 +21,7 @@ import ( "encoding/hex" "fmt" "net/http" + "net/url" "time" "github.com/google/go-github/github" @@ -54,6 +55,7 @@ type GitHubClientGetter interface { // OAuthClient is an interface for a GitHub OAuth client. type OAuthClient interface { + WithFinalRedirectURL(url string) (OAuthClient, error) // Exchanges code from GitHub OAuth redirect for user access token. Exchange(ctx context.Context, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) // Returns a URL to GitHub's OAuth 2.0 consent page. The state is a token to protect the user @@ -61,6 +63,35 @@ type OAuthClient interface { AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string } +type client struct { + *oauth2.Config +} + +func NewClient(config *oauth2.Config) client { + return client{ + config, + } +} + +func (cli client) WithFinalRedirectURL(path string) (OAuthClient, error) { + parsedURL, err := url.Parse(cli.RedirectURL) + if err != nil { + return nil, err + } + q := parsedURL.Query() + q.Set("dest", path) + parsedURL.RawQuery = q.Encode() + return NewClient( + &oauth2.Config{ + ClientID: cli.ClientID, + ClientSecret: cli.ClientSecret, + RedirectURL: parsedURL.String(), + Scopes: cli.Scopes, + Endpoint: cli.Endpoint, + }, + ), nil +} + type githubClientGetter struct{} func (gci *githubClientGetter) GetGitHubClient(accessToken string, dryRun bool) GitHubClientWrapper { @@ -92,6 +123,7 @@ func NewAgent(config *config.GitHubOAuthConfig, logger *logrus.Entry) *Agent { // redirect user to GitHub OAuth end-point for authentication. func (ga *Agent) HandleLogin(client OAuthClient) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + destPage := r.URL.Query().Get("dest") stateToken := xsrftoken.Generate(ga.gc.ClientSecret, "", "") state := hex.EncodeToString([]byte(stateToken)) oauthSession, err := ga.gc.CookieStore.New(r, oauthSessionCookie) @@ -108,8 +140,11 @@ func (ga *Agent) HandleLogin(client OAuthClient) http.HandlerFunc { ga.serverError(w, "Save oauth session", err) return } - - redirectURL := client.AuthCodeURL(state, oauth2.ApprovalForce, oauth2.AccessTypeOnline) + newClient, err := client.WithFinalRedirectURL(destPage) + if err != nil { + ga.serverError(w, "Failed to parse redirect URL", err) + } + redirectURL := newClient.AuthCodeURL(state, oauth2.ApprovalForce, oauth2.AccessTypeOnline) http.Redirect(w, r, redirectURL, http.StatusFound) } } @@ -135,7 +170,7 @@ func (ga *Agent) HandleLogout(client OAuthClient) http.HandlerFunc { loginCookie.Expires = time.Now().Add(-time.Hour * 24) http.SetCookie(w, loginCookie) } - http.Redirect(w, r, ga.gc.FinalRedirectURL, http.StatusFound) + http.Redirect(w, r, r.URL.Host, http.StatusFound) } } @@ -144,6 +179,14 @@ func (ga *Agent) HandleLogout(client OAuthClient) http.HandlerFunc { // the final destination in the config, which should be the front-end. func (ga *Agent) HandleRedirect(client OAuthClient, getter GitHubClientGetter) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + finalRedirectURL, err := r.URL.Parse(r.URL.Query().Get("dest")) + //This check prevents someone from specifying a different host to redirect to. + if finalRedirectURL.Host != "" { + ga.serverError(w, "Invalid hostname", fmt.Errorf("%s, expected prow.k8s.io", finalRedirectURL.Host)) + } + if err != nil { + ga.serverError(w, "Failed to parse final destination from OAuth redirect payload", err) + } state := r.FormValue("state") stateTokenRaw, err := hex.DecodeString(state) if err != nil { @@ -163,7 +206,7 @@ func (ga *Agent) HandleRedirect(client OAuthClient, getter GitHubClientGetter) h } secretState, ok := oauthSession.Values[stateKey].(string) if !ok { - ga.serverError(w, "Get secret state", fmt.Errorf("empty string or cannot convert to string")) + ga.serverError(w, "Get secret state", fmt.Errorf("empty string or cannot convert to string. this probably means the options passed to GitHub don't match what was expected")) return } // Validate the state parameter to prevent cross-site attack. @@ -219,7 +262,7 @@ func (ga *Agent) HandleRedirect(client OAuthClient, getter GitHubClientGetter) h Expires: time.Now().Add(time.Hour * 24 * 30), Secure: true, }) - http.Redirect(w, r, ga.gc.FinalRedirectURL, http.StatusFound) + http.Redirect(w, r, finalRedirectURL.String(), http.StatusFound) } } diff --git a/prow/githuboauth/githuboauth_test.go b/prow/githuboauth/githuboauth_test.go index 6a37e17209054..c8612bac1f59b 100644 --- a/prow/githuboauth/githuboauth_test.go +++ b/prow/githuboauth/githuboauth_test.go @@ -21,6 +21,8 @@ import ( "encoding/hex" "net/http" "net/http/httptest" + "net/url" + "strings" "testing" "github.com/google/go-github/github" @@ -36,30 +38,42 @@ import ( const mockAccessToken = "justSomeRandomSecretToken" -type MockOAuthClient struct{} +type mockOAuthClient struct { + config *oauth2.Config +} + +func (c mockOAuthClient) WithFinalRedirectURL(path string) (OAuthClient, error) { + parsedURL, err := url.Parse("www.something.com") + if err != nil { + return nil, err + } + q := parsedURL.Query() + q.Set("dest", path) + parsedURL.RawQuery = q.Encode() + return mockOAuthClient{&oauth2.Config{RedirectURL: parsedURL.String()}}, nil +} -func (c *MockOAuthClient) Exchange(ctx context.Context, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) { +func (c mockOAuthClient) Exchange(ctx context.Context, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) { return &oauth2.Token{ AccessToken: mockAccessToken, }, nil } -func (c *MockOAuthClient) AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string { - return "mock-auth-url" +func (c mockOAuthClient) AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string { + return c.config.AuthCodeURL(state, opts...) } func getMockConfig(cookie *sessions.CookieStore) *config.GitHubOAuthConfig { clientID := "mock-client-id" clientSecret := "mock-client-secret" - redirectURL := "/uni-test/redirect-url" + redirectURL := "uni-test/redirect-url" scopes := []string{} return &config.GitHubOAuthConfig{ - ClientID: clientID, - ClientSecret: clientSecret, - RedirectURL: redirectURL, - Scopes: scopes, - FinalRedirectURL: "/unit-test/final-redirect-url", + ClientID: clientID, + ClientSecret: clientSecret, + RedirectURL: redirectURL, + Scopes: scopes, CookieStore: cookie, } @@ -80,13 +94,15 @@ func isEqual(token1 *oauth2.Token, token2 *oauth2.Token) bool { } func TestHandleLogin(t *testing.T) { + dest := "wherever" + rerunStatus := "working" cookie := sessions.NewCookieStore([]byte("secret-key")) mockConfig := getMockConfig(cookie) mockLogger := logrus.WithField("uni-test", "githuboauth") mockAgent := NewAgent(mockConfig, mockLogger) - mockOAuthClient := &MockOAuthClient{} + mockOAuthClient := mockOAuthClient{} - mockRequest := httptest.NewRequest(http.MethodGet, "/mock-login", nil) + mockRequest := httptest.NewRequest(http.MethodGet, "/mock-login?dest="+dest+"?rerun="+rerunStatus, nil) mockResponse := httptest.NewRecorder() handleLoginFn := mockAgent.HandleLogin(mockOAuthClient) @@ -125,6 +141,16 @@ func TestHandleLogin(t *testing.T) { if state == "" { t.Error("Expect state parameter is not empty, found empty") } + destCount := 0 + path := mockResponse.Header().Get("Location") + for _, q := range strings.Split(path, "&") { + if q == "redirect_uri=www.something.com%3Fdest%3Dwherever%253Frerun%253Dworking" { + destCount++ + } + } + if destCount != 1 { + t.Errorf("Redirect URI in path does not include correct destination. path: %s, destination: %s", path, "?dest="+dest+"?rerun=working") + } } func TestHandleLogout(t *testing.T) { @@ -132,7 +158,7 @@ func TestHandleLogout(t *testing.T) { mockConfig := getMockConfig(cookie) mockLogger := logrus.WithField("uni-test", "githuboauth") mockAgent := NewAgent(mockConfig, mockLogger) - mockOAuthClient := &MockOAuthClient{} + mockOAuthClient := mockOAuthClient{} mockRequest := httptest.NewRequest(http.MethodGet, "/mock-logout", nil) _, err := cookie.New(mockRequest, tokenSession) @@ -172,7 +198,7 @@ func TestHandleLogoutWithLoginSession(t *testing.T) { mockConfig := getMockConfig(cookie) mockLogger := logrus.WithField("uni-test", "githuboauth") mockAgent := NewAgent(mockConfig, mockLogger) - mockOAuthClient := &MockOAuthClient{} + mockOAuthClient := mockOAuthClient{} mockRequest := httptest.NewRequest(http.MethodGet, "/mock-logout", nil) _, err := cookie.New(mockRequest, tokenSession) @@ -232,7 +258,7 @@ func TestHandleRedirectWithInvalidState(t *testing.T) { mockConfig := getMockConfig(cookie) mockLogger := logrus.WithField("uni-test", "githuboauth") mockAgent := NewAgent(mockConfig, mockLogger) - mockOAuthClient := &MockOAuthClient{} + mockOAuthClient := mockOAuthClient{} mockStateToken := createMockStateToken(mockConfig) mockRequest := httptest.NewRequest(http.MethodGet, "/mock-login", nil) @@ -262,13 +288,16 @@ func TestHandleRedirectWithValidState(t *testing.T) { mockLogger := logrus.WithField("uni-test", "githuboauth") mockAgent := NewAgent(mockConfig, mockLogger) mockLogin := "foo_name" - mockOAuthClient := &MockOAuthClient{} + mockOAuthClient := mockOAuthClient{} mockStateToken := createMockStateToken(mockConfig) - mockRequest := httptest.NewRequest(http.MethodGet, "/mock-login", nil) + dest := "somewhere" + rerunStatus := "working" + mockRequest := httptest.NewRequest(http.MethodGet, "/mock-login?dest="+dest+"?rerun="+rerunStatus, nil) mockResponse := httptest.NewRecorder() query := mockRequest.URL.Query() query.Add("state", mockStateToken) + query.Add("rerun", "working") mockRequest.URL.RawQuery = query.Encode() mockSession, err := sessions.GetRegistry(mockRequest).Get(cookie, oauthSessionCookie) @@ -321,4 +350,8 @@ func TestHandleRedirectWithValidState(t *testing.T) { if loginCookie.Value != mockLogin { t.Errorf("Mismatch github login. Got %v, expected %v", loginCookie.Value, mockLogin) } + path := mockResponse.Header().Get("Location") + if path != "/"+dest+"?rerun="+rerunStatus { + t.Errorf("Incorrect final redirect URL. Actual path: %s, Expected path: /%s", path, dest+"?rerun="+rerunStatus) + } }