diff --git a/BREAKING.md b/BREAKING.md index c26c15fac..bd37586b1 100644 --- a/BREAKING.md +++ b/BREAKING.md @@ -6,6 +6,12 @@ recommended not to use the 'latest' Docker image tag blindly but pick a version and read this documentation before upgrading. This is where you will get information about breaking changes and about what you should do to overcome those changes. +## Breaking in v4.10.0 +* Revert of `users_filter` purpose. This option now represents the complete search filter again, meaning +there is no more automatic filter computation based on username. This gives the most flexibility. +For instance, this allows administrators to choose whether they want the users to be able to sign in with +username or email. + ## Breaking in v4.7.0 * `logs_level` configuration key has been renamed to `log_level`. * `users_filter` was a search pattern for a given user with the `{0}` matcher replaced with the diff --git a/config.template.yml b/config.template.yml index 9ae92436e..39ca3887d 100644 --- a/config.template.yml +++ b/config.template.yml @@ -82,33 +82,42 @@ authentication_backend: # The base dn for every entries base_dn: dc=example,dc=com - # The attribute holding the username of the user (introduced to handle - # case insensitive search queries: #561). - # Microsoft Active Directory usually uses 'sAMAccountName' - # OpenLDAP usually uses 'uid' + # The attribute holding the username of the user. This attribute is used to populate + # the username in the session information. It was introduced due to #561 to handle case + # insensitive search queries. + # For you information, Microsoft Active Directory usually uses 'sAMAccountName' and OpenLDAP + # usually uses 'uid' username_attribute: uid # An additional dn to define the scope to all users additional_users_dn: ou=users - # This attribute is optional. The user filter used in the LDAP search queries - # is a combination of this filter and the username attribute. - # This filter is used to reduce the scope of users targeted by the LDAP search query. - # For instance, if the username attribute is set to 'uid', the computed filter is - # (&(uid=)(objectClass=person)) + # 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`. + # - {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 '(&(objectCategory=person)(objectClass=user))' - # OpenLDAP '(objectClass=person)' or '(objectClass=inetOrgPerson)' - users_filter: (objectClass=person) + # - Microsoft Active Directory: (&({username_attribute}={input})(objectCategory=person)(objectClass=user)) + # - OpenLDAP: (&({username_attribute}={input})(objectClass=person))' or '(&({username_attribute}={input})(objectClass=inetOrgPerson)) + # + # To allow sign in both with username and email, one can use a filter like + # (&(|({username_attribute}={input})({mail_attribute={input}))(objectClass=person)) + users_filter: (&({username_attribute}={input})(objectClass=person)) # An additional dn to define the scope of groups additional_groups_dn: ou=groups - # The groups filter used for retrieving groups of a given user. - # {0} is a matcher replaced by username (as provided in login portal). - # {1} is a matcher replaced by username (as stored in LDAP). - # {dn} is a matcher replaced by user DN. - # 'member={dn}' by default. + # The groups filter used in search queries to find the groups of the user. + # - {input} is a placeholder replaced by what the user inputs in the login form. + # - {username} is a placeholder replace by the username stored in LDAP (based on `username_attribute`). + # - {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. groups_filter: (&(member={dn})(objectclass=groupOfNames)) # The attribute holding the name of the group diff --git a/docs/configuration/authentication/ldap.md b/docs/configuration/authentication/ldap.md index 757e155ec..eb6866c6e 100644 --- a/docs/configuration/authentication/ldap.md +++ b/docs/configuration/authentication/ldap.md @@ -26,33 +26,42 @@ authentication_backend: # The base dn for every entries base_dn: dc=example,dc=com - # The attribute holding the username of the user (introduced to handle - # case insensitive search queries: #561). - # Microsoft Active Directory usually uses 'sAMAccountName' - # OpenLDAP usually uses 'uid' + # The attribute holding the username of the user. This attribute is used to populate + # the username in the session information. It was introduced due to #561 to handle case + # insensitive search queries. + # For you information, Microsoft Active Directory usually uses 'sAMAccountName' and OpenLDAP + # usually uses 'uid' username_attribute: uid # An additional dn to define the scope to all users additional_users_dn: ou=users - # This attribute is optional. The user filter used in the LDAP search queries - # is a combination of this filter and the username attribute. - # This filter is used to reduce the scope of users targeted by the LDAP search query. - # For instance, if the username attribute is set to 'uid', the computed filter is - # (&(uid=)(objectClass=person)) + # 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`. + # - {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 '(&(objectCategory=person)(objectClass=user))' - # OpenLDAP '(objectClass=person)' or '(objectClass=inetOrgPerson)' - users_filter: (objectClass=person) + # - Microsoft Active Directory: (&({username_attribute}={input})(objectCategory=person)(objectClass=user)) + # - OpenLDAP: (&({username_attribute}={input})(objectClass=person))' or '(&({username_attribute}={input})(objectClass=inetOrgPerson)) + # + # To allow sign in both with username and email, one can use a filter like + # (&(|({username_attribute}={input})({mail_attribute={input}))(objectClass=person)) + users_filter: (&({username_attribute}={input})(objectClass=person)) # An additional dn to define the scope of groups additional_groups_dn: ou=groups - # The groups filter used for retrieving groups of a given user. - # {0} is a matcher replaced by username (as provided in login portal). - # {1} is a matcher replaced by username (as stored in LDAP). - # {dn} is a matcher replaced by user DN. - # 'member={dn}' by default. + # The groups filter used in search queries to find the groups of the user. + # - {input} is a placeholder replaced by what the user inputs in the login form. + # - {username} is a placeholder replace by the username stored in LDAP (based on `username_attribute`). + # - {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. groups_filter: (&(member={dn})(objectclass=groupOfNames)) # The attribute holding the name of the group diff --git a/internal/authentication/ldap_user_provider.go b/internal/authentication/ldap_user_provider.go index 5a9d77741..02d535277 100644 --- a/internal/authentication/ldap_user_provider.go +++ b/internal/authentication/ldap_user_provider.go @@ -68,21 +68,21 @@ func (p *LDAPUserProvider) connect(userDN string, password string) (LDAPConnecti } // CheckUserPassword checks if provided password matches for the given user. -func (p *LDAPUserProvider) CheckUserPassword(username string, password string) (bool, error) { +func (p *LDAPUserProvider) CheckUserPassword(inputUsername string, password string) (bool, error) { adminClient, err := p.connect(p.configuration.User, p.configuration.Password) if err != nil { return false, err } defer adminClient.Close() - profile, err := p.getUserProfile(adminClient, username) + profile, err := p.getUserProfile(adminClient, inputUsername) if err != nil { return false, err } conn, err := p.connect(profile.DN, password) if err != nil { - return false, fmt.Errorf("Authentication of user %s failed. Cause: %s", username, err) + return false, fmt.Errorf("Authentication of user %s failed. Cause: %s", inputUsername, err) } defer conn.Close() @@ -91,13 +91,14 @@ func (p *LDAPUserProvider) CheckUserPassword(username string, password string) ( // OWASP recommends to escape some special characters // https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/LDAP_Injection_Prevention_Cheat_Sheet.md -const SpecialLDAPRunes = "\\,#+<>;\"=" +const specialLDAPRunes = ",#+<>;\"=" -func (p *LDAPUserProvider) ldapEscape(input string) string { - for _, c := range SpecialLDAPRunes { - input = strings.ReplaceAll(input, string(c), fmt.Sprintf("\\%c", c)) +func (p *LDAPUserProvider) ldapEscape(inputUsername string) string { + inputUsername = ldap.EscapeFilter(inputUsername) + for _, c := range specialLDAPRunes { + inputUsername = strings.ReplaceAll(inputUsername, string(c), fmt.Sprintf("\\%c", c)) } - return input + return inputUsername } type ldapUserProfile struct { @@ -106,12 +107,26 @@ type ldapUserProfile struct { Username string } -func (p *LDAPUserProvider) getUserProfile(conn LDAPConnection, username string) (*ldapUserProfile, error) { - username = p.ldapEscape(username) - userFilter := fmt.Sprintf("(%s=%s)", p.configuration.UsernameAttribute, username) - if p.configuration.UsersFilter != "" { - userFilter = fmt.Sprintf("(&%s%s)", userFilter, p.configuration.UsersFilter) - } +func (p *LDAPUserProvider) resolveUsersFilter(userFilter string, inputUsername string) string { + inputUsername = p.ldapEscape(inputUsername) + + // We temporarily keep placeholder {0} for backward compatibility + userFilter = strings.ReplaceAll(userFilter, "{0}", inputUsername) + + // The {username} placeholder is equivalent to {0}, it's the new way, a named placeholder. + userFilter = strings.ReplaceAll(userFilter, "{input}", inputUsername) + + // {username_attribute} and {mail_attribute} are replaced by the content of the attribute defined + // in configuration. + userFilter = strings.ReplaceAll(userFilter, "{username_attribute}", p.configuration.UsernameAttribute) + userFilter = strings.ReplaceAll(userFilter, "{mail_attribute}", p.configuration.MailAttribute) + return userFilter +} + +func (p *LDAPUserProvider) getUserProfile(conn LDAPConnection, inputUsername string) (*ldapUserProfile, error) { + userFilter := p.resolveUsersFilter(p.configuration.UsersFilter, inputUsername) + logging.Logger().Tracef("Computed user filter is %s", userFilter) + baseDN := p.configuration.BaseDN if p.configuration.AdditionalUsersDN != "" { baseDN = p.configuration.AdditionalUsersDN + "," + baseDN @@ -129,15 +144,15 @@ func (p *LDAPUserProvider) getUserProfile(conn LDAPConnection, username string) sr, err := conn.Search(searchRequest) if err != nil { - return nil, fmt.Errorf("Cannot find user DN of user %s. Cause: %s", username, err) + return nil, fmt.Errorf("Cannot find user DN of user %s. Cause: %s", inputUsername, err) } if len(sr.Entries) == 0 { - return nil, fmt.Errorf("No user %s found", username) + return nil, fmt.Errorf("No user %s found", inputUsername) } if len(sr.Entries) > 1 { - return nil, fmt.Errorf("Multiple users %s found", username) + return nil, fmt.Errorf("Multiple users %s found", inputUsername) } userProfile := ldapUserProfile{ @@ -148,51 +163,55 @@ func (p *LDAPUserProvider) getUserProfile(conn LDAPConnection, username string) userProfile.Emails = attr.Values } else if attr.Name == p.configuration.UsernameAttribute { if len(attr.Values) != 1 { - return nil, fmt.Errorf("User %s cannot have multiple value for attribute %s", username, p.configuration.UsernameAttribute) + return nil, fmt.Errorf("User %s cannot have multiple value for attribute %s", + inputUsername, p.configuration.UsernameAttribute) } userProfile.Username = attr.Values[0] } } if userProfile.DN == "" { - return nil, fmt.Errorf("No DN has been found for user %s", username) + return nil, fmt.Errorf("No DN has been found for user %s", inputUsername) } return &userProfile, nil } -func (p *LDAPUserProvider) createGroupsFilter(conn LDAPConnection, username string) (string, error) { - if strings.Contains(p.configuration.GroupsFilter, "{0}") { - return strings.Replace(p.configuration.GroupsFilter, "{0}", username, -1), nil - } else if strings.Contains(p.configuration.GroupsFilter, "{dn}") { - profile, err := p.getUserProfile(conn, username) - if err != nil { - return "", err - } - return strings.Replace(p.configuration.GroupsFilter, "{dn}", ldap.EscapeFilter(profile.DN), -1), nil - } else if strings.Contains(p.configuration.GroupsFilter, "{1}") { - profile, err := p.getUserProfile(conn, username) - if err != nil { - return "", err - } - return strings.Replace(p.configuration.GroupsFilter, "{1}", profile.Username, -1), nil +func (p *LDAPUserProvider) resolveGroupsFilter(inputUsername string, profile *ldapUserProfile) (string, error) { + inputUsername = p.ldapEscape(inputUsername) + + // We temporarily keep placeholder {0} for backward compatibility + groupFilter := strings.ReplaceAll(p.configuration.GroupsFilter, "{0}", inputUsername) + groupFilter = strings.ReplaceAll(groupFilter, "{input}", inputUsername) + if profile != nil { + // We temporarily keep placeholder {1} for backward compatibility + groupFilter = strings.ReplaceAll(groupFilter, "{1}", ldap.EscapeFilter(profile.Username)) + groupFilter = strings.ReplaceAll(groupFilter, "{username}", ldap.EscapeFilter(profile.Username)) + groupFilter = strings.ReplaceAll(groupFilter, "{dn}", ldap.EscapeFilter(profile.DN)) } - return p.configuration.GroupsFilter, nil + + return groupFilter, nil } // GetDetails retrieve the groups a user belongs to. -func (p *LDAPUserProvider) GetDetails(username string) (*UserDetails, error) { +func (p *LDAPUserProvider) GetDetails(inputUsername string) (*UserDetails, error) { conn, err := p.connect(p.configuration.User, p.configuration.Password) if err != nil { return nil, err } defer conn.Close() - groupsFilter, err := p.createGroupsFilter(conn, username) + profile, err := p.getUserProfile(conn, inputUsername) if err != nil { - return nil, fmt.Errorf("Unable to create group filter for user %s. Cause: %s", username, err) + return nil, err } + groupsFilter, err := p.resolveGroupsFilter(inputUsername, profile) + if err != nil { + return nil, fmt.Errorf("Unable to create group filter for user %s. Cause: %s", inputUsername, err) + } + logging.Logger().Tracef("Computed groups filter is %s", groupsFilter) + groupBaseDN := p.configuration.BaseDN if p.configuration.AdditionalGroupsDN != "" { groupBaseDN = p.configuration.AdditionalGroupsDN + "," + groupBaseDN @@ -207,24 +226,19 @@ func (p *LDAPUserProvider) GetDetails(username string) (*UserDetails, error) { sr, err := conn.Search(searchGroupRequest) if err != nil { - return nil, fmt.Errorf("Unable to retrieve groups of user %s. Cause: %s", username, err) + return nil, fmt.Errorf("Unable to retrieve groups of user %s. Cause: %s", inputUsername, err) } groups := make([]string, 0) for _, res := range sr.Entries { if len(res.Attributes) == 0 { - logging.Logger().Warningf("No groups retrieved from LDAP for user %s", username) + logging.Logger().Warningf("No groups retrieved from LDAP for user %s", inputUsername) break } // append all values of the document. Normally there should be only one per document. groups = append(groups, res.Attributes[0].Values...) } - profile, err := p.getUserProfile(conn, username) - if err != nil { - return nil, err - } - return &UserDetails{ Username: profile.Username, Emails: profile.Emails, @@ -233,14 +247,14 @@ func (p *LDAPUserProvider) GetDetails(username string) (*UserDetails, error) { } // UpdatePassword update the password of the given user. -func (p *LDAPUserProvider) UpdatePassword(username string, newPassword string) error { +func (p *LDAPUserProvider) UpdatePassword(inputUsername string, newPassword string) error { client, err := p.connect(p.configuration.User, p.configuration.Password) if err != nil { return fmt.Errorf("Unable to update password. Cause: %s", err) } - profile, err := p.getUserProfile(client, username) + profile, err := p.getUserProfile(client, inputUsername) if err != nil { return fmt.Errorf("Unable to update password. Cause: %s", err) diff --git a/internal/authentication/ldap_user_provider_test.go b/internal/authentication/ldap_user_provider_test.go index 75dd4bc57..034e53aac 100644 --- a/internal/authentication/ldap_user_provider_test.go +++ b/internal/authentication/ldap_user_provider_test.go @@ -58,7 +58,7 @@ func TestShouldCreateTLSConnectionWhenSchemeIsLDAPS(t *testing.T) { require.NoError(t, err) } -func TestEscapeSpecialChars(t *testing.T) { +func TestEscapeSpecialCharsFromUserInput(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -72,7 +72,9 @@ func TestEscapeSpecialChars(t *testing.T) { // Escape assert.Equal(t, "test\\,abc", ldap.ldapEscape("test,abc")) - assert.Equal(t, "test\\\\abc", ldap.ldapEscape("test\\abc")) + assert.Equal(t, "test\\5cabc", ldap.ldapEscape("test\\abc")) + assert.Equal(t, "test\\2aabc", ldap.ldapEscape("test*abc")) + assert.Equal(t, "test \\28abc\\29", ldap.ldapEscape("test (abc)")) assert.Equal(t, "test\\#abc", ldap.ldapEscape("test#abc")) assert.Equal(t, "test\\+abc", ldap.ldapEscape("test+abc")) assert.Equal(t, "test\\ { u2fApi.ensureSupport().then(() => setU2fSupported(true)) }, [setU2fSupported]); + useEffect(() => { + u2fApi.ensureSupport().then( + () => setU2fSupported(true), + () => console.error("U2F not supported")); + }, [setU2fSupported]); const initiateRegistration = (initiateRegistrationFunc: () => Promise) => { return async () => {