From 3f84f95dd65f04cd1aff9e3e01029c9d08a88a66 Mon Sep 17 00:00:00 2001 From: Abhishek Bhardwaj Date: Wed, 19 Mar 2025 00:10:36 +0530 Subject: [PATCH 1/4] fix(auth): validate password strength in admin.createUser The admin.createUser endpoint wasn't checking password strength against configured rules, while admin.updateUser was doing this validation. This creates a security gap where users created via the admin API could have weak passwords that don't meet the configured requirements. This commit adds the missing password strength validation to ensure consistent security across all user creation paths. Fixes #1959 --- internal/api/admin.go | 6 ++++ internal/api/admin_test.go | 56 ++++++++++++++++++++++++++++---------- 2 files changed, 47 insertions(+), 15 deletions(-) diff --git a/internal/api/admin.go b/internal/api/admin.go index 9d3be8c77..0c7915b33 100644 --- a/internal/api/admin.go +++ b/internal/api/admin.go @@ -381,6 +381,12 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error { params.Password = &password } + if params.Password != nil { + if err := a.checkPasswordStrength(ctx, *params.Password); err != nil { + return err + } + } + var user *models.User if params.PasswordHash != "" { user, err = models.NewUserWithPasswordHash(params.Phone, params.Email, params.PasswordHash, aud, params.UserMetaData) diff --git a/internal/api/admin_test.go b/internal/api/admin_test.go index c72694f78..5d7e0e7e2 100644 --- a/internal/api/admin_test.go +++ b/internal/api/admin_test.go @@ -238,7 +238,7 @@ func (ts *AdminTestSuite) TestAdminUserCreate() { desc: "Only phone", params: map[string]interface{}{ "phone": "123456789", - "password": "test1", + "password": "StrongPassword123!", }, expected: map[string]interface{}{ "email": "", @@ -246,7 +246,7 @@ func (ts *AdminTestSuite) TestAdminUserCreate() { "isAuthenticated": true, "provider": "phone", "providers": []string{"phone"}, - "password": "test1", + "password": "StrongPassword123!", }, }, { @@ -254,7 +254,7 @@ func (ts *AdminTestSuite) TestAdminUserCreate() { params: map[string]interface{}{ "email": "test1@example.com", "phone": "123456789", - "password": "test1", + "password": "StrongPassword123!", }, expected: map[string]interface{}{ "email": "test1@example.com", @@ -262,7 +262,7 @@ func (ts *AdminTestSuite) TestAdminUserCreate() { "isAuthenticated": true, "provider": "email", "providers": []string{"email", "phone"}, - "password": "test1", + "password": "StrongPassword123!", }, }, { @@ -300,7 +300,7 @@ func (ts *AdminTestSuite) TestAdminUserCreate() { params: map[string]interface{}{ "email": "test4@example.com", "phone": "", - "password": "test1", + "password": "StrongPassword123!", "ban_duration": "24h", }, expected: map[string]interface{}{ @@ -309,7 +309,7 @@ func (ts *AdminTestSuite) TestAdminUserCreate() { "isAuthenticated": true, "provider": "email", "providers": []string{"email"}, - "password": "test1", + "password": "StrongPassword123!", }, }, { @@ -332,7 +332,7 @@ func (ts *AdminTestSuite) TestAdminUserCreate() { params: map[string]interface{}{ "id": "fc56ab41-2010-4870-a9b9-767c1dc573fb", "email": "test6@example.com", - "password": "test", + "password": "StrongPassword123!", // Updated to meet requirements }, expected: map[string]interface{}{ "id": "fc56ab41-2010-4870-a9b9-767c1dc573fb", @@ -341,7 +341,7 @@ func (ts *AdminTestSuite) TestAdminUserCreate() { "isAuthenticated": true, "provider": "email", "providers": []string{"email"}, - "password": "test", + "password": "StrongPassword123!", }, }, } @@ -711,7 +711,7 @@ func (ts *AdminTestSuite) TestAdminUserCreateWithDisabledLogin() { }, userData: map[string]interface{}{ "email": "test1@example.com", - "password": "test1", + "password": "StrongPassword123!", }, expected: http.StatusOK, }, @@ -727,7 +727,7 @@ func (ts *AdminTestSuite) TestAdminUserCreateWithDisabledLogin() { }, userData: map[string]interface{}{ "phone": "123456789", - "password": "test1", + "password": "StrongPassword123!", }, expected: http.StatusOK, }, @@ -739,7 +739,7 @@ func (ts *AdminTestSuite) TestAdminUserCreateWithDisabledLogin() { }, userData: map[string]interface{}{ "email": "test2@example.com", - "password": "test2", + "password": "StrongPassword123!", }, expected: http.StatusOK, }, @@ -860,20 +860,31 @@ func (ts *AdminTestSuite) TestAdminUserCreateValidationErrors() { cases := []struct { desc string params map[string]interface{} + code int }{ { desc: "create user without email and phone", params: map[string]interface{}{ - "password": "test_password", + "password": "StrongPassword123!", + }, + code: http.StatusBadRequest, + }, + { + desc: "weak password that doesn't meet the minimum length", + params: map[string]interface{}{ + "email": "test@example.com", + "password": "weak", }, + code: http.StatusUnprocessableEntity, }, { desc: "create user with password and password hash", params: map[string]interface{}{ "email": "test@example.com", - "password": "test_password", + "password": "StrongPassword123!", "password_hash": "$2y$10$Tk6yEdmTbb/eQ/haDMaCsuCsmtPVprjHMcij1RqiJdLGPDXnL3L1a", }, + code: http.StatusBadRequest, }, { desc: "invalid ban duration", @@ -881,6 +892,7 @@ func (ts *AdminTestSuite) TestAdminUserCreateValidationErrors() { "email": "test@example.com", "ban_duration": "never", }, + code: http.StatusBadRequest, }, { desc: "custom id is nil", @@ -888,6 +900,7 @@ func (ts *AdminTestSuite) TestAdminUserCreateValidationErrors() { "id": "00000000-0000-0000-0000-000000000000", "email": "test@example.com", }, + code: http.StatusBadRequest, }, { desc: "bad id format", @@ -895,8 +908,13 @@ func (ts *AdminTestSuite) TestAdminUserCreateValidationErrors() { "id": "bad_uuid_format", "email": "test@example.com", }, + code: http.StatusBadRequest, }, } + + originalMinLength := ts.Config.Password.MinLength + ts.Config.Password.MinLength = 8 + for _, c := range cases { ts.Run(c.desc, func() { var buffer bytes.Buffer @@ -905,12 +923,20 @@ func (ts *AdminTestSuite) TestAdminUserCreateValidationErrors() { req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", ts.token)) w := httptest.NewRecorder() ts.API.handler.ServeHTTP(w, req) - require.Equal(ts.T(), http.StatusBadRequest, w.Code, w) + + require.Equal(ts.T(), c.code, w.Code, "Expected status code %d but got %d for test case: %s", c.code, w.Code, c.desc) data := map[string]interface{}{} require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data)) - require.Equal(ts.T(), data["error_code"], apierrors.ErrorCodeValidationFailed) + + if c.code == http.StatusBadRequest { + require.Equal(ts.T(), data["error_code"], apierrors.ErrorCodeValidationFailed) + } else if c.code == http.StatusUnprocessableEntity && c.desc == "weak password that doesn't meet the minimum length" { + require.Equal(ts.T(), "weak_password", data["error_code"]) + } }) } + + ts.Config.Password.MinLength = originalMinLength } From 62934abb404fea59aaddd6ef9e87052c91b6a053 Mon Sep 17 00:00:00 2001 From: Abhishek Bhardwaj Date: Mon, 24 Mar 2025 17:09:50 +0530 Subject: [PATCH 2/4] feat: Add BypassPasswordCheck option for admin APIs This commit adds the ability for admin users to bypass password strength validation when creating users, updating users, or generating signup links. This feature is useful for temporary account creation where passwords will be changed on first login. - Add BypassPasswordCheck field to AdminUserParams and GenerateLinkParams - Modify adminUserCreate, adminUserUpdate, and adminGenerateLink to respect this flag - Update validateSignupParams to accept bypass parameter - Add comprehensive tests for password bypass functionality This gives administrators more flexibility while maintaining security by restricting the bypass capability to authenticated admin endpoints only. --- internal/api/admin.go | 13 +++-- internal/api/admin_test.go | 106 +++++++++++++++++++++++++++++++++++- internal/api/mail.go | 3 +- internal/api/mail_test.go | 64 ++++++++++++++++++++++ internal/api/signup.go | 11 ++-- internal/api/signup_test.go | 24 ++++++++ 6 files changed, 211 insertions(+), 10 deletions(-) diff --git a/internal/api/admin.go b/internal/api/admin.go index 0c7915b33..71d6ec6b7 100644 --- a/internal/api/admin.go +++ b/internal/api/admin.go @@ -32,6 +32,7 @@ type AdminUserParams struct { UserMetaData map[string]interface{} `json:"user_metadata"` AppMetaData map[string]interface{} `json:"app_metadata"` BanDuration string `json:"ban_duration"` + BypassPasswordCheck bool `json:"bypass_password_check"` } type adminUserDeleteParams struct { @@ -179,8 +180,10 @@ func (a *API) adminUserUpdate(w http.ResponseWriter, r *http.Request) error { if params.Password != nil { password := *params.Password - if err := a.checkPasswordStrength(ctx, password); err != nil { - return err + if !params.BypassPasswordCheck { + if err := a.checkPasswordStrength(ctx, password); err != nil { + return err + } } if err := user.SetPassword(ctx, password, config.Security.DBEncryption.Encrypt, config.Security.DBEncryption.EncryptionKeyID, config.Security.DBEncryption.EncryptionKey); err != nil { @@ -382,8 +385,10 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error { } if params.Password != nil { - if err := a.checkPasswordStrength(ctx, *params.Password); err != nil { - return err + if !params.BypassPasswordCheck { + if err := a.checkPasswordStrength(ctx, *params.Password); err != nil { + return err + } } } diff --git a/internal/api/admin_test.go b/internal/api/admin_test.go index 5d7e0e7e2..ab11f73fc 100644 --- a/internal/api/admin_test.go +++ b/internal/api/admin_test.go @@ -332,7 +332,7 @@ func (ts *AdminTestSuite) TestAdminUserCreate() { params: map[string]interface{}{ "id": "fc56ab41-2010-4870-a9b9-767c1dc573fb", "email": "test6@example.com", - "password": "StrongPassword123!", // Updated to meet requirements + "password": "StrongPassword123!", }, expected: map[string]interface{}{ "id": "fc56ab41-2010-4870-a9b9-767c1dc573fb", @@ -940,3 +940,107 @@ func (ts *AdminTestSuite) TestAdminUserCreateValidationErrors() { ts.Config.Password.MinLength = originalMinLength } + +func (ts *AdminTestSuite) TestAdminUserCreateWithBypassCheck() { + originalMinLength := ts.Config.Password.MinLength + ts.Config.Password.MinLength = 20 + + weakPassword := "short" + + ts.Run("Without bypass flag, should fail validation", func() { + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ + "email": "test@example.com", + "password": weakPassword, + })) + + req := httptest.NewRequest(http.MethodPost, "/admin/users", &buffer) + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", ts.token)) + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + + require.Equal(ts.T(), http.StatusUnprocessableEntity, w.Code) + }) + + ts.Run("With bypass flag, should succeed despite weak password", func() { + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ + "email": "test@example.com", + "password": weakPassword, + "bypass_password_check": true, + })) + + req := httptest.NewRequest(http.MethodPost, "/admin/users", &buffer) + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", ts.token)) + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + + require.Equal(ts.T(), http.StatusOK, w.Code) + + data := models.User{} + require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data)) + require.Equal(ts.T(), "test@example.com", data.GetEmail()) + + u, err := models.FindUserByID(ts.API.db, data.ID) + require.NoError(ts.T(), err) + isAuthenticated, _, err := u.Authenticate(context.Background(), ts.API.db, weakPassword, + ts.API.config.Security.DBEncryption.DecryptionKeys, + ts.API.config.Security.DBEncryption.Encrypt, + ts.API.config.Security.DBEncryption.EncryptionKeyID) + require.NoError(ts.T(), err) + require.True(ts.T(), isAuthenticated) + }) + + ts.Config.Password.MinLength = originalMinLength +} + +func (ts *AdminTestSuite) TestAdminUserUpdateWithBypassCheck() { + u, err := models.NewUser("", "test@example.com", "original", ts.Config.JWT.Aud, nil) + require.NoError(ts.T(), err, "Error making new user") + require.NoError(ts.T(), ts.API.db.Create(u), "Error creating user") + + originalMinLength := ts.Config.Password.MinLength + ts.Config.Password.MinLength = 20 + + weakPassword := "short" + + ts.Run("Without bypass flag, update should fail validation", func() { + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ + "password": weakPassword, + })) + + req := httptest.NewRequest(http.MethodPut, fmt.Sprintf("/admin/users/%s", u.ID), &buffer) + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", ts.token)) + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + + require.Equal(ts.T(), http.StatusUnprocessableEntity, w.Code) + }) + + ts.Run("With bypass flag, update should succeed despite weak password", func() { + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ + "password": weakPassword, + "bypass_password_check": true, + })) + + req := httptest.NewRequest(http.MethodPut, fmt.Sprintf("/admin/users/%s", u.ID), &buffer) + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", ts.token)) + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + + require.Equal(ts.T(), http.StatusOK, w.Code) + + updatedUser, err := models.FindUserByID(ts.API.db, u.ID) + require.NoError(ts.T(), err) + isAuthenticated, _, err := updatedUser.Authenticate(context.Background(), ts.API.db, weakPassword, + ts.API.config.Security.DBEncryption.DecryptionKeys, + ts.API.config.Security.DBEncryption.Encrypt, + ts.API.config.Security.DBEncryption.EncryptionKeyID) + require.NoError(ts.T(), err) + require.True(ts.T(), isAuthenticated) + }) + + ts.Config.Password.MinLength = originalMinLength +} diff --git a/internal/api/mail.go b/internal/api/mail.go index 31fc309ab..b5026bbf1 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -34,6 +34,7 @@ type GenerateLinkParams struct { Password string `json:"password"` Data map[string]interface{} `json:"data"` RedirectTo string `json:"redirect_to"` + BypassPasswordCheck bool `json:"bypass_password_check"` } type GenerateLinkResponse struct { @@ -102,7 +103,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { Aud: aud, } - if err := a.validateSignupParams(ctx, signupParams); err != nil { + if err := a.validateSignupParams(ctx, signupParams, params.BypassPasswordCheck); err != nil { return err } diff --git a/internal/api/mail_test.go b/internal/api/mail_test.go index c13c18e69..571df2d14 100644 --- a/internal/api/mail_test.go +++ b/internal/api/mail_test.go @@ -255,3 +255,67 @@ func (ts *MailTestSuite) setURIAllowListMap(uris ...string) { ts.Config.URIAllowListMap[uri] = g } } + +func (ts *MailTestSuite) TestGenerateLinkWithBypassCheck() { + claims := &AccessTokenClaims{ + Role: "supabase_admin", + } + token, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString([]byte(ts.Config.JWT.Secret)) + require.NoError(ts.T(), err, "Error generating admin jwt") + + ts.setURIAllowListMap("http://localhost:8000/**") + + originalMinLength := ts.Config.Password.MinLength + ts.Config.Password.MinLength = 20 + + weakPassword := "short" + + customDomainUrl, err := url.ParseRequestURI("https://example.gotrue.com") + require.NoError(ts.T(), err) + + originalHosts := ts.API.config.Mailer.ExternalHosts + ts.API.config.Mailer.ExternalHosts = []string{ + "example.gotrue.com", + } + + ts.Run("Generate signup link without bypass should fail", func() { + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(GenerateLinkParams{ + Email: "non-existent-user@example.com", + Password: weakPassword, + Type: "signup", + })) + + req := httptest.NewRequest(http.MethodPost, customDomainUrl.String()+"/admin/generate_link", &buffer) + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + + require.Equal(ts.T(), http.StatusUnprocessableEntity, w.Code) + }) + + ts.Run("Generate signup link with bypass should succeed", func() { + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(GenerateLinkParams{ + Email: "non-existent-user@example.com", + Password: weakPassword, + Type: "signup", + BypassPasswordCheck: true, + })) + + req := httptest.NewRequest(http.MethodPost, customDomainUrl.String()+"/admin/generate_link", &buffer) + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + + require.Equal(ts.T(), http.StatusOK, w.Code) + + data := make(map[string]interface{}) + require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data)) + require.Contains(ts.T(), data, "action_link") + require.Equal(ts.T(), "signup", data["verification_type"]) + }) + + ts.Config.Password.MinLength = originalMinLength + ts.API.config.Mailer.ExternalHosts = originalHosts +} diff --git a/internal/api/signup.go b/internal/api/signup.go index f3f89a77c..868ad60e6 100644 --- a/internal/api/signup.go +++ b/internal/api/signup.go @@ -29,16 +29,19 @@ type SignupParams struct { CodeChallenge string `json:"code_challenge"` } -func (a *API) validateSignupParams(ctx context.Context, p *SignupParams) error { +func (a *API) validateSignupParams(ctx context.Context, p *SignupParams, bypassPasswordCheck bool) error { config := a.config if p.Password == "" { return badRequestError(apierrors.ErrorCodeValidationFailed, "Signup requires a valid password") } - if err := a.checkPasswordStrength(ctx, p.Password); err != nil { - return err + if !bypassPasswordCheck { + if err := a.checkPasswordStrength(ctx, p.Password); err != nil { + return err + } } + if p.Email != "" && p.Phone != "" { return badRequestError(apierrors.ErrorCodeValidationFailed, "Only an email address or phone number should be provided on signup.") } @@ -123,7 +126,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error { params.ConfigureDefaults() - if err := a.validateSignupParams(ctx, params); err != nil { + if err := a.validateSignupParams(ctx, params, false); err != nil { return err } diff --git a/internal/api/signup_test.go b/internal/api/signup_test.go index 3f4783261..7e0c02da2 100644 --- a/internal/api/signup_test.go +++ b/internal/api/signup_test.go @@ -2,6 +2,7 @@ package api import ( "bytes" + "context" "encoding/json" "fmt" "net/http" @@ -151,3 +152,26 @@ func (ts *SignupTestSuite) TestVerifySignup() { require.NotEmpty(ts.T(), v.Get("expires_in")) require.NotEmpty(ts.T(), v.Get("refresh_token")) } + +func (ts *SignupTestSuite) TestValidateSignupParamsWithBypass() { + originalMinLength := ts.Config.Password.MinLength + ts.Config.Password.MinLength = 20 + + weakPassword := "short" + + params := &SignupParams{ + Email: "non-existing@example.com", + Password: weakPassword, + Provider: "email", + Aud: ts.Config.JWT.Aud, + } + + err := ts.API.validateSignupParams(context.Background(), params, false) + require.Error(ts.T(), err, "Should fail validation without bypass") + require.Contains(ts.T(), err.Error(), "should be at least") + + err = ts.API.validateSignupParams(context.Background(), params, true) + require.NoError(ts.T(), err, "Should bypass validation with flag set") + + ts.Config.Password.MinLength = originalMinLength +} From 312c766cdd479d46f235b9a23897f39b690f413e Mon Sep 17 00:00:00 2001 From: Abhishek Bhardwaj Date: Wed, 2 Apr 2025 23:50:10 +0530 Subject: [PATCH 3/4] feat: Add EnforcePasswordCheck option for admin APIs Add EnforcePasswordCheck parameter to Admin APIs to enforce password strength validation when: - Creating users via adminUserCreate - Updating users via adminUserUpdate - Generating signup links via adminGenerateLink - Validating signup params via validateSignupParams The bool EnforcePasswordCheck is by default false. Setting the bool to true will ensure the password strength validation --- internal/api/admin.go | 6 ++-- internal/api/admin_test.go | 61 ++++++++++++++++++++++++++----------- internal/api/mail.go | 4 +-- internal/api/mail_test.go | 31 +++++++++---------- internal/api/signup.go | 4 +-- internal/api/signup_test.go | 8 ++--- 6 files changed, 69 insertions(+), 45 deletions(-) diff --git a/internal/api/admin.go b/internal/api/admin.go index 71d6ec6b7..f6068ea0f 100644 --- a/internal/api/admin.go +++ b/internal/api/admin.go @@ -32,7 +32,7 @@ type AdminUserParams struct { UserMetaData map[string]interface{} `json:"user_metadata"` AppMetaData map[string]interface{} `json:"app_metadata"` BanDuration string `json:"ban_duration"` - BypassPasswordCheck bool `json:"bypass_password_check"` + EnforcePasswordCheck bool `json:"enforce_password_check"` } type adminUserDeleteParams struct { @@ -180,7 +180,7 @@ func (a *API) adminUserUpdate(w http.ResponseWriter, r *http.Request) error { if params.Password != nil { password := *params.Password - if !params.BypassPasswordCheck { + if params.EnforcePasswordCheck { if err := a.checkPasswordStrength(ctx, password); err != nil { return err } @@ -385,7 +385,7 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error { } if params.Password != nil { - if !params.BypassPasswordCheck { + if params.EnforcePasswordCheck { if err := a.checkPasswordStrength(ctx, *params.Password); err != nil { return err } diff --git a/internal/api/admin_test.go b/internal/api/admin_test.go index ab11f73fc..c4395f690 100644 --- a/internal/api/admin_test.go +++ b/internal/api/admin_test.go @@ -505,6 +505,7 @@ func (ts *AdminTestSuite) TestAdminUserUpdatePasswordFailed() { var buffer bytes.Buffer require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ "password": "", + "enforce_password_check": true, })) // Setup request @@ -874,6 +875,7 @@ func (ts *AdminTestSuite) TestAdminUserCreateValidationErrors() { params: map[string]interface{}{ "email": "test@example.com", "password": "weak", + "enforce_password_check": true, }, code: http.StatusUnprocessableEntity, }, @@ -941,17 +943,18 @@ func (ts *AdminTestSuite) TestAdminUserCreateValidationErrors() { ts.Config.Password.MinLength = originalMinLength } -func (ts *AdminTestSuite) TestAdminUserCreateWithBypassCheck() { +func (ts *AdminTestSuite) TestAdminUserCreateWithEnforcePasswordCheck() { originalMinLength := ts.Config.Password.MinLength ts.Config.Password.MinLength = 20 weakPassword := "short" - ts.Run("Without bypass flag, should fail validation", func() { + ts.Run("With enforce flag, should fail validation", func() { var buffer bytes.Buffer require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ - "email": "test@example.com", - "password": weakPassword, + "email": "test@example.com", + "password": weakPassword, + "enforce_password_check": true, })) req := httptest.NewRequest(http.MethodPost, "/admin/users", &buffer) @@ -962,12 +965,12 @@ func (ts *AdminTestSuite) TestAdminUserCreateWithBypassCheck() { require.Equal(ts.T(), http.StatusUnprocessableEntity, w.Code) }) - ts.Run("With bypass flag, should succeed despite weak password", func() { + ts.Run("Without enforce flag, should succeed despite weak password", func() { var buffer bytes.Buffer require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ - "email": "test@example.com", - "password": weakPassword, - "bypass_password_check": true, + "email": "test@example.com", + "password": weakPassword, + "enforce_password_check": false, })) req := httptest.NewRequest(http.MethodPost, "/admin/users", &buffer) @@ -994,17 +997,17 @@ func (ts *AdminTestSuite) TestAdminUserCreateWithBypassCheck() { ts.Config.Password.MinLength = originalMinLength } -func (ts *AdminTestSuite) TestAdminUserUpdateWithBypassCheck() { +func (ts *AdminTestSuite) TestAdminUserUpdateWithEnforcePasswordCheck() { u, err := models.NewUser("", "test@example.com", "original", ts.Config.JWT.Aud, nil) require.NoError(ts.T(), err, "Error making new user") require.NoError(ts.T(), ts.API.db.Create(u), "Error creating user") originalMinLength := ts.Config.Password.MinLength - ts.Config.Password.MinLength = 20 + ts.Config.Password.MinLength = 20 // Set a high minimum to ensure our test password is "weak" weakPassword := "short" - ts.Run("Without bypass flag, update should fail validation", func() { + ts.Run("Without enforce_password_check parameter (default), should accept weak password", func() { var buffer bytes.Buffer require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ "password": weakPassword, @@ -1015,14 +1018,23 @@ func (ts *AdminTestSuite) TestAdminUserUpdateWithBypassCheck() { w := httptest.NewRecorder() ts.API.handler.ServeHTTP(w, req) - require.Equal(ts.T(), http.StatusUnprocessableEntity, w.Code) + require.Equal(ts.T(), http.StatusOK, w.Code, "Expected weak password to be accepted when enforce_password_check is not provided") + + updatedUser, err := models.FindUserByID(ts.API.db, u.ID) + require.NoError(ts.T(), err) + isAuthenticated, _, err := updatedUser.Authenticate(context.Background(), ts.API.db, weakPassword, + ts.API.config.Security.DBEncryption.DecryptionKeys, + ts.API.config.Security.DBEncryption.Encrypt, + ts.API.config.Security.DBEncryption.EncryptionKeyID) + require.NoError(ts.T(), err) + require.True(ts.T(), isAuthenticated, "Should be able to authenticate with the weak password") }) - ts.Run("With bypass flag, update should succeed despite weak password", func() { + ts.Run("With enforce_password_check=false, should accept weak password", func() { var buffer bytes.Buffer require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ - "password": weakPassword, - "bypass_password_check": true, + "password": weakPassword, + "enforce_password_check": false, })) req := httptest.NewRequest(http.MethodPut, fmt.Sprintf("/admin/users/%s", u.ID), &buffer) @@ -1030,7 +1042,7 @@ func (ts *AdminTestSuite) TestAdminUserUpdateWithBypassCheck() { w := httptest.NewRecorder() ts.API.handler.ServeHTTP(w, req) - require.Equal(ts.T(), http.StatusOK, w.Code) + require.Equal(ts.T(), http.StatusOK, w.Code, "Expected weak password to be accepted when enforce_password_check=false") updatedUser, err := models.FindUserByID(ts.API.db, u.ID) require.NoError(ts.T(), err) @@ -1039,7 +1051,22 @@ func (ts *AdminTestSuite) TestAdminUserUpdateWithBypassCheck() { ts.API.config.Security.DBEncryption.Encrypt, ts.API.config.Security.DBEncryption.EncryptionKeyID) require.NoError(ts.T(), err) - require.True(ts.T(), isAuthenticated) + require.True(ts.T(), isAuthenticated, "Should be able to authenticate with the weak password") + }) + + ts.Run("With enforce_password_check=true, should reject weak password", func() { + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ + "password": weakPassword, + "enforce_password_check": true, + })) + + req := httptest.NewRequest(http.MethodPut, fmt.Sprintf("/admin/users/%s", u.ID), &buffer) + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", ts.token)) + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + + require.Equal(ts.T(), http.StatusUnprocessableEntity, w.Code, "Expected weak password to be rejected when enforce_password_check=true") }) ts.Config.Password.MinLength = originalMinLength diff --git a/internal/api/mail.go b/internal/api/mail.go index b5026bbf1..0ffd00131 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -34,7 +34,7 @@ type GenerateLinkParams struct { Password string `json:"password"` Data map[string]interface{} `json:"data"` RedirectTo string `json:"redirect_to"` - BypassPasswordCheck bool `json:"bypass_password_check"` + EnforcePasswordCheck bool `json:"enforce_password_check"` } type GenerateLinkResponse struct { @@ -103,7 +103,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { Aud: aud, } - if err := a.validateSignupParams(ctx, signupParams, params.BypassPasswordCheck); err != nil { + if err := a.validateSignupParams(ctx, signupParams, params.EnforcePasswordCheck); err != nil { return err } diff --git a/internal/api/mail_test.go b/internal/api/mail_test.go index 571df2d14..f27d76cae 100644 --- a/internal/api/mail_test.go +++ b/internal/api/mail_test.go @@ -256,7 +256,7 @@ func (ts *MailTestSuite) setURIAllowListMap(uris ...string) { } } -func (ts *MailTestSuite) TestGenerateLinkWithBypassCheck() { +func (ts *MailTestSuite) TestGenerateLinkWithEnforcePasswordCheck() { claims := &AccessTokenClaims{ Role: "supabase_admin", } @@ -278,12 +278,13 @@ func (ts *MailTestSuite) TestGenerateLinkWithBypassCheck() { "example.gotrue.com", } - ts.Run("Generate signup link without bypass should fail", func() { + ts.Run("Generate signup link without enforce should succeed", func() { var buffer bytes.Buffer require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(GenerateLinkParams{ - Email: "non-existent-user@example.com", - Password: weakPassword, - Type: "signup", + Email: "non-existent-user@example.com", + Password: weakPassword, + Type: "signup", + EnforcePasswordCheck: false, })) req := httptest.NewRequest(http.MethodPost, customDomainUrl.String()+"/admin/generate_link", &buffer) @@ -291,16 +292,17 @@ func (ts *MailTestSuite) TestGenerateLinkWithBypassCheck() { w := httptest.NewRecorder() ts.API.handler.ServeHTTP(w, req) - require.Equal(ts.T(), http.StatusUnprocessableEntity, w.Code) + require.Equal(ts.T(), http.StatusOK, w.Code) + // ... }) - ts.Run("Generate signup link with bypass should succeed", func() { + ts.Run("Generate signup link with enforce should fail", func() { var buffer bytes.Buffer require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(GenerateLinkParams{ - Email: "non-existent-user@example.com", - Password: weakPassword, - Type: "signup", - BypassPasswordCheck: true, + Email: "non-existent1-user@example.com", + Password: weakPassword, + Type: "signup", + EnforcePasswordCheck: true, })) req := httptest.NewRequest(http.MethodPost, customDomainUrl.String()+"/admin/generate_link", &buffer) @@ -308,12 +310,7 @@ func (ts *MailTestSuite) TestGenerateLinkWithBypassCheck() { w := httptest.NewRecorder() ts.API.handler.ServeHTTP(w, req) - require.Equal(ts.T(), http.StatusOK, w.Code) - - data := make(map[string]interface{}) - require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data)) - require.Contains(ts.T(), data, "action_link") - require.Equal(ts.T(), "signup", data["verification_type"]) + require.Equal(ts.T(), http.StatusUnprocessableEntity, w.Code) }) ts.Config.Password.MinLength = originalMinLength diff --git a/internal/api/signup.go b/internal/api/signup.go index 868ad60e6..549d5d45f 100644 --- a/internal/api/signup.go +++ b/internal/api/signup.go @@ -29,14 +29,14 @@ type SignupParams struct { CodeChallenge string `json:"code_challenge"` } -func (a *API) validateSignupParams(ctx context.Context, p *SignupParams, bypassPasswordCheck bool) error { +func (a *API) validateSignupParams(ctx context.Context, p *SignupParams, enforcePasswordCheck bool) error { config := a.config if p.Password == "" { return badRequestError(apierrors.ErrorCodeValidationFailed, "Signup requires a valid password") } - if !bypassPasswordCheck { + if enforcePasswordCheck { if err := a.checkPasswordStrength(ctx, p.Password); err != nil { return err } diff --git a/internal/api/signup_test.go b/internal/api/signup_test.go index 7e0c02da2..5182330e8 100644 --- a/internal/api/signup_test.go +++ b/internal/api/signup_test.go @@ -153,7 +153,7 @@ func (ts *SignupTestSuite) TestVerifySignup() { require.NotEmpty(ts.T(), v.Get("refresh_token")) } -func (ts *SignupTestSuite) TestValidateSignupParamsWithBypass() { +func (ts *SignupTestSuite) TestValidateSignupParamsWithEnforcePasswordCheck() { originalMinLength := ts.Config.Password.MinLength ts.Config.Password.MinLength = 20 @@ -167,11 +167,11 @@ func (ts *SignupTestSuite) TestValidateSignupParamsWithBypass() { } err := ts.API.validateSignupParams(context.Background(), params, false) - require.Error(ts.T(), err, "Should fail validation without bypass") - require.Contains(ts.T(), err.Error(), "should be at least") + require.NoError(ts.T(), err, "Should bypass validation with enforcePasswordCheck=false") err = ts.API.validateSignupParams(context.Background(), params, true) - require.NoError(ts.T(), err, "Should bypass validation with flag set") + require.Error(ts.T(), err, "Should fail validation with enforcePasswordCheck=true") + require.Contains(ts.T(), err.Error(), "should be at least") ts.Config.Password.MinLength = originalMinLength } From 30160574c52c0168a5b1134b0e87e33d725f0c0d Mon Sep 17 00:00:00 2001 From: Abhishek Bhardwaj Date: Tue, 15 Apr 2025 20:41:19 +0530 Subject: [PATCH 4/4] Added documentation for the enforce_password_check parameter as well as for the missing /admin/users API POST endpoint --- openapi.yaml | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/openapi.yaml b/openapi.yaml index 009a140f5..8a13018b5 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -1222,6 +1222,8 @@ paths: redirect_to: type: string format: uri + enforce_password_check: + type: boolean responses: 200: description: User profile and generated link information. @@ -1401,6 +1403,57 @@ paths: $ref: "#/components/responses/UnauthorizedResponse" 403: $ref: "#/components/responses/ForbiddenResponse" + post: + summary: Create a new user . + tags: + - admin + security: + - APIKeyAuth: [] + AdminAuth: [] + requestBody: + content: + application/json: + schema: + type: object + properties: + id: + type: string + format: uuid + email: + type: string + format: email + phone: + type: string + format: phone + password: + type: string + password_hash: + type: string + email_confirm: + type: boolean + phone_confirm: + type: boolean + user_metadata: + type: object + app_metadata: + type: object + ban_duration: + type: string + enforce_password_check: + type: boolean + responses: + 200: + description: User was successfully created. + content: + application/json: + schema: + $ref: "#/components/schemas/UserSchema" + 400: + $ref: "#/components/responses/BadRequestResponse" + 401: + $ref: "#/components/responses/UnauthorizedResponse" + 403: + $ref: "#/components/responses/ForbiddenResponse" /admin/users/{userId}: parameters: @@ -1446,6 +1499,8 @@ paths: application/json: schema: $ref: "#/components/schemas/UserSchema" + enforce_password_check: + type: boolean responses: 200: description: User's account data was updated.