diff options
author | Stan Hu <stanhu@gmail.com> | 2021-09-16 11:27:57 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2021-09-19 20:51:47 +0300 |
commit | 159460852e41a77b7bb5d201e127f593681c370c (patch) | |
tree | 057947a8d7429cea6a481a4f8e40c022e7e48fd2 /internal | |
parent | fecd9ca44bfc63e82f1cb2fde515b1e348678f7e (diff) |
fix: handle 403 errors when artifacts loaded from GitLab artifacts browser
When the artifacts server is enabled, GitLab Rails links certain files
(e.g. HTML) to the GitLab Pages server so that the file can be rendered
instead of downloaded. However, if public pipelines are not enabled for
a public project and the user never authenticated with the Pages server,
the GitLab Artifacts API would return a 403 error. This error would be
forwarded to the user, and Pages would halt further processing.
To fix this problem, when we encounter a 403 error, we attempt to
authenticate the user if no token is available.
Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/25192
Changelog: fixed
Diffstat (limited to 'internal')
-rw-r--r-- | internal/handlers/handlers.go | 7 | ||||
-rw-r--r-- | internal/handlers/handlers_test.go | 46 |
2 files changed, 50 insertions, 3 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..62ea3a33 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -85,6 +85,52 @@ 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() + reqURL, _ := url.Parse("/") + r := &http.Request{URL: reqURL} + 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() |