From 23a1b4c536f0cba171febab300d82e72bba4912c Mon Sep 17 00:00:00 2001 From: Samir Patel <48686912+54mir@users.noreply.github.com> Date: Tue, 4 Jan 2022 16:02:25 -0600 Subject: [PATCH] revisions and docs --- authn/authenticate.go | 31 +++++++++++++++++++++-------- authn/authenticate_internal_test.go | 8 +++++--- install/featurebase.conf | 1 + 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/authn/authenticate.go b/authn/authenticate.go index 76c9f8d5d..6a5221600 100644 --- a/authn/authenticate.go +++ b/authn/authenticate.go @@ -17,6 +17,7 @@ import ( "golang.org/x/oauth2" ) +// Auth holds state, configuration, and utilities needed for authentication. type Auth struct { logger logger.Logger cookieName string @@ -26,10 +27,11 @@ type Auth struct { secure *securecookie.SecureCookie groupEndpoint string logoutEndpoint string - fbURL string + fbURL string // fbURL is the domain FB is hosted on, used for post logout redirection oAuthConfig *oauth2.Config } +// NewAuth is a constructor that returns a new auth object func NewAuth(logger logger.Logger, url string, scopes []string, authUrl, tokenUrl, groupEndpoint, logout, clientID, clientSecret, hashKey, blockKey string) (*Auth, error) { auth := &Auth{ logger: logger, @@ -85,6 +87,8 @@ type UserInfo struct { UserName string `json:"username"` } +// Authenticate reads and validates a cookie, redirects if invalid or missing, otherwise returns +// the group membership information stored in the cookie. func (a *Auth) Authenticate(w http.ResponseWriter, r *http.Request) ([]Group, error) { cookie, err := a.readCookie(w, r) if err != nil { @@ -108,18 +112,20 @@ func (a *Auth) Authenticate(w http.ResponseWriter, r *http.Request) ([]Group, er } +// Login redirects user to the IdP authorize endpoint for auth code func (a *Auth) Login(w http.ResponseWriter, r *http.Request) { authUrl := a.oAuthConfig.AuthCodeURL(a.oAuthConfig.Endpoint.AuthURL) http.Redirect(w, r, authUrl, http.StatusTemporaryRedirect) } +// Logout clears out user cookie and redirects user to IdP's logout endpoint func (a *Auth) Logout(w http.ResponseWriter, r *http.Request) { http.SetCookie(w, a.getEmptyCookie()) redirect := fmt.Sprintf("%s?post_logout_redirect_uri=%s/", a.logoutEndpoint, a.fbURL) http.Redirect(w, r, redirect, http.StatusTemporaryRedirect) } -// Gets user information from IdP and sets a secure cookie +// Redirect gets user information from IdP and sets a secure cookie func (a *Auth) Redirect(w http.ResponseWriter, r *http.Request) { code := r.FormValue("code") token, err := a.getToken(r, code) @@ -140,6 +146,7 @@ func (a *Auth) Redirect(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "/", http.StatusTemporaryRedirect) } +// GetUserInfo reads user's cookie and returns their username and userId func (a *Auth) GetUserInfo(w http.ResponseWriter, r *http.Request) *UserInfo { var resp UserInfo cookie, err := a.readCookie(w, r) @@ -154,6 +161,7 @@ func (a *Auth) GetUserInfo(w http.ResponseWriter, r *http.Request) *UserInfo { } +// getToken exhanges authorization code for an oAuth2 token func (a *Auth) getToken(r *http.Request, code string) (*oauth2.Token, error) { token, err := a.oAuthConfig.Exchange(r.Context(), code) if err != nil { @@ -162,6 +170,7 @@ func (a *Auth) getToken(r *http.Request, code string) (*oauth2.Token, error) { return token, nil } +// newCookieValue parses a jwt `token` and returns relevant information in a cookie value struct func (a *Auth) newCookieValue(token *oauth2.Token) (*CookieValue, error) { if token == nil { return nil, errors.New("baking cookie due to nil token") @@ -169,9 +178,14 @@ func (a *Auth) newCookieValue(token *oauth2.Token) (*CookieValue, error) { if token.AccessToken == "" { return nil, errors.New("no access token provided") } - accessParsed, err := jwt.Parse(token.AccessToken, nil) - if accessParsed == nil || accessParsed.Claims == nil { - return nil, errors.Wrap(err, "parsing jwt claims from access tokens") + + // We are using ParseUnverified here because we're using the OAuth2.0 authZ code flow + // which assumes that the IdP gives good responses. This means that if the IdP is + // insecure, then we are too. But that's the way OAuth works, unfortunately. + // Also, we assume the jwt is not tampered with bc we communicate with the IdP over HTTPS only. + accessParsed, _, err := new(jwt.Parser).ParseUnverified(token.AccessToken, jwt.MapClaims{}) + if accessParsed == nil || accessParsed.Claims == nil || err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("%v parsing jwt claims from access tokens", accessParsed)) } claims := accessParsed.Claims.(jwt.MapClaims) @@ -189,6 +203,7 @@ func (a *Auth) newCookieValue(token *oauth2.Token) (*CookieValue, error) { }, nil } +// getGroupMembership uses a oauth2 token to retrieve group membership information from IdP func (a *Auth) getGroupMembership(token *oauth2.Token) (Groups, error) { var groups Groups req, err := http.NewRequest("GET", a.groupEndpoint, nil) @@ -197,8 +212,7 @@ func (a *Auth) getGroupMembership(token *oauth2.Token) (Groups, error) { } req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token.AccessToken)) - client := http.DefaultClient - response, err := client.Do(req) + response, err := http.DefaultClient.Do(req) if err != nil { return groups, errors.Wrap(err, "getting group membership info") } @@ -216,6 +230,7 @@ func (a *Auth) getGroupMembership(token *oauth2.Token) (Groups, error) { return groups, nil } +// readCookie decodes an encrypted and signed cookie and returns the contained info func (a *Auth) readCookie(w http.ResponseWriter, r *http.Request) (*CookieValue, error) { cookie, err := r.Cookie(a.cookieName) if err != nil { @@ -263,7 +278,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 { - return errors.Wrap(err, "setting cookie") + return errors.Wrap(err, "creating cookie value from token") } a.setCookie(w, cv) diff --git a/authn/authenticate_internal_test.go b/authn/authenticate_internal_test.go index 07cce1396..860283433 100644 --- a/authn/authenticate_internal_test.go +++ b/authn/authenticate_internal_test.go @@ -71,11 +71,13 @@ func TestAuth(t *testing.T) { } if w.Result().Cookies()[0].Value == "" { - t.Fatalf("expected some value, got: %+v", w.Result().Cookies()[0].Value) + t.Errorf("expected something, got empty string") } - if w.Result().Cookies()[0].Path != "/" { - t.Fatalf("expected path to be /, got: %+v", w.Result().Cookies()[0].Path) + + if got, want := w.Result().Cookies()[0].Path, "/"; got != want { + t.Fatalf("path=%s, want %s", got, want) } + }) t.Run("GetEmptyCookie", func(t *testing.T) { c := a.getEmptyCookie() diff --git a/install/featurebase.conf b/install/featurebase.conf index 033db191d..94903194c 100644 --- a/install/featurebase.conf +++ b/install/featurebase.conf @@ -386,3 +386,4 @@ log-path = "/var/log/molecula/featurebase.log" # hash-key = "" # block-key = "" # permissions = "" +# query-log-path = ""