From b7e381dad550e1cf6cbee24864d4a572f95c9de7 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Fri, 23 May 2025 08:01:55 -0700 Subject: [PATCH 01/13] feat: hooks round 5 (Option 2) - add before-user-created hook This commit explores one possible implementation of this hook by: - Adding a `triggerBeforeUserCreated` method to the `*API` object in `internal/api/hooks.go` - Adding a `triggerBeforeUserCreatedExternal` method to the `*API` object in `internal/api/hooks.go` - Updating callers of `signupNewUser` to first call `triggerBeforeUserCreated` in: - internal/api/anonymous.go - internal/api/external.go - internal/api/invite.go - internal/api/mail.go - internal/api/signup.go - Updating callers of `signupNewUser` to first call `triggerBeforeUserCreatedExternal` in: - internal/api/external.go - internal/api/samlacs.go - internal/api/token_oidc.go - internal/api/web3.go - Add end to end tests in `internal/api/e2e_test.go` --- internal/api/anonymous.go | 3 + internal/api/e2e_test.go | 258 +++++++++++++++++++++++++++++ internal/api/external.go | 14 +- internal/api/hooks.go | 99 +++++++++++ internal/api/invite.go | 59 ++++--- internal/api/mail.go | 68 +++++--- internal/api/samlacs.go | 8 +- internal/api/signup.go | 3 + internal/api/token_oidc.go | 4 + internal/api/web3.go | 11 +- internal/e2e/e2eapi/e2eapi_test.go | 1 - 11 files changed, 464 insertions(+), 64 deletions(-) create mode 100644 internal/api/e2e_test.go create mode 100644 internal/api/hooks.go diff --git a/internal/api/anonymous.go b/internal/api/anonymous.go index 33aa04aee..69679b9bc 100644 --- a/internal/api/anonymous.go +++ b/internal/api/anonymous.go @@ -30,6 +30,9 @@ func (a *API) SignupAnonymously(w http.ResponseWriter, r *http.Request) error { if err != nil { return err } + if err := a.triggerBeforeUserCreated(r, db, newUser); err != nil { + return err + } var grantParams models.GrantParams grantParams.FillGrantParams(r) diff --git a/internal/api/e2e_test.go b/internal/api/e2e_test.go new file mode 100644 index 000000000..5880bd915 --- /dev/null +++ b/internal/api/e2e_test.go @@ -0,0 +1,258 @@ +package api_test + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "slices" + "testing" + "time" + + "github.com/gofrs/uuid" + jwt "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/require" + "github.com/supabase/auth/internal/api" + "github.com/supabase/auth/internal/conf" + "github.com/supabase/auth/internal/e2e" + "github.com/supabase/auth/internal/e2e/e2eapi" + "github.com/supabase/auth/internal/e2e/e2ehooks" + "github.com/supabase/auth/internal/hooks/v0hooks" + "github.com/supabase/auth/internal/models" +) + +func TestE2EHooks(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + + globalCfg := e2e.Must(e2e.Config()) + inst, err := e2ehooks.New(globalCfg) + require.NoError(t, err) + defer inst.Close() + + apiSrv := inst.APIServer + hookRec := inst.HookRecorder + + var currentUser *models.User + + // Basic tests for Before/After User Created hooks + t.Run("UserHooks", func(t *testing.T) { + + // Signup a user + var signupUser *models.User + email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + t.Run("Signup", func(t *testing.T) { + req := &api.SignupParams{ + Email: email, + Password: "password", + } + res := new(models.User) + err := e2eapi.Do(ctx, http.MethodPost, apiSrv.URL+"/signup", req, res) + require.NoError(t, err) + + signupUser = res + + require.Equal(t, email, signupUser.Email.String()) + }) + + t.Run("BeforeUserCreated", func(t *testing.T) { + calls := hookRec.BeforeUserCreated.GetCalls() + require.Equal(t, 1, len(calls)) + call := calls[0] + + hookReq := &v0hooks.BeforeUserCreatedInput{} + err := call.Unmarshal(hookReq) + require.NoError(t, err) + + u := hookReq.User + require.Equal(t, signupUser.ID, u.ID) + require.Equal(t, signupUser.Aud, u.Aud) + require.Equal(t, signupUser.Email, u.Email) + require.Equal(t, signupUser.AppMetaData, u.AppMetaData) + + require.True(t, u.CreatedAt.IsZero()) + require.True(t, u.UpdatedAt.IsZero()) + + err = signupUser.Confirm(inst.Conn) + require.NoError(t, err) + + latest, err := models.FindUserByID(inst.Conn, signupUser.ID) + require.NoError(t, err) + + // Assign currentUser for next tests. + currentUser = latest + require.NotNil(t, currentUser) + }) + }) + + // Basic tests for CustomizeAccessToken + t.Run("CustomizeAccessToken", func(t *testing.T) { + require.NotNil(t, currentUser) + + type M = map[string]any + + copyMap := func(t *testing.T, m M) (out M) { + b, err := json.Marshal(m) + require.NoError(t, err) + err = json.Unmarshal(b, &out) + require.NoError(t, err) + return out + } + checkClaims := func(t *testing.T, in, out M, exclude ...string) { + if aud, ok := in["aud"].([]any); ok && len(aud) > 0 { + require.Equal(t, aud[0].(string), out["aud"]) + } + if aud, ok := in["aud"].(string); ok { + require.Equal(t, aud, out["aud"]) + } + + for _, k := range []string{ + "iss", + "sub", + "exp", + "iat", + "aal", + "role", + "amr", + "session_id", + "is_anonymous", + "app_metadata", + "user_metadata", + "phone", + "email", + } { + if !slices.Contains(exclude, k) { + require.Equal(t, in[k], out[k]) + } + } + } + + cases := []struct { + desc string + from func(claimsIn M) (claimsOut M) + errStr string + check func( + t *testing.T, + claimsIn, claimsOut M, + ) + }{ + { + desc: `empty claims`, + from: func(in M) M { return M{"claims": M{}} }, + errStr: "500: error generating jwt token", + }, + + { + desc: `add app_metadata claims`, + from: func(in M) M { + out := copyMap(t, in) + out["claims"].(M)["app_metadata"].(M)["bool_true"] = true + out["claims"].(M)["app_metadata"].(M)["string_hello"] = "hello" + return out + }, + check: func( + t *testing.T, + in, out M, + ) { + checkClaims(t, in, out, "app_metadata") + + for k := range in { + if k == "app_metadata" { + require.Equal(t, + out["app_metadata"].(M)["bool_true"], + true, + ) + require.Equal(t, + out["app_metadata"].(M)["string_hello"], + "hello", + ) + continue + } + } + }, + }, + } + + for _, tc := range cases { + t.Run(string(tc.desc), func(t *testing.T) { + var claimsIn, claimsOut M + hr := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("content-type", "application/json") + w.WriteHeader(http.StatusOK) + + err := json.NewDecoder(r.Body).Decode(&claimsIn) + require.NoError(t, err) + + claimsOut = tc.from(copyMap(t, claimsIn)) + err = json.NewEncoder(w).Encode(claimsOut) + require.NoError(t, err) + }) + + hookRec.CustomizeAccessToken.ClearCalls() + hookRec.CustomizeAccessToken.SetHandler(hr) + req := &api.PasswordGrantParams{ + Email: string(currentUser.Email), + Password: "password", + } + + res := new(api.AccessTokenResponse) + err := e2eapi.Do(ctx, http.MethodPost, apiSrv.URL+"/token?grant_type=password", req, res) + + // always verify the hook request before checking response + { + calls := hookRec.CustomizeAccessToken.GetCalls() + require.Equal(t, 1, len(calls)) + call := calls[0] + + hookReq := &v0hooks.CustomAccessTokenInput{} + err := call.Unmarshal(hookReq) + require.NoError(t, err) + require.Equal(t, currentUser.ID, hookReq.UserID) + require.Equal(t, currentUser.ID.String(), hookReq.Claims.Subject) + } + + // check if we expected an error + if tc.errStr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errStr) + return + } + require.True(t, len(res.Token) > 0) + + // parse the token we got back + p := jwt.NewParser(jwt.WithValidMethods(globalCfg.JWT.ValidMethods)) + token, err := p.ParseWithClaims( + res.Token, + &api.AccessTokenClaims{}, + func(token *jwt.Token, + ) (any, error) { + if kid, ok := token.Header["kid"]; ok { + if kidStr, ok := kid.(string); ok { + return conf.FindPublicKeyByKid(kidStr, &globalCfg.JWT) + } + } + if alg, ok := token.Header["alg"]; ok { + if alg == jwt.SigningMethodHS256.Name { + // preserve backward compatibility for cases where the kid is not set + return []byte(globalCfg.JWT.Secret), nil + } + } + return nil, fmt.Errorf("missing kid") + }) + require.NoError(t, err) + + tokenClaims := M{} + { + b, err := json.Marshal(token.Claims) + require.NoError(t, err) + err = json.Unmarshal(b, &tokenClaims) + require.NoError(t, err) + } + + if tc.check != nil { + tc.check(t, claimsIn["claims"].(M), tokenClaims) + } + }) + } + }) +} diff --git a/internal/api/external.go b/internal/api/external.go index dfaa86a03..7e0ce527e 100644 --- a/internal/api/external.go +++ b/internal/api/external.go @@ -203,15 +203,24 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re } + targetUser := getTargetUser(ctx) + inviteToken := getInviteToken(ctx) + if targetUser == nil && inviteToken == "" { + if err := a.triggerBeforeUserCreatedExternal( + r, db, userData, providerType); err != nil { + return err + } + } + var user *models.User var token *AccessTokenResponse err = db.Transaction(func(tx *storage.Connection) error { var terr error - if targetUser := getTargetUser(ctx); targetUser != nil { + if targetUser != nil { if user, terr = a.linkIdentityToUser(r, ctx, tx, userData, providerType); terr != nil { return terr } - } else if inviteToken := getInviteToken(ctx); inviteToken != "" { + } else if inviteToken != "" { if user, terr = a.processInvite(r, tx, userData, inviteToken, providerType); terr != nil { return terr } @@ -334,6 +343,7 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http. return nil, terr } user.Identities = append(user.Identities, *identity) + case models.AccountExists: user = decision.User identity = decision.Identities[0] diff --git a/internal/api/hooks.go b/internal/api/hooks.go new file mode 100644 index 000000000..74a62a0bc --- /dev/null +++ b/internal/api/hooks.go @@ -0,0 +1,99 @@ +package api + +import ( + "net/http" + "strings" + + "github.com/fatih/structs" + "github.com/supabase/auth/internal/api/apierrors" + "github.com/supabase/auth/internal/api/provider" + "github.com/supabase/auth/internal/hooks/v0hooks" + "github.com/supabase/auth/internal/models" + "github.com/supabase/auth/internal/storage" +) + +func (a *API) triggerBeforeUserCreated( + r *http.Request, + conn *storage.Connection, + user *models.User, +) error { + if !a.hooksMgr.Enabled(v0hooks.BeforeUserCreated) { + return nil + } + if conn.TX != nil { + // TODO(cstockton): remove this + panic("unable to trigger hooks during transaction") + } + + req := v0hooks.NewBeforeUserCreatedInput(r, user) + res := new(v0hooks.BeforeUserCreatedOutput) + return a.hooksMgr.InvokeHook(conn, r, req, res) +} + +func (a *API) triggerBeforeUserCreatedExternal( + r *http.Request, + conn *storage.Connection, + userData *provider.UserProvidedData, + providerType string, +) error { + if !a.hooksMgr.Enabled(v0hooks.BeforeUserCreated) { + return nil + } + if conn.TX != nil { + // TODO(cstockton): remove this + panic("unable to trigger hooks during transaction") + } + + ctx := r.Context() + aud := a.requestAud(ctx, r) + config := a.config + + var identityData map[string]interface{} + if userData.Metadata != nil { + identityData = structs.Map(userData.Metadata) + } + + var ( + err error + decision models.AccountLinkingResult + ) + err = a.db.Transaction(func(tx *storage.Connection) error { + decision, err = models.DetermineAccountLinking( + tx, config, userData.Emails, aud, + providerType, userData.Metadata.Subject) + if err != nil { + return err + } + return nil + }) + if err != nil { + return err + } + + if decision.Decision != models.CreateAccount { + return nil + } + if config.DisableSignup { + return apierrors.NewUnprocessableEntityError( + apierrors.ErrorCodeSignupDisabled, + "Signups not allowed for this instance") + } + + params := &SignupParams{ + Provider: providerType, + Email: decision.CandidateEmail.Email, + Aud: aud, + Data: identityData, + } + + isSSOUser := false + if strings.HasPrefix(decision.LinkingDomain, "sso:") { + isSSOUser = true + } + + user, err := params.ToUserModel(isSSOUser) + if err != nil { + return err + } + return a.triggerBeforeUserCreated(r, conn, user) +} diff --git a/internal/api/invite.go b/internal/api/invite.go index 2c3f8cd7f..797931004 100644 --- a/internal/api/invite.go +++ b/internal/api/invite.go @@ -37,41 +37,38 @@ func (a *API) Invite(w http.ResponseWriter, r *http.Request) error { if err != nil && !models.IsNotFoundError(err) { return apierrors.NewInternalServerError("Database error finding user").WithInternalError(err) } + if user != nil && user.IsConfirmed() { + return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailExists, DuplicateEmailMsg) + } - err = db.Transaction(func(tx *storage.Connection) error { - if user != nil { - if user.IsConfirmed() { - return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailExists, DuplicateEmailMsg) - } - } else { - signupParams := SignupParams{ - Email: params.Email, - Data: params.Data, - Aud: aud, - Provider: "email", - } + signupParams := SignupParams{ + Email: params.Email, + Data: params.Data, + Aud: aud, + Provider: "email", + } - // because params above sets no password, this method - // is not computationally hard so it can be used within - // a database transaction - user, err = signupParams.ToUserModel(false /* <- isSSOUser */) - if err != nil { - return err - } + user, err = signupParams.ToUserModel(false /* <- isSSOUser */) + if err != nil { + return err + } + if err := a.triggerBeforeUserCreated(r, db, user); err != nil { + return err + } - user, err = a.signupNewUser(tx, user) - if err != nil { - return err - } - identity, err := a.createNewIdentity(tx, user, "email", structs.Map(provider.Claims{ - Subject: user.ID.String(), - Email: user.GetEmail(), - })) - if err != nil { - return err - } - user.Identities = []models.Identity{*identity} + err = db.Transaction(func(tx *storage.Connection) error { + user, err = a.signupNewUser(tx, user) + if err != nil { + return err + } + identity, err := a.createNewIdentity(tx, user, "email", structs.Map(provider.Claims{ + Subject: user.ID.String(), + Email: user.GetEmail(), + })) + if err != nil { + return err } + user.Identities = []models.Identity{*identity} if terr := models.NewAuditLogEntry(r, tx, adminUser, models.UserInvitedAction, "", map[string]interface{}{ "user_id": user.ID, diff --git a/internal/api/mail.go b/internal/api/mail.go index f90d9a74c..d2e6c3430 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -92,8 +92,16 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { hashedToken := crypto.GenerateTokenHash(params.Email, otp) - var signupUser *models.User - if params.Type == mail.SignupVerification && user == nil { + var ( + triggerSignupHook bool + signupVerificationUser *models.User + inviteVerificationUser *models.User + ) + switch params.Type { + case mail.SignupVerification: + if user != nil { + break + } signupParams := &SignupParams{ Email: params.Email, Password: params.Password, @@ -101,15 +109,41 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { Provider: "email", Aud: aud, } - if err := a.validateSignupParams(ctx, signupParams); err != nil { return err } - signupUser, err = signupParams.ToUserModel(false /* <- isSSOUser */) + signupVerificationUser, err = signupParams.ToUserModel(false) + if err != nil { + return err + } + triggerSignupHook = true + + case mail.InviteVerification: + if user == nil { + break + } + if user.IsConfirmed() { + return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailExists, DuplicateEmailMsg) + } + signupParams := &SignupParams{ + Email: params.Email, + Data: params.Data, + Provider: "email", + Aud: aud, + } + + inviteVerificationUser, err = signupParams.ToUserModel(false) if err != nil { return err } + triggerSignupHook = true + + } + if triggerSignupHook { + if err := a.triggerBeforeUserCreated(r, db, user); err != nil { + return err + } } err = db.Transaction(func(tx *storage.Connection) error { @@ -133,27 +167,8 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { return terr } case mail.InviteVerification: - if user != nil { - if user.IsConfirmed() { - return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailExists, DuplicateEmailMsg) - } - } else { - signupParams := &SignupParams{ - Email: params.Email, - Data: params.Data, - Provider: "email", - Aud: aud, - } - - // because params above sets no password, this - // method is not computationally hard so it can - // be used within a database transaction - user, terr = signupParams.ToUserModel(false /* <- isSSOUser */) - if terr != nil { - return terr - } - - user, terr = a.signupNewUser(tx, user) + if user == nil { + user, terr = a.signupNewUser(tx, inviteVerificationUser) if terr != nil { return terr } @@ -166,6 +181,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { } user.Identities = []models.Identity{*identity} } + if terr = models.NewAuditLogEntry(r, tx, adminUser, models.UserInvitedAction, "", map[string]interface{}{ "user_id": user.ID, "user_email": user.Email, @@ -198,7 +214,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { // password here to generate a new user, use // signupUser which is a model generated from // SignupParams above - user, terr = a.signupNewUser(tx, signupUser) + user, terr = a.signupNewUser(tx, signupVerificationUser) if terr != nil { return terr } diff --git a/internal/api/samlacs.go b/internal/api/samlacs.go index 3f64fc235..fb56e4cb5 100644 --- a/internal/api/samlacs.go +++ b/internal/api/samlacs.go @@ -281,12 +281,18 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error { } } + providerType := "sso:" + ssoProvider.ID.String() + if err := a.triggerBeforeUserCreatedExternal( + r, db, &userProvidedData, providerType); err != nil { + return err + } + if err := db.Transaction(func(tx *storage.Connection) error { var terr error var user *models.User // accounts potentially created via SAML can contain non-unique email addresses in the auth.users table - if user, terr = a.createAccountFromExternalIdentity(tx, r, &userProvidedData, "sso:"+ssoProvider.ID.String()); terr != nil { + if user, terr = a.createAccountFromExternalIdentity(tx, r, &userProvidedData, providerType); terr != nil { return terr } if flowState != nil { diff --git a/internal/api/signup.go b/internal/api/signup.go index 09ac43524..7901f0caf 100644 --- a/internal/api/signup.go +++ b/internal/api/signup.go @@ -183,6 +183,9 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error { if err != nil { return err } + if err := a.triggerBeforeUserCreated(r, db, signupUser); err != nil { + return err + } } err = db.Transaction(func(tx *storage.Connection) error { diff --git a/internal/api/token_oidc.go b/internal/api/token_oidc.go index fbc4243ba..8a04bd78f 100644 --- a/internal/api/token_oidc.go +++ b/internal/api/token_oidc.go @@ -224,6 +224,10 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R grantParams.FillGrantParams(r) + if err := a.triggerBeforeUserCreatedExternal(r, db, userData, providerType); err != nil { + return err + } + if err := db.Transaction(func(tx *storage.Connection) error { var user *models.User var terr error diff --git a/internal/api/web3.go b/internal/api/web3.go index c9beeea1e..19d1d6841 100644 --- a/internal/api/web3.go +++ b/internal/api/web3.go @@ -102,8 +102,9 @@ func (a *API) web3GrantSolana(ctx context.Context, w http.ResponseWriter, r *htt return apierrors.NewOAuthError("invalid_grant", "Solana message was issued too far in the future") } + const providerType = "web3" providerId := strings.Join([]string{ - "web3", + providerType, params.Chain, parsedMessage.Address, }, ":") @@ -126,14 +127,18 @@ func (a *API) web3GrantSolana(ctx context.Context, w http.ResponseWriter, r *htt var grantParams models.GrantParams grantParams.FillGrantParams(r) + if err := a.triggerBeforeUserCreatedExternal(r, db, &userData, providerType); err != nil { + return err + } + err = db.Transaction(func(tx *storage.Connection) error { - user, terr := a.createAccountFromExternalIdentity(tx, r, &userData, "web3") + user, terr := a.createAccountFromExternalIdentity(tx, r, &userData, providerType) if terr != nil { return terr } if terr := models.NewAuditLogEntry(r, tx, user, models.LoginAction, "", map[string]interface{}{ - "provider": "web3", + "provider": providerType, "chain": params.Chain, "network": parsedMessage.ChainID, "address": parsedMessage.Address, diff --git a/internal/e2e/e2eapi/e2eapi_test.go b/internal/e2e/e2eapi/e2eapi_test.go index 391de9514..8887deca5 100644 --- a/internal/e2e/e2eapi/e2eapi_test.go +++ b/internal/e2e/e2eapi/e2eapi_test.go @@ -172,7 +172,6 @@ func TestDo(t *testing.T) { err := Do(ctx, http.MethodPost, ts.URL, nil, nil) require.Error(t, err) require.Equal(t, sentinel, err) - }) } }) From 6b1b4a98d89bfd686418a199ba19b6c09792e511 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Fri, 23 May 2025 14:19:15 -0700 Subject: [PATCH 02/13] fix: add unit tests and fix bug within mail.go Adding the e2e unit tests for the admin routes revealed a bug in the handling of invite type within adminGenerateLink. --- internal/api/e2e_test.go | 169 ++++++++++++++++++++++++----- internal/api/mail.go | 46 ++++---- internal/e2e/e2eapi/e2eapi.go | 65 ++++++++++- internal/e2e/e2eapi/e2eapi_test.go | 59 +++++++++- internal/e2e/e2ehooks/e2ehooks.go | 22 +++- 5 files changed, 302 insertions(+), 59 deletions(-) diff --git a/internal/api/e2e_test.go b/internal/api/e2e_test.go index 5880bd915..13b9765c8 100644 --- a/internal/api/e2e_test.go +++ b/internal/api/e2e_test.go @@ -1,6 +1,7 @@ package api_test import ( + "bytes" "context" "encoding/json" "fmt" @@ -14,6 +15,7 @@ import ( "github.com/stretchr/testify/require" "github.com/supabase/auth/internal/api" "github.com/supabase/auth/internal/conf" + "github.com/supabase/auth/internal/dump" "github.com/supabase/auth/internal/e2e" "github.com/supabase/auth/internal/e2e/e2eapi" "github.com/supabase/auth/internal/e2e/e2ehooks" @@ -26,6 +28,8 @@ func TestE2EHooks(t *testing.T) { defer cancel() globalCfg := e2e.Must(e2e.Config()) + globalCfg.External.AnonymousUsers.Enabled = true + inst, err := e2ehooks.New(globalCfg) require.NoError(t, err) defer inst.Close() @@ -35,26 +39,7 @@ func TestE2EHooks(t *testing.T) { var currentUser *models.User - // Basic tests for Before/After User Created hooks - t.Run("UserHooks", func(t *testing.T) { - - // Signup a user - var signupUser *models.User - email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" - t.Run("Signup", func(t *testing.T) { - req := &api.SignupParams{ - Email: email, - Password: "password", - } - res := new(models.User) - err := e2eapi.Do(ctx, http.MethodPost, apiSrv.URL+"/signup", req, res) - require.NoError(t, err) - - signupUser = res - - require.Equal(t, email, signupUser.Email.String()) - }) - + runBeforeUserCreated := func(t *testing.T, expUser *models.User) { t.Run("BeforeUserCreated", func(t *testing.T) { calls := hookRec.BeforeUserCreated.GetCalls() require.Equal(t, 1, len(calls)) @@ -63,26 +48,158 @@ func TestE2EHooks(t *testing.T) { hookReq := &v0hooks.BeforeUserCreatedInput{} err := call.Unmarshal(hookReq) require.NoError(t, err) + require.Equal(t, v0hooks.BeforeUserCreated, hookReq.Metadata.Name) u := hookReq.User - require.Equal(t, signupUser.ID, u.ID) - require.Equal(t, signupUser.Aud, u.Aud) - require.Equal(t, signupUser.Email, u.Email) - require.Equal(t, signupUser.AppMetaData, u.AppMetaData) + require.Equal(t, expUser.ID, u.ID) + require.Equal(t, expUser.Aud, u.Aud) + require.Equal(t, expUser.Email, u.Email) + require.Equal(t, expUser.AppMetaData, u.AppMetaData) require.True(t, u.CreatedAt.IsZero()) require.True(t, u.UpdatedAt.IsZero()) - err = signupUser.Confirm(inst.Conn) + err = expUser.Confirm(inst.Conn) require.NoError(t, err) - latest, err := models.FindUserByID(inst.Conn, signupUser.ID) + latest, err := models.FindUserByID(inst.Conn, expUser.ID) require.NoError(t, err) // Assign currentUser for next tests. currentUser = latest require.NotNil(t, currentUser) }) + } + + // Basic tests for user hooks + t.Run("UserHooks", func(t *testing.T) { + + t.Run("Signup", func(t *testing.T) { + defer hookRec.BeforeUserCreated.ClearCalls() + + email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + req := &api.SignupParams{ + Email: email, + Password: "password", + } + res := new(models.User) + err := e2eapi.Do(ctx, http.MethodPost, apiSrv.URL+"/signup", req, res) + require.NoError(t, err) + require.Equal(t, email, res.Email.String()) + + runBeforeUserCreated(t, res) + }) + + t.Run("SignupAnonymously", func(t *testing.T) { + defer hookRec.BeforeUserCreated.ClearCalls() + + req := &api.SignupParams{} + res := new(api.AccessTokenResponse) + err := e2eapi.Do(ctx, http.MethodPost, apiSrv.URL+"/signup", req, res) + require.NoError(t, err) + + runBeforeUserCreated(t, res.User) + }) + + t.Run("ExternalCallback", func(t *testing.T) { + defer hookRec.BeforeUserCreated.ClearCalls() + + req := &api.SignupParams{} + res := new(api.AccessTokenResponse) + err := e2eapi.Do(ctx, http.MethodPost, apiSrv.URL+"/signup", req, res) + require.NoError(t, err) + + runBeforeUserCreated(t, res.User) + }) + + t.Run("AdminEndpoints", func(t *testing.T) { + t.Run("Invite", func(t *testing.T) { + defer hookRec.BeforeUserCreated.ClearCalls() + + email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + req := &api.InviteParams{ + Email: email, + } + res := new(models.User) + + body := new(bytes.Buffer) + err := json.NewEncoder(body).Encode(req) + require.NoError(t, err) + + httpReq, err := http.NewRequestWithContext( + ctx, "POST", "/invite", body) + require.NoError(t, err) + + httpRes, err := inst.DoAdmin(httpReq) + require.NoError(t, err) + + err = json.NewDecoder(httpRes.Body).Decode(res) + require.NoError(t, err) + + runBeforeUserCreated(t, res) + }) + + t.Run("AdminGenerateLink", func(t *testing.T) { + + t.Run("SignupVerification", func(t *testing.T) { + defer hookRec.BeforeUserCreated.ClearCalls() + + email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + req := &api.GenerateLinkParams{ + Type: "signup", + Email: email, + Password: "pass1234", + } + res := new(api.GenerateLinkResponse) + + body := new(bytes.Buffer) + err := json.NewEncoder(body).Encode(req) + require.NoError(t, err) + + httpReq, err := http.NewRequestWithContext( + ctx, "POST", "/admin/generate_link", body) + require.NoError(t, err) + + httpRes, err := inst.DoAdmin(httpReq) + require.NoError(t, err) + require.Equal(t, 200, httpRes.StatusCode) + + err = json.NewDecoder(httpRes.Body).Decode(res) + require.NoError(t, err) + + runBeforeUserCreated(t, &res.User) + }) + + t.Run("InviteVerification", func(t *testing.T) { + defer hookRec.BeforeUserCreated.ClearCalls() + + email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + req := &api.GenerateLinkParams{ + Type: "invite", + Email: email, + } + res := new(api.GenerateLinkResponse) + + body := new(bytes.Buffer) + err := json.NewEncoder(body).Encode(req) + require.NoError(t, err) + + httpReq, err := http.NewRequestWithContext( + ctx, "POST", "/admin/generate_link", body) + require.NoError(t, err) + + httpRes, err := inst.DoAdmin(httpReq) + require.NoError(t, err) + require.Equal(t, 200, httpRes.StatusCode) + + err = json.NewDecoder(httpRes.Body).Decode(res) + require.NoError(t, err) + dump.Dump(res) + + runBeforeUserCreated(t, &res.User) + }) + }) + }) }) // Basic tests for CustomizeAccessToken diff --git a/internal/api/mail.go b/internal/api/mail.go index d2e6c3430..f06811db0 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -93,15 +93,11 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { hashedToken := crypto.GenerateTokenHash(params.Email, otp) var ( - triggerSignupHook bool - signupVerificationUser *models.User - inviteVerificationUser *models.User + signupUser *models.User + inviteUser *models.User ) - switch params.Type { - case mail.SignupVerification: - if user != nil { - break - } + switch { + case params.Type == mail.SignupVerification && user == nil: signupParams := &SignupParams{ Email: params.Email, Password: params.Password, @@ -109,23 +105,20 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { Provider: "email", Aud: aud, } + if err := a.validateSignupParams(ctx, signupParams); err != nil { return err } - signupVerificationUser, err = signupParams.ToUserModel(false) + signupUser, err = signupParams.ToUserModel(false /* <- isSSOUser */) if err != nil { return err } - triggerSignupHook = true - - case mail.InviteVerification: - if user == nil { - break - } - if user.IsConfirmed() { - return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailExists, DuplicateEmailMsg) + if err := a.triggerBeforeUserCreated(r, db, signupUser); err != nil { + return err } + + case params.Type == mail.InviteVerification && user == nil: signupParams := &SignupParams{ Email: params.Email, Data: params.Data, @@ -133,15 +126,11 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { Aud: aud, } - inviteVerificationUser, err = signupParams.ToUserModel(false) + inviteUser, err = signupParams.ToUserModel(false /* <- isSSOUser */) if err != nil { return err } - triggerSignupHook = true - - } - if triggerSignupHook { - if err := a.triggerBeforeUserCreated(r, db, user); err != nil { + if err := a.triggerBeforeUserCreated(r, db, inviteUser); err != nil { return err } } @@ -167,8 +156,12 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { return terr } case mail.InviteVerification: - if user == nil { - user, terr = a.signupNewUser(tx, inviteVerificationUser) + if user != nil { + if user.IsConfirmed() { + return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailExists, DuplicateEmailMsg) + } + } else { + user, terr = a.signupNewUser(tx, inviteUser) if terr != nil { return terr } @@ -181,7 +174,6 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { } user.Identities = []models.Identity{*identity} } - if terr = models.NewAuditLogEntry(r, tx, adminUser, models.UserInvitedAction, "", map[string]interface{}{ "user_id": user.ID, "user_email": user.Email, @@ -214,7 +206,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { // password here to generate a new user, use // signupUser which is a model generated from // SignupParams above - user, terr = a.signupNewUser(tx, signupVerificationUser) + user, terr = a.signupNewUser(tx, signupUser) if terr != nil { return terr } diff --git a/internal/e2e/e2eapi/e2eapi.go b/internal/e2e/e2eapi/e2eapi.go index f28e89e51..1be6b5311 100644 --- a/internal/e2e/e2eapi/e2eapi.go +++ b/internal/e2e/e2eapi/e2eapi.go @@ -9,7 +9,9 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" + jwt "github.com/golang-jwt/jwt/v5" "github.com/supabase/auth/internal/api" "github.com/supabase/auth/internal/api/apierrors" "github.com/supabase/auth/internal/conf" @@ -22,6 +24,8 @@ type Instance struct { Config *conf.GlobalConfiguration Conn *storage.Connection APIServer *httptest.Server + APIClient *http.Client + apiURL *url.URL closers []func() } @@ -30,9 +34,16 @@ func New(globalCfg *conf.GlobalConfiguration) (*Instance, error) { o := new(Instance) o.Config = globalCfg - conn, err := test.SetupDBConnection(globalCfg) + if err := o.init(); err != nil { + return nil, err + } + return o, nil +} + +func (o *Instance) init() error { + conn, err := test.SetupDBConnection(o.Config) if err != nil { - return nil, fmt.Errorf("error setting up db connection: %w", err) + return fmt.Errorf("error setting up db connection: %w", err) } o.addCloser(func() { if conn.Store != nil { @@ -46,12 +57,24 @@ func New(globalCfg *conf.GlobalConfiguration) (*Instance, error) { apiVer = "1" } - a := api.NewAPIWithVersion(globalCfg, conn, apiVer) + a := api.NewAPIWithVersion(o.Config, conn, apiVer) apiSrv := httptest.NewServer(a) o.addCloser(apiSrv) o.APIServer = apiSrv + o.APIClient = apiSrv.Client() - return o, nil + return o.initURL() +} + +func (o *Instance) initURL() error { + u, err := url.Parse(o.APIServer.URL) + if err != nil { + defer o.Close() + + return err + } + o.apiURL = u + return nil } func (o *Instance) Close() error { @@ -70,6 +93,40 @@ func (o *Instance) addCloser(v any) { } } +func (o *Instance) Do( + req *http.Request, +) (*http.Response, error) { + req.URL = o.apiURL.ResolveReference(req.URL) + + return o.APIClient.Do(req) +} + +func (o *Instance) DoAdmin( + req *http.Request, +) (*http.Response, error) { + return o.doAdmin(req, []byte(o.Config.JWT.Secret)) +} + +func (o *Instance) doAdmin( + req *http.Request, + key any, +) (*http.Response, error) { + tok := jwt.NewWithClaims( + jwt.SigningMethodHS256, + &api.AccessTokenClaims{ + Role: "supabase_admin", + }, + ) + + adminJwt, err := tok.SignedString(key) + if err != nil { + return nil, err + } + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", adminJwt)) + + return o.Do(req) +} + func Do( ctx context.Context, method string, diff --git a/internal/e2e/e2eapi/e2eapi_test.go b/internal/e2e/e2eapi/e2eapi_test.go index 8887deca5..098d4c831 100644 --- a/internal/e2e/e2eapi/e2eapi_test.go +++ b/internal/e2e/e2eapi/e2eapi_test.go @@ -1,7 +1,9 @@ package e2eapi import ( + "bytes" "context" + "encoding/json" "errors" "fmt" "io" @@ -29,7 +31,7 @@ func TestInstance(t *testing.T) { require.NoError(t, err) defer inst.Close() - email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + email := "e2eapitest_" + uuid.Must(uuid.NewV4()).String() + "@localhost" req := &api.SignupParams{ Email: email, Password: "password", @@ -40,6 +42,50 @@ func TestInstance(t *testing.T) { require.Equal(t, email, res.Email.String()) }) + t.Run("DoAdmin", func(t *testing.T) { + globalCfg := e2e.Must(e2e.Config()) + inst, err := New(globalCfg) + require.NoError(t, err) + defer inst.Close() + + email := "e2eapitest_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + req := &api.InviteParams{ + Email: email, + } + res := new(models.User) + + body := new(bytes.Buffer) + err = json.NewEncoder(body).Encode(req) + require.NoError(t, err) + + httpReq, err := http.NewRequestWithContext( + ctx, "POST", "/invite", body) + require.NoError(t, err) + + httpRes, err := inst.DoAdmin(httpReq) + require.NoError(t, err) + + err = json.NewDecoder(httpRes.Body).Decode(res) + require.NoError(t, err) + require.Equal(t, email, res.Email.String()) + }) + + t.Run("DoAdminFailure", func(t *testing.T) { + globalCfg := e2e.Must(e2e.Config()) + inst, err := New(globalCfg) + require.NoError(t, err) + defer inst.Close() + + httpReq, err := http.NewRequestWithContext( + ctx, "POST", "/invite", nil) + require.NoError(t, err) + + httpRes, err := inst.doAdmin(httpReq, new(int)) + require.Error(t, err) + require.Nil(t, httpRes) + + }) + t.Run("Failure", func(t *testing.T) { globalCfg := e2e.Must(e2e.Config()) globalCfg.DB.Driver = "" @@ -49,6 +95,17 @@ func TestInstance(t *testing.T) { require.Error(t, err) require.Nil(t, inst) }) + + t.Run("InitURLFailure", func(t *testing.T) { + globalCfg := e2e.Must(e2e.Config()) + inst, err := New(globalCfg) + require.NoError(t, err) + defer inst.Close() + + inst.APIServer.URL = "\x01" + err = inst.initURL() + require.Error(t, err) + }) }) } diff --git a/internal/e2e/e2ehooks/e2ehooks.go b/internal/e2e/e2ehooks/e2ehooks.go index 0bd0bd522..65340750a 100644 --- a/internal/e2e/e2ehooks/e2ehooks.go +++ b/internal/e2e/e2ehooks/e2ehooks.go @@ -68,7 +68,27 @@ func NewHook(name v0hooks.Name) *Hook { o := &Hook{ name: name, } - o.SetHandler(HandleSuccess()) + switch name { + case v0hooks.CustomizeAccessToken: + // This hooks returns the exact claims given. + hr := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("content-type", "application/json") + w.WriteHeader(http.StatusOK) + + var v any + if err := json.NewDecoder(r.Body).Decode(&v); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + } + if err := json.NewEncoder(w).Encode(&v); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + } + }) + o.SetHandler(hr) + + default: + o.SetHandler(HandleSuccess()) + } + return o } From ee2da27eef22a731c625edd6150e1c70e8fa5c06 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Fri, 23 May 2025 14:24:18 -0700 Subject: [PATCH 03/13] fix: remove a debug line left in test --- internal/api/e2e_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/api/e2e_test.go b/internal/api/e2e_test.go index 13b9765c8..5ac82f2f1 100644 --- a/internal/api/e2e_test.go +++ b/internal/api/e2e_test.go @@ -15,7 +15,6 @@ import ( "github.com/stretchr/testify/require" "github.com/supabase/auth/internal/api" "github.com/supabase/auth/internal/conf" - "github.com/supabase/auth/internal/dump" "github.com/supabase/auth/internal/e2e" "github.com/supabase/auth/internal/e2e/e2eapi" "github.com/supabase/auth/internal/e2e/e2ehooks" @@ -194,7 +193,6 @@ func TestE2EHooks(t *testing.T) { err = json.NewDecoder(httpRes.Body).Decode(res) require.NoError(t, err) - dump.Dump(res) runBeforeUserCreated(t, &res.User) }) From 879689e3c07574c3e64d20aed5ac630bdb467303 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Fri, 23 May 2025 14:51:51 -0700 Subject: [PATCH 04/13] chore: fix tedious staticcheck --- internal/e2e/e2ehooks/e2ehooks.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/e2e/e2ehooks/e2ehooks.go b/internal/e2e/e2ehooks/e2ehooks.go index 65340750a..e638cfa60 100644 --- a/internal/e2e/e2ehooks/e2ehooks.go +++ b/internal/e2e/e2ehooks/e2ehooks.go @@ -69,6 +69,7 @@ func NewHook(name v0hooks.Name) *Hook { name: name, } switch name { + //exhaustive:ignore case v0hooks.CustomizeAccessToken: // This hooks returns the exact claims given. hr := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From 91966f3c2981498d148e53d93c676cb5375d7ee1 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Fri, 23 May 2025 14:52:59 -0700 Subject: [PATCH 05/13] chore: fully fix exhaustive check --- internal/e2e/e2ehooks/e2ehooks.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/e2e/e2ehooks/e2ehooks.go b/internal/e2e/e2ehooks/e2ehooks.go index e638cfa60..186d497a2 100644 --- a/internal/e2e/e2ehooks/e2ehooks.go +++ b/internal/e2e/e2ehooks/e2ehooks.go @@ -68,8 +68,9 @@ func NewHook(name v0hooks.Name) *Hook { o := &Hook{ name: name, } - switch name { + //exhaustive:ignore + switch name { case v0hooks.CustomizeAccessToken: // This hooks returns the exact claims given. hr := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From c97ff3cfd2c0fb5e3e0599544686a4529d030e2b Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Wed, 28 May 2025 08:54:49 -0700 Subject: [PATCH 06/13] fix: only assign currentUser in signup test Otherwise test ordering can change the test result. --- internal/api/e2e_test.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/internal/api/e2e_test.go b/internal/api/e2e_test.go index 5ac82f2f1..72e16a932 100644 --- a/internal/api/e2e_test.go +++ b/internal/api/e2e_test.go @@ -38,7 +38,8 @@ func TestE2EHooks(t *testing.T) { var currentUser *models.User - runBeforeUserCreated := func(t *testing.T, expUser *models.User) { + runBeforeUserCreated := func(t *testing.T, expUser *models.User) *models.User { + var latest *models.User t.Run("BeforeUserCreated", func(t *testing.T) { calls := hookRec.BeforeUserCreated.GetCalls() require.Equal(t, 1, len(calls)) @@ -61,13 +62,11 @@ func TestE2EHooks(t *testing.T) { err = expUser.Confirm(inst.Conn) require.NoError(t, err) - latest, err := models.FindUserByID(inst.Conn, expUser.ID) + latest, err = models.FindUserByID(inst.Conn, expUser.ID) require.NoError(t, err) - - // Assign currentUser for next tests. - currentUser = latest - require.NotNil(t, currentUser) + require.NotNil(t, latest) }) + return latest } // Basic tests for user hooks @@ -86,7 +85,7 @@ func TestE2EHooks(t *testing.T) { require.NoError(t, err) require.Equal(t, email, res.Email.String()) - runBeforeUserCreated(t, res) + currentUser = runBeforeUserCreated(t, res) }) t.Run("SignupAnonymously", func(t *testing.T) { @@ -202,6 +201,8 @@ func TestE2EHooks(t *testing.T) { // Basic tests for CustomizeAccessToken t.Run("CustomizeAccessToken", func(t *testing.T) { + defer hookRec.CustomizeAccessToken.ClearCalls() + require.NotNil(t, currentUser) type M = map[string]any @@ -290,6 +291,8 @@ func TestE2EHooks(t *testing.T) { for _, tc := range cases { t.Run(string(tc.desc), func(t *testing.T) { + defer hookRec.CustomizeAccessToken.ClearCalls() + var claimsIn, claimsOut M hr := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Add("content-type", "application/json") From 78be229b31b4930af16363f85cfc4faa736b890a Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Wed, 28 May 2025 09:28:18 -0700 Subject: [PATCH 07/13] fix: ensure errors remain same as they were before --- internal/api/e2e_test.go | 47 +++++++++++++++++++++++++++++++++++----- internal/api/token.go | 12 ++++++---- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/internal/api/e2e_test.go b/internal/api/e2e_test.go index 72e16a932..4c200dbc1 100644 --- a/internal/api/e2e_test.go +++ b/internal/api/e2e_test.go @@ -36,8 +36,6 @@ func TestE2EHooks(t *testing.T) { apiSrv := inst.APIServer hookRec := inst.HookRecorder - var currentUser *models.User - runBeforeUserCreated := func(t *testing.T, expUser *models.User) *models.User { var latest *models.User t.Run("BeforeUserCreated", func(t *testing.T) { @@ -85,7 +83,7 @@ func TestE2EHooks(t *testing.T) { require.NoError(t, err) require.Equal(t, email, res.Email.String()) - currentUser = runBeforeUserCreated(t, res) + runBeforeUserCreated(t, res) }) t.Run("SignupAnonymously", func(t *testing.T) { @@ -203,6 +201,22 @@ func TestE2EHooks(t *testing.T) { t.Run("CustomizeAccessToken", func(t *testing.T) { defer hookRec.CustomizeAccessToken.ClearCalls() + var currentUser *models.User + t.Run("CreateUserForHookTests", func(t *testing.T) { + defer hookRec.BeforeUserCreated.ClearCalls() + + email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + req := &api.SignupParams{ + Email: email, + Password: "password", + } + res := new(models.User) + err := e2eapi.Do(ctx, http.MethodPost, apiSrv.URL+"/signup", req, res) + require.NoError(t, err) + require.Equal(t, email, res.Email.String()) + + currentUser = runBeforeUserCreated(t, res) + }) require.NotNil(t, currentUser) type M = map[string]any @@ -253,9 +267,32 @@ func TestE2EHooks(t *testing.T) { ) }{ { - desc: `empty claims`, + desc: `claims empty`, from: func(in M) M { return M{"claims": M{}} }, - errStr: "500: error generating jwt token", + errStr: "500: output claims do not conform to the expected schema", + }, + + { + desc: `claims nil`, + from: func(in M) M { return M{"claims": nil} }, + errStr: "500: output claims do not conform to the expected schema", + }, + + { + desc: `claims field missing`, + from: func(in M) M { return M{} }, + errStr: "500: output claims do not conform to the expected schema", + }, + + { + desc: `claims are top level`, + from: func(in M) M { + return M{ + "myclaim": "aaa", + "other_claim": "bbb", + } + }, + errStr: "500: output claims do not conform to the expected schema", }, { diff --git a/internal/api/token.go b/internal/api/token.go index 0f041faf5..8a86b3016 100644 --- a/internal/api/token.go +++ b/internal/api/token.go @@ -510,13 +510,17 @@ func validateTokenClaims(outputClaims map[string]interface{}) error { for _, desc := range result.Errors() { errorMessages += fmt.Sprintf("- %s\n", desc) - fmt.Printf("- %s\n", desc) } - return fmt.Errorf( + err = fmt.Errorf( "output claims do not conform to the expected schema: \n%s", errorMessages) - } - + if err != nil { + httpError := &apierrors.HTTPError{ + HTTPStatus: http.StatusInternalServerError, + Message: err.Error(), + } + return httpError + } return nil } From 753f600dee1a2d80503141244f2cd87d88b7431a Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Wed, 28 May 2025 11:21:32 -0700 Subject: [PATCH 08/13] fix: improve error message for missing claims field (issue#2038) --- internal/api/e2e_test.go | 38 +++++++++++------------ internal/hooks/hookshttp/hookshttp.go | 4 +++ internal/hooks/hookspgfunc/hookspgfunc.go | 5 +++ internal/hooks/v0hooks/v0hooks.go | 34 +++++++++++++++++++- 4 files changed, 61 insertions(+), 20 deletions(-) diff --git a/internal/api/e2e_test.go b/internal/api/e2e_test.go index 4c200dbc1..43db52b83 100644 --- a/internal/api/e2e_test.go +++ b/internal/api/e2e_test.go @@ -201,10 +201,9 @@ func TestE2EHooks(t *testing.T) { t.Run("CustomizeAccessToken", func(t *testing.T) { defer hookRec.CustomizeAccessToken.ClearCalls() + // setup user to test with var currentUser *models.User - t.Run("CreateUserForHookTests", func(t *testing.T) { - defer hookRec.BeforeUserCreated.ClearCalls() - + { email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" req := &api.SignupParams{ Email: email, @@ -216,8 +215,9 @@ func TestE2EHooks(t *testing.T) { require.Equal(t, email, res.Email.String()) currentUser = runBeforeUserCreated(t, res) - }) - require.NotNil(t, currentUser) + require.NotNil(t, currentUser) + hookRec.CustomizeAccessToken.ClearCalls() + } type M = map[string]any @@ -266,32 +266,32 @@ func TestE2EHooks(t *testing.T) { claimsIn, claimsOut M, ) }{ - { - desc: `claims empty`, - from: func(in M) M { return M{"claims": M{}} }, - errStr: "500: output claims do not conform to the expected schema", - }, - - { - desc: `claims nil`, - from: func(in M) M { return M{"claims": nil} }, - errStr: "500: output claims do not conform to the expected schema", - }, - { desc: `claims field missing`, from: func(in M) M { return M{} }, - errStr: "500: output claims do not conform to the expected schema", + errStr: "500: output claims field is missing", }, { - desc: `claims are top level`, + desc: `claims field missing with top level keys`, from: func(in M) M { return M{ "myclaim": "aaa", "other_claim": "bbb", } }, + errStr: "500: output claims field is missing", + }, + + { + desc: `claims field nil`, + from: func(in M) M { return M{"claims": nil} }, + errStr: "500: output claims do not conform to the expected schema", + }, + + { + desc: `claims field empty`, + from: func(in M) M { return M{"claims": M{}} }, errStr: "500: output claims do not conform to the expected schema", }, diff --git a/internal/hooks/hookshttp/hookshttp.go b/internal/hooks/hookshttp/hookshttp.go index e8c39397b..e11897e85 100644 --- a/internal/hooks/hookshttp/hookshttp.go +++ b/internal/hooks/hookshttp/hookshttp.go @@ -92,6 +92,10 @@ func (o *Dispatcher) Dispatch( } if data != nil { if err := json.Unmarshal(data, res); err != nil { + e := new(apierrors.HTTPError) + if errors.As(err, &e) { + return e + } return apierrors.NewInternalServerError( "Error unmarshaling JSON output.").WithInternalError(err) } diff --git a/internal/hooks/hookspgfunc/hookspgfunc.go b/internal/hooks/hookspgfunc/hookspgfunc.go index 5d26a006d..ce8ee2a32 100644 --- a/internal/hooks/hookspgfunc/hookspgfunc.go +++ b/internal/hooks/hookspgfunc/hookspgfunc.go @@ -3,6 +3,7 @@ package hookspgfunc import ( "context" "encoding/json" + "errors" "fmt" "time" @@ -58,6 +59,10 @@ func (o *Dispatcher) Dispatch( } if data != nil { if err := json.Unmarshal(data, res); err != nil { + e := new(apierrors.HTTPError) + if errors.As(err, &e) { + return e + } return apierrors.NewInternalServerError( "Error unmarshaling JSON output.").WithInternalError(err) } diff --git a/internal/hooks/v0hooks/v0hooks.go b/internal/hooks/v0hooks/v0hooks.go index 33a054ec1..b0acb4821 100644 --- a/internal/hooks/v0hooks/v0hooks.go +++ b/internal/hooks/v0hooks/v0hooks.go @@ -1,11 +1,13 @@ package v0hooks import ( + "encoding/json" "net/http" "time" "github.com/gofrs/uuid" "github.com/golang-jwt/jwt/v5" + "github.com/supabase/auth/internal/api/apierrors" "github.com/supabase/auth/internal/mailer" "github.com/supabase/auth/internal/models" "github.com/supabase/auth/internal/utilities" @@ -139,7 +141,37 @@ type CustomAccessTokenInput struct { } type CustomAccessTokenOutput struct { - Claims map[string]interface{} `json:"claims"` + Claims map[string]any `json:"claims"` +} + +func (o *CustomAccessTokenOutput) UnmarshalJSON(b []byte) error { + var m map[string]any + if err := json.Unmarshal(b, &m); err != nil { + return err + } + + // First check if the claims field is missing + if _, ok := m["claims"]; !ok { + httpError := &apierrors.HTTPError{ + HTTPStatus: http.StatusInternalServerError, + Message: "output claims field is missing", + } + return httpError + } + + // This check allows us to skip an additional unmarshal for valid inputs + if v, ok := m["claims"].(map[string]any); ok { + o.Claims = v + return nil + } + + // The Claims field is not a map[string]any so we unmarshal again just + // to get the correct error type. + type raw CustomAccessTokenOutput + if err := json.Unmarshal(b, (*raw)(o)); err != nil { + return err + } + return nil } type SendSMSInput struct { From 680392b5e5391a4c333f82dd752c3ee1e8e1f248 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Thu, 29 May 2025 07:41:23 -0700 Subject: [PATCH 09/13] chore: remove panics from code in favor of errors --- internal/api/hooks.go | 18 ++++++++++++------ internal/api/hooks_test.go | 13 +++++++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/internal/api/hooks.go b/internal/api/hooks.go index 74a62a0bc..f850ab1ad 100644 --- a/internal/api/hooks.go +++ b/internal/api/hooks.go @@ -20,9 +20,8 @@ func (a *API) triggerBeforeUserCreated( if !a.hooksMgr.Enabled(v0hooks.BeforeUserCreated) { return nil } - if conn.TX != nil { - // TODO(cstockton): remove this - panic("unable to trigger hooks during transaction") + if err := checkTX(conn); err != nil { + return err } req := v0hooks.NewBeforeUserCreatedInput(r, user) @@ -39,9 +38,8 @@ func (a *API) triggerBeforeUserCreatedExternal( if !a.hooksMgr.Enabled(v0hooks.BeforeUserCreated) { return nil } - if conn.TX != nil { - // TODO(cstockton): remove this - panic("unable to trigger hooks during transaction") + if err := checkTX(conn); err != nil { + return err } ctx := r.Context() @@ -97,3 +95,11 @@ func (a *API) triggerBeforeUserCreatedExternal( } return a.triggerBeforeUserCreated(r, conn, user) } + +func checkTX(conn *storage.Connection) error { + if conn.TX != nil { + return apierrors.NewInternalServerError( + "unable to trigger hooks during transaction") + } + return nil +} diff --git a/internal/api/hooks_test.go b/internal/api/hooks_test.go index 8195f1104..9670e431c 100644 --- a/internal/api/hooks_test.go +++ b/internal/api/hooks_test.go @@ -47,6 +47,19 @@ func TestHooks(t *testing.T) { defer api.db.Close() suite.Run(t, ts) + + t.Run("CheckTX", func(t *testing.T) { + require.NoError(t, checkTX(api.db)) + + err := api.db.Transaction(func(tx *storage.Connection) error { + require.Error(t, checkTX(tx)) + + err := checkTX(tx) + require.Error(t, err) + return nil + }) + require.NoError(t, err) + }) } func (ts *HooksTestSuite) SetupTest() { From aef9121711d4835160ef36f2157f5e5faad69854 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Thu, 29 May 2025 09:55:20 -0700 Subject: [PATCH 10/13] fix: bug with factor type not being included in hook response --- internal/api/mfa.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/api/mfa.go b/internal/api/mfa.go index 8ce34e52d..83422be18 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -631,9 +631,10 @@ func (a *API) verifyTOTPFactor(w http.ResponseWriter, r *http.Request, params *V if config.Hook.MFAVerificationAttempt.Enabled { input := v0hooks.MFAVerificationAttemptInput{ - UserID: user.ID, - FactorID: factor.ID, - Valid: valid, + UserID: user.ID, + FactorID: factor.ID, + FactorType: factor.FactorType, + Valid: valid, } output := v0hooks.MFAVerificationAttemptOutput{} From 83143c4f61815d2cba9284be9baad07622f95568 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Thu, 29 May 2025 09:56:35 -0700 Subject: [PATCH 11/13] feat: add test coverage for mfa flows to cover mfa type bug --- internal/api/e2e_test.go | 274 ++++++++++++++++++++++++++++-- internal/e2e/e2eapi/e2eapi.go | 8 + internal/e2e/e2ehooks/e2ehooks.go | 21 ++- 3 files changed, 288 insertions(+), 15 deletions(-) diff --git a/internal/api/e2e_test.go b/internal/api/e2e_test.go index 43db52b83..59899895d 100644 --- a/internal/api/e2e_test.go +++ b/internal/api/e2e_test.go @@ -5,13 +5,16 @@ import ( "context" "encoding/json" "fmt" + "math/rand" "net/http" "slices" + "strings" "testing" "time" "github.com/gofrs/uuid" jwt "github.com/golang-jwt/jwt/v5" + "github.com/pquerna/otp/totp" "github.com/stretchr/testify/require" "github.com/supabase/auth/internal/api" "github.com/supabase/auth/internal/conf" @@ -22,12 +25,32 @@ import ( "github.com/supabase/auth/internal/models" ) +type M = map[string]any + +func genEmail() string { + return "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" +} + +func genPhone() string { + var sb strings.Builder + sb.WriteString("1") + for i := 0; i < 9; i++ { + sb.WriteString(fmt.Sprintf("%d", rand.Intn(9))) + } + phone := sb.String() + return phone +} + func TestE2EHooks(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) defer cancel() globalCfg := e2e.Must(e2e.Config()) globalCfg.External.AnonymousUsers.Enabled = true + globalCfg.External.Phone.Enabled = true + globalCfg.MFA.Phone.EnrollEnabled = true + globalCfg.MFA.TOTP.EnrollEnabled = true + globalCfg.MFA.Phone.VerifyEnabled = true inst, err := e2ehooks.New(globalCfg) require.NoError(t, err) @@ -39,6 +62,8 @@ func TestE2EHooks(t *testing.T) { runBeforeUserCreated := func(t *testing.T, expUser *models.User) *models.User { var latest *models.User t.Run("BeforeUserCreated", func(t *testing.T) { + defer hookRec.BeforeUserCreated.ClearCalls() + calls := hookRec.BeforeUserCreated.GetCalls() require.Equal(t, 1, len(calls)) call := calls[0] @@ -67,13 +92,28 @@ func TestE2EHooks(t *testing.T) { return latest } + getAccessToken := func( + t *testing.T, + email, pass string, + ) *api.AccessTokenResponse { + req := &api.PasswordGrantParams{ + Email: email, + Password: pass, + } + + res := new(api.AccessTokenResponse) + err := e2eapi.Do(ctx, http.MethodPost, apiSrv.URL+"/token?grant_type=password", req, res) + require.NoError(t, err) + return res + } + // Basic tests for user hooks t.Run("UserHooks", func(t *testing.T) { - t.Run("Signup", func(t *testing.T) { + t.Run("SignupEmail", func(t *testing.T) { defer hookRec.BeforeUserCreated.ClearCalls() - email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + email := genEmail() req := &api.SignupParams{ Email: email, Password: "password", @@ -86,6 +126,22 @@ func TestE2EHooks(t *testing.T) { runBeforeUserCreated(t, res) }) + t.Run("SignupPhone", func(t *testing.T) { + defer hookRec.BeforeUserCreated.ClearCalls() + + phone := genPhone() + req := &api.SignupParams{ + Phone: phone, + Password: "password", + } + res := new(models.User) + err := e2eapi.Do(ctx, http.MethodPost, apiSrv.URL+"/signup", req, res) + require.NoError(t, err) + require.Equal(t, phone, res.Phone.String()) + + runBeforeUserCreated(t, res) + }) + t.Run("SignupAnonymously", func(t *testing.T) { defer hookRec.BeforeUserCreated.ClearCalls() @@ -112,7 +168,7 @@ func TestE2EHooks(t *testing.T) { t.Run("Invite", func(t *testing.T) { defer hookRec.BeforeUserCreated.ClearCalls() - email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + email := genEmail() req := &api.InviteParams{ Email: email, } @@ -140,7 +196,7 @@ func TestE2EHooks(t *testing.T) { t.Run("SignupVerification", func(t *testing.T) { defer hookRec.BeforeUserCreated.ClearCalls() - email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + email := genEmail() req := &api.GenerateLinkParams{ Type: "signup", Email: email, @@ -169,7 +225,7 @@ func TestE2EHooks(t *testing.T) { t.Run("InviteVerification", func(t *testing.T) { defer hookRec.BeforeUserCreated.ClearCalls() - email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + email := genEmail() req := &api.GenerateLinkParams{ Type: "invite", Email: email, @@ -197,6 +253,203 @@ func TestE2EHooks(t *testing.T) { }) }) + t.Run("MFAVerificationAttempt", func(t *testing.T) { + defer hookRec.MFAVerification.ClearCalls() + + type flowResult struct { + factorRes *api.EnrollFactorResponse + challengeRes *api.ChallengeFactorResponse + mfaUser *models.User + mfaUserAccessToken *api.AccessTokenResponse + } + + runMFAFlow := func(t *testing.T) *flowResult { + factorRes := new(api.EnrollFactorResponse) + challengeRes := new(api.ChallengeFactorResponse) + mfaUser := new(models.User) + mfaUserAccessToken := new(api.AccessTokenResponse) + + t.Run("MFAFlow", func(t *testing.T) { + t.Run("Signup", func(t *testing.T) { + email := genEmail() + const password = "password" + req := &api.SignupParams{ + Email: email, + Password: password, + } + err := e2eapi.Do(ctx, http.MethodPost, apiSrv.URL+"/signup", req, mfaUser) + require.NoError(t, err) + require.Equal(t, email, mfaUser.Email.String()) + + mfaUser = runBeforeUserCreated(t, mfaUser) + mfaUserAccessToken = getAccessToken(t, string(mfaUser.Email), password) + + phone := genPhone() + domain := strings.Split(email, "@")[1] + + // enroll factor + t.Run("MFAEnroll", func(t *testing.T) { + req := &api.EnrollFactorParams{ + FriendlyName: "totp_" + email, + Phone: phone, + Issuer: domain, + FactorType: models.TOTP, + } + + body := new(bytes.Buffer) + err = json.NewEncoder(body).Encode(req) + require.NoError(t, err) + + httpReq, err := http.NewRequestWithContext( + ctx, "POST", "/factors/", body) + require.NoError(t, err) + + httpRes, err := inst.DoAuth(httpReq, mfaUserAccessToken.Token) + require.NoError(t, err) + require.Equal(t, 200, httpRes.StatusCode) + + err = json.NewDecoder(httpRes.Body).Decode(factorRes) + require.NoError(t, err) + }) + + // challenge factor + t.Run("MFAChallenge", func(t *testing.T) { + req := &models.Factor{ + ID: factorRes.ID, + } + + body := new(bytes.Buffer) + err = json.NewEncoder(body).Encode(req) + require.NoError(t, err) + + url := fmt.Sprintf("/factors/%v/challenge", factorRes.ID) + httpReq, err := http.NewRequestWithContext( + ctx, "POST", url, body) + require.NoError(t, err) + + httpRes, err := inst.DoAuth(httpReq, mfaUserAccessToken.Token) + require.NoError(t, err) + require.Equal(t, 200, httpRes.StatusCode) + + err = json.NewDecoder(httpRes.Body).Decode(challengeRes) + require.NoError(t, err) + }) + }) + }) + return &flowResult{ + factorRes: factorRes, + challengeRes: challengeRes, + mfaUser: mfaUser, + mfaUserAccessToken: mfaUserAccessToken, + } + } + + t.Run("MFAVerifySuccess", func(t *testing.T) { + defer hookRec.MFAVerification.ClearCalls() + + flowRes := runMFAFlow(t) + verifyRes := new(api.AccessTokenResponse) + + mfaCode, err := totp.GenerateCode(flowRes.factorRes.TOTP.Secret, time.Now().UTC()) + require.NoError(t, err) + + req := &api.VerifyFactorParams{ + ChallengeID: flowRes.challengeRes.ID, + Code: mfaCode, + } + + body := new(bytes.Buffer) + err = json.NewEncoder(body).Encode(req) + require.NoError(t, err) + + url := fmt.Sprintf("/factors/%v/verify", flowRes.factorRes.ID) + httpReq, err := http.NewRequestWithContext( + ctx, "POST", url, body) + require.NoError(t, err) + + httpRes, err := inst.DoAuth(httpReq, flowRes.mfaUserAccessToken.Token) + require.NoError(t, err) + require.Equal(t, 200, httpRes.StatusCode) + + // verify the mfa was accepted + err = json.NewDecoder(httpRes.Body).Decode(verifyRes) + require.NoError(t, err) + require.NotEmpty(t, verifyRes.Token) + + calls := hookRec.MFAVerification.GetCalls() + require.Equal(t, 1, len(calls)) + call := calls[0] + + hookReq := M{} + err = call.Unmarshal(&hookReq) + require.NoError(t, err) + + // verify hook request + require.Equal(t, flowRes.factorRes.ID.String(), hookReq["factor_id"]) + require.Equal(t, flowRes.factorRes.Type, hookReq["factor_type"]) + require.Equal(t, flowRes.mfaUser.ID.String(), hookReq["user_id"]) + require.Equal(t, true, hookReq["valid"]) + }) + + t.Run("MFAVerifyFailure", func(t *testing.T) { + defer hookRec.MFAVerification.ClearCalls() + + const errorMsg = "sentinel error message" + { + hr := e2ehooks.HandleJSON(M{ + "decision": "reject", + "message": errorMsg, + }) + hookRec.MFAVerification.SetHandler(hr) + } + + flowRes := runMFAFlow(t) + errorRes := new(api.HTTPError) + + mfaCode, err := totp.GenerateCode(flowRes.factorRes.TOTP.Secret, time.Now().UTC()) + require.NoError(t, err) + + req := &api.VerifyFactorParams{ + ChallengeID: flowRes.challengeRes.ID, + Code: mfaCode, + } + + body := new(bytes.Buffer) + err = json.NewEncoder(body).Encode(req) + require.NoError(t, err) + + url := fmt.Sprintf("/factors/%v/verify", flowRes.factorRes.ID) + httpReq, err := http.NewRequestWithContext( + ctx, "POST", url, body) + require.NoError(t, err) + + httpRes, err := inst.DoAuth(httpReq, flowRes.mfaUserAccessToken.Token) + require.NoError(t, err) + require.Equal(t, 403, httpRes.StatusCode) + + // verify the mfa rejection + err = json.NewDecoder(httpRes.Body).Decode(errorRes) + require.NoError(t, err) + require.Equal(t, 403, errorRes.HTTPStatus) + require.Equal(t, "mfa_verification_rejected", errorRes.ErrorCode) + require.Equal(t, errorMsg, errorRes.Message) + + calls := hookRec.MFAVerification.GetCalls() + require.Equal(t, 1, len(calls)) + call := calls[0] + + hookReq := M{} + err = call.Unmarshal(&hookReq) + require.NoError(t, err) + + // verify hook request + require.Equal(t, flowRes.factorRes.ID.String(), hookReq["factor_id"]) + require.Equal(t, flowRes.factorRes.Type, hookReq["factor_type"]) + require.Equal(t, flowRes.mfaUser.ID.String(), hookReq["user_id"]) + require.Equal(t, true, hookReq["valid"]) + }) + }) + // Basic tests for CustomizeAccessToken t.Run("CustomizeAccessToken", func(t *testing.T) { defer hookRec.CustomizeAccessToken.ClearCalls() @@ -204,7 +457,7 @@ func TestE2EHooks(t *testing.T) { // setup user to test with var currentUser *models.User { - email := "e2etesthooks_" + uuid.Must(uuid.NewV4()).String() + "@localhost" + email := genEmail() req := &api.SignupParams{ Email: email, Password: "password", @@ -219,8 +472,6 @@ func TestE2EHooks(t *testing.T) { hookRec.CustomizeAccessToken.ClearCalls() } - type M = map[string]any - copyMap := func(t *testing.T, m M) (out M) { b, err := json.Marshal(m) require.NoError(t, err) @@ -345,13 +596,8 @@ func TestE2EHooks(t *testing.T) { hookRec.CustomizeAccessToken.ClearCalls() hookRec.CustomizeAccessToken.SetHandler(hr) - req := &api.PasswordGrantParams{ - Email: string(currentUser.Email), - Password: "password", - } - res := new(api.AccessTokenResponse) - err := e2eapi.Do(ctx, http.MethodPost, apiSrv.URL+"/token?grant_type=password", req, res) + res := getAccessToken(t, string(currentUser.Email), "password") // always verify the hook request before checking response { diff --git a/internal/e2e/e2eapi/e2eapi.go b/internal/e2e/e2eapi/e2eapi.go index 1be6b5311..551035432 100644 --- a/internal/e2e/e2eapi/e2eapi.go +++ b/internal/e2e/e2eapi/e2eapi.go @@ -101,6 +101,14 @@ func (o *Instance) Do( return o.APIClient.Do(req) } +func (o *Instance) DoAuth( + req *http.Request, + jwt string, +) (*http.Response, error) { + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", jwt)) + return o.Do(req) +} + func (o *Instance) DoAdmin( req *http.Request, ) (*http.Response, error) { diff --git a/internal/e2e/e2ehooks/e2ehooks.go b/internal/e2e/e2ehooks/e2ehooks.go index 186d497a2..637f4f090 100644 --- a/internal/e2e/e2ehooks/e2ehooks.go +++ b/internal/e2e/e2ehooks/e2ehooks.go @@ -50,9 +50,16 @@ func New(globalCfg *conf.GlobalConfiguration) (*Instance, error) { } func HandleSuccess() http.Handler { + return HandleJSON(map[string]any{}) +} + +func HandleJSON(m map[string]any) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Add("content-type", "application/json") - _, _ = io.WriteString(w, "{}") + + if err := json.NewEncoder(w).Encode(&m); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + } }) } @@ -87,6 +94,18 @@ func NewHook(name v0hooks.Name) *Hook { }) o.SetHandler(hr) + case v0hooks.MFAVerification: + hr := HandleJSON(map[string]any{ + "decision": "continue", + }) + o.SetHandler(hr) + + case v0hooks.PasswordVerification: + hr := HandleJSON(map[string]any{ + "decision": "continue", + }) + o.SetHandler(hr) + default: o.SetHandler(HandleSuccess()) } From f9857532550d8cf96bc69f757d6fd79abe5ef37f Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Thu, 29 May 2025 10:00:43 -0700 Subject: [PATCH 12/13] fix: add nosec to math/rand usage in unit test --- internal/api/e2e_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/api/e2e_test.go b/internal/api/e2e_test.go index 59899895d..94f8db6b5 100644 --- a/internal/api/e2e_test.go +++ b/internal/api/e2e_test.go @@ -35,6 +35,7 @@ func genPhone() string { var sb strings.Builder sb.WriteString("1") for i := 0; i < 9; i++ { + // #nosec G404 sb.WriteString(fmt.Sprintf("%d", rand.Intn(9))) } phone := sb.String() From ea5064712dea5e1b342e519bb7aa68c93d087a96 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Thu, 29 May 2025 10:10:10 -0700 Subject: [PATCH 13/13] fix: customize access token tests cant use getAccessToken --- internal/api/e2e_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/api/e2e_test.go b/internal/api/e2e_test.go index 94f8db6b5..8902a7c06 100644 --- a/internal/api/e2e_test.go +++ b/internal/api/e2e_test.go @@ -450,7 +450,6 @@ func TestE2EHooks(t *testing.T) { require.Equal(t, true, hookReq["valid"]) }) }) - // Basic tests for CustomizeAccessToken t.Run("CustomizeAccessToken", func(t *testing.T) { defer hookRec.CustomizeAccessToken.ClearCalls() @@ -597,8 +596,13 @@ func TestE2EHooks(t *testing.T) { hookRec.CustomizeAccessToken.ClearCalls() hookRec.CustomizeAccessToken.SetHandler(hr) + req := &api.PasswordGrantParams{ + Email: string(currentUser.Email), + Password: "password", + } - res := getAccessToken(t, string(currentUser.Email), "password") + res := new(api.AccessTokenResponse) + err := e2eapi.Do(ctx, http.MethodPost, apiSrv.URL+"/token?grant_type=password", req, res) // always verify the hook request before checking response {