fix: removed deprecated smtp/ldap options (#1912)

This removes the deprecated options from 4.25. This includes the LDAP filters which allow {0} or {1} placeholders. The new aliases are documented. Additionally it refactors the keys validator to use uniform messages for most replaced keys.
authelia · Apr 16, 2021 · cc4f47f · cc4f47f
1 parent 168dbf7
commit cc4f47f
Show file tree

Hide file tree

Showing 21 changed files with 292 additions and 423 deletions.
diff --git a/cmd/authelia/main.go b/cmd/authelia/main.go
@@ -36,7 +36,7 @@ func startServer() {
 		os.Exit(1)
 	}
 
-	autheliaCertPool, errs, nonFatalErrs := utils.NewX509CertPool(config.CertificatesDirectory, config)
+	autheliaCertPool, errs, nonFatalErrs := utils.NewX509CertPool(config.CertificatesDirectory)
 	if len(errs) > 0 {
 		for _, err := range errs {
 			logger.Error(err)
@@ -89,8 +89,8 @@ func startServer() {
 	switch {
 	case config.AuthenticationBackend.File != nil:
 		userProvider = authentication.NewFileUserProvider(config.AuthenticationBackend.File)
-	case config.AuthenticationBackend.Ldap != nil:
-		userProvider = authentication.NewLDAPUserProvider(*config.AuthenticationBackend.Ldap, autheliaCertPool)
+	case config.AuthenticationBackend.LDAP != nil:
+		userProvider = authentication.NewLDAPUserProvider(*config.AuthenticationBackend.LDAP, autheliaCertPool)
 	default:
 		logger.Fatalf("Unrecognized authentication backend")
 	}

diff --git a/config.template.yml b/config.template.yml
@@ -162,8 +162,6 @@ authentication_backend:
     ## - {input} is a placeholder replaced by what the user inputs in the login form.
     ## - {username_attribute} is a mandatory placeholder replaced by what is configured in `username_attribute`.
     ## - {mail_attribute} is a placeholder replaced by what is configured in `mail_attribute`.
-    ## - DON'T USE - {0} is an alias for {input} supported for backward compatibility but it will be deprecated in later
-    ##   versions, so please don't use it.
     ##
     ## Recommended settings are as follows:
     ## - Microsoft Active Directory: (&({username_attribute}={input})(objectCategory=person)(objectClass=user))
@@ -185,10 +183,6 @@ authentication_backend:
     ## - {dn} is a matcher replaced by the user distinguished name, aka, user DN.
     ## - {username_attribute} is a placeholder replaced by what is configured in `username_attribute`.
     ## - {mail_attribute} is a placeholder replaced by what is configured in `mail_attribute`.
-    ## - DON'T USE - {0} is an alias for {input} supported for backward compatibility but it will be deprecated in later
-    ##   versions, so please don't use it.
-    ## - DON'T USE - {1} is an alias for {username} supported for backward compatibility but it will be deprecated in
-    ##   later version, so please don't use it.
     ##
     ## If your groups use the `groupOfUniqueNames` structure use this instead:
     ##    (&(uniquemember={dn})(objectclass=groupOfUniqueNames))

diff --git a/docs/configuration/authentication/ldap.md b/docs/configuration/authentication/ldap.md
@@ -148,7 +148,11 @@ Similar to [additional_users_dn](#additional_users_dn) but it applies to group s
 
 ### groups_filter
 
-Similar to [users_filter](#users_filter) but it applies to group searches.
+Similar to [users_filter](#users_filter) but it applies to group searches. In order to include groups the memeber is not
+a direct member of, but is a member of another group that is a member of those (i.e. recursive groups), you may try
+using the following filter which is currently only tested against Microsoft Active Directory:
+
+`(&(member:1.2.840.113556.1.4.1941:={dn})(objectClass=group)(objectCategory=group))`
 
 ### mail_attribute
 

diff --git a/internal/authentication/ldap_user_provider.go b/internal/authentication/ldap_user_provider.go
@@ -62,27 +62,6 @@ func NewLDAPUserProviderWithFactory(configuration schema.LDAPAuthenticationBacke
 }
 
 func (p *LDAPUserProvider) parseDynamicConfiguration() {
-	// Deprecated: This is temporary for deprecation notice purposes. TODO: Remove in 4.28.
-	if strings.Contains(p.configuration.UsersFilter, "{0}") {
-		p.logger.Warnf("DEPRECATION NOTICE: LDAP Users Filter will no longer support replacing `{0}` in 4.28.0. Please use `{input}` instead.")
-
-		p.configuration.UsersFilter = strings.ReplaceAll(p.configuration.UsersFilter, "{0}", "{input}")
-	}
-
-	// Deprecated: This is temporary for deprecation notice purposes. TODO: Remove in 4.28.
-	if strings.Contains(p.configuration.GroupsFilter, "{0}") {
-		p.logger.Warnf("DEPRECATION NOTICE: LDAP Groups Filter will no longer support replacing `{0}` in 4.28.0. Please use `{input}` instead.")
-
-		p.configuration.GroupsFilter = strings.ReplaceAll(p.configuration.GroupsFilter, "{0}", "{input}")
-	}
-
-	// Deprecated: This is temporary for deprecation notice purposes. TODO: Remove in 4.28.
-	if strings.Contains(p.configuration.GroupsFilter, "{1}") {
-		p.logger.Warnf("DEPRECATION NOTICE: LDAP Groups Filter will no longer support replacing `{1}` in 4.28.0. Please use `{username}` instead.")
-
-		p.configuration.GroupsFilter = strings.ReplaceAll(p.configuration.GroupsFilter, "{1}", "{username}")
-	}
-
 	p.configuration.UsersFilter = strings.ReplaceAll(p.configuration.UsersFilter, "{username_attribute}", p.configuration.UsernameAttribute)
 	p.configuration.UsersFilter = strings.ReplaceAll(p.configuration.UsersFilter, "{mail_attribute}", p.configuration.MailAttribute)
 	p.configuration.UsersFilter = strings.ReplaceAll(p.configuration.UsersFilter, "{display_name_attribute}", p.configuration.DisplayNameAttribute)

diff --git a/internal/authentication/ldap_user_provider_test.go b/internal/authentication/ldap_user_provider_test.go
@@ -713,8 +713,8 @@ func TestShouldParseDynamicConfiguration(t *testing.T) {
 			UsernameAttribute:    "uid",
 			MailAttribute:        "mail",
 			DisplayNameAttribute: "displayname",
-			UsersFilter:          "(&(|({username_attribute}={0})({mail_attribute}={0})({display_name_attribute}={0}))(objectCategory=person)(objectClass=user)(!userAccountControl:1.2.840.113556.1.4.803:=2)(!pwdLastSet=0))",
-			GroupsFilter:         "(&(|(member={dn})(member={0})(member={1}))(objectClass=group))",
+			UsersFilter:          "(&(|({username_attribute}={input})({mail_attribute}={input})({display_name_attribute}={input}))(objectCategory=person)(objectClass=user)(!userAccountControl:1.2.840.113556.1.4.803:=2)(!pwdLastSet=0))",
+			GroupsFilter:         "(&(|(member={dn})(member={input})(member={username}))(objectClass=group))",
 			AdditionalUsersDN:    "ou=users",
 			AdditionalGroupsDN:   "ou=groups",
 			BaseDN:               "dc=example,dc=com",

diff --git a/internal/configuration/config.template.yml b/internal/configuration/config.template.yml
@@ -162,8 +162,6 @@ authentication_backend:
     ## - {input} is a placeholder replaced by what the user inputs in the login form.
     ## - {username_attribute} is a mandatory placeholder replaced by what is configured in `username_attribute`.
     ## - {mail_attribute} is a placeholder replaced by what is configured in `mail_attribute`.
-    ## - DON'T USE - {0} is an alias for {input} supported for backward compatibility but it will be deprecated in later
-    ##   versions, so please don't use it.
     ##
     ## Recommended settings are as follows:
     ## - Microsoft Active Directory: (&({username_attribute}={input})(objectCategory=person)(objectClass=user))
@@ -185,10 +183,6 @@ authentication_backend:
     ## - {dn} is a matcher replaced by the user distinguished name, aka, user DN.
     ## - {username_attribute} is a placeholder replaced by what is configured in `username_attribute`.
     ## - {mail_attribute} is a placeholder replaced by what is configured in `mail_attribute`.
-    ## - DON'T USE - {0} is an alias for {input} supported for backward compatibility but it will be deprecated in later
-    ##   versions, so please don't use it.
-    ## - DON'T USE - {1} is an alias for {username} supported for backward compatibility but it will be deprecated in
-    ##   later version, so please don't use it.
     ##
     ## If your groups use the `groupOfUniqueNames` structure use this instead:
     ##    (&(uniquemember={dn})(objectclass=groupOfUniqueNames))

diff --git a/internal/configuration/reader_test.go b/internal/configuration/reader_test.go
@@ -195,7 +195,7 @@ func TestShouldParseConfigFile(t *testing.T) {
 	assert.Equal(t, "duo_secret_from_env", config.DuoAPI.SecretKey)
 
 	assert.Equal(t, "session_secret_from_env", config.Session.Secret)
-	assert.Equal(t, "ldap_secret_from_env", config.AuthenticationBackend.Ldap.Password)
+	assert.Equal(t, "ldap_secret_from_env", config.AuthenticationBackend.LDAP.Password)
 	assert.Equal(t, "smtp_secret_from_env", config.Notifier.SMTP.Password)
 	assert.Equal(t, "redis_secret_from_env", config.Session.Redis.Password)
 	assert.Equal(t, "redis-sentinel_secret_from_env", config.Session.Redis.HighAvailability.SentinelPassword)
@@ -253,7 +253,7 @@ func TestShouldNotParseConfigFileWithOldOrUnexpectedKeys(t *testing.T) {
 		return errors[i].Error() < errors[j].Error()
 	})
 	assert.EqualError(t, errors[0], "config key not expected: loggy_file")
-	assert.EqualError(t, errors[1], "config key replaced: logs_level is now log_level")
+	assert.EqualError(t, errors[1], "invalid configuration key 'logs_level' was replaced by 'log_level'")
 }
 
 func TestShouldValidateConfigurationTemplate(t *testing.T) {

diff --git a/internal/configuration/schema/authentication.go b/internal/configuration/schema/authentication.go
@@ -17,8 +17,6 @@ type LDAPAuthenticationBackendConfiguration struct {
 	Password             string     `mapstructure:"password"`
 	StartTLS             bool       `mapstructure:"start_tls"`
 	TLS                  *TLSConfig `mapstructure:"tls"`
-	SkipVerify           *bool      `mapstructure:"skip_verify"`         // Deprecated: Replaced with LDAPAuthenticationBackendConfiguration.TLS.SkipVerify. TODO: Remove in 4.28.
-	MinimumTLSVersion    string     `mapstructure:"minimum_tls_version"` // Deprecated: Replaced with LDAPAuthenticationBackendConfiguration.TLS.MinimumVersion. TODO: Remove in 4.28.
 }
 
 // FileAuthenticationBackendConfiguration represents the configuration related to file-based backend.
@@ -41,7 +39,7 @@ type PasswordConfiguration struct {
 type AuthenticationBackendConfiguration struct {
 	DisableResetPassword bool                                    `mapstructure:"disable_reset_password"`
 	RefreshInterval      string                                  `mapstructure:"refresh_interval"`
-	Ldap                 *LDAPAuthenticationBackendConfiguration `mapstructure:"ldap"`
+	LDAP                 *LDAPAuthenticationBackendConfiguration `mapstructure:"ldap"`
 	File                 *FileAuthenticationBackendConfiguration `mapstructure:"file"`
 }
 

diff --git a/internal/configuration/schema/notifier.go b/internal/configuration/schema/notifier.go
@@ -18,8 +18,6 @@ type SMTPNotifierConfiguration struct {
 	DisableRequireTLS   bool       `mapstructure:"disable_require_tls"`
 	DisableHTMLEmails   bool       `mapstructure:"disable_html_emails"`
 	TLS                 *TLSConfig `mapstructure:"tls"`
-	TrustedCert         string     `mapstructure:"trusted_cert"`        // Deprecated: Replaced with Global Option CertificatesDirectory. TODO: Remove in 4.28.
-	DisableVerifyCert   *bool      `mapstructure:"disable_verify_cert"` // Deprecated: Replaced with LDAPAuthenticationBackendConfiguration.TLS.SkipVerify. TODO: Remove in 4.28.
 }
 
 // NotifierConfiguration represents the configuration of the notifier to use when sending notifications to users.

diff --git a/internal/configuration/validator/authentication.go b/internal/configuration/validator/authentication.go
@@ -10,6 +10,32 @@ import (
 	"github.com/authelia/authelia/internal/utils"
 )
 
+// ValidateAuthenticationBackend validates and update authentication backend configuration.
+func ValidateAuthenticationBackend(configuration *schema.AuthenticationBackendConfiguration, validator *schema.StructValidator) {
+	if configuration.LDAP == nil && configuration.File == nil {
+		validator.Push(errors.New("Please provide `ldap` or `file` object in `authentication_backend`"))
+	}
+
+	if configuration.LDAP != nil && configuration.File != nil {
+		validator.Push(errors.New("You cannot provide both `ldap` and `file` objects in `authentication_backend`"))
+	}
+
+	if configuration.File != nil {
+		validateFileAuthenticationBackend(configuration.File, validator)
+	} else if configuration.LDAP != nil {
+		validateLDAPAuthenticationBackend(configuration.LDAP, validator)
+	}
+
+	if configuration.RefreshInterval == "" {
+		configuration.RefreshInterval = schema.RefreshIntervalDefault
+	} else {
+		_, err := utils.ParseDurationString(configuration.RefreshInterval)
+		if err != nil && configuration.RefreshInterval != schema.ProfileRefreshDisabled && configuration.RefreshInterval != schema.ProfileRefreshAlways {
+			validator.Push(fmt.Errorf("Auth Backend `refresh_interval` is configured to '%s' but it must be either a duration notation or one of 'disable', or 'always'. Error from parser: %s", configuration.RefreshInterval, err))
+		}
+	}
+}
+
 //nolint:gocyclo // TODO: Consider refactoring/simplifying, time permitting.
 func validateFileAuthenticationBackend(configuration *schema.FileAuthenticationBackendConfiguration, validator *schema.StructValidator) {
 	if configuration.Path == "" {
@@ -72,58 +98,15 @@ func validateFileAuthenticationBackend(configuration *schema.FileAuthenticationB
 	}
 }
 
-// Wrapper for test purposes to exclude the hostname from the return.
-func validateLdapURLSimple(ldapURL string, validator *schema.StructValidator) (finalURL string) {
-	finalURL, _ = validateLdapURL(ldapURL, validator)
-
-	return finalURL
-}
-
-func validateLdapURL(ldapURL string, validator *schema.StructValidator) (finalURL string, hostname string) {
-	parsedURL, err := url.Parse(ldapURL)
-
-	if err != nil {
-		validator.Push(errors.New("Unable to parse URL to ldap server. The scheme is probably missing: ldap:// or ldaps://"))
-		return "", ""
-	}
-
-	if !(parsedURL.Scheme == schemeLDAP || parsedURL.Scheme == schemeLDAPS) {
-		validator.Push(errors.New("Unknown scheme for ldap url, should be ldap:// or ldaps://"))
-		return "", ""
-	}
-
-	return parsedURL.String(), parsedURL.Hostname()
-}
-
-//nolint:gocyclo // TODO: Consider refactoring/simplifying, time permitting.
-func validateLdapAuthenticationBackend(configuration *schema.LDAPAuthenticationBackendConfiguration, validator *schema.StructValidator) {
+func validateLDAPAuthenticationBackend(configuration *schema.LDAPAuthenticationBackendConfiguration, validator *schema.StructValidator) {
 	if configuration.Implementation == "" {
 		configuration.Implementation = schema.DefaultLDAPAuthenticationBackendConfiguration.Implementation
 	}
 
-	nilTLS := configuration.TLS == nil
-	if nilTLS {
+	if configuration.TLS == nil {
 		configuration.TLS = schema.DefaultLDAPAuthenticationBackendConfiguration.TLS
 	}
 
-	// Deprecated. Maps deprecated values to the new ones. TODO: Remove in 4.28 (if block).
-	if configuration.SkipVerify != nil {
-		validator.PushWarning(errors.New("DEPRECATED: LDAP Auth Backend `skip_verify` option has been replaced by `authentication_backend.ldap.tls.skip_verify` (will be removed in 4.28.0)"))
-
-		if nilTLS {
-			configuration.TLS.SkipVerify = *configuration.SkipVerify
-		}
-	}
-
-	// Deprecated. Maps deprecated values to the new ones. TODO: Remove in 4.28 (if block).
-	if configuration.MinimumTLSVersion != "" {
-		validator.PushWarning(errors.New("DEPRECATED: LDAP Auth Backend `minimum_tls_version` option has been replaced by `authentication_backend.ldap.tls.minimum_version` (will be removed in 4.28.0)"))
-
-		if nilTLS {
-			configuration.TLS.MinimumVersion = configuration.MinimumTLSVersion
-		}
-	}
-
 	if configuration.TLS.MinimumVersion == "" {
 		configuration.TLS.MinimumVersion = schema.DefaultLDAPAuthenticationBackendConfiguration.TLS.MinimumVersion
 	}
@@ -134,17 +117,28 @@ func validateLdapAuthenticationBackend(configuration *schema.LDAPAuthenticationB
 
 	switch configuration.Implementation {
 	case schema.LDAPImplementationCustom:
-		setDefaultImplementationCustomLdapAuthenticationBackend(configuration)
+		setDefaultImplementationCustomLDAPAuthenticationBackend(configuration)
 	case schema.LDAPImplementationActiveDirectory:
-		setDefaultImplementationActiveDirectoryLdapAuthenticationBackend(configuration)
+		setDefaultImplementationActiveDirectoryLDAPAuthenticationBackend(configuration)
 	default:
 		validator.Push(fmt.Errorf("authentication backend ldap implementation must be blank or one of the following values `%s`, `%s`", schema.LDAPImplementationCustom, schema.LDAPImplementationActiveDirectory))
 	}
 
+	if strings.Contains(configuration.UsersFilter, "{0}") {
+		validator.Push(fmt.Errorf("authentication backend ldap users filter must not contain removed placeholders" +
+			", {0} has been replaced with {input}"))
+	}
+
+	if strings.Contains(configuration.GroupsFilter, "{0}") ||
+		strings.Contains(configuration.GroupsFilter, "{1}") {
+		validator.Push(fmt.Errorf("authentication backend ldap groups filter must not contain removed " +
+			"placeholders, {0} has been replaced with {input} and {1} has been replaced with {username}"))
+	}
+
 	if configuration.URL == "" {
 		validator.Push(errors.New("Please provide a URL to the LDAP server"))
 	} else {
-		ldapURL, serverName := validateLdapURL(configuration.URL, validator)
+		ldapURL, serverName := validateLDAPURL(configuration.URL, validator)
 
 		configuration.URL = ldapURL
 
@@ -153,6 +147,33 @@ func validateLdapAuthenticationBackend(configuration *schema.LDAPAuthenticationB
 		}
 	}
 
+	validateLDAPRequiredParameters(configuration, validator)
+}
+
+// Wrapper for test purposes to exclude the hostname from the return.
+func validateLDAPURLSimple(ldapURL string, validator *schema.StructValidator) (finalURL string) {
+	finalURL, _ = validateLDAPURL(ldapURL, validator)
+
+	return finalURL
+}
+
+func validateLDAPURL(ldapURL string, validator *schema.StructValidator) (finalURL string, hostname string) {
+	parsedURL, err := url.Parse(ldapURL)
+
+	if err != nil {
+		validator.Push(errors.New("Unable to parse URL to ldap server. The scheme is probably missing: ldap:// or ldaps://"))
+		return "", ""
+	}
+
+	if !(parsedURL.Scheme == schemeLDAP || parsedURL.Scheme == schemeLDAPS) {
+		validator.Push(errors.New("Unknown scheme for ldap url, should be ldap:// or ldaps://"))
+		return "", ""
+	}
+
+	return parsedURL.String(), parsedURL.Hostname()
+}
+
+func validateLDAPRequiredParameters(configuration *schema.LDAPAuthenticationBackendConfiguration, validator *schema.StructValidator) {
 	// TODO: see if it's possible to disable this check if disable_reset_password is set and when anonymous/user binding is supported (#101 and #387)
 	if configuration.User == "" {
 		validator.Push(errors.New("Please provide a user name to connect to the LDAP server"))
@@ -193,7 +214,7 @@ func validateLdapAuthenticationBackend(configuration *schema.LDAPAuthenticationB
 	}
 }
 
-func setDefaultImplementationActiveDirectoryLdapAuthenticationBackend(configuration *schema.LDAPAuthenticationBackendConfiguration) {
+func setDefaultImplementationActiveDirectoryLDAPAuthenticationBackend(configuration *schema.LDAPAuthenticationBackendConfiguration) {
 	if configuration.UsersFilter == "" {
 		configuration.UsersFilter = schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.UsersFilter
 	}
@@ -219,7 +240,7 @@ func setDefaultImplementationActiveDirectoryLdapAuthenticationBackend(configurat
 	}
 }
 
-func setDefaultImplementationCustomLdapAuthenticationBackend(configuration *schema.LDAPAuthenticationBackendConfiguration) {
+func setDefaultImplementationCustomLDAPAuthenticationBackend(configuration *schema.LDAPAuthenticationBackendConfiguration) {
 	if configuration.UsernameAttribute == "" {
 		configuration.UsernameAttribute = schema.DefaultLDAPAuthenticationBackendConfiguration.UsernameAttribute
 	}
@@ -236,29 +257,3 @@ func setDefaultImplementationCustomLdapAuthenticationBackend(configuration *sche
 		configuration.DisplayNameAttribute = schema.DefaultLDAPAuthenticationBackendConfiguration.DisplayNameAttribute
 	}
 }
-
-// ValidateAuthenticationBackend validates and update authentication backend configuration.
-func ValidateAuthenticationBackend(configuration *schema.AuthenticationBackendConfiguration, validator *schema.StructValidator) {
-	if configuration.Ldap == nil && configuration.File == nil {
-		validator.Push(errors.New("Please provide `ldap` or `file` object in `authentication_backend`"))
-	}
-
-	if configuration.Ldap != nil && configuration.File != nil {
-		validator.Push(errors.New("You cannot provide both `ldap` and `file` objects in `authentication_backend`"))
-	}
-
-	if configuration.File != nil {
-		validateFileAuthenticationBackend(configuration.File, validator)
-	} else if configuration.Ldap != nil {
-		validateLdapAuthenticationBackend(configuration.Ldap, validator)
-	}
-
-	if configuration.RefreshInterval == "" {
-		configuration.RefreshInterval = schema.RefreshIntervalDefault
-	} else {
-		_, err := utils.ParseDurationString(configuration.RefreshInterval)
-		if err != nil && configuration.RefreshInterval != schema.ProfileRefreshDisabled && configuration.RefreshInterval != schema.ProfileRefreshAlways {
-			validator.Push(fmt.Errorf("Auth Backend `refresh_interval` is configured to '%s' but it must be either a duration notation or one of 'disable', or 'always'. Error from parser: %s", configuration.RefreshInterval, err))
-		}
-	}
-}