From d30b49cde8ce46271cf8a481b7ae74b5b2dfd163 Mon Sep 17 00:00:00 2001 From: "Sergio C. Arteaga" Date: Mon, 22 Mar 2021 19:20:11 +0100 Subject: [PATCH] Add some more tests for users handlers (#1194) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sergio CastaƱo Arteaga --- cmd/hub/handlers/user/handlers.go | 5 +- cmd/hub/handlers/user/handlers_test.go | 95 ++++++++++++++++++++++++++ internal/util/config_test.go | 3 +- 3 files changed, 101 insertions(+), 2 deletions(-) diff --git a/cmd/hub/handlers/user/handlers.go b/cmd/hub/handlers/user/handlers.go index 2bdfacd2..ee806f1c 100644 --- a/cmd/hub/handlers/user/handlers.go +++ b/cmd/hub/handlers/user/handlers.go @@ -8,6 +8,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "math/big" "net" "net/http" @@ -122,7 +123,7 @@ func (h *Handlers) BasicAuth(next http.Handler) http.Handler { if !ok || !areCredentialsValid([]byte(user), []byte(pass)) { w.Header().Set("WWW-Authenticate", "Basic realm="+realm+`"`) w.WriteHeader(http.StatusUnauthorized) - _, _ = w.Write([]byte("Unauthorized\n")) + _, _ = io.WriteString(w, "Unauthorized\n") return } next.ServeHTTP(w, r) @@ -461,6 +462,8 @@ func (h *Handlers) registerUserWithOauth( u, err = h.newUserFromGoogleProfile(ctx, providerConfig, oauthToken) case "oidc": u, err = h.newUserFromOIDProfile(ctx, oauthToken) + default: + err = fmt.Errorf("invalid provider: %s", provider) } if err != nil { return "", err diff --git a/cmd/hub/handlers/user/handlers_test.go b/cmd/hub/handlers/user/handlers_test.go index a19696c0..db9d075c 100644 --- a/cmd/hub/handlers/user/handlers_test.go +++ b/cmd/hub/handlers/user/handlers_test.go @@ -495,6 +495,100 @@ func TestLogout(t *testing.T) { }) } +func TestOauthCallback(t *testing.T) { + t.Run("invalid oauth code or state", func(t *testing.T) { + state := &OauthState{ + Random: "abcd", + RedirectURL: "/", + } + + testCases := []struct { + description string + url string + cookie *http.Cookie + }{ + { + "oauth code not provided", + "/", + nil, + }, + { + "oauth state not provided", + "/?code=1234", + nil, + }, + { + "state cookie not provided", + "/?code=1234&state=" + state.String(), + nil, + }, + { + "invalid state cookie", + "/?code=1234&state=" + state.String(), + &http.Cookie{ + Name: oauthStateCookieName, + Value: "something not expected", + }, + }, + } + for _, tc := range testCases { + tc := tc + t.Run(tc.description, func(t *testing.T) { + t.Parallel() + w := httptest.NewRecorder() + r, _ := http.NewRequest("GET", tc.url, nil) + if tc.cookie != nil { + r.AddCookie(tc.cookie) + } + + hw := newHandlersWrapper() + hw.h.OauthCallback(w, r) + resp := w.Result() + defer resp.Body.Close() + + assert.Equal(t, http.StatusSeeOther, resp.StatusCode) + redirectURL, err := resp.Location() + require.NoError(t, err) + assert.Equal(t, oauthFailedURL, redirectURL.String()) + }) + } + }) +} + +func TestOauthRedirect(t *testing.T) { + t.Parallel() + w := httptest.NewRecorder() + r, _ := http.NewRequest("GET", "/", nil) + rctx := &chi.Context{ + URLParams: chi.RouteParams{ + Keys: []string{"provider"}, + Values: []string{"github"}, + }, + } + r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) + + hw := newHandlersWrapper() + hw.h.OauthRedirect(w, r) + resp := w.Result() + defer resp.Body.Close() + + require.Len(t, resp.Cookies(), 1) + assert.Equal(t, oauthStateCookieName, resp.Cookies()[0].Name) + assert.NotEmpty(t, resp.Cookies()[0].Value) + assert.Equal(t, "/", resp.Cookies()[0].Path) + assert.True(t, resp.Cookies()[0].HttpOnly) + assert.False(t, resp.Cookies()[0].Secure) + assert.Equal(t, http.StatusSeeOther, resp.StatusCode) + state := &OauthState{ + Random: resp.Cookies()[0].Value, + RedirectURL: "/", + } + expectedRedirectURL := hw.h.oauthConfig["github"].AuthCodeURL(state.String()) + redirectURL, err := resp.Location() + require.NoError(t, err) + assert.Equal(t, expectedRedirectURL, redirectURL.String()) +} + func TestRegisterPasswordResetCode(t *testing.T) { t.Run("invalid input", func(t *testing.T) { t.Parallel() @@ -1173,6 +1267,7 @@ type handlersWrapper struct { func newHandlersWrapper() *handlersWrapper { cfg := viper.New() cfg.Set("server.baseURL", "baseURL") + cfg.Set("server.oauth.github", map[string]string{}) um := &user.ManagerMock{} h, _ := NewHandlers(context.Background(), um, cfg) diff --git a/internal/util/config_test.go b/internal/util/config_test.go index ab3c3f53..e948ceca 100644 --- a/internal/util/config_test.go +++ b/internal/util/config_test.go @@ -1,6 +1,7 @@ package util import ( + "io" "os" "path/filepath" "testing" @@ -25,7 +26,7 @@ func TestSetupConfig(t *testing.T) { f, err := os.Create(name) require.NoError(t, err) defer os.Remove(name) - _, err = f.Write([]byte(`key1: value1`)) + _, err = io.WriteString(f, `key1: value1`) require.NoError(t, err) // Check SetupConfig now succeeds