refactor: apply changes from code review

config.template.yml

@@ -191,11 +191,12 @@ authentication_backend:
   ## Disable both the HTML element and the API for reset password functionality.
   disable_reset_password: false
 
-  ## Enable the use of external reset password link.
-  enable_external_reset_password: false
+  ## Password Reset Options.
+  password_reset:
 
-  ## External reset password url that redirects the user to an external reset portal.
-  external_reset_password_url: ""
+    ## External reset password url that redirects the user to an external reset portal. This disables the internal reset
+    ## functionality.
+    custom_url: ""
 
   ## The amount of time to wait before we refresh data from the authentication backend. Uses duration notation.
   ## To disable this feature set it to 'disable', this will slightly reduce security because for Authelia, users will

docs/configuration/authentication/index.md

@@ -18,8 +18,8 @@ There are two ways to store the users along with their password:
 ```yaml
 authentication_backend:
   disable_reset_password: false
-  enable_external_reset_password: false
-  external_reset_password_url: ""
+  password_reset:
+    custom_url: ""
   file: {}
   ldap: {}
 ```
@@ -38,29 +38,20 @@ required: no
 
 This setting controls if users can reset their password from the web frontend or not.
 
-### enable_external_reset_password
-<div markdown="1">
-type: boolean
-{: .label .label-config .label-purple } 
-default: false
-{: .label .label-config .label-blue }
-required: no
-{: .label .label-config .label-green }
-</div>
+### password_reset
 
-This setting controls if users can reset their password from an external reset password portal.
-
-### external_reset_password_url
+#### custom_url
 <div markdown="1">
 type: string
 {: .label .label-config .label-purple } 
 default: ""
 {: .label .label-config .label-blue }
-required: no (yes if enable_external_reset_password is true)
+required: no
 {: .label .label-config .label-green }
 </div>
 
-The custom reset password URL, which redirects users to the custom reset password portal. 
+The custom password reset URL. This replaces the inbuilt password reset functionality and disables the endpoints if
+this is configured to anything other than nothing or an empty string.
 
 ### file
 

internal/configuration/config.template.yml

@@ -191,11 +191,12 @@ authentication_backend:
   ## Disable both the HTML element and the API for reset password functionality.
   disable_reset_password: false
 
-  ## Enable the use of external reset password link.
-  enable_external_reset_password: false
+  ## Password Reset Options.
+  password_reset:
 
-  ## External reset password url that redirects the user to an external reset portal.
-  external_reset_password_url: ""
+    ## External reset password url that redirects the user to an external reset portal. This disables the internal reset
+    ## functionality.
+    custom_url: ""
 
   ## The amount of time to wait before we refresh data from the authentication backend. Uses duration notation.
   ## To disable this feature set it to 'disable', this will slightly reduce security because for Authelia, users will

internal/configuration/schema/authentication.go

@@ -1,6 +1,9 @@
 package schema
 
-import "time"
+import (
+	"net/url"
+	"time"
+)
 
 // LDAPAuthenticationBackendConfiguration represents the configuration related to LDAP server.
 type LDAPAuthenticationBackendConfiguration struct {
@@ -45,12 +48,18 @@ type PasswordConfiguration struct {
 
 // AuthenticationBackendConfiguration represents the configuration related to the authentication backend.
 type AuthenticationBackendConfiguration struct {
-	DisableResetPassword        bool                                    `koanf:"disable_reset_password"`
-	EnableExternalResetPassword bool                                    `koanf:"enable_external_reset_password"`
-	ExternalResetPasswordURL    string                                  `koanf:"external_reset_password_url"`
-	RefreshInterval             string                                  `koanf:"refresh_interval"`
-	LDAP                        *LDAPAuthenticationBackendConfiguration `koanf:"ldap"`
-	File                        *FileAuthenticationBackendConfiguration `koanf:"file"`
+	LDAP *LDAPAuthenticationBackendConfiguration `koanf:"ldap"`
+	File *FileAuthenticationBackendConfiguration `koanf:"file"`
+
+	PasswordReset PasswordResetAuthenticationBackendConfiguration `koanf:"password_reset"`
+
+	DisableResetPassword bool   `koanf:"disable_reset_password"`
+	RefreshInterval      string `koanf:"refresh_interval"`
+}
+
+// PasswordResetAuthenticationBackendConfiguration represents the configuration related to password reset functionality.
+type PasswordResetAuthenticationBackendConfiguration struct {
+	CustomURL url.URL `koanf:"custom_url"`
 }
 
 // DefaultPasswordConfiguration represents the default configuration related to Argon2id hashing.

internal/configuration/validator/access_control_test.go

@@ -32,8 +32,8 @@ func (suite *AccessControl) SetupTest() {
 func (suite *AccessControl) TestShouldValidateCompleteConfiguration() {
 	ValidateAccessControl(suite.config, suite.validator)
 
-	suite.Assert().False(suite.validator.HasWarnings())
-	suite.Assert().False(suite.validator.HasErrors())
+	suite.Assert().Len(suite.validator.Warnings(), 0)
+	suite.Assert().Len(suite.validator.Errors(), 0)
 }
 
 func (suite *AccessControl) TestShouldValidateEitherDomainsOrDomainsRegex() {
@@ -55,7 +55,7 @@ func (suite *AccessControl) TestShouldValidateEitherDomainsOrDomainsRegex() {
 
 	ValidateRules(suite.config, suite.validator)
 
-	suite.Assert().False(suite.validator.HasWarnings())
+	suite.Assert().Len(suite.validator.Warnings(), 0)
 	suite.Require().Len(suite.validator.Errors(), 1)
 
 	assert.EqualError(suite.T(), suite.validator.Errors()[0], "access control: rule #3: rule is invalid: must have the option 'domain' or 'domain_regex' configured")
@@ -66,7 +66,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidDefaultPolicy() {
 
 	ValidateAccessControl(suite.config, suite.validator)
 
-	suite.Assert().False(suite.validator.HasWarnings())
+	suite.Assert().Len(suite.validator.Warnings(), 0)
 	suite.Require().Len(suite.validator.Errors(), 1)
 
 	suite.Assert().EqualError(suite.validator.Errors()[0], "access control: option 'default_policy' must be one of 'bypass', 'one_factor', 'two_factor', 'deny' but it is configured as 'invalid'")
@@ -82,7 +82,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidNetworkGroupNetwork() {
 
 	ValidateAccessControl(suite.config, suite.validator)
 
-	suite.Assert().False(suite.validator.HasWarnings())
+	suite.Assert().Len(suite.validator.Warnings(), 0)
 	suite.Require().Len(suite.validator.Errors(), 1)
 
 	suite.Assert().EqualError(suite.validator.Errors()[0], "access control: networks: network group 'internal' is invalid: the network 'abc.def.ghi.jkl' is not a valid IP or CIDR notation")
@@ -93,7 +93,7 @@ func (suite *AccessControl) TestShouldRaiseErrorWithNoRulesDefined() {
 
 	ValidateRules(suite.config, suite.validator)
 
-	suite.Assert().False(suite.validator.HasWarnings())
+	suite.Assert().Len(suite.validator.Warnings(), 0)
 	suite.Require().Len(suite.validator.Errors(), 1)
 
 	suite.Assert().EqualError(suite.validator.Errors()[0], "access control: 'default_policy' option 'deny' is invalid: when no rules are specified it must be 'two_factor' or 'one_factor'")
@@ -106,7 +106,7 @@ func (suite *AccessControl) TestShouldRaiseWarningWithNoRulesDefined() {
 
 	ValidateRules(suite.config, suite.validator)
 
-	suite.Assert().False(suite.validator.HasErrors())
+	suite.Assert().Len(suite.validator.Errors(), 0)
 	suite.Require().Len(suite.validator.Warnings(), 1)
 
 	suite.Assert().EqualError(suite.validator.Warnings()[0], "access control: no rules have been specified so the 'default_policy' of 'two_factor' is going to be applied to all requests")
@@ -122,7 +122,7 @@ func (suite *AccessControl) TestShouldRaiseErrorsWithEmptyRules() {
 
 	ValidateRules(suite.config, suite.validator)
 
-	suite.Assert().False(suite.validator.HasWarnings())
+	suite.Assert().Len(suite.validator.Warnings(), 0)
 	suite.Require().Len(suite.validator.Errors(), 4)
 
 	suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1: rule is invalid: must have the option 'domain' or 'domain_regex' configured")
@@ -141,7 +141,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidPolicy() {
 
 	ValidateRules(suite.config, suite.validator)
 
-	suite.Assert().False(suite.validator.HasWarnings())
+	suite.Assert().Len(suite.validator.Warnings(), 0)
 	suite.Require().Len(suite.validator.Errors(), 1)
 
 	suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1 (domain 'public.example.com'): rule 'policy' option 'invalid' is invalid: must be one of 'deny', 'two_factor', 'one_factor' or 'bypass'")
@@ -158,7 +158,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidNetwork() {
 
 	ValidateRules(suite.config, suite.validator)
 
-	suite.Assert().False(suite.validator.HasWarnings())
+	suite.Assert().Len(suite.validator.Warnings(), 0)
 	suite.Require().Len(suite.validator.Errors(), 1)
 
 	suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1 (domain 'public.example.com'): the network 'abc.def.ghi.jkl/32' is not a valid Group Name, IP, or CIDR notation")
@@ -175,7 +175,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidMethod() {
 
 	ValidateRules(suite.config, suite.validator)
 
-	suite.Assert().False(suite.validator.HasWarnings())
+	suite.Assert().Len(suite.validator.Warnings(), 0)
 	suite.Require().Len(suite.validator.Errors(), 1)
 
 	suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1 (domain 'public.example.com'): 'methods' option 'HOP' is invalid: must be one of 'GET', 'HEAD', 'POST', 'PUT', 'PATCH', 'DELETE', 'TRACE', 'CONNECT', 'OPTIONS', 'COPY', 'LOCK', 'MKCOL', 'MOVE', 'PROPFIND', 'PROPPATCH', 'UNLOCK'")

internal/configuration/validator/authentication.go

@@ -1,7 +1,6 @@
 package validator
 
 import (
-	"errors"
 	"fmt"
 	"net/url"
 	"strings"
@@ -10,7 +9,7 @@ import (
 	"github.com/authelia/authelia/v4/internal/utils"
 )
 
-// ValidateAuthenticationBackend validates and updates the authentication backend config.
+// ValidateAuthenticationBackend validates and updates the authentication backend configuration.
 func ValidateAuthenticationBackend(config *schema.AuthenticationBackendConfiguration, validator *schema.StructValidator) {
 	if config.LDAP == nil && config.File == nil {
 		validator.Push(fmt.Errorf(errFmtAuthBackendNotConfigured))
@@ -35,19 +34,17 @@ func ValidateAuthenticationBackend(config *schema.AuthenticationBackendConfigura
 		}
 	}
 
-	if config.EnableExternalResetPassword && !config.DisableResetPassword {
-		validator.Push(errors.New("You cannot enable both `internal reset password` and `external reset password` processes in `authentication_backend`"))
-	}
-
-	if config.EnableExternalResetPassword {
-		err := utils.IsStringAbsURL(config.ExternalResetPasswordURL)
-		if err != nil {
-			validator.Push(fmt.Errorf("Provided reset url is invalid. Error from parser: %s", err))
+	if config.PasswordReset.CustomURL.String() != "" {
+		switch config.PasswordReset.CustomURL.Scheme {
+		case schemeHTTP, schemeHTTPS:
+			config.DisableResetPassword = false
+		default:
+			validator.Push(fmt.Errorf(errFmtAuthBackendPasswordResetCustomURLScheme, config.PasswordReset.CustomURL.String(), config.PasswordReset.CustomURL.Scheme))
 		}
 	}
 }
 
-// validateFileAuthenticationBackend validates and updates the file authentication backend config.
+// validateFileAuthenticationBackend validates and updates the file authentication backend configuration.
 func validateFileAuthenticationBackend(config *schema.FileAuthenticationBackendConfiguration, validator *schema.StructValidator) {
 	if config.Path == "" {
 		validator.Push(fmt.Errorf(errFmtFileAuthBackendPathNotConfigured))