From aea52745eeb595e45a0ea9e67cdce0fc38b65b4a Mon Sep 17 00:00:00 2001 From: Vaibhav Acharya Date: Mon, 1 Jun 2026 15:00:32 +0530 Subject: [PATCH] feat: enforce configurable minimum password length when strict --- api/admin.go | 10 ++++------ api/instance_test.go | 6 ++++-- api/password.go | 30 +++++++++++++++++++++++++----- api/password_test.go | 38 ++++++++++++++++++++++++++++---------- api/signup.go | 2 +- api/signup_test.go | 20 ++++++++++++++++++++ api/user.go | 6 ++---- api/verify.go | 6 ++---- conf/configuration.go | 8 ++++++++ conf/configuration_test.go | 18 ++++++++++++++++++ 10 files changed, 112 insertions(+), 32 deletions(-) diff --git a/api/admin.go b/api/admin.go index 14005b61c..e935122d0 100644 --- a/api/admin.go +++ b/api/admin.go @@ -137,10 +137,8 @@ func (a *API) adminUserUpdate(w http.ResponseWriter, r *http.Request) error { return err } - if params.Password != "" { - if err := validatePassword(params.Password); err != nil { - return err - } + if err := validatePassword(a.getConfig(ctx), params.Password); err != nil { + return err } err = a.db.Transaction(func(tx *storage.Connection) error { @@ -211,11 +209,11 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error { return err } - if err := a.validateEmail(ctx, params.Email); err != nil { + if err := validatePassword(a.getConfig(ctx), params.Password); err != nil { return err } - if err := validatePassword(params.Password); err != nil { + if err := a.validateEmail(ctx, params.Email); err != nil { return err } diff --git a/api/instance_test.go b/api/instance_test.go index d6fda6d40..d40ab93be 100644 --- a/api/instance_test.go +++ b/api/instance_test.go @@ -104,7 +104,8 @@ func (ts *InstanceTestSuite) TestCreate_SecureByDefaultFlipsEnabled() { func (ts *InstanceTestSuite) TestCreate_SecureByDefaultPreservesExplicitConfig() { // The secure-by-default flip only fires when Security.Strict is false, so a - // caller that explicitly enables it keeps that value untouched. + // caller that explicitly enables it (and sets other Security fields) keeps + // those values untouched. prev := ts.API.config.NewInstancesSecureByDefault ts.API.config.NewInstancesSecureByDefault = true defer func() { ts.API.config.NewInstancesSecureByDefault = prev }() @@ -115,7 +116,7 @@ func (ts *InstanceTestSuite) TestCreate_SecureByDefaultPreservesExplicitConfig() "uuid": freshUUID, "config": map[string]interface{}{ "jwt": map[string]interface{}{"secret": "testsecret"}, - "security": map[string]interface{}{"strict": true}, + "security": map[string]interface{}{"strict": true, "min_password_length": 12}, }, })) @@ -129,6 +130,7 @@ func (ts *InstanceTestSuite) TestCreate_SecureByDefaultPreservesExplicitConfig() i, err := models.GetInstanceByUUID(ts.API.db, freshUUID) require.NoError(ts.T(), err) assert.True(ts.T(), i.BaseConfig.Security.Strict) + assert.Equal(ts.T(), 12, i.BaseConfig.Security.MinPasswordLength) } func (ts *InstanceTestSuite) TestCreate_SecureByDefaultOverridesExplicitFalse() { diff --git a/api/password.go b/api/password.go index bf99728d3..b8a1ceba2 100644 --- a/api/password.go +++ b/api/password.go @@ -1,13 +1,33 @@ package api -import "github.com/netlify/gotrue/models" +import ( + "unicode/utf8" -// validatePassword rejects passwords that violate GoTrue's server-side rules. -// Empty passwords are accepted here; callers that require a non-empty password -// (e.g. signup) check that separately. -func validatePassword(password string) error { + "github.com/netlify/gotrue/conf" + "github.com/netlify/gotrue/models" +) + +// validatePassword applies the server-side password rules: +// - rejects anything longer than bcrypt's input limit (always-on) +// - enforces a minimum length when the instance opts in to strict security +// +// Empty passwords are accepted here; callers that require a non-empty +// password (e.g. signup) check that separately. +func validatePassword(config *conf.Configuration, password string) error { + if password == "" { + return nil + } if len(password) > models.MaxPasswordLength { return unprocessableEntityError("Password exceeds the maximum length of %d bytes", models.MaxPasswordLength) } + // Count runes, not bytes, so the policy matches the error wording + // ("characters") and so that a few multi-byte glyphs can't satisfy a + // length policy meant to enforce real complexity. + if config.Security.Strict && utf8.RuneCountInString(password) < config.Security.MinPasswordLength { + return unprocessableEntityError( + "Password must be at least %d characters long", + config.Security.MinPasswordLength, + ) + } return nil } diff --git a/api/password_test.go b/api/password_test.go index ad06a291d..a05478752 100644 --- a/api/password_test.go +++ b/api/password_test.go @@ -4,28 +4,46 @@ import ( "strings" "testing" + "github.com/netlify/gotrue/conf" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestValidatePassword(t *testing.T) { - tests := []struct { + strict := &conf.Configuration{Security: conf.SecurityConfiguration{ + Strict: true, + MinPasswordLength: 8, + }} + legacy := &conf.Configuration{} + + cases := []struct { name string + config *conf.Configuration password string wantErr bool }{ - {"empty is accepted here", "", false}, - {"short password", "hunter2", false}, - {"exactly 72 bytes", strings.Repeat("a", 72), false}, - {"73 bytes is rejected", strings.Repeat("a", 73), true}, + {"legacy empty", legacy, "", false}, + {"legacy single char", legacy, "a", false}, + {"legacy at 72 bytes", legacy, strings.Repeat("a", 72), false}, + {"legacy 73 bytes rejected", legacy, strings.Repeat("a", 73), true}, // 4-byte emoji * 19 = 76 bytes total - {"long emoji string rejected", strings.Repeat("😀", 19), true}, + {"legacy long emoji rejected", legacy, strings.Repeat("😀", 19), true}, + {"strict empty (callers handle)", strict, "", false}, + {"strict below min", strict, "short", true}, + {"strict at min", strict, "12345678", false}, + {"strict above min", strict, "longer-password", false}, + {"strict 73 bytes rejected (max takes precedence)", strict, strings.Repeat("a", 73), true}, + // Rune-count semantics: 3 emoji = 12 bytes but only 3 characters, + // so it must fail an 8-char policy. 8 emoji = 32 bytes / 8 chars, + // so it must pass. + {"strict 3 emoji rejected as 3 chars", strict, strings.Repeat("😀", 3), true}, + {"strict 8 emoji accepted as 8 chars", strict, strings.Repeat("😀", 8), false}, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := validatePassword(tt.password) - if tt.wantErr { + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validatePassword(tc.config, tc.password) + if tc.wantErr { require.Error(t, err) he, ok := err.(*HTTPError) require.True(t, ok, "expected *HTTPError, got %T", err) diff --git a/api/signup.go b/api/signup.go index d5b683f96..c10e0e1ef 100644 --- a/api/signup.go +++ b/api/signup.go @@ -37,7 +37,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error { if params.Password == "" { return unprocessableEntityError("Signup requires a valid password") } - if err := validatePassword(params.Password); err != nil { + if err := validatePassword(config, params.Password); err != nil { return err } if err := a.validateEmail(ctx, params.Email); err != nil { diff --git a/api/signup_test.go b/api/signup_test.go index 59b383eb7..f3393ffb5 100644 --- a/api/signup_test.go +++ b/api/signup_test.go @@ -43,6 +43,7 @@ func TestSignup(t *testing.T) { func (ts *SignupTestSuite) SetupTest() { require.NoError(ts.T(), models.TruncateAll(ts.API.db)) ts.Config.Webhook = conf.WebhookConfig{} + ts.Config.Security = conf.SecurityConfiguration{} } // TestSignup tests API /signup route @@ -259,3 +260,22 @@ func (ts *SignupTestSuite) TestVerifySignup() { assert.Equal(ts.T(), http.StatusOK, w.Code, w.Body.String()) } + +// TestSignup_StrictRejectsShortPassword exercises the validatePassword call +// at the signup entry point under the strict policy. +func (ts *SignupTestSuite) TestSignup_StrictRejectsShortPassword() { + ts.Config.Security.Strict = true + ts.Config.Security.MinPasswordLength = 8 + + var buf bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buf).Encode(map[string]interface{}{ + "email": "shortpw@example.com", + "password": "short", + })) + req := httptest.NewRequest(http.MethodPost, "/signup", &buf) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + + require.Equal(ts.T(), http.StatusUnprocessableEntity, w.Code, w.Body.String()) +} diff --git a/api/user.go b/api/user.go index a7256d6c2..902c11c33 100644 --- a/api/user.go +++ b/api/user.go @@ -62,10 +62,8 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error { return badRequestError("Could not read User Update params: %v", err) } - if params.Password != "" { - if err := validatePassword(params.Password); err != nil { - return err - } + if err := validatePassword(config, params.Password); err != nil { + return err } claims := getClaims(ctx) diff --git a/api/verify.go b/api/verify.go index f71d0dd66..ea9fc3d85 100644 --- a/api/verify.go +++ b/api/verify.go @@ -39,10 +39,8 @@ func (a *API) Verify(w http.ResponseWriter, r *http.Request) error { return unprocessableEntityError("Verify requires a token") } - if params.Password != "" { - if err := validatePassword(params.Password); err != nil { - return err - } + if err := validatePassword(config, params.Password); err != nil { + return err } var ( diff --git a/conf/configuration.go b/conf/configuration.go index 63fbdc172..b76152b8d 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -125,6 +125,10 @@ type MailerConfiguration struct { // follow-up changes. type SecurityConfiguration struct { Strict bool `json:"strict"` + + // MinPasswordLength is the minimum password length enforced when Strict. + // Falls back to 8 via ApplyDefaults when zero. + MinPasswordLength int `json:"min_password_length" split_words:"true"` } // Configuration holds all the per-instance configuration. @@ -261,6 +265,10 @@ func (config *Configuration) ApplyDefaults() { if config.Cookie.Duration == 0 { config.Cookie.Duration = 86400 } + + if config.Security.Strict && config.Security.MinPasswordLength <= 0 { + config.Security.MinPasswordLength = 8 + } } func (config *Configuration) Value() (driver.Value, error) { diff --git a/conf/configuration_test.go b/conf/configuration_test.go index 5e15d29e8..c5f04215e 100644 --- a/conf/configuration_test.go +++ b/conf/configuration_test.go @@ -79,6 +79,24 @@ func TestSecurityConfigurationDefaults(t *testing.T) { require.NoError(t, err) assert.True(t, c.Security.Strict) }) + + t.Run("min password length defaults to 8 when enabled", func(t *testing.T) { + baseEnv() + os.Setenv("GOTRUE_SECURITY_STRICT", "true") + c, err := LoadConfig("") + require.NoError(t, err) + assert.True(t, c.Security.Strict) + assert.Equal(t, 8, c.Security.MinPasswordLength) + }) + + t.Run("respects explicit min password length", func(t *testing.T) { + baseEnv() + os.Setenv("GOTRUE_SECURITY_STRICT", "true") + os.Setenv("GOTRUE_SECURITY_MIN_PASSWORD_LENGTH", "12") + c, err := LoadConfig("") + require.NoError(t, err) + assert.Equal(t, 12, c.Security.MinPasswordLength) + }) } func TestNewInstancesSecureByDefault(t *testing.T) {