Skip to content

Commit

Permalink
feat: make the password policy more configurable (ory#2118)
Browse files Browse the repository at this point in the history
Closes ory#970

Co-authored-by: aeneasr <[email protected]>
  • Loading branch information
RamiBerm and aeneasr committed Jan 18, 2022
1 parent 39aba7d commit a8855a1
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 19 deletions.
35 changes: 35 additions & 0 deletions docs/docs/guides/password-policy.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
id: password-policy
title: Configuring The Password Policy
---

The password policy is a set of rules that define the password requirements for
Kratos identities. They can be changed by modifying the following configuration
parameters:

```yaml title=path/to/kratos/config.yml
selfservice:
methods:
password:
enabled: true
config:
haveibeenpwned_enabled: true
min_password_length: 8
identifier_similarity_check_enabled: true
```
#### `haveibeenpwned_enabled`

If set to `true`, the password policy will check if the password has been found
in the [Have I Been Pwned](https://haveibeenpwned.com/) database. The default
value is `true`.

#### `min_password_length`

The minimum length of the password. The default value is `8`, the minimum
allowed value is `6`.

#### `identifier_similarity_check_enabled`

If set to `true`, the password policy will check if the password is similar to
the user identifier. The default value is `true`.
27 changes: 27 additions & 0 deletions docs/docs/reference/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -997,6 +997,33 @@ selfservice:
#
haveibeenpwned_host: ''

## Minimum Password Length ##
#
# Default value: 8
# Minimum value: 6
#
# Set this value using environment variables on
# - Linux/macOS:
# $ export SELFSERVICE_METHODS_PASSWORD_CONFIG_MIN_PASSWORD_LENGTH=<value>
# - Windows Command Line (CMD):
# > set SELFSERVICE_METHODS_PASSWORD_CONFIG_MIN_PASSWORD_LENGTH=<value>
#
min_password_length: 0

## Enables Password User Identifier Similarity Check ##
#
# If set to false the password validation does not check whether passwords and user identifiers are similar
#
# Default value: true
#
# Set this value using environment variables on
# - Linux/macOS:
# $ export SELFSERVICE_METHODS_PASSWORD_CONFIG_SELFSERVICE_METHODS_PASSWORD_CONFIG_IDENTIFIER_SIMILARITY_CHECK_ENABLED=<value>
# - Windows Command Line (CMD):
# > set SELFSERVICE_METHODS_PASSWORD_CONFIG_SELFSERVICE_METHODS_PASSWORD_CONFIG_IDENTIFIER_SIMILARITY_CHECK_ENABLED=<value>
#
identifier_similarity_check_enabled: false

## Enables Username/Email and Password Method ##
#
# Default value: true
Expand Down
4 changes: 3 additions & 1 deletion docs/sidebar.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"guides/configuring-cookies",
"guides/multi-domain-cookies",
"guides/setting-up-cors",
"guides/password-policy",
"guides/account-recovery-password-reset",
"guides/account-activation-email-verification",
"guides/zero-trust-iap-proxy-identity-access-proxy",
Expand All @@ -75,7 +76,8 @@
"guides/setting-up-password-hashing-parameters",
"guides/integration-with-other-systems-using-web-hooks",
"guides/tracing",
"guides/upgrade"
"guides/upgrade",
"guides/password-policy"
]
},
"reference/api",
Expand Down
22 changes: 14 additions & 8 deletions driver/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ const (
ViperKeyPasswordHaveIBeenPwnedHost = "selfservice.methods.password.config.haveibeenpwned_host"
ViperKeyPasswordHaveIBeenPwnedEnabled = "selfservice.methods.password.config.haveibeenpwned_enabled"
ViperKeyPasswordMaxBreaches = "selfservice.methods.password.config.max_breaches"
ViperKeyPasswordMinLength = "selfservice.methods.password.config.min_password_length"
ViperKeyPasswordIdentifierSimilarityCheckEnabled = "selfservice.methods.password.config.identifier_similarity_check_enabled"
ViperKeyIgnoreNetworkErrors = "selfservice.methods.password.config.ignore_network_errors"
ViperKeyTOTPIssuer = "selfservice.methods.totp.config.issuer"
ViperKeyWebAuthnRPDisplayName = "selfservice.methods.webauthn.config.rp.display_name"
Expand Down Expand Up @@ -195,10 +197,12 @@ type (
URL string `json:"url"`
}
PasswordPolicy struct {
HaveIBeenPwnedHost string `json:"haveibeenpwned_host"`
HaveIBeenPwnedEnabled bool `json:"haveibeenpwned_enabled"`
MaxBreaches uint `json:"max_breaches"`
IgnoreNetworkErrors bool `json:"ignore_network_errors"`
HaveIBeenPwnedHost string `json:"haveibeenpwned_host"`
HaveIBeenPwnedEnabled bool `json:"haveibeenpwned_enabled"`
MaxBreaches uint `json:"max_breaches"`
IgnoreNetworkErrors bool `json:"ignore_network_errors"`
MinPasswordLength uint `json:"min_password_length"`
IdentifierSimilarityCheckEnabled bool `json:"identifier_similarity_check_enabled"`
}
Schemas []Schema
Config struct {
Expand Down Expand Up @@ -998,10 +1002,12 @@ func (p *Config) ConfigVersion() string {

func (p *Config) PasswordPolicyConfig() *PasswordPolicy {
return &PasswordPolicy{
HaveIBeenPwnedHost: p.p.StringF(ViperKeyPasswordHaveIBeenPwnedHost, "api.pwnedpasswords.com"),
HaveIBeenPwnedEnabled: p.p.BoolF(ViperKeyPasswordHaveIBeenPwnedEnabled, true),
MaxBreaches: uint(p.p.Int(ViperKeyPasswordMaxBreaches)),
IgnoreNetworkErrors: p.p.BoolF(ViperKeyIgnoreNetworkErrors, true),
HaveIBeenPwnedHost: p.p.StringF(ViperKeyPasswordHaveIBeenPwnedHost, "api.pwnedpasswords.com"),
HaveIBeenPwnedEnabled: p.p.BoolF(ViperKeyPasswordHaveIBeenPwnedEnabled, true),
MaxBreaches: uint(p.p.Int(ViperKeyPasswordMaxBreaches)),
IgnoreNetworkErrors: p.p.BoolF(ViperKeyIgnoreNetworkErrors, true),
MinPasswordLength: uint(p.p.IntF(ViperKeyPasswordMinLength, 8)),
IdentifierSimilarityCheckEnabled: p.p.BoolF(ViperKeyPasswordIdentifierSimilarityCheckEnabled, true),
}
}

Expand Down
24 changes: 23 additions & 1 deletion driver/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func TestViperProvider(t *testing.T) {
config string
enabled bool
}{
{id: "password", enabled: true, config: `{"haveibeenpwned_host":"api.pwnedpasswords.com","haveibeenpwned_enabled":true,"ignore_network_errors":true,"max_breaches":0}`},
{id: "password", enabled: true, config: `{"haveibeenpwned_host":"api.pwnedpasswords.com","haveibeenpwned_enabled":true,"ignore_network_errors":true,"max_breaches":0,"min_password_length":8,"identifier_similarity_check_enabled":true}`},
{id: "oidc", enabled: true, config: `{"providers":[{"client_id":"a","client_secret":"b","id":"github","provider":"github","mapper_url":"http://test.kratos.ory.sh/default-identity.schema.json"}]}`},
{id: "totp", enabled: true, config: `{"issuer":"issuer.ory.sh"}`},
} {
Expand Down Expand Up @@ -1017,3 +1017,25 @@ func TestIdentitySchemaValidation(t *testing.T) {
}
})
}

func TestChangeMinPasswordLength(t *testing.T) {
t.Run("case=must fail on minimum password length below enforced minimum", func(t *testing.T) {
ctx := context.Background()

_, err := config.New(ctx, logrusx.New("", ""), os.Stderr,
configx.WithConfigFiles("stub/.kratos.yaml"),
configx.WithValue(config.ViperKeyPasswordMinLength, 5))

assert.Error(t, err)
})

t.Run("case=must not fail on minimum password length above enforced minimum", func(t *testing.T) {
ctx := context.Background()

_, err := config.New(ctx, logrusx.New("", ""), os.Stderr,
configx.WithConfigFiles("stub/.kratos.yaml"),
configx.WithValue(config.ViperKeyPasswordMinLength, 9))

assert.NoError(t, err)
})
}
13 changes: 13 additions & 0 deletions embedx/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1128,6 +1128,19 @@
"description": "If set to false the password validation fails when the network or the Have I Been Pwnd API is down.",
"type": "boolean",
"default": true
},
"min_password_length": {
"title": "Minimum Password Length",
"description": "Defines the minimum length of the password.",
"type": "integer",
"default": 8,
"minimum": 6
},
"identifier_similarity_check_enabled": {
"title": "Enable password-identifier similarity check",
"description": "If set to false the password validation does not check for similarity between the password and the user identifier.",
"type": "boolean",
"default": true
}
},
"additionalProperties": false
Expand Down
20 changes: 11 additions & 9 deletions selfservice/strategy/password/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,20 @@ func (s *DefaultPasswordValidator) fetch(hpw []byte, apiDNSName string) error {
}

func (s *DefaultPasswordValidator) Validate(ctx context.Context, identifier, password string) error {
if len(password) < 8 {
return errors.Errorf("password length must be at least 8 characters but only got %d", len(password))
}
passwordPolicyConfig := s.reg.Config(ctx).PasswordPolicyConfig()

compIdentifier, compPassword := strings.ToLower(identifier), strings.ToLower(password)
dist := levenshtein.Distance(compIdentifier, compPassword)
lcs := float32(lcsLength(compIdentifier, compPassword)) / float32(len(compPassword))
if dist < s.minIdentifierPasswordDist || lcs > s.maxIdentifierPasswordSubstrThreshold {
return errors.Errorf("the password is too similar to the user identifier")
if len(password) < int(passwordPolicyConfig.MinPasswordLength) {
return errors.Errorf("password length must be at least %d characters but only got %d", passwordPolicyConfig.MinPasswordLength, len(password))
}

passwordPolicyConfig := s.reg.Config(ctx).PasswordPolicyConfig()
if passwordPolicyConfig.IdentifierSimilarityCheckEnabled {
compIdentifier, compPassword := strings.ToLower(identifier), strings.ToLower(password)
dist := levenshtein.Distance(compIdentifier, compPassword)
lcs := float32(lcsLength(compIdentifier, compPassword)) / float32(len(compPassword))
if dist < s.minIdentifierPasswordDist || lcs > s.maxIdentifierPasswordSubstrThreshold {
return errors.Errorf("the password is too similar to the user identifier")
}
}

if !passwordPolicyConfig.HaveIBeenPwnedEnabled {
return nil
Expand Down
29 changes: 29 additions & 0 deletions selfservice/strategy/password/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,35 @@ func TestDisableHaveIBeenPwnedValidationHost(t *testing.T) {
})
}

func TestChangeMinPasswordLength(t *testing.T) {
conf, reg := internal.NewFastRegistryWithMocks(t)
s := password.NewDefaultPasswordValidatorStrategy(reg)
conf.MustSet(config.ViperKeyPasswordMinLength, 10)

t.Run("case=should not fail if password is longer than min length", func(t *testing.T) {
require.NoError(t, s.Validate(context.Background(), "", "kuobahcaas"))
})

t.Run("case=should fail if password is shorter than min length", func(t *testing.T) {
require.Error(t, s.Validate(context.Background(), "", "rfqyfjied"))
})
}

func TestChangeIdentifierSimilarityCheckEnabled(t *testing.T) {
conf, reg := internal.NewFastRegistryWithMocks(t)
s := password.NewDefaultPasswordValidatorStrategy(reg)

t.Run("case=should not fail if password is similar to identifier", func(t *testing.T) {
conf.MustSet(config.ViperKeyPasswordIdentifierSimilarityCheckEnabled, false)
require.NoError(t, s.Validate(context.Background(), "bosqwfaxee", "bosqwfaxee"))
})

t.Run("case=should fail if password is similar to identifier", func(t *testing.T) {
conf.MustSet(config.ViperKeyPasswordIdentifierSimilarityCheckEnabled, true)
require.Error(t, s.Validate(context.Background(), "bosqwfaxee", "bosqwfaxee"))
})
}

type fakeValidatorAPI struct{}

func (api *fakeValidatorAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
Expand Down

0 comments on commit a8855a1

Please sign in to comment.