From 01be853119e87fe56e25901e0c95d92e869f8d52 Mon Sep 17 00:00:00 2001 From: Tuomo Ala-Vannesluoma Date: Wed, 20 Jun 2018 22:05:46 +0300 Subject: Refactor logic to avoid existence leak --- internal/auth/auth.go | 105 ++++++++++++++++++++++++++++++++++----------- internal/auth/auth_test.go | 99 +++++++++++++++++++++++++++++++++++------- 2 files changed, 164 insertions(+), 40 deletions(-) (limited to 'internal/auth') diff --git a/internal/auth/auth.go b/internal/auth/auth.go index c8022f2c..483b471d 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -15,11 +15,12 @@ import ( ) const ( - apiURLTemplate = "%s/api/v4/projects/%d" - authorizeURLTemplate = "%s/oauth/authorize?client_id=%s&redirect_uri=%s&response_type=code&state=%s" - tokenURLTemplate = "%s/oauth/token" - tokenContentTemplate = "client_id=%s&client_secret=%s&code=%s&grant_type=authorization_code&redirect_uri=%s" - callbackPath = "/auth" + apiURLProjectsTemplate = "%s/api/v4/projects" + apiURLProjectTemplate = "%s/api/v4/projects/%d" + authorizeURLTemplate = "%s/oauth/authorize?client_id=%s&redirect_uri=%s&response_type=code&state=%s" + tokenURLTemplate = "%s/oauth/token" + tokenContentTemplate = "client_id=%s&client_secret=%s&code=%s&grant_type=authorization_code&redirect_uri=%s" + callbackPath = "/auth" ) // Auth handles authenticating users with GitLab API @@ -180,20 +181,7 @@ func (a *Auth) fetchAccessToken(code string) (tokenResponse, error) { return token, nil } -// CheckAuthentication checks if user is authenticated and has access to the project -func (a *Auth) CheckAuthentication(w http.ResponseWriter, r *http.Request, projectID uint64) bool { - - if a == nil { - httperrors.Serve500(w) - return true - } - - if a.checkSession(w, r) { - return true - } - - session := a.getSession(r) - +func (a *Auth) checkTokenExists(session *sessions.Session, w http.ResponseWriter, r *http.Request) bool { // If no access token redirect to OAuth login page if session.Values["access_token"] == nil { @@ -209,9 +197,37 @@ func (a *Auth) CheckAuthentication(w http.ResponseWriter, r *http.Request, proje return true } + return false +} + +func destroySession(session *sessions.Session, w http.ResponseWriter, r *http.Request) { + // Invalidate access token and redirect back for refreshing and re-authenticating + delete(session.Values, "access_token") + session.Save(r, w) + + http.Redirect(w, r, getRequestAddress(r), 302) +} + +// CheckAuthenticationWithoutProject checks if user is authenticated and has a valid token +func (a *Auth) CheckAuthenticationWithoutProject(w http.ResponseWriter, r *http.Request) bool { + + if a == nil { + // No auth supported + return false + } + + if a.checkSession(w, r) { + return true + } + + session := a.getSession(r) + + if a.checkTokenExists(session, w, r) { + return true + } // Access token exists, authorize request - url := fmt.Sprintf(apiURLTemplate, a.gitLabServer, projectID) + url := fmt.Sprintf(apiURLProjectsTemplate, a.gitLabServer) req, err := http.NewRequest("GET", url, nil) if err != nil { @@ -223,18 +239,57 @@ func (a *Auth) CheckAuthentication(w http.ResponseWriter, r *http.Request, proje resp, err := a.apiClient.Do(req) if checkResponseForInvalidToken(resp, err) { + destroySession(session, w, r) + return true + } - // Invalidate access token and redirect back for refreshing and re-authenticating - delete(session.Values, "access_token") - session.Save(r, w) + if err != nil || resp.StatusCode != 200 { + // We return 404 if for some reason token is not valid to avoid (not) existence leak + httperrors.Serve404(w) + return true + } - http.Redirect(w, r, getRequestAddress(r), 302) + return false +} + +// CheckAuthentication checks if user is authenticated and has access to the project +func (a *Auth) CheckAuthentication(w http.ResponseWriter, r *http.Request, projectID uint64) bool { + + if a == nil { + httperrors.Serve500(w) + return true + } + + if a.checkSession(w, r) { + return true + } + session := a.getSession(r) + + if a.checkTokenExists(session, w, r) { + return true + } + + // Access token exists, authorize request + url := fmt.Sprintf(apiURLProjectTemplate, a.gitLabServer, projectID) + req, err := http.NewRequest("GET", url, nil) + + if err != nil { + httperrors.Serve500(w) + return true + } + + req.Header.Add("Authorization", "Bearer "+session.Values["access_token"].(string)) + resp, err := a.apiClient.Do(req) + + if checkResponseForInvalidToken(resp, err) { + destroySession(session, w, r) return true } if err != nil || resp.StatusCode != 200 { - httperrors.Serve401(w) + // We return 404 if user has no access to avoid user knowing if the pages really existed or not + httperrors.Serve404(w) return true } diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 60ff6223..69f1d731 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -13,13 +13,17 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/auth" ) -func TestTryAuthenticate(t *testing.T) { - auth := auth.New("pages.gitlab-example.com", +func createAuth(t *testing.T) *auth.Auth { + return auth.New("pages.gitlab-example.com", "something-very-secret", "id", "secret", "http://pages.gitlab-example.com/auth", "http://gitlab-example.com") +} + +func TestTryAuthenticate(t *testing.T) { + auth := createAuth(t) result := httptest.NewRecorder() reqURL, err := url.Parse("/something/else") @@ -30,12 +34,7 @@ func TestTryAuthenticate(t *testing.T) { } func TestTryAuthenticateWithError(t *testing.T) { - auth := auth.New("pages.gitlab-example.com", - "something-very-secret", - "id", - "secret", - "http://pages.gitlab-example.com/auth", - "http://gitlab-example.com") + auth := createAuth(t) result := httptest.NewRecorder() reqURL, err := url.Parse("/auth?error=access_denied") @@ -48,12 +47,7 @@ func TestTryAuthenticateWithError(t *testing.T) { func TestTryAuthenticateWithCodeButInvalidState(t *testing.T) { store := sessions.NewCookieStore([]byte("something-very-secret")) - auth := auth.New("pages.gitlab-example.com", - "something-very-secret", - "id", - "secret", - "http://pages.gitlab-example.com/auth", - "http://gitlab-example.com") + auth := createAuth(t) result := httptest.NewRecorder() reqURL, err := url.Parse("/auth?code=1&state=invalid") @@ -182,7 +176,7 @@ func TestCheckAuthenticationWhenNoAccess(t *testing.T) { session.Save(r, result) assert.Equal(t, true, auth.CheckAuthentication(result, r, 1000)) - assert.Equal(t, 401, result.Code) + assert.Equal(t, 404, result.Code) } func TestCheckAuthenticationWhenInvalidToken(t *testing.T) { @@ -222,3 +216,78 @@ func TestCheckAuthenticationWhenInvalidToken(t *testing.T) { assert.Equal(t, true, auth.CheckAuthentication(result, r, 1000)) assert.Equal(t, 302, result.Code) } + +func TestCheckAuthenticationWithoutProject(t *testing.T) { + apiServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v4/projects": + assert.Equal(t, "Bearer abc", r.Header.Get("Authorization")) + w.WriteHeader(http.StatusOK) + default: + t.Logf("Unexpected r.URL.RawPath: %q", r.URL.Path) + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(http.StatusNotFound) + } + })) + + apiServer.Start() + defer apiServer.Close() + + store := sessions.NewCookieStore([]byte("something-very-secret")) + auth := auth.New("pages.gitlab-example.com", + "something-very-secret", + "id", + "secret", + "http://pages.gitlab-example.com/auth", + apiServer.URL) + + result := httptest.NewRecorder() + reqURL, err := url.Parse("/auth?code=1&state=state") + require.NoError(t, err) + r := &http.Request{URL: reqURL} + + session, _ := store.Get(r, "gitlab-pages") + session.Values["access_token"] = "abc" + session.Save(r, result) + + assert.Equal(t, false, auth.CheckAuthenticationWithoutProject(result, r)) + assert.Equal(t, 200, result.Code) +} + +func TestCheckAuthenticationWithoutProjectWhenInvalidToken(t *testing.T) { + apiServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v4/projects": + assert.Equal(t, "Bearer abc", r.Header.Get("Authorization")) + w.WriteHeader(http.StatusUnauthorized) + fmt.Fprint(w, "{\"error\":\"invalid_token\"}") + default: + t.Logf("Unexpected r.URL.RawPath: %q", r.URL.Path) + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(http.StatusNotFound) + } + })) + + apiServer.Start() + defer apiServer.Close() + + store := sessions.NewCookieStore([]byte("something-very-secret")) + auth := auth.New("pages.gitlab-example.com", + "something-very-secret", + "id", + "secret", + "http://pages.gitlab-example.com/auth", + apiServer.URL) + + result := httptest.NewRecorder() + reqURL, err := url.Parse("/auth?code=1&state=state") + require.NoError(t, err) + r := &http.Request{URL: reqURL} + + session, _ := store.Get(r, "gitlab-pages") + session.Values["access_token"] = "abc" + session.Save(r, result) + + assert.Equal(t, true, auth.CheckAuthenticationWithoutProject(result, r)) + assert.Equal(t, 302, result.Code) +} -- cgit v1.2.3