diff options
author | Tuomo Ala-Vannesluoma <tuomoav@gmail.com> | 2019-09-26 16:35:28 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2019-09-26 16:35:28 +0300 |
commit | c781a7ccd3147f750faeda631db15d06de455949 (patch) | |
tree | c2d190d278bb270eed5b2ecae0ba9e77f84e1577 /internal/auth | |
parent | 218376d484a8ec55882b037475ec3201d1c897cf (diff) |
Add support for previewing artifacts that are not public
Remove some duplicate logic on Auth module
Separate handling artifact to own handlers package
Unit test handlers by mocking auth and artifact modules
Add generate-mock step to Makefile
Use additional handler func to simplify TryMakeRequest return type
Always try with token if exists
Do not log RequestURI, log path only
Remove not used logRequest func
Diffstat (limited to 'internal/auth')
-rw-r--r-- | internal/auth/auth.go | 121 | ||||
-rw-r--r-- | internal/auth/auth_test.go | 88 |
2 files changed, 161 insertions, 48 deletions
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)) +} |