diff --git a/internal/authentication/ldap_user_provider.go b/internal/authentication/ldap_user_provider.go index 23d5518e..6142df7d 100644 --- a/internal/authentication/ldap_user_provider.go +++ b/internal/authentication/ldap_user_provider.go @@ -339,14 +339,9 @@ func (p *LDAPUserProvider) getUserProfile(client LDAPClient, username string) (p } for _, attr := range searchResult.Entries[0].Attributes { - switch attr.Name { - case p.config.DisplayNameAttribute: - userProfile.DisplayName = attr.Values[0] - case p.config.MailAttribute: - userProfile.Emails = attr.Values - case p.config.UsernameAttribute: - attrs := len(attr.Values) + attrs := len(attr.Values) + if attr.Name == p.config.UsernameAttribute { switch attrs { case 1: userProfile.Username = attr.Values[0] @@ -358,6 +353,18 @@ func (p *LDAPUserProvider) getUserProfile(client LDAPClient, username string) (p username, attrs, p.config.UsernameAttribute) } } + + if attrs == 0 { + continue + } + + if attr.Name == p.config.MailAttribute { + userProfile.Emails = attr.Values + } + + if attr.Name == p.config.DisplayNameAttribute { + userProfile.DisplayName = attr.Values[0] + } } if userProfile.Username == "" { diff --git a/internal/authentication/ldap_user_provider_startup.go b/internal/authentication/ldap_user_provider_startup.go index 71c88129..4c87c901 100644 --- a/internal/authentication/ldap_user_provider_startup.go +++ b/internal/authentication/ldap_user_provider_startup.go @@ -6,6 +6,7 @@ import ( "github.com/go-ldap/ldap/v3" "github.com/authelia/authelia/v4/internal/configuration/schema" + "github.com/authelia/authelia/v4/internal/utils" ) // StartupCheck implements the startup check provider interface. @@ -88,10 +89,16 @@ func (p *LDAPUserProvider) parseDynamicUsersConfiguration() { p.log.Tracef("Dynamically generated users filter is %s", p.config.UsersFilter) - p.usersAttributes = []string{ - p.config.DisplayNameAttribute, - p.config.MailAttribute, - p.config.UsernameAttribute, + if !utils.IsStringInSlice(p.config.UsernameAttribute, p.usersAttributes) { + p.usersAttributes = append(p.usersAttributes, p.config.UsernameAttribute) + } + + if !utils.IsStringInSlice(p.config.MailAttribute, p.usersAttributes) { + p.usersAttributes = append(p.usersAttributes, p.config.MailAttribute) + } + + if !utils.IsStringInSlice(p.config.DisplayNameAttribute, p.usersAttributes) { + p.usersAttributes = append(p.usersAttributes, p.config.DisplayNameAttribute) } if p.config.AdditionalUsersDN != "" { diff --git a/internal/authentication/ldap_user_provider_test.go b/internal/authentication/ldap_user_provider_test.go index d158fd60..a63cb109 100644 --- a/internal/authentication/ldap_user_provider_test.go +++ b/internal/authentication/ldap_user_provider_test.go @@ -545,6 +545,222 @@ func TestShouldEscapeUserInput(t *testing.T) { assert.EqualError(t, err, "user not found") } +func TestShouldReturnEmailWhenAttributeSameAsUsername(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockFactory := NewMockLDAPClientFactory(ctrl) + mockClient := NewMockLDAPClient(ctrl) + + ldapClient := newLDAPUserProvider( + schema.LDAPAuthenticationBackendConfiguration{ + URL: "ldap://127.0.0.1:389", + User: "cn=admin,dc=example,dc=com", + Password: "password", + UsernameAttribute: "mail", + MailAttribute: "mail", + DisplayNameAttribute: "displayName", + UsersFilter: "(&({username_attribute}={input})(objectClass=inetOrgPerson))", + AdditionalUsersDN: "ou=users", + BaseDN: "dc=example,dc=com", + }, + false, + nil, + mockFactory) + + assert.Equal(t, []string{"mail", "displayName"}, ldapClient.usersAttributes) + + dialURL := mockFactory.EXPECT(). + DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()). + Return(mockClient, nil) + + bind := mockClient.EXPECT(). + Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")). + Return(nil) + + search := mockClient.EXPECT(). + Search(NewSearchRequestMatcher("(&(mail=john@example.com)(objectClass=inetOrgPerson))")). + Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{ + { + DN: "uid=john,dc=example,dc=com", + Attributes: []*ldap.EntryAttribute{ + { + Name: "mail", + Values: []string{"john@example.com"}, + }, + { + Name: "displayName", + Values: []string{"John Doe"}, + }, + }, + }, + }, + }, nil) + + gomock.InOrder(dialURL, bind, search) + + client, err := ldapClient.connect() + assert.NoError(t, err) + + profile, err := ldapClient.getUserProfile(client, "john@example.com") + + assert.NoError(t, err) + require.NotNil(t, profile) + + assert.Equal(t, "uid=john,dc=example,dc=com", profile.DN) + assert.Equal(t, "john@example.com", profile.Username) + assert.Equal(t, "John Doe", profile.DisplayName) + + require.Len(t, profile.Emails, 1) + assert.Equal(t, "john@example.com", profile.Emails[0]) +} + +func TestShouldReturnUsernameAndBlankDisplayNameWhenAttributesTheSame(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockFactory := NewMockLDAPClientFactory(ctrl) + mockClient := NewMockLDAPClient(ctrl) + + ldapClient := newLDAPUserProvider( + schema.LDAPAuthenticationBackendConfiguration{ + URL: "ldap://127.0.0.1:389", + User: "cn=admin,dc=example,dc=com", + Password: "password", + UsernameAttribute: "uid", + MailAttribute: "mail", + DisplayNameAttribute: "uid", + UsersFilter: "(&(|({username_attribute}={input})({mail_attribute}={input}))(objectClass=inetOrgPerson))", + AdditionalUsersDN: "ou=users", + BaseDN: "dc=example,dc=com", + }, + false, + nil, + mockFactory) + + assert.Equal(t, []string{"uid", "mail"}, ldapClient.usersAttributes) + + dialURL := mockFactory.EXPECT(). + DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()). + Return(mockClient, nil) + + bind := mockClient.EXPECT(). + Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")). + Return(nil) + + search := mockClient.EXPECT(). + Search(NewSearchRequestMatcher("(&(|(uid=john@example.com)(mail=john@example.com))(objectClass=inetOrgPerson))")). + Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{ + { + DN: "uid=john,dc=example,dc=com", + Attributes: []*ldap.EntryAttribute{ + { + Name: "uid", + Values: []string{"john"}, + }, + { + Name: "mail", + Values: []string{"john@example.com"}, + }, + }, + }, + }, + }, nil) + + gomock.InOrder(dialURL, bind, search) + + client, err := ldapClient.connect() + assert.NoError(t, err) + + profile, err := ldapClient.getUserProfile(client, "john@example.com") + + assert.NoError(t, err) + require.NotNil(t, profile) + + assert.Equal(t, "uid=john,dc=example,dc=com", profile.DN) + assert.Equal(t, "john", profile.Username) + assert.Equal(t, "john", profile.DisplayName) + + require.Len(t, profile.Emails, 1) + assert.Equal(t, "john@example.com", profile.Emails[0]) +} + +func TestShouldReturnBlankEmailAndDisplayNameWhenAttrsLenZero(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockFactory := NewMockLDAPClientFactory(ctrl) + mockClient := NewMockLDAPClient(ctrl) + + ldapClient := newLDAPUserProvider( + schema.LDAPAuthenticationBackendConfiguration{ + URL: "ldap://127.0.0.1:389", + User: "cn=admin,dc=example,dc=com", + Password: "password", + UsernameAttribute: "uid", + MailAttribute: "mail", + DisplayNameAttribute: "displayName", + UsersFilter: "(&(|({username_attribute}={input})({mail_attribute}={input}))(objectClass=inetOrgPerson))", + AdditionalUsersDN: "ou=users", + BaseDN: "dc=example,dc=com", + }, + false, + nil, + mockFactory) + + assert.Equal(t, []string{"uid", "mail", "displayName"}, ldapClient.usersAttributes) + + dialURL := mockFactory.EXPECT(). + DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()). + Return(mockClient, nil) + + bind := mockClient.EXPECT(). + Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")). + Return(nil) + + search := mockClient.EXPECT(). + Search(NewSearchRequestMatcher("(&(|(uid=john@example.com)(mail=john@example.com))(objectClass=inetOrgPerson))")). + Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{ + { + DN: "uid=john,dc=example,dc=com", + Attributes: []*ldap.EntryAttribute{ + { + Name: "uid", + Values: []string{"john"}, + }, + { + Name: "mail", + Values: []string{}, + }, + { + Name: "displayName", + Values: []string{}, + }, + }, + }, + }, + }, nil) + + gomock.InOrder(dialURL, bind, search) + + client, err := ldapClient.connect() + assert.NoError(t, err) + + profile, err := ldapClient.getUserProfile(client, "john@example.com") + + assert.NoError(t, err) + require.NotNil(t, profile) + + assert.Equal(t, "uid=john,dc=example,dc=com", profile.DN) + assert.Equal(t, "john", profile.Username) + assert.Equal(t, "", profile.DisplayName) + + assert.Len(t, profile.Emails, 0) +} + func TestShouldCombineUsernameFilterAndUsersFilter(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -569,6 +785,8 @@ func TestShouldCombineUsernameFilterAndUsersFilter(t *testing.T) { nil, mockFactory) + assert.Equal(t, []string{"uid", "mail", "displayName"}, ldapClient.usersAttributes) + assert.True(t, ldapClient.usersFilterReplacementInput) mockClient.EXPECT().