Skip to content

Commit

Permalink
feat: add required characters password strength check (#1323)
Browse files Browse the repository at this point in the history
Adds the `GOTRUE_PASSWORD_REQUIRED_CHARACTERS` config option, which if
set, will reject passwords that do not contain at least one character of
each set of characters.

It is defined like so: `abc...xyz:0123...89`. This means that at least
one lowercase and one digit has to be present in the password to be
accepted. All other characters are also allowed. To include the `:`
character, escape it with `\:`.

When a weak password is detected, the HTTP 429 error is sent with an
additional JSON field `weak_password` that includes a `reasons` property
-- an array of the strings:

- `length` if the password is not long enough
- `characters` if the password does not use all required character sets

---------

Co-authored-by: Kang Ming <[email protected]>
  • Loading branch information
hf and kangmingtay authored Nov 28, 2023
1 parent 0540c7f commit 3991bdb
Show file tree
Hide file tree
Showing 7 changed files with 286 additions and 24 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ Rate limit the number of emails sent per hr on the following endpoints: `/signup

Minimum password length, defaults to 6.

`GOTRUE_PASSWORD_REQUIRED_CHARACTERS` - a string of character sets separated by `:`. A password must contain at least one character of each set to be accepted. To use the `:` character escape it with `\`.

`GOTRUE_SECURITY_REFRESH_TOKEN_ROTATION_ENABLED` - `bool`

If refresh token rotation is enabled, gotrue will automatically detect malicious attempts to reuse a revoked refresh token. When a malicious attempt is detected, gotrue immediately revokes all tokens that descended from the offending token.
Expand Down
4 changes: 2 additions & 2 deletions internal/api/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ func (ts *AdminTestSuite) TestAdminUserUpdatePasswordFailed() {
require.NoError(ts.T(), ts.API.db.Create(u), "Error creating user")

var updateEndpoint = fmt.Sprintf("/admin/users/%s", u.ID)
ts.Config.PasswordMinLength = 6
ts.Config.Password.MinLength = 6
ts.Run("Password doesn't meet minimum length", func() {
var buffer bytes.Buffer
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
Expand All @@ -479,7 +479,7 @@ func (ts *AdminTestSuite) TestAdminUserUpdateBannedUntilFailed() {
require.NoError(ts.T(), ts.API.db.Create(u), "Error creating user")

var updateEndpoint = fmt.Sprintf("/admin/users/%s", u.ID)
ts.Config.PasswordMinLength = 6
ts.Config.Password.MinLength = 6
ts.Run("Incorrect format for ban_duration", func() {
var buffer bytes.Buffer
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
Expand Down
20 changes: 16 additions & 4 deletions internal/api/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,6 @@ func (e *OAuthError) Cause() error {
return e
}

func invalidPasswordLengthError(passwordMinLength int) *HTTPError {
return unprocessableEntityError(fmt.Sprintf("Password should be at least %d characters", passwordMinLength))
}

func invalidSignupError(config *conf.GlobalConfiguration) *HTTPError {
var msg string
if config.External.Email.Enabled && config.External.Phone.Enabled {
Expand Down Expand Up @@ -245,6 +241,22 @@ func handleError(err error, w http.ResponseWriter, r *http.Request) {
log := observability.GetLogEntry(r)
errorID := getRequestID(r.Context())
switch e := err.(type) {
case *WeakPasswordError:
var output struct {
HTTPError
Payload struct {
Reasons []string `json:"reasons,omitempty"`
} `json:"weak_password,omitempty"`
}

output.Code = http.StatusUnprocessableEntity
output.Message = e.Message
output.Payload.Reasons = e.Reasons

if jsonErr := sendJSON(w, output.Code, output); jsonErr != nil {
handleError(jsonErr, w, r)
}

case *HTTPError:
if e.Code >= http.StatusInternalServerError {
e.ErrorID = errorID
Expand Down
43 changes: 40 additions & 3 deletions internal/api/password.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,49 @@
package api

import "context"
import (
"context"
"fmt"
"strings"
)

// WeakPasswordError encodes an error that a password does not meet strength
// requirements. It is handled specially in errors.go as it gets transformed to
// a HTTPError with a special weak_password field that encodes the Reasons
// slice.
type WeakPasswordError struct {
Message string
Reasons []string
}

func (e *WeakPasswordError) Error() string {
return e.Message
}

func (a *API) checkPasswordStrength(ctx context.Context, password string) error {
config := a.config

if len(password) < config.PasswordMinLength {
return invalidPasswordLengthError(config.PasswordMinLength)
var messages, reasons []string

if len(password) < config.Password.MinLength {
reasons = append(reasons, "length")
messages = append(messages, fmt.Sprintf("Password should be at least %d characters.", config.Password.MinLength))
}

for _, characterSet := range config.Password.RequiredCharacters {
if characterSet != "" && !strings.ContainsAny(password, characterSet) {
reasons = append(reasons, "characters")

messages = append(messages, fmt.Sprintf("Password should contain at least one character of each: %s.", strings.Join(config.Password.RequiredCharacters, ", ")))

break
}
}

if len(reasons) > 0 {
return &WeakPasswordError{
Message: strings.Join(messages, " "),
Reasons: reasons,
}
}

return nil
Expand Down
107 changes: 107 additions & 0 deletions internal/api/password_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package api

import (
"context"
"testing"

"github.com/stretchr/testify/require"
"github.com/supabase/gotrue/internal/conf"
)

func TestPasswordStrengthChecks(t *testing.T) {
examples := []struct {
MinLength int
RequiredCharacters []string

Password string
Reasons []string
}{
{
MinLength: 6,
Password: "12345",
Reasons: []string{
"length",
},
},
{
MinLength: 6,
RequiredCharacters: []string{
"a",
"b",
"c",
},
Password: "123",
Reasons: []string{
"length",
"characters",
},
},
{
MinLength: 6,
RequiredCharacters: []string{
"a",
"b",
"c",
},
Password: "a123",
Reasons: []string{
"length",
"characters",
},
},
{
MinLength: 6,
RequiredCharacters: []string{
"a",
"b",
"c",
},
Password: "ab123",
Reasons: []string{
"length",
"characters",
},
},
{
MinLength: 6,
RequiredCharacters: []string{
"a",
"b",
"c",
},
Password: "c123",
Reasons: []string{
"length",
"characters",
},
},
{
MinLength: 6,
RequiredCharacters: []string{
"a",
"b",
"c",
},
Password: "abc123",
Reasons: nil,
},
}

for i, example := range examples {
api := &API{
config: &conf.GlobalConfiguration{
Password: conf.PasswordConfiguration{
MinLength: example.MinLength,
RequiredCharacters: conf.PasswordRequiredCharacters(example.RequiredCharacters),
},
},
}

err := api.checkPasswordStrength(context.Background(), example.Password)
if example.Reasons == nil {
require.NoError(t, err, "Example %d failed with error", i)
} else {
require.Equal(t, err.(*WeakPasswordError).Reasons, example.Reasons, "Example %d failed with wrong reasons", i)
}
}
}
65 changes: 50 additions & 15 deletions internal/conf/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,41 @@ func (c *SessionsConfiguration) Validate() error {
return nil
}

type PasswordRequiredCharacters []string

func (v *PasswordRequiredCharacters) Decode(value string) error {
parts := strings.Split(value, ":")

for i := 0; i < len(parts)-1; i += 1 {
part := parts[i]

if part == "" {
continue
}

// part ended in escape character, so it should be joined with the next one
if part[len(part)-1] == '\\' {
parts[i] = part[0:len(part)-1] + ":" + parts[i+1]
parts[i+1] = ""
continue
}
}

for _, part := range parts {
if part != "" {
*v = append(*v, part)
}
}

return nil
}

type PasswordConfiguration struct {
MinLength int `json:"min_length" split_words:"true"`

RequiredCharacters PasswordRequiredCharacters `json:"required_characters" split_words:"true"`
}

// GlobalConfiguration holds all the configuration that applies to all instances.
type GlobalConfiguration struct {
API APIConfiguration
Expand All @@ -149,19 +184,19 @@ type GlobalConfiguration struct {
RateLimitTokenRefresh float64 `split_words:"true" default:"150"`
RateLimitSso float64 `split_words:"true" default:"30"`

SiteURL string `json:"site_url" split_words:"true" required:"true"`
URIAllowList []string `json:"uri_allow_list" split_words:"true"`
URIAllowListMap map[string]glob.Glob
PasswordMinLength int `json:"password_min_length" split_words:"true"`
JWT JWTConfiguration `json:"jwt"`
Mailer MailerConfiguration `json:"mailer"`
Sms SmsProviderConfiguration `json:"sms"`
DisableSignup bool `json:"disable_signup" split_words:"true"`
Webhook WebhookConfig `json:"webhook" split_words:"true"`
Security SecurityConfiguration `json:"security"`
Sessions SessionsConfiguration `json:"sessions"`
MFA MFAConfiguration `json:"MFA"`
Cookie struct {
SiteURL string `json:"site_url" split_words:"true" required:"true"`
URIAllowList []string `json:"uri_allow_list" split_words:"true"`
URIAllowListMap map[string]glob.Glob
Password PasswordConfiguration `json:"password"`
JWT JWTConfiguration `json:"jwt"`
Mailer MailerConfiguration `json:"mailer"`
Sms SmsProviderConfiguration `json:"sms"`
DisableSignup bool `json:"disable_signup" split_words:"true"`
Webhook WebhookConfig `json:"webhook" split_words:"true"`
Security SecurityConfiguration `json:"security"`
Sessions SessionsConfiguration `json:"sessions"`
MFA MFAConfiguration `json:"MFA"`
Cookie struct {
Key string `json:"key"`
Domain string `json:"domain"`
Duration int `json:"duration"`
Expand Down Expand Up @@ -516,8 +551,8 @@ func (config *GlobalConfiguration) ApplyDefaults() error {
}
}

if config.PasswordMinLength < defaultMinPasswordLength {
config.PasswordMinLength = defaultMinPasswordLength
if config.Password.MinLength < defaultMinPasswordLength {
config.Password.MinLength = defaultMinPasswordLength
}
if config.MFA.ChallengeExpiryDuration < defaultChallengeExpiryDuration {
config.MFA.ChallengeExpiryDuration = defaultChallengeExpiryDuration
Expand Down
69 changes: 69 additions & 0 deletions internal/conf/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,72 @@ func TestGlobal(t *testing.T) {
require.NotNil(t, gc)
assert.Equal(t, "X-Request-ID", gc.API.RequestIDHeader)
}

func TestPasswordRequiredCharactersDecode(t *testing.T) {
examples := []struct {
Value string
Result []string
}{
{
Value: "a:b:c",
Result: []string{
"a",
"b",
"c",
},
},
{
Value: "a\\:b:c",
Result: []string{
"a:b",
"c",
},
},
{
Value: "a:b\\:c",
Result: []string{
"a",
"b:c",
},
},
{
Value: "\\:a:b:c",
Result: []string{
":a",
"b",
"c",
},
},
{
Value: "a:b:c\\:",
Result: []string{
"a",
"b",
"c:",
},
},
{
Value: "::\\::",
Result: []string{
":",
},
},
{
Value: "",
Result: nil,
},
{
Value: " ",
Result: []string{
" ",
},
},
}

for i, example := range examples {
var into PasswordRequiredCharacters
require.NoError(t, into.Decode(example.Value), "Example %d failed with error", i)

require.Equal(t, []string(into), example.Result, "Example %d got unexpected result", i)
}
}

0 comments on commit 3991bdb

Please sign in to comment.