From d7b266a8d2d0ca45f1fe61e86d8ad60a73ac950e Mon Sep 17 00:00:00 2001 From: Mistah J <26472282+mistahj67@users.noreply.github.com> Date: Thu, 17 Oct 2024 15:19:40 -0700 Subject: [PATCH] refactor: add gorilla sessions in place of pure cookie storage + cleanup --- cmd/api/src/api/constant.go | 1 + cmd/api/src/api/v2/auth/auth.go | 9 +++++ cmd/api/src/api/v2/auth/oidc.go | 59 +++++++++++++++++++-------------- cmd/api/src/go.mod | 3 ++ cmd/api/src/go.sum | 8 +++++ 5 files changed, 56 insertions(+), 24 deletions(-) diff --git a/cmd/api/src/api/constant.go b/cmd/api/src/api/constant.go index 17ef89e83..4765cf716 100644 --- a/cmd/api/src/api/constant.go +++ b/cmd/api/src/api/constant.go @@ -22,6 +22,7 @@ const ( // Cookie Keys AuthTokenCookieName = "token" + AuthSessionCookieName = "session" AuthStateCookieName = "state" AuthPKCECookieName = "pkce" diff --git a/cmd/api/src/api/v2/auth/auth.go b/cmd/api/src/api/v2/auth/auth.go index cda7b1259..345587de2 100644 --- a/cmd/api/src/api/v2/auth/auth.go +++ b/cmd/api/src/api/v2/auth/auth.go @@ -26,10 +26,12 @@ import ( "strings" "time" + "github.com/antonlindstrom/pgstore" "github.com/crewjam/saml" "github.com/crewjam/saml/samlsp" "github.com/gofrs/uuid" "github.com/gorilla/mux" + "github.com/gorilla/sessions" "github.com/pkg/errors" "github.com/pquerna/otp/totp" "github.com/specterops/bloodhound/crypto" @@ -62,9 +64,15 @@ type ManagementResource struct { QueryParameterFilterParser model.QueryParameterFilterParser authorizer auth.Authorizer // Used for Permissions authenticator api.Authenticator // Used for secrets + ssoSessionStore sessions.Store // Used for Login through OIDC } func NewManagementResource(authConfig config.Configuration, db database.Database, authorizer auth.Authorizer, authenticator api.Authenticator) ManagementResource { + sessionStore, err := pgstore.NewPGStore(fmt.Sprintf("%s %s", authConfig.Database.PostgreSQLConnectionString(), "sslmode=disable"), []byte(authConfig.Crypto.JWT.SigningKey)) + if err != nil { + log.Fatalf("Failed to create postgres session store... %v", err) + } + return ManagementResource{ config: authConfig, secretDigester: authConfig.Crypto.Argon2.NewDigester(), @@ -72,6 +80,7 @@ func NewManagementResource(authConfig config.Configuration, db database.Database QueryParameterFilterParser: model.NewQueryParameterFilterParser(), authorizer: authorizer, authenticator: authenticator, + ssoSessionStore: sessionStore, } } diff --git a/cmd/api/src/api/v2/auth/oidc.go b/cmd/api/src/api/v2/auth/oidc.go index 257434e13..97d671c43 100644 --- a/cmd/api/src/api/v2/auth/oidc.go +++ b/cmd/api/src/api/v2/auth/oidc.go @@ -19,7 +19,6 @@ package auth import ( "fmt" "net/http" - "time" "github.com/coreos/go-oidc/v3/oidc" "github.com/specterops/bloodhound/headers" @@ -67,10 +66,11 @@ func (s ManagementResource) OIDCLoginHandler(response http.ResponseWriter, reque if state, err := config.GenerateRandomBase64String(77); err != nil { api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusInternalServerError, api.ErrorResponseDetailsInternalServerError, request), response) } else if provider, err := oidc.NewProvider(request.Context(), oidcProvider.Issuer); err != nil { + // This hits the /.well-known oidc configuration to generate the necessary endpoints for redirect dynamically + api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusInternalServerError, err.Error(), request), response) + } else if ssoLoginSession, err := s.ssoSessionStore.New(request, api.AuthSessionCookieName); err != nil { api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusInternalServerError, err.Error(), request), response) } else { - log.Debugf("provider generated endpoints %+v", provider.Endpoint()) - conf := &oauth2.Config{ ClientID: oidcProvider.ClientID, Endpoint: provider.Endpoint(), @@ -80,20 +80,20 @@ func (s ManagementResource) OIDCLoginHandler(response http.ResponseWriter, reque // use PKCE to protect against CSRF attacks // https://www.ietf.org/archive/id/draft-ietf-oauth-security-topics-22.html#name-countermeasures-6 verifier := oauth2.GenerateVerifier() - log.Debugf("LOGIN HANDLER - pkce verifier %s\n", verifier) - - // Store PKCE on web browser in secure cookie for retrieval in callback - api.SetSecureBrowserCookie(request, response, api.AuthPKCECookieName, verifier, time.Now().UTC().Add(time.Hour)) - // Store State on web browser in secure cookie for retrieval in callback - api.SetSecureBrowserCookie(request, response, api.AuthStateCookieName, state, time.Now().UTC().Add(time.Hour)) - - // Redirect user to consent page to ask for permission for the scopes specified above. - redirectURL := conf.AuthCodeURL(state, oauth2.AccessTypeOffline, oauth2.S256ChallengeOption(verifier)) - log.Debugf("Visit the URL for the auth dialog: %v", redirectURL) + // Store PKCE and state in gorilla session + ssoLoginSession.Values[api.AuthPKCECookieName] = verifier + ssoLoginSession.Values[api.AuthStateCookieName] = state + // Save session + if err = ssoLoginSession.Save(request, response); err != nil { + api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusInternalServerError, err.Error(), request), response) + } else { + // Redirect user to consent page to ask for permission for the scopes specified above. + redirectURL := conf.AuthCodeURL(state, oauth2.AccessTypeOffline, oauth2.S256ChallengeOption(verifier)) - response.Header().Add(headers.Location.String(), redirectURL) - response.WriteHeader(http.StatusFound) + response.Header().Add(headers.Location.String(), redirectURL) + response.WriteHeader(http.StatusFound) + } } } @@ -104,14 +104,28 @@ func (s ManagementResource) OIDCCallbackHandler(response http.ResponseWriter, re code = queryParams[api.QueryParameterCode] ) + gorillaSession, err := s.ssoSessionStore.Get(request, api.AuthSessionCookieName) + if err != nil { + api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusInternalServerError, api.ErrorResponseDetailsInternalServerError, request), response) + return + } + + // Delete session regardless of what happens below + defer func() { + gorillaSession.Options.MaxAge = -1 + if err = gorillaSession.Save(request, response); err != nil { + log.Warnf("Error deleting session: %v", err) + } + }() + if len(code) == 0 { api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, "missing code", request), response) - } else if pkceVerifier, err := request.Cookie(api.AuthPKCECookieName); err != nil { - api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, "missing pkce verifier", request), response) + } else if pkceVerifier, ok := gorillaSession.Values[api.AuthPKCECookieName].(string); !ok { + api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, "missing pkce verifier in session", request), response) } else if len(state) == 0 { - api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, "missing state", request), response) - } else if stateCookie, err := request.Cookie(api.AuthStateCookieName); err != nil || stateCookie.Value != state[0] { - api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, "bad state", request), response) + api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, "missing state in callback", request), response) + } else if stateCookie, ok := gorillaSession.Values[api.AuthStateCookieName].(string); !ok || stateCookie != state[0] { + api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, "missing or bad state", request), response) } else if provider, err := oidc.NewProvider(request.Context(), oidcProvider.Issuer); err != nil { api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusInternalServerError, err.Error(), request), response) } else { @@ -124,15 +138,13 @@ func (s ManagementResource) OIDCCallbackHandler(response http.ResponseWriter, re } ) - if token, err := oauth2Conf.Exchange(request.Context(), code[0], oauth2.VerifierOption(pkceVerifier.Value)); err != nil { + if token, err := oauth2Conf.Exchange(request.Context(), code[0], oauth2.VerifierOption(pkceVerifier)); err != nil { api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusForbidden, api.ErrorResponseDetailsForbidden, request), response) } else if rawIDToken, ok := token.Extra("id_token").(string); !ok { // Extract the ID Token from OAuth2 token api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, "missing id token", request), response) } else if idToken, err := oidcVerifier.Verify(request.Context(), rawIDToken); err != nil { api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, "invalid id token", request), response) } else { - log.Debugf("GOT A TOKEN %+v\n", token) - log.Debugf("ID TOKEN %+v\n", rawIDToken) // Extract custom claims var claims struct { Name string `json:"name"` @@ -143,7 +155,6 @@ func (s ManagementResource) OIDCCallbackHandler(response http.ResponseWriter, re if err := idToken.Claims(&claims); err != nil { api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusInternalServerError, err.Error(), request), response) } else { - log.Debugf("ID CLAIMS %+v\n", claims) s.authenticator.CreateSSOSession(request, response, claims.Email, oidcProvider) } } diff --git a/cmd/api/src/go.mod b/cmd/api/src/go.mod index c9bc24b5b..7c66d3bad 100644 --- a/cmd/api/src/go.mod +++ b/cmd/api/src/go.mod @@ -54,6 +54,7 @@ require ( require ( github.com/Masterminds/semver/v3 v3.2.1 // indirect + github.com/antonlindstrom/pgstore v0.0.0-20220421113606-e3a6e3fed12a // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/boombuler/barcode v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.2.0 // indirect @@ -64,6 +65,8 @@ require ( github.com/go-pkgz/expirable-cache v1.0.0 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/google/go-cmp v0.6.0 // indirect + github.com/gorilla/securecookie v1.1.1 // indirect + github.com/gorilla/sessions v1.2.1 // indirect github.com/jackc/chunkreader/v2 v2.0.1 // indirect github.com/jackc/pgconn v1.14.3 // indirect github.com/jackc/pgio v1.0.0 // indirect diff --git a/cmd/api/src/go.sum b/cmd/api/src/go.sum index a032431ab..0455f3e6b 100644 --- a/cmd/api/src/go.sum +++ b/cmd/api/src/go.sum @@ -2,6 +2,8 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03 github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/Masterminds/semver/v3 v3.2.1 h1:RN9w6+7QoMeJVGyfmbcgs28Br8cvmnucEXnY0rYXWg0= github.com/Masterminds/semver/v3 v3.2.1/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ= +github.com/antonlindstrom/pgstore v0.0.0-20220421113606-e3a6e3fed12a h1:dIdcLbck6W67B5JFMewU5Dba1yKZA3MsT67i4No/zh0= +github.com/antonlindstrom/pgstore v0.0.0-20220421113606-e3a6e3fed12a/go.mod h1:Sdr/tmSOLEnncCuXS5TwZRxuk7deH1WXVY8cve3eVBM= github.com/beevik/etree v1.1.0/go.mod h1:r8Aw8JqVegEf0w2fDnATrX9VpkMcyFeM0FhwO62wh+A= github.com/beevik/etree v1.2.0 h1:l7WETslUG/T+xOPs47dtd6jov2Ii/8/OjCldk5fYfQw= github.com/beevik/etree v1.2.0/go.mod h1:aiPf89g/1k3AShMVAzriilpcE4R/Vuor90y83zVZWFc= @@ -54,6 +56,7 @@ github.com/gofrs/uuid v4.4.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRx github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= +github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= @@ -68,6 +71,10 @@ github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/gorilla/schema v1.4.1 h1:jUg5hUjCSDZpNGLuXQOgIWGdlgrIdYvgQ0wZtdK1M3E= github.com/gorilla/schema v1.4.1/go.mod h1:Dg5SSm5PV60mhF2NFaTV1xuYYj8tV8NOPRo4FggUMnM= +github.com/gorilla/securecookie v1.1.1 h1:miw7JPhV+b/lAHSXz4qd/nN9jRiAFV5FwjeKyCS8BvQ= +github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4= +github.com/gorilla/sessions v1.2.1 h1:DHd3rPN5lE3Ts3D8rKkQ8x/0kqfeNmBAaiSi+o7FsgI= +github.com/gorilla/sessions v1.2.1/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM= github.com/jackc/chunkreader v1.0.0/go.mod h1:RT6O25fNZIuasFJRyZ4R/Y2BbhasbmZXF9QQ7T3kePo= github.com/jackc/chunkreader/v2 v2.0.0/go.mod h1:odVSm741yZoC3dpHEUXIqA9tQRhFrgOHwnPIn9lDKlk= github.com/jackc/chunkreader/v2 v2.0.1 h1:i+RDz65UE+mmpjTfyz0MoVTnzeYxroil2G82ki7MGG8= @@ -217,6 +224,7 @@ github.com/stretchr/testify v1.7.4/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/teambition/rrule-go v1.8.2 h1:lIjpjvWTj9fFUZCmuoVDrKVOtdiyzbzc93qTmRVe/J8= +github.com/teambition/rrule-go v1.8.2/go.mod h1:Ieq5AbrKGciP1V//Wq8ktsTXwSwJHDD5mD/wLBGl3p4= github.com/unrolled/secure v1.13.0 h1:sdr3Phw2+f8Px8HE5sd1EHdj1aV3yUwed/uZXChLFsk= github.com/unrolled/secure v1.13.0/go.mod h1:BmF5hyM6tXczk3MpQkFf1hpKSRqCyhqcbiQtiAF7+40= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=