diff options
Diffstat (limited to 'internal')
-rw-r--r-- | internal/artifact/artifact.go | 53 | ||||
-rw-r--r-- | internal/artifact/artifact_test.go | 59 | ||||
-rw-r--r-- | internal/auth/auth.go | 121 | ||||
-rw-r--r-- | internal/auth/auth_test.go | 88 | ||||
-rw-r--r-- | internal/handlers/handlers.go | 67 | ||||
-rw-r--r-- | internal/handlers/handlers_test.go | 161 | ||||
-rw-r--r-- | internal/interface.go | 18 | ||||
-rw-r--r-- | internal/logging/logging.go | 8 | ||||
-rw-r--r-- | internal/mocks/mocks.go | 128 |
9 files changed, 626 insertions, 77 deletions
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index 5050b426..28a595bb 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -1,6 +1,7 @@ package artifact import ( + "errors" "fmt" "io" "net/http" @@ -11,8 +12,11 @@ import ( "strings" "time" + "gitlab.com/gitlab-org/labkit/errortracking" + "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/httptransport" + "gitlab.com/gitlab-org/gitlab-pages/internal/logging" ) const ( @@ -26,7 +30,10 @@ const ( var ( // Captures subgroup + project, job ID and artifacts path - pathExtractor = regexp.MustCompile(`(?i)\A/-/(.*)/-/jobs/(\d+)/artifacts(/[^?]*)\z`) + pathExtractor = regexp.MustCompile(`(?i)\A/-/(.*)/-/jobs/(\d+)/artifacts(/[^?]*)\z`) + errCreateArtifactRequest = errors.New("Failed to create the artifact request") + errArtifactRequest = errors.New("Failed to request the artifact") + errArtifactResponse = errors.New("Artifact request response was not successful") ) // Artifact proxies requests for artifact files to the GitLab artifacts API @@ -51,8 +58,9 @@ func New(server string, timeoutSeconds int, pagesDomain string) *Artifact { // TryMakeRequest will attempt to proxy a request and write it to the argument // http.ResponseWriter, ultimately returning a bool that indicates if the -// http.ResponseWriter has been written to in any capacity. -func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Request) bool { +// http.ResponseWriter has been written to in any capacity. Additional handler func +// may be given which should return true if it did handle the response. +func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Request, token string, additionalHandler func(*http.Response) bool) bool { if a == nil || a.server == "" || host == "" { return false } @@ -62,30 +70,51 @@ func (a *Artifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Re return false } - a.makeRequest(w, reqURL) + a.makeRequest(w, r, reqURL, token, additionalHandler) + return true } -func (a *Artifact) makeRequest(w http.ResponseWriter, reqURL *url.URL) { - resp, err := a.client.Get(reqURL.String()) +func (a *Artifact) makeRequest(w http.ResponseWriter, r *http.Request, reqURL *url.URL, token string, additionalHandler func(*http.Response) bool) { + req, err := http.NewRequest("GET", reqURL.String(), nil) + if err != nil { + logging.LogRequest(r).WithError(err).Error(errCreateArtifactRequest) + errortracking.Capture(err, errortracking.WithRequest(r)) + httperrors.Serve500(w) + return + } + + if token != "" { + req.Header.Add("Authorization", "Bearer "+token) + } + resp, err := a.client.Do(req) + if err != nil { + logging.LogRequest(r).WithError(err).Error(errArtifactRequest) + errortracking.Capture(err, errortracking.WithRequest(r)) httperrors.Serve502(w) return } + if additionalHandler(resp) { + return + } + if resp.StatusCode == http.StatusNotFound { httperrors.Serve404(w) return } if resp.StatusCode == http.StatusInternalServerError { + logging.LogRequest(r).Error(errArtifactResponse) + errortracking.Capture(errArtifactResponse, errortracking.WithRequest(r)) httperrors.Serve500(w) return } - // we only cache responses within the 2xx series response codes - if (resp.StatusCode >= minStatusCode) && (resp.StatusCode <= maxStatusCode) { - w.Header().Set("Cache-Control", "max-age=3600") + // we only cache responses within the 2xx series response codes and that were not private + if token == "" { + addCacheHeader(w, resp) } w.Header().Set("Content-Type", resp.Header.Get("Content-Type")) @@ -95,6 +124,12 @@ func (a *Artifact) makeRequest(w http.ResponseWriter, reqURL *url.URL) { return } +func addCacheHeader(w http.ResponseWriter, resp *http.Response) { + if (resp.StatusCode >= minStatusCode) && (resp.StatusCode <= maxStatusCode) { + w.Header().Set("Cache-Control", "max-age=3600") + } +} + // BuildURL returns a pointer to a url.URL for where the request should be // proxied to. The returned bool will indicate if there is some sort of issue // with the url while it is being generated. diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go index ef37384e..689effb5 100644 --- a/internal/artifact/artifact_test.go +++ b/internal/artifact/artifact_test.go @@ -15,29 +15,12 @@ import ( func TestTryMakeRequest(t *testing.T) { content := "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>" contentType := "text/html; charset=utf-8" - testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", contentType) - switch r.URL.RawPath { - case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/200.html": - w.WriteHeader(http.StatusOK) - case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/max-caching.html": - w.WriteHeader(http.StatusIMUsed) - case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/non-caching.html": - w.WriteHeader(http.StatusTeapot) - case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/500.html": - w.WriteHeader(http.StatusInternalServerError) - case "/projects/group%2Fsubgroup%2Fgroup%2Fproject/jobs/1/artifacts/404.html": - w.WriteHeader(http.StatusNotFound) - default: - t.Log("Surprising r.URL.RawPath", r.URL.RawPath) - w.WriteHeader(999) - } - fmt.Fprint(w, content) - })) + testServer := makeArtifactServerStub(t, content, contentType) defer testServer.Close() cases := []struct { Path string + Token string Status int Content string Length string @@ -47,6 +30,7 @@ func TestTryMakeRequest(t *testing.T) { }{ { "/200.html", + "", http.StatusOK, content, "90", @@ -55,7 +39,18 @@ func TestTryMakeRequest(t *testing.T) { "basic successful request", }, { + "/200.html", + "token", + http.StatusOK, + content, + "90", + "", + "text/html; charset=utf-8", + "basic successful request", + }, + { "/max-caching.html", + "", http.StatusIMUsed, content, "90", @@ -65,6 +60,7 @@ func TestTryMakeRequest(t *testing.T) { }, { "/non-caching.html", + "", http.StatusTeapot, content, "90", @@ -82,7 +78,7 @@ func TestTryMakeRequest(t *testing.T) { r := &http.Request{URL: reqURL} art := artifact.New(testServer.URL, 1, "gitlab-example.io") - require.True(t, art.TryMakeRequest("group.gitlab-example.io", result, r)) + require.True(t, art.TryMakeRequest("group.gitlab-example.io", result, r, c.Token, func(resp *http.Response) bool { return false })) require.Equal(t, c.Status, result.Code) require.Equal(t, c.ContentType, result.Header().Get("Content-Type")) require.Equal(t, c.Length, result.Header().Get("Content-Length")) @@ -93,6 +89,29 @@ func TestTryMakeRequest(t *testing.T) { } } +// provide stub for testing different artifact responses +func makeArtifactServerStub(t *testing.T, content string, contentType string) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", contentType) + switch r.URL.RawPath { + case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/200.html": + w.WriteHeader(http.StatusOK) + case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/max-caching.html": + w.WriteHeader(http.StatusIMUsed) + case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/non-caching.html": + w.WriteHeader(http.StatusTeapot) + case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/500.html": + w.WriteHeader(http.StatusInternalServerError) + case "/projects/group%2Fsubgroup%2Fgroup%2Fproject/jobs/1/artifacts/404.html": + w.WriteHeader(http.StatusNotFound) + default: + t.Log("Surprising r.URL.RawPath", r.URL.RawPath) + w.WriteHeader(999) + } + fmt.Fprint(w, content) + })) +} + func TestBuildURL(t *testing.T) { cases := []struct { RawServer string diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 77bc7d8e..5b85a44e 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -364,6 +364,19 @@ func (a *Auth) fetchAccessToken(code string) (tokenResponse, error) { return token, nil } +func (a *Auth) checkSessionIsValid(w http.ResponseWriter, r *http.Request) *sessions.Session { + session, err := a.checkSession(w, r) + if err != nil { + return nil + } + + if a.checkTokenExists(session, w, r) { + return nil + } + + return session +} + 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 { @@ -424,25 +437,19 @@ func (a *Auth) IsAuthSupported() bool { return true } -// 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 - } - - session, err := a.checkSession(w, r) - if err != nil { - return true - } - - if a.checkTokenExists(session, w, r) { +func (a *Auth) checkAuthentication(w http.ResponseWriter, r *http.Request, projectID uint64) bool { + session := a.checkSessionIsValid(w, r) + if session == nil { return true } // Access token exists, authorize request - url := fmt.Sprintf(apiURLUserTemplate, a.gitLabServer) + var url string + if projectID > 0 { + url = fmt.Sprintf(apiURLProjectTemplate, a.gitLabServer, projectID) + } else { + url = fmt.Sprintf(apiURLUserTemplate, a.gitLabServer) + } req, err := http.NewRequest("GET", url, nil) if err != nil { @@ -456,19 +463,16 @@ func (a *Auth) CheckAuthenticationWithoutProject(w http.ResponseWriter, r *http. req.Header.Add("Authorization", "Bearer "+session.Values["access_token"].(string)) resp, err := a.apiClient.Do(req) - if checkResponseForInvalidToken(resp, err) { - logRequest(r).Warn("Access token was invalid, destroying session") - - destroySession(session, w, r) + if err == nil && checkResponseForInvalidToken(resp, session, w, r) { return true } if err != nil || resp.StatusCode != 200 { - // We return 404 if for some reason token is not valid to avoid (not) existence leak if err != nil { logRequest(r).WithError(err).Error("Failed to retrieve info with token") } + // We return 404 if for some reason token is not valid to avoid (not) existence leak httperrors.Serve404(w) return true } @@ -476,63 +480,81 @@ func (a *Auth) CheckAuthenticationWithoutProject(w http.ResponseWriter, r *http. 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 { - logRequest(r).Debug("Authenticate request") +// 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 { - logRequest(r).Error(errAuthNotConfigured) - errortracking.Capture(errAuthNotConfigured, errortracking.WithRequest(r)) + // No auth supported + return false + } - httperrors.Serve500(w) - return true + return a.checkAuthentication(w, r, 0) +} + +// GetTokenIfExists returns the token if it exists +func (a *Auth) GetTokenIfExists(w http.ResponseWriter, r *http.Request) (string, error) { + if a == nil { + return "", nil } session, err := a.checkSession(w, r) if err != nil { - return true + return "", errors.New("Error retrieving the session") } - if a.checkTokenExists(session, w, r) { + if session.Values["access_token"] != nil { + return session.Values["access_token"].(string), nil + } + + return "", nil +} + +// RequireAuth will trigger authentication flow if no token exists +func (a *Auth) RequireAuth(w http.ResponseWriter, r *http.Request) bool { + session := a.checkSessionIsValid(w, r) + if session == nil { return true } + return false +} - // Access token exists, authorize request - url := fmt.Sprintf(apiURLProjectTemplate, a.gitLabServer, projectID) - req, err := http.NewRequest("GET", url, 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 { + logRequest(r).Debug("Authenticate request") - if err != nil { - errortracking.Capture(err, errortracking.WithRequest(req)) + if a == nil { + logRequest(r).Error(errAuthNotConfigured) + errortracking.Capture(errAuthNotConfigured, errortracking.WithRequest(r)) httperrors.Serve500(w) return true } - req.Header.Add("Authorization", "Bearer "+session.Values["access_token"].(string)) - resp, err := a.apiClient.Do(req) + return a.checkAuthentication(w, r, projectID) +} - if checkResponseForInvalidToken(resp, err) { - logRequest(r).Warn("Access token was invalid, destroying session") +// CheckResponseForInvalidToken checks response for invalid token and destroys session if it was invalid +func (a *Auth) CheckResponseForInvalidToken(w http.ResponseWriter, r *http.Request, + resp *http.Response) bool { + if a == nil { + // No auth supported + return false + } - destroySession(session, w, r) + session, err := a.checkSession(w, r) + if err != nil { return true } - if err != nil || resp.StatusCode != 200 { - if err != nil { - logRequest(r).WithError(err).Error("Failed to retrieve info with token") - } - - // We return 404 if user has no access to avoid user knowing if the pages really existed or not - httperrors.Serve404(w) + if checkResponseForInvalidToken(resp, session, w, r) { return true } return false } -func checkResponseForInvalidToken(resp *http.Response, err error) bool { - if err == nil && resp.StatusCode == 401 { +func checkResponseForInvalidToken(resp *http.Response, session *sessions.Session, w http.ResponseWriter, r *http.Request) bool { + if resp.StatusCode == http.StatusUnauthorized { errResp := errorResponse{} // Parse response @@ -545,6 +567,9 @@ func checkResponseForInvalidToken(resp *http.Response, err error) bool { if errResp.Error == "invalid_token" { // Token is invalid + logRequest(r).Warn("Access token was invalid, destroying session") + + destroySession(session, w, r) return true } } diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 8be5e835..6ab5739f 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -1,7 +1,9 @@ package auth import ( + "bytes" "fmt" + "io/ioutil" "net/http" "net/http/httptest" "net/url" @@ -333,3 +335,89 @@ func TestGenerateKeyPair(t *testing.T) { require.Equal(t, len(signingSecret), 32) require.Equal(t, len(encryptionSecret), 32) } + +func TestGetTokenIfExistsWhenTokenExists(t *testing.T) { + store := sessions.NewCookieStore([]byte("something-very-secret")) + auth := New("pages.gitlab-example.com", + "something-very-secret", + "id", + "secret", + "http://pages.gitlab-example.com/auth", + "") + + result := httptest.NewRecorder() + reqURL, err := url.Parse("/") + require.NoError(t, err) + r := &http.Request{URL: reqURL} + r = request.WithHTTPSFlag(r, false) + + session, _ := store.Get(r, "gitlab-pages") + session.Values["access_token"] = "abc" + session.Save(r, result) + + token, err := auth.GetTokenIfExists(result, r) + require.Equal(t, "abc", token) +} + +func TestGetTokenIfExistsWhenTokenDoesNotExist(t *testing.T) { + store := sessions.NewCookieStore([]byte("something-very-secret")) + auth := New("pages.gitlab-example.com", + "something-very-secret", + "id", + "secret", + "http://pages.gitlab-example.com/auth", + "") + + result := httptest.NewRecorder() + reqURL, err := url.Parse("http://pages.gitlab-example.com/test") + require.NoError(t, err) + r := &http.Request{URL: reqURL, Host: "pages.gitlab-example.com", RequestURI: "/test"} + r = request.WithHTTPSFlag(r, false) + + session, _ := store.Get(r, "gitlab-pages") + session.Save(r, result) + + token, err := auth.GetTokenIfExists(result, r) + require.Equal(t, "", token) + require.Equal(t, nil, err) +} + +func TestCheckResponseForInvalidTokenWhenInvalidToken(t *testing.T) { + auth := New("pages.gitlab-example.com", + "something-very-secret", + "id", + "secret", + "http://pages.gitlab-example.com/auth", + "") + + result := httptest.NewRecorder() + reqURL, err := url.Parse("http://pages.gitlab-example.com/test") + require.NoError(t, err) + r := &http.Request{URL: reqURL, Host: "pages.gitlab-example.com", RequestURI: "/test"} + r = request.WithHTTPSFlag(r, false) + + resp := &http.Response{StatusCode: http.StatusUnauthorized, Body: ioutil.NopCloser(bytes.NewReader([]byte("{\"error\":\"invalid_token\"}")))} + + require.Equal(t, true, auth.CheckResponseForInvalidToken(result, r, resp)) + require.Equal(t, http.StatusFound, result.Result().StatusCode) + require.Equal(t, "http://pages.gitlab-example.com/test", result.Header().Get("Location")) +} + +func TestCheckResponseForInvalidTokenWhenNotInvalidToken(t *testing.T) { + auth := New("pages.gitlab-example.com", + "something-very-secret", + "id", + "secret", + "http://pages.gitlab-example.com/auth", + "") + + result := httptest.NewRecorder() + reqURL, err := url.Parse("/something") + require.NoError(t, err) + r := &http.Request{URL: reqURL} + r = request.WithHTTPSFlag(r, false) + + resp := &http.Response{StatusCode: 200, Body: ioutil.NopCloser(bytes.NewReader([]byte("ok")))} + + require.Equal(t, false, auth.CheckResponseForInvalidToken(result, r, resp)) +} diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go new file mode 100644 index 00000000..5791e793 --- /dev/null +++ b/internal/handlers/handlers.go @@ -0,0 +1,67 @@ +package handlers + +import ( + "net/http" + + "gitlab.com/gitlab-org/gitlab-pages/internal" + "gitlab.com/gitlab-org/gitlab-pages/internal/logging" +) + +// Handlers take care of handling specific requests +type Handlers struct { + Auth internal.Auth + Artifact internal.Artifact +} + +// New when provided the arguments defined herein, returns a pointer to an +// Handlers that is used to handle requests. +func New(auth internal.Auth, artifact internal.Artifact) *Handlers { + return &Handlers{ + Auth: auth, + Artifact: artifact, + } +} + +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 { + + 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") + + // Authenticate user + if a.Auth.RequireAuth(w, r) { + return true + } + } else { + logging.LogRequest(r).Debug("Artifact API response was 404 with authentication") + } + } + + if a.Auth.CheckResponseForInvalidToken(w, r, resp) { + return true + } + + return false + } +} + +// HandleArtifactRequest handles all artifact related requests, will return true if request was handled here +func (a *Handlers) HandleArtifactRequest(host string, w http.ResponseWriter, r *http.Request) bool { + // In the event a host is prefixed with the artifact prefix an artifact + // value is created, and an attempt to proxy the request is made + + // Always try to add token to the request if it exists + token, err := a.Auth.GetTokenIfExists(w, r) + if err != nil { + return true + } + + return a.Artifact.TryMakeRequest(host, w, r, token, a.checkIfLoginRequiredOrInvalidToken(w, r, token)) +} diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go new file mode 100644 index 00000000..aa664266 --- /dev/null +++ b/internal/handlers/handlers_test.go @@ -0,0 +1,161 @@ +package handlers + +import ( + "errors" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "gitlab.com/gitlab-org/gitlab-pages/internal/mocks" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" +) + +func TestNotHandleArtifactRequestReturnsFalse(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockArtifact := mocks.NewMockArtifact(mockCtrl) + mockArtifact.EXPECT(). + TryMakeRequest(gomock.Any(), gomock.Any(), gomock.Any(), "", gomock.Any()). + Return(false). + Times(1) + + mockAuth := mocks.NewMockAuth(mockCtrl) + mockAuth.EXPECT(). + GetTokenIfExists(gomock.Any(), gomock.Any()). + Return("", nil). + Times(1) + + handlers := New(mockAuth, mockArtifact) + + result := httptest.NewRecorder() + reqURL, err := url.Parse("/something") + require.NoError(t, err) + r := &http.Request{URL: reqURL} + + require.Equal(t, false, handlers.HandleArtifactRequest("host", result, r)) +} + +func TestHandleArtifactRequestedReturnsTrue(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockArtifact := mocks.NewMockArtifact(mockCtrl) + mockArtifact.EXPECT(). + TryMakeRequest(gomock.Any(), gomock.Any(), gomock.Any(), "", gomock.Any()). + Return(true). + Times(1) + + mockAuth := mocks.NewMockAuth(mockCtrl) + mockAuth.EXPECT(). + GetTokenIfExists(gomock.Any(), gomock.Any()). + Return("", nil). + Times(1) + + handlers := New(mockAuth, mockArtifact) + + result := httptest.NewRecorder() + reqURL, err := url.Parse("/something") + require.NoError(t, err) + r := &http.Request{URL: reqURL} + + require.Equal(t, true, handlers.HandleArtifactRequest("host", result, r)) +} + +func TestNotFoundWithTokenIsNotHandled(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockAuth := mocks.NewMockAuth(mockCtrl) + 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: http.StatusNotFound} + handled := handlers.checkIfLoginRequiredOrInvalidToken(w, r, "token")(response) + + require.False(t, handled) +} + +func TestNotFoundWithoutTokenIsNotHandledWhenNotAuthSupport(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockAuth := mocks.NewMockAuth(mockCtrl) + mockAuth.EXPECT().IsAuthSupported().Return(false) + + handlers := New(mockAuth, nil) + + w := httptest.NewRecorder() + reqURL, _ := url.Parse("/") + r := &http.Request{URL: reqURL} + response := &http.Response{StatusCode: http.StatusNotFound} + handled := handlers.checkIfLoginRequiredOrInvalidToken(w, r, "")(response) + + require.False(t, handled) +} +func TestNotFoundWithoutTokenIsHandled(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockAuth := mocks.NewMockAuth(mockCtrl) + mockAuth.EXPECT().IsAuthSupported().Return(true) + mockAuth.EXPECT().RequireAuth(gomock.Any(), gomock.Any()).Times(1).Return(true) + + handlers := New(mockAuth, nil) + + w := httptest.NewRecorder() + reqURL, _ := url.Parse("/") + r := &http.Request{URL: reqURL} + response := &http.Response{StatusCode: http.StatusNotFound} + handled := handlers.checkIfLoginRequiredOrInvalidToken(w, r, "")(response) + + require.True(t, handled) +} +func TestInvalidTokenResponseIsHandled(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockAuth := mocks.NewMockAuth(mockCtrl) + mockAuth.EXPECT().CheckResponseForInvalidToken(gomock.Any(), gomock.Any(), gomock.Any()). + Return(true) + + handlers := New(mockAuth, nil) + + w := httptest.NewRecorder() + reqURL, _ := url.Parse("/") + r := &http.Request{URL: reqURL} + response := &http.Response{StatusCode: http.StatusUnauthorized} + handled := handlers.checkIfLoginRequiredOrInvalidToken(w, r, "token")(response) + + require.True(t, handled) +} + +func TestHandleArtifactRequestButGetTokenFails(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockArtifact := mocks.NewMockArtifact(mockCtrl) + mockArtifact.EXPECT(). + TryMakeRequest(gomock.Any(), gomock.Any(), gomock.Any(), "", gomock.Any()). + Times(0) + + mockAuth := mocks.NewMockAuth(mockCtrl) + mockAuth.EXPECT().GetTokenIfExists(gomock.Any(), gomock.Any()).Return("", errors.New("error when retrieving token")) + + handlers := New(mockAuth, mockArtifact) + + result := httptest.NewRecorder() + reqURL, err := url.Parse("/something") + require.NoError(t, err) + r := &http.Request{URL: reqURL} + + require.Equal(t, true, handlers.HandleArtifactRequest("host", result, r)) +} diff --git a/internal/interface.go b/internal/interface.go new file mode 100644 index 00000000..3e82ee3a --- /dev/null +++ b/internal/interface.go @@ -0,0 +1,18 @@ +package internal + +import ( + "net/http" +) + +// Artifact allows to handle artifact related requests +type Artifact interface { + TryMakeRequest(host string, w http.ResponseWriter, r *http.Request, token string, responseHandler func(*http.Response) bool) bool +} + +// Auth handles the authentication logic +type Auth interface { + IsAuthSupported() bool + RequireAuth(w http.ResponseWriter, r *http.Request) bool + GetTokenIfExists(w http.ResponseWriter, r *http.Request) (string, error) + CheckResponseForInvalidToken(w http.ResponseWriter, r *http.Request, resp *http.Response) bool +} diff --git a/internal/logging/logging.go b/internal/logging/logging.go index f0682573..9269261d 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -78,3 +78,11 @@ func AccessLogger(handler http.Handler, format string) (http.Handler, error) { log.WithAccessLogger(accessLogger), ), nil } + +// LogRequest will inject request host and path to the logged messages +func LogRequest(r *http.Request) *logrus.Entry { + return log.WithFields(log.Fields{ + "host": r.Host, + "path": r.URL.Path, + }) +} diff --git a/internal/mocks/mocks.go b/internal/mocks/mocks.go new file mode 100644 index 00000000..2816205d --- /dev/null +++ b/internal/mocks/mocks.go @@ -0,0 +1,128 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: internal/interface.go + +// Package mocks is a generated GoMock package. +package mocks + +import ( + gomock "github.com/golang/mock/gomock" + http "net/http" + reflect "reflect" +) + +// MockArtifact is a mock of Artifact interface +type MockArtifact struct { + ctrl *gomock.Controller + recorder *MockArtifactMockRecorder +} + +// MockArtifactMockRecorder is the mock recorder for MockArtifact +type MockArtifactMockRecorder struct { + mock *MockArtifact +} + +// NewMockArtifact creates a new mock instance +func NewMockArtifact(ctrl *gomock.Controller) *MockArtifact { + mock := &MockArtifact{ctrl: ctrl} + mock.recorder = &MockArtifactMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockArtifact) EXPECT() *MockArtifactMockRecorder { + return m.recorder +} + +// TryMakeRequest mocks base method +func (m *MockArtifact) TryMakeRequest(host string, w http.ResponseWriter, r *http.Request, token string, responseHandler func(*http.Response) bool) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "TryMakeRequest", host, w, r, token, responseHandler) + ret0, _ := ret[0].(bool) + return ret0 +} + +// TryMakeRequest indicates an expected call of TryMakeRequest +func (mr *MockArtifactMockRecorder) TryMakeRequest(host, w, r, token, responseHandler interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TryMakeRequest", reflect.TypeOf((*MockArtifact)(nil).TryMakeRequest), host, w, r, token, responseHandler) +} + +// MockAuth is a mock of Auth interface +type MockAuth struct { + ctrl *gomock.Controller + recorder *MockAuthMockRecorder +} + +// MockAuthMockRecorder is the mock recorder for MockAuth +type MockAuthMockRecorder struct { + mock *MockAuth +} + +// NewMockAuth creates a new mock instance +func NewMockAuth(ctrl *gomock.Controller) *MockAuth { + mock := &MockAuth{ctrl: ctrl} + mock.recorder = &MockAuthMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockAuth) EXPECT() *MockAuthMockRecorder { + return m.recorder +} + +// IsAuthSupported mocks base method +func (m *MockAuth) IsAuthSupported() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsAuthSupported") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsAuthSupported indicates an expected call of IsAuthSupported +func (mr *MockAuthMockRecorder) IsAuthSupported() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsAuthSupported", reflect.TypeOf((*MockAuth)(nil).IsAuthSupported)) +} + +// RequireAuth mocks base method +func (m *MockAuth) RequireAuth(w http.ResponseWriter, r *http.Request) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RequireAuth", w, r) + ret0, _ := ret[0].(bool) + return ret0 +} + +// RequireAuth indicates an expected call of RequireAuth +func (mr *MockAuthMockRecorder) RequireAuth(w, r interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RequireAuth", reflect.TypeOf((*MockAuth)(nil).RequireAuth), w, r) +} + +// GetTokenIfExists mocks base method +func (m *MockAuth) GetTokenIfExists(w http.ResponseWriter, r *http.Request) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetTokenIfExists", w, r) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetTokenIfExists indicates an expected call of GetTokenIfExists +func (mr *MockAuthMockRecorder) GetTokenIfExists(w, r interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTokenIfExists", reflect.TypeOf((*MockAuth)(nil).GetTokenIfExists), w, r) +} + +// CheckResponseForInvalidToken mocks base method +func (m *MockAuth) CheckResponseForInvalidToken(w http.ResponseWriter, r *http.Request, resp *http.Response) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CheckResponseForInvalidToken", w, r, resp) + ret0, _ := ret[0].(bool) + return ret0 +} + +// CheckResponseForInvalidToken indicates an expected call of CheckResponseForInvalidToken +func (mr *MockAuthMockRecorder) CheckResponseForInvalidToken(w, r, resp interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckResponseForInvalidToken", reflect.TypeOf((*MockAuth)(nil).CheckResponseForInvalidToken), w, r, resp) +} |