Merge branch 'sh-handle-api-artifacts-403' into 'master'

fix: Handle 403 errors when artifacts loaded from GitLab artifacts browser

See merge request gitlab-org/gitlab-pages!584

2 files changed, 54 insertions, 15 deletions

diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go
index fb47fc55..76724387 100644
--- a/internal/handlers/handlers.go
+++ b/internal/handlers/handlers.go
@@ -24,21 +24,22 @@ func New(auth internal.Auth, artifact internal.Artifact) *Handlers {
 
 func (a *Handlers) checkIfLoginRequiredOrInvalidToken(w http.ResponseWriter, r *http.Request, token string) func(*http.Response) bool {
 	return func(resp *http.Response) bool {
-		if resp.StatusCode == http.StatusNotFound {
+		// API will return 403 if the project does not have public pipelines (public_builds flag)
+		if resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusForbidden {
 			if token == "" {
 				if !a.Auth.IsAuthSupported() {
 					// Auth is not supported, probably means no access or does not exist but we cannot try with auth
 					return false
 				}
 
-				logging.LogRequest(r).Debug("Artifact API response was 404 without token, try with authentication")
+				logging.LogRequest(r).Debugf("Artifact API response was %d without token, try with authentication", resp.StatusCode)
 
 				// Authenticate user
 				if a.Auth.RequireAuth(w, r) {
 					return true
 				}
 			} else {
-				logging.LogRequest(r).Debug("Artifact API response was 404 with authentication")
+				logging.LogRequest(r).Debugf("Artifact API response was %d with authentication", resp.StatusCode)
 			}
 		}
 
diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go
index 710a167c..33a0cdb2 100644
--- a/internal/handlers/handlers_test.go
+++ b/internal/handlers/handlers_test.go
@@ -58,9 +58,7 @@ func TestHandleArtifactRequestedReturnsTrue(t *testing.T) {
 	handlers := New(mockAuth, mockArtifact)
 
 	result := httptest.NewRecorder()
-	reqURL, err := url.Parse("/something")
-	require.NoError(t, err)
-	r := &http.Request{URL: reqURL}
+	r := httptest.NewRequest(http.MethodGet, "/something", nil)
 
 	require.Equal(t, true, handlers.HandleArtifactRequest("host", result, r))
 }
@@ -85,6 +83,51 @@ func TestNotFoundWithTokenIsNotHandled(t *testing.T) {
 	require.False(t, handled)
 }
 
+func TestForbiddenWithTokenIsNotHandled(t *testing.T) {
+	cases := map[string]struct {
+		StatusCode int
+		Token      string
+		Handled    bool
+	}{
+		"403 Forbidden with token": {
+			http.StatusForbidden,
+			"token",
+			false,
+		},
+		"403 Forbidden with no token": {
+			http.StatusForbidden,
+			"",
+			true,
+		},
+	}
+
+	for tn, tc := range cases {
+		t.Run(tn, func(t *testing.T) {
+			mockCtrl := gomock.NewController(t)
+			defer mockCtrl.Finish()
+
+			mockAuth := mocks.NewMockAuth(mockCtrl)
+			if tc.Token == "" {
+				mockAuth.EXPECT().IsAuthSupported().Return(true)
+				mockAuth.EXPECT().RequireAuth(gomock.Any(), gomock.Any()).Return(true)
+			} else {
+				mockAuth.EXPECT().CheckResponseForInvalidToken(gomock.Any(), gomock.Any(), gomock.Any()).
+					Return(false)
+			}
+
+			handlers := New(mockAuth, nil)
+
+			w := httptest.NewRecorder()
+			r := httptest.NewRequest(http.MethodGet, "/", nil)
+			response := &http.Response{StatusCode: tc.StatusCode}
+			// nolint:bodyclose // TODO investigate https://gitlab.com/gitlab-org/gitlab-pages/-/issues/606
+			handled := handlers.checkIfLoginRequiredOrInvalidToken(w, r, tc.Token)(response)
+
+			require.Equal(t, tc.Handled, handled)
+		})
+	}
+}
+
 func TestNotFoundWithoutTokenIsNotHandledWhenNotAuthSupport(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
@@ -95,8 +138,7 @@ func TestNotFoundWithoutTokenIsNotHandledWhenNotAuthSupport(t *testing.T) {
 	handlers := New(mockAuth, nil)
 
 	w := httptest.NewRecorder()
-	reqURL, _ := url.Parse("/")
-	r := &http.Request{URL: reqURL}
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
 	response := &http.Response{StatusCode: http.StatusNotFound}
 	// nolint:bodyclose // TODO investigate https://gitlab.com/gitlab-org/gitlab-pages/-/issues/606
 	handled := handlers.checkIfLoginRequiredOrInvalidToken(w, r, "")(response)
@@ -114,8 +156,7 @@ func TestNotFoundWithoutTokenIsHandled(t *testing.T) {
 	handlers := New(mockAuth, nil)
 
 	w := httptest.NewRecorder()
-	reqURL, _ := url.Parse("/")
-	r := &http.Request{URL: reqURL}
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
 	response := &http.Response{StatusCode: http.StatusNotFound}
 	// nolint:bodyclose // TODO investigate https://gitlab.com/gitlab-org/gitlab-pages/-/issues/606
 	handled := handlers.checkIfLoginRequiredOrInvalidToken(w, r, "")(response)
@@ -133,8 +174,7 @@ func TestInvalidTokenResponseIsHandled(t *testing.T) {
 	handlers := New(mockAuth, nil)
 
 	w := httptest.NewRecorder()
-	reqURL, _ := url.Parse("/")
-	r := &http.Request{URL: reqURL}
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
 	response := &http.Response{StatusCode: http.StatusUnauthorized}
 	// nolint:bodyclose // TODO investigate https://gitlab.com/gitlab-org/gitlab-pages/-/issues/606
 	handled := handlers.checkIfLoginRequiredOrInvalidToken(w, r, "token")(response)
@@ -157,9 +197,7 @@ func TestHandleArtifactRequestButGetTokenFails(t *testing.T) {
 	handlers := New(mockAuth, mockArtifact)
 
 	result := httptest.NewRecorder()
-	reqURL, err := url.Parse("/something")
-	require.NoError(t, err)
-	r := &http.Request{URL: reqURL}
+	r := httptest.NewRequest(http.MethodGet, "/something", nil)
 
 	require.Equal(t, true, handlers.HandleArtifactRequest("host", result, r))
 }