fix(configuration): remove unused password policy option (#3149)

Removes the min score option from the ZXCVBN policy and adds tests.

api/openapi.yml

@@ -730,9 +730,6 @@ components:
             max_length:
               type: integer
               description: The maximum password length when using the standard mode.
-            min_score:
-              type: integer
-              description: The minimum password score when using the zxcvbn mode.
             require_uppercase:
               type: boolean
               description: If uppercase characters are required when using the standard mode.

config.template.yml

@@ -364,7 +364,6 @@ password_policy:
   ## zxcvbn is a well known and used password strength algorithm. It does not have tunable settings.
   zxcvbn:
     enabled: false
-    min_score: 0
 
 ##
 ## Access Control Configuration

internal/configuration/config.template.yml

@@ -364,7 +364,6 @@ password_policy:
   ## zxcvbn is a well known and used password strength algorithm. It does not have tunable settings.
   zxcvbn:
     enabled: false
-    min_score: 0
 
 ##
 ## Access Control Configuration

internal/configuration/schema/password_policy.go

@@ -11,16 +11,15 @@ type PasswordPolicyStandardParams struct {
 	RequireSpecial   bool `koanf:"require_special"`
 }
 
-// PasswordPolicyZxcvbnParams represents the configuration related to zxcvbn parameters of password policy.
-type PasswordPolicyZxcvbnParams struct {
-	Enabled  bool `koanf:"enabled"`
-	MinScore int  `koanf:"min_score"`
+// PasswordPolicyZXCVBNParams represents the configuration related to ZXCVBN parameters of password policy.
+type PasswordPolicyZXCVBNParams struct {
+	Enabled bool `koanf:"enabled"`
 }
 
 // PasswordPolicyConfiguration represents the configuration related to password policy.
 type PasswordPolicyConfiguration struct {
 	Standard PasswordPolicyStandardParams `koanf:"standard"`
-	Zxcvbn   PasswordPolicyZxcvbnParams   `koanf:"zxcvbn"`
+	ZXCVBN   PasswordPolicyZXCVBNParams   `koanf:"zxcvbn"`
 }
 
 // DefaultPasswordPolicyConfiguration is the default password policy configuration.
@@ -30,8 +29,7 @@ var DefaultPasswordPolicyConfiguration = PasswordPolicyConfiguration{
 		MinLength: 8,
 		MaxLength: 0,
 	},
-	Zxcvbn: PasswordPolicyZxcvbnParams{
-		Enabled:  false,
-		MinScore: 0,
+	ZXCVBN: PasswordPolicyZXCVBNParams{
+		Enabled: false,
 	},
 }

internal/configuration/validator/const.go

@@ -243,6 +243,11 @@ const (
 	errFmtServerBufferSize           = "server: option '%s_buffer_size' must be above 0 but it is configured as '%d'"
 )
 
+const (
+	errFmtPasswordPolicyMinLengthNotGreaterThanZero = "password_policy: standard: option 'min_length' must be greater than 0 but is configured as %d"
+	errPasswordPolicyMultipleDefined                = "password_policy: only a single password policy mechanism can be specified"
+)
+
 // Error constants.
 const (
 	/*
@@ -519,7 +524,6 @@ var ValidKeys = []string{
 	"password_policy.standard.require_number",
 	"password_policy.standard.require_special",
 	"password_policy.zxcvbn.enabled",
-	"password_policy.zxcvbn.min_score",
 }
 
 var replacedKeys = map[string]string{

internal/configuration/validator/keys_test.go

@@ -54,7 +54,7 @@ func TestAllSpecificErrorKeys(t *testing.T) {
 
 	var uniqueValues []string
 
-	// Setup configKeys and uniqueValues expected.
+	// Setup configKeys and uniqueValues expectedErrs.
 	for key, value := range specificErrorKeys {
 		configKeys = append(configKeys, key)
 

internal/configuration/validator/password_policy.go

@@ -1,33 +1,27 @@
 package validator
 
 import (
-	"errors"
+	"fmt"
 
 	"github.com/authelia/authelia/v4/internal/configuration/schema"
 	"github.com/authelia/authelia/v4/internal/utils"
 )
 
 // ValidatePasswordPolicy validates and update Password Policy configuration.
-func ValidatePasswordPolicy(configuration *schema.PasswordPolicyConfiguration, validator *schema.StructValidator) {
-	if !utils.IsBoolCountLessThanN(1, true, configuration.Standard.Enabled, configuration.Zxcvbn.Enabled) {
-		validator.Push(errors.New("password_policy:only one password policy can be enabled at a time"))
+func ValidatePasswordPolicy(config *schema.PasswordPolicyConfiguration, validator *schema.StructValidator) {
+	if !utils.IsBoolCountLessThanN(1, true, config.Standard.Enabled, config.ZXCVBN.Enabled) {
+		validator.Push(fmt.Errorf(errPasswordPolicyMultipleDefined))
 	}
 
-	if configuration.Standard.Enabled {
-		if configuration.Standard.MinLength == 0 {
-			configuration.Standard.MinLength = schema.DefaultPasswordPolicyConfiguration.Standard.MinLength
-		} else if configuration.Standard.MinLength < 0 {
-			validator.Push(errors.New("password_policy: min_length must be > 0"))
+	if config.Standard.Enabled {
+		if config.Standard.MinLength == 0 {
+			config.Standard.MinLength = schema.DefaultPasswordPolicyConfiguration.Standard.MinLength
+		} else if config.Standard.MinLength < 0 {
+			validator.Push(fmt.Errorf(errFmtPasswordPolicyMinLengthNotGreaterThanZero, config.Standard.MinLength))
 		}
 
-		if configuration.Standard.MaxLength == 0 {
-			configuration.Standard.MaxLength = schema.DefaultPasswordPolicyConfiguration.Standard.MaxLength
-		}
-	} else if configuration.Zxcvbn.Enabled {
-		if configuration.Zxcvbn.MinScore == 0 {
-			configuration.Zxcvbn.MinScore = schema.DefaultPasswordPolicyConfiguration.Zxcvbn.MinScore
-		} else if configuration.Zxcvbn.MinScore < 0 || configuration.Zxcvbn.MinScore > 4 {
-			validator.Push(errors.New("min_score must be between 0 and 4"))
+		if config.Standard.MaxLength == 0 {
+			config.Standard.MaxLength = schema.DefaultPasswordPolicyConfiguration.Standard.MaxLength
 		}
 	}
 }

internal/configuration/validator/password_policy_test.go

@@ -0,0 +1,113 @@
+package validator
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/authelia/authelia/v4/internal/configuration/schema"
+)
+
+func TestValidatePasswordPolicy(t *testing.T) {
+	testCases := []struct {
+		desc           string
+		have, expected *schema.PasswordPolicyConfiguration
+		expectedErrs   []string
+	}{
+		{
+			desc: "ShouldRaiseErrorsWhenMisconfigured",
+			have: &schema.PasswordPolicyConfiguration{
+				Standard: schema.PasswordPolicyStandardParams{
+					Enabled:   true,
+					MinLength: -1,
+				},
+				ZXCVBN: schema.PasswordPolicyZXCVBNParams{
+					Enabled: true,
+				},
+			},
+			expected: &schema.PasswordPolicyConfiguration{
+				Standard: schema.PasswordPolicyStandardParams{
+					Enabled:   true,
+					MinLength: -1,
+				},
+				ZXCVBN: schema.PasswordPolicyZXCVBNParams{
+					Enabled: true,
+				},
+			},
+			expectedErrs: []string{
+				"password_policy: only a single password policy mechanism can be specified",
+				"password_policy: standard: option 'min_length' must be greater than 0 but is configured as -1",
+			},
+		},
+		{
+			desc: "ShouldNotRaiseErrorsStandard",
+			have: &schema.PasswordPolicyConfiguration{
+				Standard: schema.PasswordPolicyStandardParams{
+					Enabled:   true,
+					MinLength: 8,
+				},
+			},
+			expected: &schema.PasswordPolicyConfiguration{
+				Standard: schema.PasswordPolicyStandardParams{
+					Enabled:   true,
+					MinLength: 8,
+				},
+			},
+		},
+		{
+			desc: "ShouldNotRaiseErrorsZXCVBN",
+			have: &schema.PasswordPolicyConfiguration{
+				ZXCVBN: schema.PasswordPolicyZXCVBNParams{
+					Enabled: true,
+				},
+			},
+			expected: &schema.PasswordPolicyConfiguration{
+				ZXCVBN: schema.PasswordPolicyZXCVBNParams{
+					Enabled: true,
+				},
+			},
+		},
+		{
+			desc: "ShouldSetDefaultstandard",
+			have: &schema.PasswordPolicyConfiguration{
+				Standard: schema.PasswordPolicyStandardParams{
+					Enabled:   true,
+					MinLength: 0,
+				},
+			},
+			expected: &schema.PasswordPolicyConfiguration{
+				Standard: schema.PasswordPolicyStandardParams{
+					Enabled:   true,
+					MinLength: 8,
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			validator := &schema.StructValidator{}
+			ValidatePasswordPolicy(tc.have, validator)
+
+			assert.Len(t, validator.Warnings(), 0)
+
+			assert.Equal(t, tc.expected.Standard.MaxLength, tc.have.Standard.MaxLength)
+			assert.Equal(t, tc.expected.Standard.MinLength, tc.have.Standard.MinLength)
+			assert.Equal(t, tc.expected.Standard.RequireNumber, tc.have.Standard.RequireNumber)
+			assert.Equal(t, tc.expected.Standard.RequireSpecial, tc.have.Standard.RequireSpecial)
+			assert.Equal(t, tc.expected.Standard.RequireUppercase, tc.have.Standard.RequireUppercase)
+			assert.Equal(t, tc.expected.Standard.RequireLowercase, tc.have.Standard.RequireLowercase)
+
+			errs := validator.Errors()
+			require.Len(t, errs, len(tc.expectedErrs))
+
+			for i := 0; i < len(errs); i++ {
+				t.Run(fmt.Sprintf("Err%d", i+1), func(t *testing.T) {
+					assert.EqualError(t, errs[i], tc.expectedErrs[i])
+				})
+			}
+		})
+	}
+}

internal/handlers/handler_configuration_password_policy.go

@@ -18,9 +18,8 @@ func PasswordPolicyConfigurationGet(ctx *middlewares.AutheliaCtx) {
 		policyResponse.RequireUppercase = ctx.Configuration.PasswordPolicy.Standard.RequireUppercase
 		policyResponse.RequireNumber = ctx.Configuration.PasswordPolicy.Standard.RequireNumber
 		policyResponse.RequireSpecial = ctx.Configuration.PasswordPolicy.Standard.RequireSpecial
-	} else if ctx.Configuration.PasswordPolicy.Zxcvbn.Enabled {
+	} else if ctx.Configuration.PasswordPolicy.ZXCVBN.Enabled {
 		policyResponse.Mode = "zxcvbn"
-		policyResponse.MinScore = ctx.Configuration.PasswordPolicy.Zxcvbn.MinScore
 	}
 
 	var err error

internal/handlers/types.go

@@ -120,7 +120,6 @@ type PassworPolicyBody struct {
 	Mode             string `json:"mode"`
 	MinLength        int    `json:"min_length"`
 	MaxLength        int    `json:"max_length"`
-	MinScore         int    `json:"min_score"`
 	RequireUppercase bool   `json:"require_uppercase"`
 	RequireLowercase bool   `json:"require_lowercase"`
 	RequireNumber    bool   `json:"require_number"`

internal/middlewares/password_policy_test.go

@@ -23,7 +23,7 @@ func TestNewPasswordPolicyProvider(t *testing.T) {
 		},
 		{
 			desc:     "ShouldReturnUnconfiguredProviderWhenZxcvbn",
-			have:     schema.PasswordPolicyConfiguration{Zxcvbn: schema.PasswordPolicyZxcvbnParams{Enabled: true}},
+			have:     schema.PasswordPolicyConfiguration{ZXCVBN: schema.PasswordPolicyZXCVBNParams{Enabled: true}},
 			expected: PasswordPolicyProvider{},
 		},
 		{

web/src/components/PasswordMeter.test.tsx

@@ -12,7 +12,6 @@ it("renders without crashing", () => {
             policy={{
                 max_length: 0,
                 min_length: 4,
-                min_score: 0,
                 require_lowercase: false,
                 require_number: false,
                 require_special: false,
@@ -30,7 +29,6 @@ it("renders adjusted height without crashing", () => {
             policy={{
                 max_length: 0,
                 min_length: 4,
-                min_score: 0,
                 require_lowercase: false,
                 require_number: false,
                 require_special: false,

web/src/models/PasswordPolicy.ts

@@ -8,7 +8,6 @@ export interface PasswordPolicyConfiguration {
     mode: PasswordPolicyMode;
     min_length: number;
     max_length: number;
-    min_score: number;
     require_uppercase: boolean;
     require_lowercase: boolean;
     require_number: boolean;

web/src/services/PasswordPolicyConfiguration.ts

@@ -6,7 +6,6 @@ interface PasswordPolicyConfigurationPayload {
     mode: ModePasswordPolicy;
     min_length: number;
     max_length: number;
-    min_score: number;
     require_uppercase: boolean;
     require_lowercase: boolean;
     require_number: boolean;

web/src/views/ResetPassword/ResetPasswordStep2.tsx

@@ -32,7 +32,6 @@ const ResetPasswordStep2 = function () {
     const [pPolicy, setPPolicy] = useState<PasswordPolicyConfiguration>({
         max_length: 0,
         min_length: 8,
-        min_score: 0,
         require_lowercase: false,
         require_number: false,
         require_special: false,