Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions api/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down
6 changes: 4 additions & 2 deletions api/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 }()
Expand All @@ -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},
},
}))

Expand All @@ -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() {
Expand Down
30 changes: 25 additions & 5 deletions api/password.go
Original file line number Diff line number Diff line change
@@ -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
}
38 changes: 28 additions & 10 deletions api/password_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion api/signup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 20 additions & 0 deletions api/signup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
}
6 changes: 2 additions & 4 deletions api/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 2 additions & 4 deletions api/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
8 changes: 8 additions & 0 deletions conf/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down
18 changes: 18 additions & 0 deletions conf/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading