diff --git a/authn/authenticate.go b/authn/authenticate.go index af16bf28b..d732848b8 100644 --- a/authn/authenticate.go +++ b/authn/authenticate.go @@ -86,7 +86,7 @@ type UserInfo struct { } func (a *Auth) Authenticate(w http.ResponseWriter, r *http.Request) ([]Group, error) { - cookie, err := a.readCookie(r) + cookie, err := a.readCookie(w, r) if err != nil { http.Redirect(w, r, "/signin", http.StatusTemporaryRedirect) return nil, err @@ -94,7 +94,7 @@ func (a *Auth) Authenticate(w http.ResponseWriter, r *http.Request) ([]Group, er if cookie.Token.Expiry.Before(time.Now().Add(a.refreshWithin)) { err = a.refreshToken(w, cookie) if err != nil { - //log error + a.logger.Errorf("refreshing access token: ", err) if cookie.Token.Expiry.Before(time.Now()) { http.Redirect(w, r, "/signin", http.StatusTemporaryRedirect) return nil, err @@ -114,14 +114,7 @@ func (a *Auth) Login(w http.ResponseWriter, r *http.Request) { } func (a *Auth) Logout(w http.ResponseWriter, r *http.Request) { - newCookie := &http.Cookie{ - Name: a.cookieName, - Value: "", - Path: "/", - Secure: true, - HttpOnly: true, - SameSite: http.SameSiteStrictMode, - } + newCookie := a.getEmptyCookie() http.SetCookie(w, newCookie) redirect := fmt.Sprintf("%s?post_logout_redirect_uri=%s/", a.logoutEndpoint, a.fbURL) http.Redirect(w, r, redirect, http.StatusTemporaryRedirect) @@ -146,9 +139,9 @@ func (a *Auth) Redirect(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "/", http.StatusTemporaryRedirect) } -func (a *Auth) GetUserInfo(r *http.Request) *UserInfo { +func (a *Auth) GetUserInfo(w http.ResponseWriter, r *http.Request) *UserInfo { var resp UserInfo - cookie, err := a.readCookie(r) + cookie, err := a.readCookie(w, r) if err != nil { //add logging return &resp @@ -171,15 +164,18 @@ func (a *Auth) newCookieValue(token *oauth2.Token) (*CookieValue, error) { if token == nil { return nil, errors.New("baking cookie due to nil token") } + if token.AccessToken == "" { + return nil, errors.New("no access token provided") + } accessParsed, err := jwt.Parse(token.AccessToken, nil) - if token == nil { - a.logger.Errorf("parsing jwt claims from access tokens: %v", err) + if accessParsed == nil || accessParsed.Claims == nil { + return nil, errors.Wrap(err, "parsing jwt claims from access tokens") } claims := accessParsed.Claims.(jwt.MapClaims) groups, err := a.getGroupMembership(token) if err != nil { - a.logger.Errorf("getting group memebership %v", err) + return nil, errors.Wrap(err, "getting group memebership") } // not needed at this point in the logic and makes the encoded cookie too large token.AccessToken = "" @@ -219,7 +215,7 @@ func (a *Auth) getGroupMembership(token *oauth2.Token) (Groups, error) { return groups, nil } -func (a *Auth) readCookie(r *http.Request) (*CookieValue, error) { +func (a *Auth) readCookie(w http.ResponseWriter, r *http.Request) (*CookieValue, error) { cookie, err := r.Cookie(a.cookieName) if err != nil { return nil, errors.Wrap(err, "cookie not found") @@ -228,6 +224,8 @@ func (a *Auth) readCookie(r *http.Request) (*CookieValue, error) { var value CookieValue err = a.secure.Decode(a.cookieName, cookie.Value, &value) if err != nil { + newCookie := a.getEmptyCookie() + http.SetCookie(w, newCookie) return nil, errors.Wrap(err, "decoding cookie") } @@ -266,7 +264,7 @@ func (a *Auth) refreshToken(w http.ResponseWriter, cookie *CookieValue) error { if newToken.Expiry != cookie.Token.Expiry { cv, err := a.newCookieValue(newToken) if err != nil { - errors.New("setting cookie") + errors.Wrap(err, "setting cookie") } a.setCookie(w, cv) @@ -285,3 +283,14 @@ func decodeHex(hexstr string) ([]byte, error) { } return data, nil } + +func (a *Auth) getEmptyCookie() *http.Cookie { + return &http.Cookie{ + Name: a.cookieName, + Value: "", + Path: "/", + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, + } +} diff --git a/authn/authenticate_test.go b/authn/authenticate_test.go index 97ede1da1..42c8a3eb0 100644 --- a/authn/authenticate_test.go +++ b/authn/authenticate_test.go @@ -1,74 +1,98 @@ -package authn_test +package authn import ( - "io/ioutil" - gohttp "net/http" "net/http/httptest" "os" "strings" "testing" + "time" - "github.com/molecula/featurebase/v2/authn" "github.com/molecula/featurebase/v2/logger" - "github.com/molecula/featurebase/v2/server" + "golang.org/x/oauth2" ) func TestAuth(t *testing.T) { + var ( + ClientId = "e9088663-eb08-41d7-8f65-efb5f54bbb71" + ClientSecret = "DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF" + AuthorizeURL = "https://login.microsoftonline.com/4a137d66-d161-4ae4-b1e6-07e9920874b8/oauth2/v2.0/authorize" + TokenURL = "https://login.microsoftonline.com/4a137d66-d161-4ae4-b1e6-07e9920874b8/oauth2/v2.0/token" + GroupEndpointURL = "https://graph.microsoft.com/v1.0/me/transitiveMemberOf/microsoft.graph.group?$count=true" + LogoutURL = "https://login.microsoftonline.com/common/oauth2/v2.0/logout" + Scopes = []string{"https://graph.microsoft.com/.default", "offline_access"} + Key = "DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF" + ShortKey = "DEADBEEFD" + ) - settings := server.Config{} - settings.Auth.Enable = true - settings.Auth.ClientId = "e9088663-eb08-41d7-8f65-efb5f54bbb71" - settings.Auth.ClientSecret = "DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF" - settings.Auth.AuthorizeURL = "https://login.microsoftonline.com/4a137d66-d161-4ae4-b1e6-07e9920874b8/oauth2/v2.0/authorize" - settings.Auth.TokenURL = "https://login.microsoftonline.com/4a137d66-d161-4ae4-b1e6-07e9920874b8/oauth2/v2.0/token" - settings.Auth.GroupEndpointURL = "https://graph.microsoft.com/v1.0/me/transitiveMemberOf/microsoft.graph.group?$count=true" - settings.Auth.LogoutURL = "https://login.microsoftonline.com/common/oauth2/v2.0/logout" - settings.Auth.Scopes = []string{"https://graph.microsoft.com/.default", "offline_access"} - settings.Auth.HashKey = "DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF" - settings.Auth.BlockKey = "DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF" - - a, err := authn.NewAuth( + a, err := NewAuth( logger.NewStandardLogger(os.Stdout), "http://localhost:10101/", - settings.Auth.Scopes, - settings.Auth.AuthorizeURL, - settings.Auth.TokenURL, - settings.Auth.GroupEndpointURL, - settings.Auth.LogoutURL, - settings.Auth.ClientId, - settings.Auth.ClientSecret, - settings.Auth.HashKey, - settings.Auth.BlockKey, + Scopes, + AuthorizeURL, + TokenURL, + GroupEndpointURL, + LogoutURL, + ClientId, + ClientSecret, + Key, + Key, ) if err != nil { t.Errorf("building auth object%s", err) } + tokenNoAT := oauth2.Token{ + TokenType: "Bearer", + RefreshToken: "abcdef", + Expiry: time.Now().Add(time.Hour), + } + tokenAT := oauth2.Token{ + TokenType: "Bearer", + RefreshToken: "abcdef", + AccessToken: "aasdf", + Expiry: time.Now().Add(time.Hour), + } + // expiredToken := oauth2.Token{ + // TokenType: "Bearer", + // RefreshToken: "abcdef", + // Expiry: time.Now(), + // } + grp := Group{ + UserID: "snowstorm", + GroupID: "abcd123-A", + GroupName: "Romantic Painters", + } + validCV := CookieValue{ + UserID: "snowstorm", + UserName: "J.M.W. Turner", + GroupMembership: []Group{grp}, + Token: &tokenAT, + } - t.Run("Login", func(t *testing.T) { + // t.Run("Login", func(t *testing.T) { - r := httptest.NewRequest(gohttp.MethodGet, "/login", nil) - w := httptest.NewRecorder() - a.Login(w, r) - res := w.Result() - defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) - if err != nil { - t.Errorf("expected no errors reading response, got: %+v", err) - } + // r := httptest.NewRequest(gohttp.MethodGet, "/login", nil) + // w := httptest.NewRecorder() + // a.Login(w, r) + // res := w.Result() + // defer res.Body.Close() + // data, err := ioutil.ReadAll(res.Body) + // if err != nil { + // t.Errorf("expected no errors reading response, got: %+v", err) + // } - // redir := "http://localhost:10101/" + // // redir := "http://localhost:10101/" - // redirecturl := fmt.Sprintf("%s?client_id=%s&redirect_uri=%s&response_type=%s&scope=%s+%s&state=%s", settings.Auth.AuthorizeURL, settings.Auth.ClientId, redir, "code", settings.Auth.Scopes[0], settings.Auth.Scopes[1], settings.Auth.AuthorizeURL) + // // redirecturl := fmt.Sprintf("%s?client_id=%s&redirect_uri=%s&response_type=%s&scope=%s+%s&state=%s", settings.Auth.AuthorizeURL, settings.Auth.ClientId, redir, "code", settings.Auth.Scopes[0], settings.Auth.Scopes[1], settings.Auth.AuthorizeURL) - if res.Status != "307 Temporary Redirect" { - t.Errorf("expected status code 307 Temporary Redirect, got: %v", err) - } + // if res.Status != "307 Temporary Redirect" { + // t.Errorf("expected status code 307 Temporary Redirect, got: %v", err) + // } - if !strings.Contains(string(data), settings.Auth.AuthorizeURL) { - t.Errorf("expected url: %v, %v", settings.Auth.AuthorizeURL, string(data)) - } + // if !strings.Contains(string(data), settings.Auth.AuthorizeURL) { + // t.Errorf("expected url: %v, %v", settings.Auth.AuthorizeURL, string(data)) + // } - }) + // }) // t.Run("Logout", func(t *testing.T) { // r := httptest.NewRequest(gohttp.MethodGet, "/login", nil) // w := httptest.NewRecorder() @@ -100,24 +124,71 @@ func TestAuth(t *testing.T) { // }) - t.Run("Logout", func(t *testing.T) { - r := httptest.NewRequest(gohttp.MethodGet, "/logout", nil) - w := httptest.NewRecorder() - a.Logout(w, r) - }) - t.Run("Authenticate", func(t *testing.T) { - r := httptest.NewRequest(gohttp.MethodGet, "/authenticate", nil) - w := httptest.NewRecorder() - a.Authenticate(w, r) - }) - // t.Run("Redirect", func(t *testing.T) { - // r := httptest.NewRequest(gohttp.MethodGet, "/login", nil) + // t.Run("Logout", func(t *testing.T) { + // r := httptest.NewRequest(gohttp.MethodGet, "/logout", nil) // w := httptest.NewRecorder() - // a.Redirect(w, r) + // a.Logout(w, r) // }) - t.Run("GetUserInfo", func(t *testing.T) { - r := httptest.NewRequest(gohttp.MethodGet, "/userinfo", nil) - a.GetUserInfo(r) + // t.Run("Authenticate", func(t *testing.T) { + // r := httptest.NewRequest(gohttp.MethodGet, "/authenticate", nil) + // w := httptest.NewRecorder() + // a.Authenticate(w, r) + // }) + // // t.Run("Redirect", func(t *testing.T) { + // // r := httptest.NewRequest(gohttp.MethodGet, "/login", nil) + // // w := httptest.NewRecorder() + // // a.Redirect(w, r) + // // }) + t.Run("SetCookie", func(t *testing.T) { + w := httptest.NewRecorder() + err := a.setCookie(w, &validCV) + if err != nil { + t.Errorf("expected no errors, got: %v", err) + } + + if w.Result().Cookies()[0].Value == "" { + t.Errorf("expected some value, got: %+v", w.Result().Cookies()[0].Value) + } + if w.Result().Cookies()[0].Path != "/" { + t.Errorf("expected path to be /, got: %+v", w.Result().Cookies()[0].Path) + } + }) + t.Run("GetEmptyCookie", func(t *testing.T) { + c := a.getEmptyCookie() + if c.Value != "" { + t.Errorf("expected empty cookie, got: %+v", c.Value) + } + }) + t.Run("KeyLength", func(t *testing.T) { + _, err := NewAuth( + logger.NewStandardLogger(os.Stdout), + "http://localhost:10101/", + Scopes, + AuthorizeURL, + TokenURL, + GroupEndpointURL, + LogoutURL, + ClientId, + ClientSecret, + Key, + ShortKey, + ) + if err == nil || !strings.Contains(err.Error(), "decoding block key") { + t.Errorf("expected error decoding block key got: %v", err) + } + }) + t.Run("NewCookieValue", func(t *testing.T) { + _, err := a.newCookieValue(&tokenAT) + if err == nil || !strings.Contains(err.Error(), "jwt claims") { + t.Errorf("expected failure regarding jwt claims, got: %v", err) + } + + }) + t.Run("NewCookieValue-1", func(t *testing.T) { + _, err := a.newCookieValue(&tokenNoAT) + if err == nil || !strings.Contains(err.Error(), "access token") { + t.Errorf("expected failure regarding access token, got: %v", err) + } }) } diff --git a/http/handler.go b/http/handler.go index 895b9c368..42c46ab17 100644 --- a/http/handler.go +++ b/http/handler.go @@ -3428,7 +3428,7 @@ func (h *Handler) handleUserInfo(w http.ResponseWriter, r *http.Request) { w.Write([]byte("Auth Off")) //nolint:errcheck return } - if err := json.NewEncoder(w).Encode(h.auth.GetUserInfo(r)); err != nil { + if err := json.NewEncoder(w).Encode(h.auth.GetUserInfo(w, r)); err != nil { h.logger.Errorf("writing user info: %s", err) } } diff --git a/http/handler_internal_test.go b/http/handler_internal_test.go index 28b80ce07..e0d148c9e 100644 --- a/http/handler_internal_test.go +++ b/http/handler_internal_test.go @@ -5,7 +5,6 @@ import ( "bytes" "encoding/hex" "encoding/json" - "fmt" "io/ioutil" gohttp "net/http" "net/http/httptest" @@ -226,13 +225,17 @@ func TestAuth(t *testing.T) { hOff := Handler{} - validToken := oauth2.Token{ + token := oauth2.Token{ TokenType: "Bearer", RefreshToken: "abcdef", Expiry: time.Now().Add(time.Hour), } - // emptyToken := oauth2.Token{} + expiredToken := oauth2.Token{ + TokenType: "Bearer", + RefreshToken: "abcdef", + Expiry: time.Now(), + } grp := authn.Group{ UserID: "snowstorm", @@ -244,22 +247,46 @@ func TestAuth(t *testing.T) { UserID: "snowstorm", UserName: "J.M.W. Turner", GroupMembership: []authn.Group{grp}, - Token: &validToken, + Token: &token, + } + + emptyCV := authn.CookieValue{ + UserID: "narcissus", + UserName: "Caravaggio", + GroupMembership: []authn.Group{}, + Token: &token, + } + expiredCV := authn.CookieValue{ + UserID: "narcissus", + UserName: "Caravaggio", + GroupMembership: []authn.Group{}, + Token: &expiredToken, } secure := securecookie.New(hashKey, blockKey) validEncodedCV, _ := secure.Encode("molecula-chip", validCV) + noGroupEncodedCV, _ := secure.Encode("molecula-chip", emptyCV) + expiredEncodedCV, _ := secure.Encode("molecula-chip", expiredCV) + validCookie := &gohttp.Cookie{ Name: "molecula-chip", Value: validEncodedCV, Path: "/", Secure: true, HttpOnly: true, - Expires: validToken.Expiry, + Expires: token.Expiry, + } + noGroupCookie := &gohttp.Cookie{ + Name: "molecula-chip", + Value: noGroupEncodedCV, + Path: "/", + Secure: true, + HttpOnly: true, + Expires: token.Expiry, } expiredCookie := &gohttp.Cookie{ Name: "molecula-chip", - Value: validEncodedCV, + Value: expiredEncodedCV, Path: "/", Secure: true, HttpOnly: true, @@ -271,7 +298,7 @@ func TestAuth(t *testing.T) { Path: "/", Secure: true, HttpOnly: true, - Expires: validToken.Expiry, + Expires: token.Expiry, } unEncodedCookie := &gohttp.Cookie{ Name: "molecula-chip", @@ -279,7 +306,7 @@ func TestAuth(t *testing.T) { Path: "/", Secure: true, HttpOnly: true, - Expires: validToken.Expiry, + Expires: token.Expiry, } tests := []struct { @@ -321,27 +348,35 @@ func TestAuth(t *testing.T) { cookie: validCookie, handler: func(w gohttp.ResponseWriter, r *gohttp.Request) { h.handleCheckAuthentication(w, r) }, fn: func(w *httptest.ResponseRecorder, data []byte) { - fmt.Printf("w %+v \n\n", w) + if w.Result().StatusCode != 200 { + t.Errorf("expected http code 200, got: %+v", w.Result().StatusCode) + } }, }, { name: "Authenticate-NoGroups", path: "/auth", kind: "type1", - cookie: validCookie, + cookie: noGroupCookie, handler: func(w gohttp.ResponseWriter, r *gohttp.Request) { h.handleCheckAuthentication(w, r) }, fn: func(w *httptest.ResponseRecorder, data []byte) { - fmt.Printf("w %+v \n\n", w) + // status forbidden + if w.Result().StatusCode != 403 { + t.Errorf("expected http code 403, got: %+v", w.Result().StatusCode) + } }, }, { - name: "Authenticate-BadCookie", + name: "Authenticate-MalformedCookie", path: "/auth", kind: "type1", cookie: unEncodedCookie, handler: func(w gohttp.ResponseWriter, r *gohttp.Request) { h.handleCheckAuthentication(w, r) }, fn: func(w *httptest.ResponseRecorder, data []byte) { - fmt.Printf("w %+v \n\n", w) + // redirect to signin + if w.Result().StatusCode != 307 { + t.Errorf("expected http code 307, got: %+v", w.Result().StatusCode) + } }, }, { @@ -351,7 +386,10 @@ func TestAuth(t *testing.T) { cookie: expiredCookie, handler: func(w gohttp.ResponseWriter, r *gohttp.Request) { h.handleCheckAuthentication(w, r) }, fn: func(w *httptest.ResponseRecorder, data []byte) { - fmt.Printf("w %+v \n\n", w) + // redirect to signin + if w.Result().StatusCode != 307 { + t.Errorf("expected http code 307, got: %+v", w.Result().StatusCode) + } }, }, { @@ -361,7 +399,10 @@ func TestAuth(t *testing.T) { cookie: emptyCookie, handler: func(w gohttp.ResponseWriter, r *gohttp.Request) { h.handleCheckAuthentication(w, r) }, fn: func(w *httptest.ResponseRecorder, data []byte) { - fmt.Printf("w %+v \n\n", w) + // redirect to signin + if w.Result().StatusCode != 307 { + t.Errorf("expected http code 307, got: %+v", w.Result().StatusCode) + } }, }, { @@ -514,47 +555,24 @@ func TestAuth(t *testing.T) { test.fn(w, data) }) case "type2": - r := httptest.NewRequest(gohttp.MethodGet, test.path, nil) - w := httptest.NewRecorder() - r.Form = url.Values{} - r.Header.Set("Content-Type", "application/x-www-form-urlencoded") - r.Form.Add("code", "junk") + t.Run(test.name, func(t *testing.T) { + r := httptest.NewRequest(gohttp.MethodGet, test.path, nil) + w := httptest.NewRecorder() + r.Form = url.Values{} + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + r.Form.Add("code", "junk") - test.handler(w, r) - data, err := readResponse(w) - if err != nil { - t.Errorf("expected no errors reading response, got: %+v", err) - } + test.handler(w, r) + data, err := readResponse(w) + if err != nil { + t.Errorf("expected no errors reading response, got: %+v", err) + } - test.fn(w, data) + test.fn(w, data) + }) } } - t.Run("GetUserInfo", func(t *testing.T) { - r := httptest.NewRequest(gohttp.MethodGet, "/userinfo", nil) - w := httptest.NewRecorder() - - h.handleUserInfo(w, r) - - data, err := readResponse(w) - if err != nil { - t.Errorf("expected no errors reading response, got: %+v", err) - } - - uinfo := authn.UserInfo{} - - err = json.Unmarshal(data, &uinfo) - if err != nil { - t.Errorf("unmarshalling userinfo") - } - - if uinfo.UserID != "" && uinfo.UserName != "" { - - t.Errorf("expected http code 400, got: %+v", uinfo) - } - - }) - }