Skip to content

Commit

Permalink
feat: add required characters password strength check
Browse files Browse the repository at this point in the history
  • Loading branch information
hf committed Nov 28, 2023
1 parent 5524653 commit 457743a
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 24 deletions.
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
63 changes: 48 additions & 15 deletions internal/conf/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,39 @@ func (c *SessionsConfiguration) Validate() error {
return nil
}

type PasswordRequiredCharacters []string

func (v *PasswordRequiredCharacters) Decode(value string) {
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] = parts[i] + ":" + parts[i+1]
parts[i+1] = ""
i += 1
}
}

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

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

RequiredCharacters []string `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 +182,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 +549,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

0 comments on commit 457743a

Please sign in to comment.