From 36cf662458326ef2f1e70f9ee304715152616048 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Sun, 3 Apr 2022 10:48:26 +1000 Subject: [PATCH] refactor: misc password policy refactoring (#3102) Add tests and makes the password policy a provider so the configuration can be loaded to memory on startup. --- config.template.yml | 19 +++ docs/configuration/password_policy.md | 63 +++++--- docs/features/password-policy.md | 29 ++-- internal/commands/helpers.go | 3 + internal/configuration/config.template.yml | 19 +++ .../configuration/schema/configuration.go | 15 +- internal/handlers/errors.go | 5 - .../handlers/handler_reset_password_step2.go | 54 +------ internal/middlewares/const.go | 4 + internal/middlewares/password_policy.go | 61 ++++++++ internal/middlewares/password_policy_test.go | 145 ++++++++++++++++++ internal/middlewares/types.go | 1 + internal/suites/Standalone/configuration.yml | 2 +- web/package.json | 2 +- web/pnpm-lock.yaml | 2 +- 15 files changed, 322 insertions(+), 102 deletions(-) delete mode 100644 internal/handlers/errors.go create mode 100644 internal/middlewares/password_policy.go create mode 100644 internal/middlewares/password_policy_test.go diff --git a/config.template.yml b/config.template.yml index 159e9b8f3..04afa858d 100644 --- a/config.template.yml +++ b/config.template.yml @@ -321,6 +321,25 @@ authentication_backend: # memory: 1024 # parallelism: 8 +## +## Password Policy Configuration. +## +password_policy: + + ## The standard policy allows you to tune individual settings manually. + standard: + enabled: false + min_length: 8 + max_length: 0 + require_uppercase: true + require_lowercase: true + require_number: true + require_special: true + + ## zxcvbn is a well known and used password strength algorithm. It does not have tunable settings. + zxcvbn: + enabled: false + ## ## Access Control Configuration ## diff --git a/docs/configuration/password_policy.md b/docs/configuration/password_policy.md index 9ce29cce9..d05b3eb4e 100644 --- a/docs/configuration/password_policy.md +++ b/docs/configuration/password_policy.md @@ -6,6 +6,7 @@ nav_order: 17 --- # Password Policy + _Authelia_ allows administrators to configure an enforced password policy. ## Configuration @@ -13,7 +14,7 @@ _Authelia_ allows administrators to configure an enforced password policy. ```yaml password_policy: standard: - enabled: false + enabled: false min_length: 8 max_length: 0 require_uppercase: true @@ -34,77 +35,95 @@ required: no {: .label .label-config .label-green } -This section allows you to enable standard security policies. -#### enabled +This section allows you to enable standard security policies. + +#### enabled +
type: bool {: .label .label-config .label-purple } required: no {: .label .label-config .label-green }
-Enables standard password policy -#### min_length +Enables standard password policy. + +#### min_length +
type: integer {: .label .label-config .label-purple } required: no {: .label .label-config .label-green }
-Determines the minimum allowed password length -#### max_length +Determines the minimum allowed password length. + +#### max_length +
type: integer {: .label .label-config .label-purple } required: no {: .label .label-config .label-green }
-Determines the maximum allowed password length -#### require_uppercase +Determines the maximum allowed password length. + +#### require_uppercase +
type: bool {: .label .label-config .label-purple } required: no {: .label .label-config .label-green }
-Indicates that at least one UPPERCASE letter must be provided as part of the password -#### require_lowercase +Indicates that at least one UPPERCASE letter must be provided as part of the password. + +#### require_lowercase +
type: bool {: .label .label-config .label-purple } required: no {: .label .label-config .label-green }
-Indicates that at least one lowercase letter must be provided as part of the password -#### require_number +Indicates that at least one lowercase letter must be provided as part of the password. + +#### require_number +
type: bool {: .label .label-config .label-purple } required: no {: .label .label-config .label-green }
-Indicates that at least one number must be provided as part of the password -#### require_special +Indicates that at least one number must be provided as part of the password. + +#### require_special +
type: bool {: .label .label-config .label-purple } required: no {: .label .label-config .label-green }
-Indicates that at least one special character must be provided as part of the password +Indicates that at least one special character must be provided as part of the password. ### zxcvbn -This password policy enables advanced password strengh metering, using [Dropbox zxcvbn package](https://github.com/dropbox/zxcvbn). -Note that this password policy do not restrict the user's entry, just warns the user that if their password is too weak +This password policy enables advanced password strength metering, using [zxcvbn](https://github.com/dropbox/zxcvbn). +Note that this password policy do not restrict the user's entry it just gives the user feedback as to how strong their +password is. -#### enabled +#### enabled +
type: bool {: .label .label-config .label-purple } required: no {: .label .label-config .label-green }
-Enables standard password policy -Note: -* only one password policy can be applied at a time +_**Important Note:** only one password policy can be applied at a time._ + +Enables zxcvbn password policy. + + diff --git a/docs/features/password-policy.md b/docs/features/password-policy.md index 4ca1ae1ee..afe658d6e 100644 --- a/docs/features/password-policy.md +++ b/docs/features/password-policy.md @@ -6,17 +6,22 @@ nav_order: 8 --- # Password Policy -Password policy enforces the security by requering the users to use strong passwords + +Password policy enforces the security by requiring the users to use strong passwords. + Currently, two methods are supported: + ## classic -* this mode of operation allows administrators to set the rules that user passwords must comply with -* the available options are: - * Minimum password length - * Require Uppercase - * Require Lowercase - * Require Numbers - * Require Special characters -* when changing the password users must meet these rules + +This mode of operation allows administrators to set the rules that user passwords must comply with when changing their +password. + +The available options are: + * Minimum password length + * Require Uppercase + * Require Lowercase + * Require Numbers + * Require Special characters

@@ -25,8 +30,10 @@ Currently, two methods are supported: ## zxcvbn -* this mode uses zxcvbn for password strength checking (see: https://github.com/dropbox/zxcvbn) -* in this mode of operation, the user is not forced to follow any rules. the user is notified if their passwords is weak or strong + +This mode uses [zxcvbn](https://github.com/dropbox/zxcvbn) for password strength checking. In this mode of operation, +the user is not forced to follow any rules. The user is notified if their passwords is weak or strong. +

diff --git a/internal/commands/helpers.go b/internal/commands/helpers.go index 24424ade5..ee09f7433 100644 --- a/internal/commands/helpers.go +++ b/internal/commands/helpers.go @@ -71,6 +71,8 @@ func getProviders() (providers middlewares.Providers, warnings []error, errors [ totpProvider := totp.NewTimeBasedProvider(config.TOTP) + passwordPolicyProvider := middlewares.NewPasswordPolicyProvider(config.PasswordPolicy) + return middlewares.Providers{ Authorizer: authorizer, UserProvider: userProvider, @@ -81,5 +83,6 @@ func getProviders() (providers middlewares.Providers, warnings []error, errors [ Notifier: notifier, SessionProvider: sessionProvider, TOTP: totpProvider, + PasswordPolicy: passwordPolicyProvider, }, warnings, errors } diff --git a/internal/configuration/config.template.yml b/internal/configuration/config.template.yml index 159e9b8f3..04afa858d 100644 --- a/internal/configuration/config.template.yml +++ b/internal/configuration/config.template.yml @@ -321,6 +321,25 @@ authentication_backend: # memory: 1024 # parallelism: 8 +## +## Password Policy Configuration. +## +password_policy: + + ## The standard policy allows you to tune individual settings manually. + standard: + enabled: false + min_length: 8 + max_length: 0 + require_uppercase: true + require_lowercase: true + require_number: true + require_special: true + + ## zxcvbn is a well known and used password strength algorithm. It does not have tunable settings. + zxcvbn: + enabled: false + ## ## Access Control Configuration ## diff --git a/internal/configuration/schema/configuration.go b/internal/configuration/schema/configuration.go index fe2803ada..bcaf2f344 100644 --- a/internal/configuration/schema/configuration.go +++ b/internal/configuration/schema/configuration.go @@ -10,16 +10,15 @@ type Configuration struct { Log LogConfiguration `koanf:"log"` IdentityProviders IdentityProvidersConfiguration `koanf:"identity_providers"` AuthenticationBackend AuthenticationBackendConfiguration `koanf:"authentication_backend"` + Session SessionConfiguration `koanf:"session"` TOTP TOTPConfiguration `koanf:"totp"` - Webauthn WebauthnConfiguration `koanf:"webauthn"` DuoAPI *DuoAPIConfiguration `koanf:"duo_api"` AccessControl AccessControlConfiguration `koanf:"access_control"` + NTP NTPConfiguration `koanf:"ntp"` Regulation RegulationConfiguration `koanf:"regulation"` - - Server ServerConfiguration `koanf:"server"` - Session SessionConfiguration `koanf:"session"` - NTP NTPConfiguration `koanf:"ntp"` - Storage StorageConfiguration `koanf:"storage"` - Notifier *NotifierConfiguration `koanf:"notifier"` - PasswordPolicy PasswordPolicyConfiguration `koanf:"password_policy"` + Storage StorageConfiguration `koanf:"storage"` + Notifier *NotifierConfiguration `koanf:"notifier"` + Server ServerConfiguration `koanf:"server"` + Webauthn WebauthnConfiguration `koanf:"webauthn"` + PasswordPolicy PasswordPolicyConfiguration `koanf:"password_policy"` } diff --git a/internal/handlers/errors.go b/internal/handlers/errors.go deleted file mode 100644 index 5111dc95d..000000000 --- a/internal/handlers/errors.go +++ /dev/null @@ -1,5 +0,0 @@ -package handlers - -import "errors" - -var errPasswordPolicyNoMet = errors.New("the supplied password does not met the security policy") diff --git a/internal/handlers/handler_reset_password_step2.go b/internal/handlers/handler_reset_password_step2.go index c3f9dbb85..a6fd44dee 100644 --- a/internal/handlers/handler_reset_password_step2.go +++ b/internal/handlers/handler_reset_password_step2.go @@ -2,7 +2,6 @@ package handlers import ( "fmt" - "regexp" "github.com/authelia/authelia/v4/internal/middlewares" "github.com/authelia/authelia/v4/internal/utils" @@ -28,7 +27,7 @@ func ResetPasswordPost(ctx *middlewares.AutheliaCtx) { return } - if err := validatePassword(ctx, requestBody.Password); err != nil { + if err = ctx.Providers.PasswordPolicy.Check(requestBody.Password); err != nil { ctx.Error(err, messagePasswordWeak) return } @@ -60,54 +59,3 @@ func ResetPasswordPost(ctx *middlewares.AutheliaCtx) { ctx.ReplyOK() } - -// validatePassword validates if the password met the password policy rules. -func validatePassword(ctx *middlewares.AutheliaCtx, password string) error { - // password validation applies only to standard passwor policy. - if !ctx.Configuration.PasswordPolicy.Standard.Enabled { - return nil - } - - requireLowercase := ctx.Configuration.PasswordPolicy.Standard.RequireLowercase - requireUppercase := ctx.Configuration.PasswordPolicy.Standard.RequireUppercase - requireNumber := ctx.Configuration.PasswordPolicy.Standard.RequireNumber - requireSpecial := ctx.Configuration.PasswordPolicy.Standard.RequireSpecial - minLength := ctx.Configuration.PasswordPolicy.Standard.MinLength - maxlength := ctx.Configuration.PasswordPolicy.Standard.MaxLength - - var patterns []string - - if (minLength > 0 && len(password) < minLength) || (maxlength > 0 && len(password) > maxlength) { - return errPasswordPolicyNoMet - } - - if requireLowercase { - patterns = append(patterns, "[a-z]+") - } - - if requireUppercase { - patterns = append(patterns, "[A-Z]+") - } - - if requireNumber { - patterns = append(patterns, "[0-9]+") - } - - if requireSpecial { - patterns = append(patterns, "[^a-zA-Z0-9]+") - } - - for _, pattern := range patterns { - re, err := regexp.Compile(pattern) - - if err != nil { - return err - } - - if found := re.MatchString(password); !found { - return errPasswordPolicyNoMet - } - } - - return nil -} diff --git a/internal/middlewares/const.go b/internal/middlewares/const.go index 54b350a7e..8e0d4aca1 100644 --- a/internal/middlewares/const.go +++ b/internal/middlewares/const.go @@ -1,6 +1,8 @@ package middlewares import ( + "errors" + "github.com/valyala/fasthttp" ) @@ -56,3 +58,5 @@ const ( var protoHostSeparator = []byte("://") var validOverrideAssets = []string{"favicon.ico", "logo.png"} + +var errPasswordPolicyNoMet = errors.New("the supplied password does not met the security policy") diff --git a/internal/middlewares/password_policy.go b/internal/middlewares/password_policy.go new file mode 100644 index 000000000..f2894361c --- /dev/null +++ b/internal/middlewares/password_policy.go @@ -0,0 +1,61 @@ +package middlewares + +import ( + "regexp" + + "github.com/authelia/authelia/v4/internal/configuration/schema" +) + +// NewPasswordPolicyProvider returns a new password policy provider. +func NewPasswordPolicyProvider(config schema.PasswordPolicyConfiguration) (provider PasswordPolicyProvider) { + if !config.Standard.Enabled { + return provider + } + + provider.min, provider.max = config.Standard.MinLength, config.Standard.MaxLength + + if config.Standard.RequireLowercase { + provider.patterns = append(provider.patterns, *regexp.MustCompile(`[a-z]+`)) + } + + if config.Standard.RequireUppercase { + provider.patterns = append(provider.patterns, *regexp.MustCompile(`[A-Z]+`)) + } + + if config.Standard.RequireNumber { + provider.patterns = append(provider.patterns, *regexp.MustCompile(`[0-9]+`)) + } + + if config.Standard.RequireSpecial { + provider.patterns = append(provider.patterns, *regexp.MustCompile(`[^a-zA-Z0-9]+`)) + } + + return provider +} + +// PasswordPolicyProvider handles password policy checking. +type PasswordPolicyProvider struct { + patterns []regexp.Regexp + min, max int +} + +// Check checks the password against the policy. +func (p PasswordPolicyProvider) Check(password string) (err error) { + patterns := len(p.patterns) + + if (p.min > 0 && len(password) < p.min) || (p.max > 0 && len(password) > p.max) { + return errPasswordPolicyNoMet + } + + if patterns == 0 { + return nil + } + + for i := 0; i < patterns; i++ { + if !p.patterns[i].MatchString(password) { + return errPasswordPolicyNoMet + } + } + + return nil +} diff --git a/internal/middlewares/password_policy_test.go b/internal/middlewares/password_policy_test.go new file mode 100644 index 000000000..35e2d8811 --- /dev/null +++ b/internal/middlewares/password_policy_test.go @@ -0,0 +1,145 @@ +package middlewares + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/authelia/authelia/v4/internal/configuration/schema" +) + +func TestNewPasswordPolicyProvider(t *testing.T) { + testCases := []struct { + desc string + have schema.PasswordPolicyConfiguration + expected PasswordPolicyProvider + }{ + { + desc: "ShouldReturnUnconfiguredProvider", + have: schema.PasswordPolicyConfiguration{}, + expected: PasswordPolicyProvider{}, + }, + { + desc: "ShouldReturnUnconfiguredProviderWhenZxcvbn", + have: schema.PasswordPolicyConfiguration{Zxcvbn: schema.PasswordPolicyZxcvbnParams{Enabled: true}}, + expected: PasswordPolicyProvider{}, + }, + { + desc: "ShouldReturnConfiguredProviderWithMin", + have: schema.PasswordPolicyConfiguration{Standard: schema.PasswordPolicyStandardParams{Enabled: true, MinLength: 8}}, + expected: PasswordPolicyProvider{min: 8}, + }, + { + desc: "ShouldReturnConfiguredProviderWitHMinMax", + have: schema.PasswordPolicyConfiguration{Standard: schema.PasswordPolicyStandardParams{Enabled: true, MinLength: 8, MaxLength: 100}}, + expected: PasswordPolicyProvider{min: 8, max: 100}, + }, + { + desc: "ShouldReturnConfiguredProviderWithMinLowercase", + have: schema.PasswordPolicyConfiguration{Standard: schema.PasswordPolicyStandardParams{Enabled: true, MinLength: 8, RequireLowercase: true}}, + expected: PasswordPolicyProvider{min: 8, patterns: []regexp.Regexp{*regexp.MustCompile(`[a-z]+`)}}, + }, + { + desc: "ShouldReturnConfiguredProviderWithMinLowercaseUppercase", + have: schema.PasswordPolicyConfiguration{Standard: schema.PasswordPolicyStandardParams{Enabled: true, MinLength: 8, RequireLowercase: true, RequireUppercase: true}}, + expected: PasswordPolicyProvider{min: 8, patterns: []regexp.Regexp{*regexp.MustCompile(`[a-z]+`), *regexp.MustCompile(`[A-Z]+`)}}, + }, + { + desc: "ShouldReturnConfiguredProviderWithMinLowercaseUppercaseNumber", + have: schema.PasswordPolicyConfiguration{Standard: schema.PasswordPolicyStandardParams{Enabled: true, MinLength: 8, RequireLowercase: true, RequireUppercase: true, RequireNumber: true}}, + expected: PasswordPolicyProvider{min: 8, patterns: []regexp.Regexp{*regexp.MustCompile(`[a-z]+`), *regexp.MustCompile(`[A-Z]+`), *regexp.MustCompile(`[0-9]+`)}}, + }, + { + desc: "ShouldReturnConfiguredProviderWithMinLowercaseUppercaseSpecial", + have: schema.PasswordPolicyConfiguration{Standard: schema.PasswordPolicyStandardParams{Enabled: true, MinLength: 8, RequireLowercase: true, RequireUppercase: true, RequireSpecial: true}}, + expected: PasswordPolicyProvider{min: 8, patterns: []regexp.Regexp{*regexp.MustCompile(`[a-z]+`), *regexp.MustCompile(`[A-Z]+`), *regexp.MustCompile(`[^a-zA-Z0-9]+`)}}, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + actual := NewPasswordPolicyProvider(tc.have) + assert.Equal(t, tc.expected, actual) + }) + } +} + +func TestPasswordPolicyProvider_Validate(t *testing.T) { + testCases := []struct { + desc string + config schema.PasswordPolicyConfiguration + have []string + expected []error + }{ + { + desc: "ShouldValidateAllPasswords", + config: schema.PasswordPolicyConfiguration{}, + have: []string{"a", "1", "a really str0ng pass12nm3kjl12word@@#4"}, + expected: []error{nil, nil, nil}, + }, + { + desc: "ShouldValidatePasswordMinLength", + config: schema.PasswordPolicyConfiguration{Standard: schema.PasswordPolicyStandardParams{Enabled: true, MinLength: 8}}, + have: []string{"a", "b123", "1111111", "aaaaaaaa", "1o23nm1kio2n3k12jn"}, + expected: []error{errPasswordPolicyNoMet, errPasswordPolicyNoMet, errPasswordPolicyNoMet, nil, nil}, + }, + { + desc: "ShouldValidatePasswordMaxLength", + config: schema.PasswordPolicyConfiguration{Standard: schema.PasswordPolicyStandardParams{Enabled: true, MaxLength: 30}}, + have: []string{ + "a1234567894654wkjnkjasnskjandkjansdkjnas", + "012345678901234567890123456789a", + "0123456789012345678901234567890123456789", + "012345678901234567890123456789", + "1o23nm1kio2n3k12jn", + }, + expected: []error{errPasswordPolicyNoMet, errPasswordPolicyNoMet, errPasswordPolicyNoMet, nil, nil}, + }, + { + desc: "ShouldValidatePasswordAdvancedLowerUpperMin8", + config: schema.PasswordPolicyConfiguration{Standard: schema.PasswordPolicyStandardParams{Enabled: true, MinLength: 8, RequireLowercase: true, RequireUppercase: true}}, + have: []string{"a", "b123", "1111111", "aaaaaaaa", "1o23nm1kio2n3k12jn", "ANJKJQ@#NEK!@#NJK!@#", "qjik2nkjAkjlmn123"}, + expected: []error{errPasswordPolicyNoMet, errPasswordPolicyNoMet, errPasswordPolicyNoMet, errPasswordPolicyNoMet, errPasswordPolicyNoMet, errPasswordPolicyNoMet, nil}, + }, + { + desc: "ShouldValidatePasswordAdvancedAllMax100Min8", + config: schema.PasswordPolicyConfiguration{Standard: schema.PasswordPolicyStandardParams{Enabled: true, MinLength: 8, MaxLength: 100, RequireLowercase: true, RequireUppercase: true, RequireNumber: true, RequireSpecial: true}}, + have: []string{ + "a", + "b123", + "1111111", + "aaaaaaaa", + "1o23nm1kio2n3k12jn", + "ANJKJQ@#NEK!@#NJK!@#", + "qjik2nkjAkjlmn123", + "qjik2n@jAkjlmn123", + "qjik2n@jAkjlmn123qjik2n@jAkjlmn123qjik2n@jAkjlmn123qjik2n@jAkjlmn123qjik2n@jAkjlmn123qjik2n@jAkjlmn123", + }, + expected: []error{ + errPasswordPolicyNoMet, + errPasswordPolicyNoMet, + errPasswordPolicyNoMet, + errPasswordPolicyNoMet, + errPasswordPolicyNoMet, + errPasswordPolicyNoMet, + errPasswordPolicyNoMet, + nil, + errPasswordPolicyNoMet, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + require.Equal(t, len(tc.have), len(tc.expected)) + for i := 0; i < len(tc.have); i++ { + provider := NewPasswordPolicyProvider(tc.config) + t.Run(tc.have[i], func(t *testing.T) { + assert.Equal(t, tc.expected[i], provider.Check(tc.have[i])) + }) + } + }) + } +} diff --git a/internal/middlewares/types.go b/internal/middlewares/types.go index 585f7cdfe..aacbd36fb 100644 --- a/internal/middlewares/types.go +++ b/internal/middlewares/types.go @@ -39,6 +39,7 @@ type Providers struct { StorageProvider storage.Provider Notifier notification.Notifier TOTP totp.Provider + PasswordPolicy PasswordPolicyProvider } // RequestHandler represents an Authelia request handler. diff --git a/internal/suites/Standalone/configuration.yml b/internal/suites/Standalone/configuration.yml index cf6723d3d..919f26931 100644 --- a/internal/suites/Standalone/configuration.yml +++ b/internal/suites/Standalone/configuration.yml @@ -103,7 +103,7 @@ ntp: password_policy: standard: # Enables standard password Policy - enabled: false + enabled: false min_length: 8 max_length: 0 require_uppercase: true diff --git a/web/package.json b/web/package.json index b50d6c59b..06bcfb6b6 100644 --- a/web/package.json +++ b/web/package.json @@ -24,7 +24,7 @@ "react-loading": "2.0.3", "react-otp-input": "2.4.0", "react-router-dom": "6.3.0", - "zxcvbn": "^4.4.2" + "zxcvbn": "4.4.2" }, "scripts": { "prepare": "cd .. && husky install .github", diff --git a/web/pnpm-lock.yaml b/web/pnpm-lock.yaml index ab6037ab4..77ab76882 100644 --- a/web/pnpm-lock.yaml +++ b/web/pnpm-lock.yaml @@ -56,7 +56,7 @@ specifiers: vite-plugin-istanbul: 2.5.1 vite-plugin-svgr: 1.1.0 vite-tsconfig-paths: 3.4.1 - zxcvbn: ^4.4.2 + zxcvbn: 4.4.2 dependencies: '@fortawesome/fontawesome-svg-core': 6.1.1