From 4f501122d31bdbfde431250ffb05d9cd45a1d25f Mon Sep 17 00:00:00 2001 From: Joel Lee Date: Mon, 4 Dec 2023 13:32:18 +0800 Subject: [PATCH] Add password verification hook (#1328) ## What kind of change does this PR introduce? Similar to the MFA Verification Hook, this hook should allow for developers to customize the behaviour of Supabase after a failed password verification attempt. Example use cases include: - blocking a user after multiple failed attempts. - Imposing additional restrictions on top of password verification. --------- Co-authored-by: joel@joellee.org Co-authored-by: Kang Ming --- internal/api/mfa.go | 29 ++++++++++++++-- internal/api/token.go | 31 ++++++++++++++++- internal/api/token_test.go | 61 ++++++++++++++++++++++++++++++++++ internal/conf/configuration.go | 4 ++- internal/hooks/auth_hooks.go | 31 +++++++++++++---- 5 files changed, 146 insertions(+), 10 deletions(-) diff --git a/internal/api/mfa.go b/internal/api/mfa.go index d9df7092a..0ce5a2e53 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -236,7 +236,6 @@ func (a *API) runHook(ctx context.Context, name string, input, output any) ([]by func (a *API) invokeHook(ctx context.Context, input, output any) error { config := a.config - switch input.(type) { case *hooks.MFAVerificationAttemptInput: hookOutput, ok := output.(*hooks.MFAVerificationAttemptOutput) @@ -263,6 +262,32 @@ func (a *API) invokeHook(ctx context.Context, input, output any) error { return httpError.WithInternalError(&hookOutput.HookError) } + return nil + case *hooks.PasswordVerificationAttemptInput: + hookOutput, ok := output.(*hooks.PasswordVerificationAttemptOutput) + if !ok { + panic("output should be *hooks.PasswordVerificationAttemptOutput") + } + + if _, err := a.runHook(ctx, config.Hook.PasswordVerificationAttempt.HookName, input, output); err != nil { + return internalServerError("Error invoking password verification hook.").WithInternalError(err) + } + + if hookOutput.IsError() { + httpCode := hookOutput.HookError.HTTPCode + + if httpCode == 0 { + httpCode = http.StatusInternalServerError + } + + httpError := &HTTPError{ + Code: httpCode, + Message: hookOutput.HookError.Message, + } + + return httpError.WithInternalError(&hookOutput.HookError) + } + return nil default: @@ -335,7 +360,7 @@ func (a *API) VerifyFactor(w http.ResponseWriter, r *http.Request) error { return err } - if output.Decision == hooks.MFAHookRejection { + if output.Decision == hooks.HookRejection { if err := models.Logout(a.db, user.ID); err != nil { return err } diff --git a/internal/api/token.go b/internal/api/token.go index f4d5584d2..8deaf98f0 100644 --- a/internal/api/token.go +++ b/internal/api/token.go @@ -13,6 +13,7 @@ import ( "github.com/golang-jwt/jwt" "github.com/supabase/gotrue/internal/conf" + "github.com/supabase/gotrue/internal/hooks" "github.com/supabase/gotrue/internal/metering" "github.com/supabase/gotrue/internal/models" "github.com/supabase/gotrue/internal/storage" @@ -140,7 +141,35 @@ func (a *API) ResourceOwnerPasswordGrant(ctx context.Context, w http.ResponseWri return internalServerError("Database error querying schema").WithInternalError(err) } - if user.IsBanned() || !user.Authenticate(ctx, params.Password) { + if user.IsBanned() { + return oauthError("invalid_grant", InvalidLoginMessage) + } + isValidPassword := user.Authenticate(ctx, params.Password) + if config.Hook.PasswordVerificationAttempt.Enabled { + + input := hooks.PasswordVerificationAttemptInput{ + UserID: user.ID, + Valid: isValidPassword, + } + output := hooks.PasswordVerificationAttemptOutput{} + err := a.invokeHook(ctx, &input, &output) + if err != nil { + return err + } + + if output.Decision == hooks.HookRejection { + if output.Message == "" { + output.Message = hooks.DefaultPasswordHookRejectionMessage + } + if output.ShouldLogoutUser { + if err := models.Logout(a.db, user.ID); err != nil { + return err + } + } + return forbiddenError(output.Message) + } + } + if !isValidPassword { return oauthError("invalid_grant", InvalidLoginMessage) } diff --git a/internal/api/token_test.go b/internal/api/token_test.go index d2c19b50c..179d39b88 100644 --- a/internal/api/token_test.go +++ b/internal/api/token_test.go @@ -588,3 +588,64 @@ func (ts *TokenTestSuite) TestMagicLinkPKCESignIn() { require.NotEmpty(ts.T(), verifyResp.Token) } + +func (ts *TokenTestSuite) TestPasswordVerificationHook() { + type verificationHookTestcase struct { + desc string + uri string + hookFunctionSQL string + expectedCode int + } + cases := []verificationHookTestcase{ + { + desc: "Default success", + uri: "pg-functions://postgres/auth/password_verification_hook", + hookFunctionSQL: ` + create or replace function password_verification_hook(input jsonb) + returns json as $$ + begin + return json_build_object('decision', 'continue'); + end; $$ language plpgsql;`, + expectedCode: http.StatusOK, + }, { + desc: "Reject- Enabled", + uri: "pg-functions://postgres/auth/password_verification_hook_reject", + hookFunctionSQL: ` + create or replace function password_verification_hook_reject(input jsonb) + returns json as $$ + begin + return json_build_object('decision', 'reject'); + end; $$ language plpgsql;`, + expectedCode: http.StatusForbidden, + }, + } + for _, c := range cases { + ts.T().Run(c.desc, func(t *testing.T) { + ts.Config.Hook.PasswordVerificationAttempt.Enabled = true + ts.Config.Hook.PasswordVerificationAttempt.URI = c.uri + require.NoError(ts.T(), ts.Config.Hook.PasswordVerificationAttempt.ValidateAndPopulateExtensibilityPoint()) + + err := ts.API.db.RawQuery(c.hookFunctionSQL).Exec() + require.NoError(t, err) + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ + "email": "test@example.com", + "password": "password", + })) + + req := httptest.NewRequest(http.MethodPost, "http://localhost/token?grant_type=password", &buffer) + req.Header.Set("Content-Type", "application/json") + + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + + assert.Equal(ts.T(), c.expectedCode, w.Code) + cleanupHookSQL := fmt.Sprintf("drop function if exists %s", ts.Config.Hook.PasswordVerificationAttempt.HookName) + require.NoError(ts.T(), ts.API.db.RawQuery(cleanupHookSQL).Exec()) + // Reset so it doesn't affect other tests + ts.Config.Hook.PasswordVerificationAttempt.Enabled = false + + }) + } + +} diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 6006e6992..162e233d3 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -441,7 +441,8 @@ type WebhookConfig struct { // Moving away from the existing HookConfig so we can get a fresh start. type HookConfiguration struct { - MFAVerificationAttempt ExtensibilityPointConfiguration `json:"mfa_verification_attempt" split_words:"true"` + MFAVerificationAttempt ExtensibilityPointConfiguration `json:"mfa_verification_attempt" split_words:"true"` + PasswordVerificationAttempt ExtensibilityPointConfiguration `json:"password_verification_attempt" split_words:"true"` } type ExtensibilityPointConfiguration struct { @@ -453,6 +454,7 @@ type ExtensibilityPointConfiguration struct { func (h *HookConfiguration) Validate() error { points := []ExtensibilityPointConfiguration{ h.MFAVerificationAttempt, + h.PasswordVerificationAttempt, } for _, point := range points { if err := point.ValidateAndPopulateExtensibilityPoint(); err != nil { diff --git a/internal/hooks/auth_hooks.go b/internal/hooks/auth_hooks.go index f76527e68..a2fc70184 100644 --- a/internal/hooks/auth_hooks.go +++ b/internal/hooks/auth_hooks.go @@ -17,8 +17,7 @@ const ( // Hook Names const ( - MFAHookRejection = "reject" - MFAHookContinue = "continue" + HookRejection = "reject" ) type HookOutput interface { @@ -33,10 +32,21 @@ type MFAVerificationAttemptInput struct { } type MFAVerificationAttemptOutput struct { - Decision string `json:"decision,omitempty"` - Message string `json:"message,omitempty"` + Decision string `json:"decision"` + Message string `json:"message"` + HookError AuthHookError `json:"error"` +} + +type PasswordVerificationAttemptInput struct { + UserID uuid.UUID `json:"user_id"` + Valid bool `json:"valid"` +} - HookError AuthHookError `json:"error,omitempty"` +type PasswordVerificationAttemptOutput struct { + Decision string `json:"decision"` + Message string `json:"message"` + ShouldLogoutUser bool `json:"should_logout_user"` + HookError AuthHookError `json:"error"` } func (mf *MFAVerificationAttemptOutput) IsError() bool { @@ -47,6 +57,14 @@ func (mf *MFAVerificationAttemptOutput) Error() string { return mf.HookError.Message } +func (p *PasswordVerificationAttemptOutput) IsError() bool { + return p.HookError.Message != "" +} + +func (p *PasswordVerificationAttemptOutput) Error() string { + return p.HookError.Message +} + type AuthHookError struct { HTTPCode int `json:"http_code,omitempty"` Message string `json:"message,omitempty"` @@ -57,5 +75,6 @@ func (a *AuthHookError) Error() string { } const ( - DefaultMFAHookRejectionMessage = "Further MFA verification attempts will be rejected." + DefaultMFAHookRejectionMessage = "Further MFA verification attempts will be rejected." + DefaultPasswordHookRejectionMessage = "Further password verification attempts will be rejected." )