From ba04d1072b327f2bd95e3fc82cb74fb0945b4029 Mon Sep 17 00:00:00 2001 From: Amir Zarrinkafsh Date: Sat, 28 Nov 2020 00:30:27 +1100 Subject: [PATCH] [BUGFIX] Make username_attribute a mandatory placeholder in users_filter (#1449) * [BUGFIX] Make username_attribute a mandatory placeholder in users_filter Not including the `username_attribute` in the `users_filter` will cause issues with the LDAP session refresh and will result in session resets when the refresh interval has expired. This change makes said attribute mandatory for the `users_filter`. * Update version referenced in docs for fix --- config.template.yml | 2 +- docs/configuration/authentication/ldap.md | 7 +- .../configuration/validator/authentication.go | 7 +- .../validator/authentication_test.go | 91 +++---------------- 4 files changed, 24 insertions(+), 83 deletions(-) diff --git a/config.template.yml b/config.template.yml index d1dedcd2d..98c2aa92b 100644 --- a/config.template.yml +++ b/config.template.yml @@ -133,7 +133,7 @@ authentication_backend: # The users filter used in search queries to find the user profile based on input filled in login form. # Various placeholders are available to represent the user input and back reference other options of the configuration: # - {input} is a placeholder replaced by what the user inputs in the login form. - # - {username_attribute} is a placeholder replaced by what is configured in `username_attribute`. + # - {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. # diff --git a/docs/configuration/authentication/ldap.md b/docs/configuration/authentication/ldap.md index 738d1ed14..ae76f3076 100644 --- a/docs/configuration/authentication/ldap.md +++ b/docs/configuration/authentication/ldap.md @@ -78,7 +78,7 @@ authentication_backend: # The users filter used in search queries to find the user profile based on input filled in login form. # Various placeholders are available to represent the user input and back reference other options of the configuration: # - {input} is a placeholder replaced by what the user inputs in the login form. - # - {username_attribute} is a placeholder replaced by what is configured in `username_attribute`. + # - {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. # @@ -185,6 +185,11 @@ In order to avoid such problems, we highly recommended you follow https://www.ie `sAMAccountName` for Active Directory and `uid` for other implementations as the attribute holding the unique identifier for your users. +As of versions > `4.24.0` the `users_filter` must include the `username_attribute` placeholder, not including this will +result in Authelia throwing an error. +In versions <= `4.24.0` not including the `username_attribute` placeholder will cause issues with the session refresh +and will result in session resets when the refresh interval has expired, default of 5 minutes. + ## Loading a password from a secret instead of inside the configuration Password can also be defined using a [secret](../secrets.md). diff --git a/internal/configuration/validator/authentication.go b/internal/configuration/validator/authentication.go index 5082b0f10..abba521a3 100644 --- a/internal/configuration/validator/authentication.go +++ b/internal/configuration/validator/authentication.go @@ -137,7 +137,12 @@ func validateLdapAuthenticationBackend(configuration *schema.LDAPAuthenticationB validator.Push(errors.New("Please provide a users filter with `users_filter` attribute")) } else { if !strings.HasPrefix(configuration.UsersFilter, "(") || !strings.HasSuffix(configuration.UsersFilter, ")") { - validator.Push(errors.New("The users filter should contain enclosing parenthesis. For instance uid={input} should be (uid={input})")) + validator.Push(errors.New("The users filter should contain enclosing parenthesis. For instance {username_attribute}={input} should be ({username_attribute}={input})")) + } + + if !strings.Contains(configuration.UsersFilter, "{username_attribute}") { + validator.Push(errors.New("Unable to detect {username_attribute} placeholder in users_filter, your configuration is broken. " + + "Please review configuration options listed at https://docs.authelia.com/configuration/authentication/ldap.html")) } // This test helps the user know that users_filter is broken after the breaking change induced by this commit. diff --git a/internal/configuration/validator/authentication_test.go b/internal/configuration/validator/authentication_test.go index 338b0d8b5..9f85955ab 100644 --- a/internal/configuration/validator/authentication_test.go +++ b/internal/configuration/validator/authentication_test.go @@ -165,7 +165,7 @@ func (suite *LdapAuthenticationBackendSuite) SetupTest() { suite.configuration.Ldap.Password = "password" suite.configuration.Ldap.BaseDN = "base_dn" suite.configuration.Ldap.UsernameAttribute = "uid" - suite.configuration.Ldap.UsersFilter = "(uid={input})" + suite.configuration.Ldap.UsersFilter = "({username_attribute}={input})" suite.configuration.Ldap.GroupsFilter = "(cn={input})" } @@ -267,10 +267,10 @@ func (suite *LdapAuthenticationBackendSuite) TestShouldSetDefaultRefreshInterval } func (suite *LdapAuthenticationBackendSuite) TestShouldRaiseWhenUsersFilterDoesNotContainEnclosingParenthesis() { - suite.configuration.Ldap.UsersFilter = "uid={input}" + suite.configuration.Ldap.UsersFilter = "{username_attribute}={input}" ValidateAuthenticationBackend(&suite.configuration, suite.validator) require.Len(suite.T(), suite.validator.Errors(), 1) - assert.EqualError(suite.T(), suite.validator.Errors()[0], "The users filter should contain enclosing parenthesis. For instance uid={input} should be (uid={input})") + assert.EqualError(suite.T(), suite.validator.Errors()[0], "The users filter should contain enclosing parenthesis. For instance {username_attribute}={input} should be ({username_attribute}={input})") } func (suite *LdapAuthenticationBackendSuite) TestShouldRaiseWhenGroupsFilterDoesNotContainEnclosingParenthesis() { @@ -280,8 +280,15 @@ func (suite *LdapAuthenticationBackendSuite) TestShouldRaiseWhenGroupsFilterDoes assert.EqualError(suite.T(), suite.validator.Errors()[0], "The groups filter should contain enclosing parenthesis. For instance cn={input} should be (cn={input})") } +func (suite *LdapAuthenticationBackendSuite) TestShouldRaiseWhenUsersFilterDoesNotContainUsernameAttribute() { + suite.configuration.Ldap.UsersFilter = "(&({mail_attribute}={input})(objectClass=person))" + ValidateAuthenticationBackend(&suite.configuration, suite.validator) + require.Len(suite.T(), suite.validator.Errors(), 1) + assert.EqualError(suite.T(), suite.validator.Errors()[0], "Unable to detect {username_attribute} placeholder in users_filter, your configuration is broken. Please review configuration options listed at https://docs.authelia.com/configuration/authentication/ldap.html") +} + func (suite *LdapAuthenticationBackendSuite) TestShouldHelpDetectNoInputPlaceholder() { - suite.configuration.Ldap.UsersFilter = "(objectClass=person)" + suite.configuration.Ldap.UsersFilter = "(&({username_attribute}={mail_attribute})(objectClass=person))" ValidateAuthenticationBackend(&suite.configuration, suite.validator) require.Len(suite.T(), suite.validator.Errors(), 1) assert.EqualError(suite.T(), suite.validator.Errors()[0], "Unable to detect {input} placeholder in users_filter, your configuration might be broken. Please review configuration options listed at https://docs.authelia.com/configuration/authentication/ldap.html") @@ -308,79 +315,3 @@ func (suite *LdapAuthenticationBackendSuite) TestShouldAdaptLDAPURL() { func TestLdapAuthenticationBackend(t *testing.T) { suite.Run(t, new(LdapAuthenticationBackendSuite)) } - -type ActiveDirectoryAuthenticationBackendSuite struct { - suite.Suite - configuration schema.AuthenticationBackendConfiguration - validator *schema.StructValidator -} - -func (suite *ActiveDirectoryAuthenticationBackendSuite) SetupTest() { - suite.validator = schema.NewStructValidator() - suite.configuration = schema.AuthenticationBackendConfiguration{} - suite.configuration.Ldap = &schema.LDAPAuthenticationBackendConfiguration{} - suite.configuration.Ldap.Implementation = schema.LDAPImplementationActiveDirectory - suite.configuration.Ldap.URL = "ldap://ldap" - suite.configuration.Ldap.User = "user" - suite.configuration.Ldap.Password = "password" - suite.configuration.Ldap.BaseDN = "base_dn" -} - -func (suite *ActiveDirectoryAuthenticationBackendSuite) TestShouldSetActiveDirectoryDefaults() { - ValidateAuthenticationBackend(&suite.configuration, suite.validator) - - assert.Len(suite.T(), suite.validator.Errors(), 0) - - assert.Equal(suite.T(), - suite.configuration.Ldap.UsersFilter, - schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.UsersFilter) - assert.Equal(suite.T(), - suite.configuration.Ldap.UsernameAttribute, - schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.UsernameAttribute) - assert.Equal(suite.T(), - suite.configuration.Ldap.DisplayNameAttribute, - schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.DisplayNameAttribute) - assert.Equal(suite.T(), - suite.configuration.Ldap.MailAttribute, - schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.MailAttribute) - assert.Equal(suite.T(), - suite.configuration.Ldap.GroupsFilter, - schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.GroupsFilter) - assert.Equal(suite.T(), - suite.configuration.Ldap.GroupNameAttribute, - schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.GroupNameAttribute) -} - -func (suite *ActiveDirectoryAuthenticationBackendSuite) TestShouldOnlySetDefaultsIfNotManuallyConfigured() { - suite.configuration.Ldap.UsersFilter = "(&({username_attribute}={input})(objectCategory=person)(objectClass=user)(!userAccountControl:1.2.840.113556.1.4.803:=2))" - suite.configuration.Ldap.UsernameAttribute = "cn" - suite.configuration.Ldap.MailAttribute = "userPrincipalName" - suite.configuration.Ldap.DisplayNameAttribute = "name" - suite.configuration.Ldap.GroupsFilter = "(&(member={dn})(objectClass=group)(objectCategory=group))" - suite.configuration.Ldap.GroupNameAttribute = "distinguishedName" - - ValidateAuthenticationBackend(&suite.configuration, suite.validator) - - assert.NotEqual(suite.T(), - suite.configuration.Ldap.UsersFilter, - schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.UsersFilter) - assert.NotEqual(suite.T(), - suite.configuration.Ldap.UsernameAttribute, - schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.UsernameAttribute) - assert.NotEqual(suite.T(), - suite.configuration.Ldap.DisplayNameAttribute, - schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.DisplayNameAttribute) - assert.NotEqual(suite.T(), - suite.configuration.Ldap.MailAttribute, - schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.MailAttribute) - assert.NotEqual(suite.T(), - suite.configuration.Ldap.GroupsFilter, - schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.GroupsFilter) - assert.NotEqual(suite.T(), - suite.configuration.Ldap.GroupNameAttribute, - schema.DefaultLDAPAuthenticationBackendImplementationActiveDirectoryConfiguration.GroupNameAttribute) -} - -func TestActiveDirectoryAuthenticationBackend(t *testing.T) { - suite.Run(t, new(ActiveDirectoryAuthenticationBackendSuite)) -}