Refactor logic to avoid existence leak

2 files changed, 164 insertions, 40 deletions

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)
+}