github-starred/vikunja
2
0
Fork 0
mirror of https://github.com/go-vikunja/vikunja.git synced 2026-11-10 15:16:20 -06:00
Code Issues 690 Packages Projects Releases 13 Wiki Activity

feat(ldap): query only the user's groups during group sync

Browse Source 
Support {userdn} and {username} placeholders in auth.ldap.groupsyncfilter.
When present, every matching group is a membership and the member
attribute is not fetched, so the cost scales with the user's own
memberships and nested AD groups can be resolved with
LDAP_MATCHING_RULE_IN_CHAIN. Filters without a placeholder keep the
previous client-side member matching.

The group search now uses paged results in both modes so directories
with more groups than the server's size limit no longer fail the login.
This commit is contained in:
kolaente
2026-10-19 13:42:19 +02:00
committed by Tink
parent 413b5bead2
commit eff486337f
2 changed files with 200 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/modules/auth/ldap/ldap.go
+56 -24
View File
@@ -21,6 +21,7 @@ import (
"crypto/tls"
"errors"
"fmt"
"slices"
"strings"
"code.vikunja.io/api/pkg/config"
@@ -229,7 +230,8 @@ func AuthenticateUserInLDAP(s *xorm.Session, username, password string, syncGrou
}
}
err = syncUserGroups(s, l, u, userdn)
ldapUsername := sr.Entries[0].GetAttributeValue(config.AuthLdapAttributeUsername.GetString())
err = syncUserGroups(s, l, u, userdn, ldapUsername)
return u, err
}
@@ -293,44 +295,74 @@ func getOrCreateLdapUser(s *xorm.Session, entry *ldap.Entry) (u *user.User, err
return
}
func syncUserGroups(s *xorm.Session, l *ldap.Conn, u *user.User, userdn string) (err error) {
const (
groupSyncFilterUserDN = "{userdn}"
groupSyncFilterUsername = "{username}"
)
// Below AD's default MaxPageSize of 1000.
const groupSearchPageSize = 500
// buildGroupSyncFilter returns perUser = true when the template references the
// user, in which case every group matching the filter is a membership.
func buildGroupSyncFilter(template, userDN, username string) (filter string, perUser bool, err error) {
hasUsername := strings.Contains(template, groupSyncFilterUsername)
if !hasUsername && !strings.Contains(template, groupSyncFilterUserDN) {
return template, false, nil
}
// An empty value would turn e.g. (memberUid={username}*) into (memberUid=*) and grant every group.
if hasUsername && username == "" {
return "", false, fmt.Errorf("group sync filter uses %s but the user has no %s attribute", groupSyncFilterUsername, config.AuthLdapAttributeUsername.GetString())
}
return strings.NewReplacer(
groupSyncFilterUserDN, ldap.EscapeFilter(userDN),
groupSyncFilterUsername, ldap.EscapeFilter(username),
).Replace(template), true, nil
}
func syncUserGroups(s *xorm.Session, l *ldap.Conn, u *user.User, userdn, ldapUsername string) (err error) {
filter, perUser, err := buildGroupSyncFilter(config.AuthLdapGroupSyncFilter.GetString(), userdn, ldapUsername)
if err != nil {
return err
}
memberAttribute := config.AuthLdapAttributeMemberID.GetString()
attributes := []string{"cn", "description"}
if !perUser {
attributes = append(attributes, memberAttribute)
}
searchRequest := ldap.NewSearchRequest(
config.AuthLdapBaseDN.GetString(),
ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
config.AuthLdapGroupSyncFilter.GetString(),
[]string{
"dn",
"cn",
config.AuthLdapAttributeMemberID.GetString(),
"description",
},
filter,
attributes,
nil,
)
sr, err := l.Search(searchRequest)
sr, err := l.SearchWithPaging(searchRequest, groupSearchPageSize)
if err != nil {
log.Errorf("Error searching for LDAP groups: %v", err)
return err
}
log.Debugf("Found %d LDAP groups for user %s with filter %s", len(sr.Entries), userdn, filter)
var teams []*models.Team
isUser := func(member string) bool { return member == userdn || member == u.Username }
for _, group := range sr.Entries {
groupName := group.GetAttributeValue("cn")
members := group.GetAttributeValues(config.AuthLdapAttributeMemberID.GetString())
description := group.GetAttributeValue("description")
log.Debugf("Group %s has %d members", groupName, len(members))
for _, member := range members {
if member == userdn || member == u.Username {
teams = append(teams, &models.Team{
Name: groupName,
ExternalID: group.DN,
Description: description,
})
}
if !perUser && !slices.ContainsFunc(group.GetAttributeValues(memberAttribute), isUser) {
continue
}
teams = append(teams, &models.Team{
Name: group.GetAttributeValue("cn"),
ExternalID: group.DN,
Description: group.GetAttributeValue("description"),
})
}
err = models.SyncExternalTeamsForUser(s, u, teams, user.IssuerLDAP, "LDAP")
pkg/modules/auth/ldap/ldap_test.go
+144
View File
@@ -102,6 +102,7 @@ func TestLdapLogin(t *testing.T) {
"issuer": "ldap",
"external_id": "cn=git,ou=people,dc=planetexpress,dc=com",
}, false)
assertLdapTeamCount(t, user.ID, 2)
})
t.Run("should sync groups using service account rebind", func(t *testing.T) {
@@ -162,6 +163,56 @@ func TestLdapLogin(t *testing.T) {
}, false)
})
t.Run("should sync groups with per-user filter", func(t *testing.T) {
origFilter := config.AuthLdapGroupSyncFilter.GetString()
config.AuthLdapGroupSyncFilter.Set("(&(objectclass=groupOfNames)(member={userdn}))")
defer config.AuthLdapGroupSyncFilter.Set(origFilter)
tests := []struct {
name string
username string
groups []string
}{
{
name: "professor",
username: "professor",
groups: []string{
"admin_staff",
"git",
},
},
{
name: "non-ascii dn",
username: "bender",
groups: []string{
"ship_crew",
"git",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
user, err := AuthenticateUserInLDAP(s, tt.username, tt.username, true, "")
require.NoError(t, err)
require.NoError(t, s.Commit())
for _, group := range tt.groups {
db.AssertExists(t, "teams", map[string]interface{}{
"name": group + " (LDAP)",
"issuer": "ldap",
"external_id": "cn=" + group + ",ou=people,dc=planetexpress,dc=com",
}, false)
}
assertLdapTeamCount(t, user.ID, 2)
})
}
})
t.Run("should sync avatar when enabled", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
@@ -321,3 +372,96 @@ func TestSanitizedUserQueryPreventsInjection(t *testing.T) {
})
}
}
func assertLdapTeamCount(t *testing.T, userID int64, expected int64) {
t.Helper()
s := db.NewSession()
defer s.Close()
count, err := s.
Table("teams").
Join("INNER", "team_members", "team_members.team_id = teams.id").
Where("teams.issuer = ? AND team_members.user_id = ?", user2.IssuerLDAP, userID).
Count()
require.NoError(t, err)
assert.Equal(t, expected, count)
}
func TestBuildGroupSyncFilter(t *testing.T) {
tests := []struct {
name string
template string
userDN string
username string
expectedFilter string
expectedPerUser bool
expectedErr bool
}{
{
name: "no placeholder",
template: "(&(objectclass=*)(|(objectclass=group)(objectclass=groupOfNames)))",
userDN: "cn=professor,ou=people,dc=planetexpress,dc=com",
username: "professor",
expectedFilter: "(&(objectclass=*)(|(objectclass=group)(objectclass=groupOfNames)))",
},
{
name: "userdn",
template: "(&(objectclass=groupOfNames)(member={userdn}))",
userDN: "cn=professor,ou=people,dc=planetexpress,dc=com",
expectedFilter: "(&(objectclass=groupOfNames)(member=cn=professor,ou=people,dc=planetexpress,dc=com))",
expectedPerUser: true,
},
{
name: "username",
template: "(&(objectclass=posixGroup)(memberUid={username}))",
username: "professor",
expectedFilter: "(&(objectclass=posixGroup)(memberUid=professor))",
expectedPerUser: true,
},
{
name: "both placeholders, repeated",
template: "(|(member={userdn})(memberUid={username})(uniqueMember={userdn}))",
userDN: "cn=a,dc=b",
username: "a",
expectedFilter: "(|(member=cn=a,dc=b)(memberUid=a)(uniqueMember=cn=a,dc=b))",
expectedPerUser: true,
},
{
name: "injection attempt in username",
template: "(&(objectclass=posixGroup)(memberUid={username}))",
username: "x)(|(objectclass=*",
expectedFilter: `(&(objectclass=posixGroup)(memberUid=x\29\28|\28objectclass=\2a))`,
expectedPerUser: true,
},
{
name: "placeholder in value is not substituted again",
template: "(|(member={userdn})(memberUid={username}))",
userDN: "cn={username},dc=example",
username: "professor",
expectedFilter: "(|(member=cn={username},dc=example)(memberUid=professor))",
expectedPerUser: true,
},
{
name: "empty username",
template: "(&(objectclass=posixGroup)(memberUid={username}*))",
userDN: "cn=professor,ou=people,dc=planetexpress,dc=com",
expectedErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
filter, perUser, err := buildGroupSyncFilter(tt.template, tt.userDN, tt.username)
if tt.expectedErr {
require.Error(t, err)
return
}
require.NoError(t, err)
assert.Equal(t, tt.expectedFilter, filter)
assert.Equal(t, tt.expectedPerUser, perUser)
_, err = ldap.CompileFilter(filter)
require.NoError(t, err)
})
}
}