diff --git a/services/auth/source/ldap/source_search.go b/services/auth/source/ldap/source_search.go index 68ebba3917..16f13029f9 100644 --- a/services/auth/source/ldap/source_search.go +++ b/services/auth/source/ldap/source_search.go @@ -196,22 +196,39 @@ func checkRestricted(l *ldap.Conn, ls *Source, userDN string) bool { } // List all group memberships of a user -func (source *Source) listLdapGroupMemberships(l *ldap.Conn, uid string) []string { +func (source *Source) listLdapGroupMemberships(l *ldap.Conn, uid string, applyGroupFilter bool) []string { var ldapGroups []string - groupFilter := fmt.Sprintf("(%s=%s)", source.GroupMemberUID, ldap.EscapeFilter(uid)) + var searchFilter string + + groupFilter, ok := source.sanitizedGroupFilter(source.GroupFilter) + if !ok { + return ldapGroups + } + + groupDN, ok := source.sanitizedGroupDN(source.GroupDN) + if !ok { + return ldapGroups + } + + if applyGroupFilter { + searchFilter = fmt.Sprintf("(&(%s)(%s=%s))", groupFilter, source.GroupMemberUID, ldap.EscapeFilter(uid)) + } else { + searchFilter = fmt.Sprintf("(%s=%s)", source.GroupMemberUID, ldap.EscapeFilter(uid)) + } + result, err := l.Search(ldap.NewSearchRequest( - source.GroupDN, + groupDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, - groupFilter, + searchFilter, []string{}, nil, )) if err != nil { - log.Error("Failed group search using filter[%s]: %v", groupFilter, err) + log.Error("Failed group search in LDAP with filter [%s]: %v", searchFilter, err) return ldapGroups } @@ -238,9 +255,7 @@ func (source *Source) mapLdapGroupsToTeams() map[string]map[string][]string { } // getMappedMemberships : returns the organizations and teams to modify the users membership -func (source *Source) getMappedMemberships(l *ldap.Conn, uid string) (map[string][]string, map[string][]string) { - // get all LDAP group memberships for user - usersLdapGroups := source.listLdapGroupMemberships(l, uid) +func (source *Source) getMappedMemberships(usersLdapGroups []string, uid string) (map[string][]string, map[string][]string) { // unmarshall LDAP group team map from configs ldapGroupsToTeams := source.mapLdapGroupsToTeams() membershipsToAdd := map[string][]string{} @@ -260,6 +275,14 @@ func (source *Source) getMappedMemberships(l *ldap.Conn, uid string) (map[string return membershipsToAdd, membershipsToRemove } +func (source *Source) getUserAttributeListedInGroup(entry *ldap.Entry) string { + if strings.ToLower(source.UserUID) == "dn" { + return entry.DN + } + + return entry.GetAttributeValue(source.UserUID) +} + // SearchEntry : search an LDAP source if an entry (name, passwd) is valid and in the specific filter func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchResult { // See https://tools.ietf.org/search/rfc4513#section-5.1.2 @@ -375,58 +398,30 @@ func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchR firstname := sr.Entries[0].GetAttributeValue(source.AttributeName) surname := sr.Entries[0].GetAttributeValue(source.AttributeSurname) mail := sr.Entries[0].GetAttributeValue(source.AttributeMail) - uid := sr.Entries[0].GetAttributeValue(source.UserUID) - if source.UserUID == "dn" || source.UserUID == "DN" { - uid = sr.Entries[0].DN - } + + teamsToAdd := make(map[string][]string) + teamsToRemove := make(map[string][]string) // Check group membership - if source.GroupsEnabled && source.GroupFilter != "" { - groupFilter, ok := source.sanitizedGroupFilter(source.GroupFilter) - if !ok { - return nil - } - groupDN, ok := source.sanitizedGroupDN(source.GroupDN) - if !ok { + if source.GroupsEnabled { + userAttributeListedInGroup := source.getUserAttributeListedInGroup(sr.Entries[0]) + usersLdapGroups := source.listLdapGroupMemberships(l, userAttributeListedInGroup, true) + + if source.GroupFilter != "" && len(usersLdapGroups) == 0 { return nil } - log.Trace("Fetching groups '%v' with filter '%s' and base '%s'", source.GroupMemberUID, groupFilter, groupDN) - groupSearch := ldap.NewSearchRequest( - groupDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, groupFilter, - []string{source.GroupMemberUID}, - nil) - - srg, err := l.Search(groupSearch) - if err != nil { - log.Error("LDAP group search failed: %v", err) - return nil - } else if len(srg.Entries) < 1 { - log.Error("LDAP group search failed: 0 entries") - return nil - } - - isMember := false - Entries: - for _, group := range srg.Entries { - for _, member := range group.GetAttributeValues(source.GroupMemberUID) { - if (source.UserUID == "dn" && member == sr.Entries[0].DN) || member == uid { - isMember = true - break Entries - } - } - } - - if !isMember { - log.Error("LDAP group membership test failed") - return nil + if source.GroupTeamMap != "" || source.GroupTeamMapRemoval { + teamsToAdd, teamsToRemove = source.getMappedMemberships(usersLdapGroups, userAttributeListedInGroup) } } if isAttributeSSHPublicKeySet { sshPublicKey = sr.Entries[0].GetAttributeValues(source.AttributeSSHPublicKey) } + isAdmin := checkAdmin(l, source, userDN) + var isRestricted bool if !isAdmin { isRestricted = checkRestricted(l, source, userDN) @@ -436,12 +431,6 @@ func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchR Avatar = sr.Entries[0].GetRawAttributeValue(source.AttributeAvatar) } - teamsToAdd := make(map[string][]string) - teamsToRemove := make(map[string][]string) - if source.GroupsEnabled && (source.GroupTeamMap != "" || source.GroupTeamMapRemoval) { - teamsToAdd, teamsToRemove = source.getMappedMemberships(l, uid) - } - if !directBind && source.AttributesInBind { // binds user (checking password) after looking-up attributes in BindDN context err = bindUser(l, userDN, passwd) @@ -520,19 +509,29 @@ func (source *Source) SearchEntries() ([]*SearchResult, error) { return nil, err } - result := make([]*SearchResult, len(sr.Entries)) + result := make([]*SearchResult, 0, len(sr.Entries)) - for i, v := range sr.Entries { + for _, v := range sr.Entries { teamsToAdd := make(map[string][]string) teamsToRemove := make(map[string][]string) - if source.GroupsEnabled && (source.GroupTeamMap != "" || source.GroupTeamMapRemoval) { - userAttributeListedInGroup := v.GetAttributeValue(source.UserUID) - if source.UserUID == "dn" || source.UserUID == "DN" { - userAttributeListedInGroup = v.DN + + if source.GroupsEnabled { + userAttributeListedInGroup := source.getUserAttributeListedInGroup(v) + + if source.GroupFilter != "" { + usersLdapGroups := source.listLdapGroupMemberships(l, userAttributeListedInGroup, true) + if len(usersLdapGroups) == 0 { + continue + } + } + + if source.GroupTeamMap != "" || source.GroupTeamMapRemoval { + usersLdapGroups := source.listLdapGroupMemberships(l, userAttributeListedInGroup, false) + teamsToAdd, teamsToRemove = source.getMappedMemberships(usersLdapGroups, userAttributeListedInGroup) } - teamsToAdd, teamsToRemove = source.getMappedMemberships(l, userAttributeListedInGroup) } - result[i] = &SearchResult{ + + user := &SearchResult{ Username: v.GetAttributeValue(source.AttributeUsername), Name: v.GetAttributeValue(source.AttributeName), Surname: v.GetAttributeValue(source.AttributeSurname), @@ -541,16 +540,22 @@ func (source *Source) SearchEntries() ([]*SearchResult, error) { LdapTeamAdd: teamsToAdd, LdapTeamRemove: teamsToRemove, } - if !result[i].IsAdmin { - result[i].IsRestricted = checkRestricted(l, source, v.DN) + + if !user.IsAdmin { + user.IsRestricted = checkRestricted(l, source, v.DN) } + if isAttributeSSHPublicKeySet { - result[i].SSHPublicKey = v.GetAttributeValues(source.AttributeSSHPublicKey) + user.SSHPublicKey = v.GetAttributeValues(source.AttributeSSHPublicKey) } + if isAtributeAvatarSet { - result[i].Avatar = v.GetRawAttributeValue(source.AttributeAvatar) + user.Avatar = v.GetRawAttributeValue(source.AttributeAvatar) } - result[i].LowerName = strings.ToLower(result[i].Username) + + user.LowerName = strings.ToLower(user.Username) + + result = append(result, user) } return result, nil diff --git a/tests/integration/auth_ldap_test.go b/tests/integration/auth_ldap_test.go index 88571ecd61..9dfa4c73d8 100644 --- a/tests/integration/auth_ldap_test.go +++ b/tests/integration/auth_ldap_test.go @@ -11,12 +11,14 @@ import ( "testing" "code.gitea.io/gitea/models" + auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/services/auth" + "code.gitea.io/gitea/services/auth/source/ldap" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -102,13 +104,28 @@ func getLDAPServerHost() string { return host } -func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string, groupMapParams ...string) { +func getLDAPServerPort() string { + port := os.Getenv("TEST_LDAP_PORT") + if len(port) == 0 { + port = "389" + } + return port +} + +func addAuthSourceLDAP(t *testing.T, sshKeyAttribute, groupFilter string, groupMapParams ...string) { groupTeamMapRemoval := "off" groupTeamMap := "" if len(groupMapParams) == 2 { groupTeamMapRemoval = groupMapParams[0] groupTeamMap = groupMapParams[1] } + + // Modify user filter to test group filter explicitly + userFilter := "(&(objectClass=inetOrgPerson)(memberOf=cn=git,ou=people,dc=planetexpress,dc=com)(uid=%s))" + if groupFilter != "" { + userFilter = "(&(objectClass=inetOrgPerson)(uid=%s))" + } + session := loginUser(t, "user1") csrf := GetCSRF(t, session, "/admin/auths/new") req := NewRequestWithValues(t, "POST", "/admin/auths/new", map[string]string{ @@ -116,11 +133,11 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string, groupMapParams ...s "type": "2", "name": "ldap", "host": getLDAPServerHost(), - "port": "389", + "port": getLDAPServerPort(), "bind_dn": "uid=gitea,ou=service,dc=planetexpress,dc=com", "bind_password": "password", "user_base": "ou=people,dc=planetexpress,dc=com", - "filter": "(&(objectClass=inetOrgPerson)(memberOf=cn=git,ou=people,dc=planetexpress,dc=com)(uid=%s))", + "filter": userFilter, "admin_filter": "(memberOf=cn=admin_staff,ou=people,dc=planetexpress,dc=com)", "restricted_filter": "(uid=leela)", "attribute_username": "uid", @@ -133,6 +150,7 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string, groupMapParams ...s "groups_enabled": "on", "group_dn": "ou=people,dc=planetexpress,dc=com", "group_member_uid": "member", + "group_filter": groupFilter, "group_team_map": groupTeamMap, "group_team_map_removal": groupTeamMapRemoval, "user_uid": "DN", @@ -146,7 +164,7 @@ func TestLDAPUserSignin(t *testing.T) { return } defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "") + addAuthSourceLDAP(t, "", "") u := gitLDAPUsers[0] @@ -163,7 +181,7 @@ func TestLDAPUserSignin(t *testing.T) { func TestLDAPAuthChange(t *testing.T) { defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "") + addAuthSourceLDAP(t, "", "") session := loginUser(t, "user1") req := NewRequest(t, "GET", "/admin/auths") @@ -221,7 +239,7 @@ func TestLDAPUserSync(t *testing.T) { return } defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "") + addAuthSourceLDAP(t, "", "") auth.SyncExternalUsers(context.Background(), true) session := loginUser(t, "user1") @@ -266,13 +284,72 @@ func TestLDAPUserSync(t *testing.T) { } } +func TestLDAPUserSyncWithGroupFilter(t *testing.T) { + if skipLDAPTests() { + t.Skip() + return + } + defer tests.PrepareTestEnv(t)() + addAuthSourceLDAP(t, "", "(cn=git)") + + // Assert a user not a member of the LDAP group "cn=git" cannot login + // This test may look like TestLDAPUserSigninFailed but it is not. + // The later test uses user filter containing group membership filter (memberOf) + // This test is for the case when LDAP user records may not be linked with + // all groups the user is a member of, the user filter is modified accordingly inside + // the addAuthSourceLDAP based on the value of the groupFilter + u := otherLDAPUsers[0] + testLoginFailed(t, u.UserName, u.Password, translation.NewLocale("en-US").Tr("form.username_password_incorrect")) + + auth.SyncExternalUsers(context.Background(), true) + + // Assert members of LDAP group "cn=git" are added + for _, gitLDAPUser := range gitLDAPUsers { + unittest.BeanExists(t, &user_model.User{ + Name: gitLDAPUser.UserName, + }) + } + + // Assert everyone else is not added + for _, gitLDAPUser := range otherLDAPUsers { + unittest.AssertNotExistsBean(t, &user_model.User{ + Name: gitLDAPUser.UserName, + }) + } + + ldapSource := unittest.AssertExistsAndLoadBean(t, &auth_model.Source{ + Name: "ldap", + }) + ldapConfig := ldapSource.Cfg.(*ldap.Source) + ldapConfig.GroupFilter = "(cn=ship_crew)" + auth_model.UpdateSource(ldapSource) + + auth.SyncExternalUsers(context.Background(), true) + + for _, gitLDAPUser := range gitLDAPUsers { + if gitLDAPUser.UserName == "fry" || gitLDAPUser.UserName == "leela" || gitLDAPUser.UserName == "bender" { + // Assert members of the LDAP group "cn-ship_crew" are still active + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ + Name: gitLDAPUser.UserName, + }) + assert.True(t, user.IsActive, "User %s should be active", gitLDAPUser.UserName) + } else { + // Assert everyone else is inactive + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ + Name: gitLDAPUser.UserName, + }) + assert.False(t, user.IsActive, "User %s should be inactive", gitLDAPUser.UserName) + } + } +} + func TestLDAPUserSigninFailed(t *testing.T) { if skipLDAPTests() { t.Skip() return } defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "") + addAuthSourceLDAP(t, "", "") u := otherLDAPUsers[0] testLoginFailed(t, u.UserName, u.Password, translation.NewLocale("en-US").Tr("form.username_password_incorrect")) @@ -284,7 +361,7 @@ func TestLDAPUserSSHKeySync(t *testing.T) { return } defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "sshPublicKey") + addAuthSourceLDAP(t, "sshPublicKey", "") auth.SyncExternalUsers(context.Background(), true) @@ -317,7 +394,7 @@ func TestLDAPGroupTeamSyncAddMember(t *testing.T) { return } defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "on", `{"cn=ship_crew,ou=people,dc=planetexpress,dc=com":{"org26": ["team11"]},"cn=admin_staff,ou=people,dc=planetexpress,dc=com": {"non-existent": ["non-existent"]}}`) + addAuthSourceLDAP(t, "", "", "on", `{"cn=ship_crew,ou=people,dc=planetexpress,dc=com":{"org26": ["team11"]},"cn=admin_staff,ou=people,dc=planetexpress,dc=com": {"non-existent": ["non-existent"]}}`) org, err := organization.GetOrgByName("org26") assert.NoError(t, err) team, err := organization.GetTeam(db.DefaultContext, org.ID, "team11") @@ -362,7 +439,7 @@ func TestLDAPGroupTeamSyncRemoveMember(t *testing.T) { return } defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "on", `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`) + addAuthSourceLDAP(t, "", "", "on", `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`) org, err := organization.GetOrgByName("org26") assert.NoError(t, err) team, err := organization.GetTeam(db.DefaultContext, org.ID, "team11") @@ -398,7 +475,7 @@ func TestBrokenLDAPMapUserSignin(t *testing.T) { return } defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "on", `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`) + addAuthSourceLDAP(t, "", "", "on", `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`) u := gitLDAPUsers[0]