fix(auth): use binddn as group sync dn instead of userbind

This commit is contained in:
karl Einziger 2026-06-02 10:20:04 +02:00 committed by kolaente
parent 9e880e98a5
commit 0efae572cd
4 changed files with 90 additions and 8 deletions

View File

@ -849,6 +849,11 @@
"default_value": "(&(objectclass=*)(|(objectclass=group)(objectclass=groupOfNames)))",
"comment": "The filter to search for group objects in the ldap directory. Only used when `groupsyncenabled` is set to `true`."
},
{
"key": "groupsyncuseserviceaccount",
"default_value": "false",
"comment": "If true, Vikunja re-binds as the service account (binddn/bindpassword) before searching for groups during group sync. Enable this when the authenticating user does not have sufficient rights to enumerate group membership in the directory."
},
{
"key": "avatarsyncattribute",
"default_value": "",

View File

@ -92,14 +92,15 @@ const (
AuthLdapVerifyTLS Key = `auth.ldap.verifytls`
AuthLdapBindDN Key = `auth.ldap.binddn`
// #nosec G101
AuthLdapBindPassword Key = `auth.ldap.bindpassword`
AuthLdapGroupSyncEnabled Key = `auth.ldap.groupsyncenabled`
AuthLdapGroupSyncFilter Key = `auth.ldap.groupsyncfilter`
AuthLdapAvatarSyncAttribute Key = `auth.ldap.avatarsyncattribute`
AuthLdapAttributeUsername Key = `auth.ldap.attribute.username`
AuthLdapAttributeEmail Key = `auth.ldap.attribute.email`
AuthLdapAttributeDisplayname Key = `auth.ldap.attribute.displayname`
AuthLdapAttributeMemberID Key = `auth.ldap.attribute.memberid`
AuthLdapBindPassword Key = `auth.ldap.bindpassword`
AuthLdapGroupSyncEnabled Key = `auth.ldap.groupsyncenabled`
AuthLdapGroupSyncFilter Key = `auth.ldap.groupsyncfilter`
AuthLdapGroupSyncUseServiceAccount Key = `auth.ldap.groupsyncuseserviceaccount`
AuthLdapAvatarSyncAttribute Key = `auth.ldap.avatarsyncattribute`
AuthLdapAttributeUsername Key = `auth.ldap.attribute.username`
AuthLdapAttributeEmail Key = `auth.ldap.attribute.email`
AuthLdapAttributeDisplayname Key = `auth.ldap.attribute.displayname`
AuthLdapAttributeMemberID Key = `auth.ldap.attribute.memberid`
LegalImprintURL Key = `legal.imprinturl`
LegalPrivacyURL Key = `legal.privacyurl`
@ -389,6 +390,7 @@ func InitDefaultConfig() {
AuthLdapVerifyTLS.setDefault(true)
AuthLdapGroupSyncEnabled.setDefault(false)
AuthLdapGroupSyncFilter.setDefault("(&(objectclass=*)(|(objectclass=group)(objectclass=groupOfNames)))")
AuthLdapGroupSyncUseServiceAccount.setDefault(false)
AuthLdapAttributeUsername.setDefault("uid")
AuthLdapAttributeEmail.setDefault("mail")
AuthLdapAttributeDisplayname.setDefault("displayName")

View File

@ -250,6 +250,23 @@ func AuthenticateUserInLDAP(s *xorm.Session, username, password string, syncGrou
return
}
// After verifying the user's password above the connection is bound as the
// end user. Many directories restrict group searches to service accounts, so
// re-bind as the service account before enumerating groups when configured.
if config.AuthLdapGroupSyncUseServiceAccount.GetBool() {
bindDN := config.AuthLdapBindDN.GetString()
bindPassword := config.AuthLdapBindPassword.GetString()
if bindDN != "" && bindPassword != "" {
if err = l.Bind(bindDN, bindPassword); err != nil {
return nil, fmt.Errorf("could not re-bind service account for group sync: %w", err)
}
} else {
if err = l.UnauthenticatedBind(""); err != nil {
return nil, fmt.Errorf("could not re-bind anonymously for group sync: %w", err)
}
}
}
err = syncUserGroups(s, l, u, userdn)
return u, err

View File

@ -104,6 +104,64 @@ func TestLdapLogin(t *testing.T) {
}, false)
})
t.Run("should sync groups using service account rebind", func(t *testing.T) {
// Verifies that re-binding as the service account before the group
// search works correctly — the fix for directories where regular users
// cannot enumerate group membership.
origFlag := config.AuthLdapGroupSyncUseServiceAccount.GetBool()
config.AuthLdapGroupSyncUseServiceAccount.Set(true)
defer config.AuthLdapGroupSyncUseServiceAccount.Set(origFlag)
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
user, err := AuthenticateUserInLDAP(s, "professor", "professor", true, "")
require.NoError(t, err)
assert.Equal(t, "professor", user.Username)
require.NoError(t, s.Commit())
db.AssertExists(t, "teams", map[string]interface{}{
"name": "admin_staff (LDAP)",
"issuer": "ldap",
"external_id": "cn=admin_staff,ou=people,dc=planetexpress,dc=com",
}, false)
db.AssertExists(t, "teams", map[string]interface{}{
"name": "git (LDAP)",
"issuer": "ldap",
"external_id": "cn=git,ou=people,dc=planetexpress,dc=com",
}, false)
})
t.Run("should sync groups using user binding", func(t *testing.T) {
// Verifies the flag=false path where the connection stays bound as the
// authenticated user during the group search. Works on directories that
// grant regular users read access to group objects.
origFlag := config.AuthLdapGroupSyncUseServiceAccount.GetBool()
config.AuthLdapGroupSyncUseServiceAccount.Set(false)
defer config.AuthLdapGroupSyncUseServiceAccount.Set(origFlag)
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
user, err := AuthenticateUserInLDAP(s, "professor", "professor", true, "")
require.NoError(t, err)
assert.Equal(t, "professor", user.Username)
require.NoError(t, s.Commit())
db.AssertExists(t, "teams", map[string]interface{}{
"name": "admin_staff (LDAP)",
"issuer": "ldap",
"external_id": "cn=admin_staff,ou=people,dc=planetexpress,dc=com",
}, false)
db.AssertExists(t, "teams", map[string]interface{}{
"name": "git (LDAP)",
"issuer": "ldap",
"external_id": "cn=git,ou=people,dc=planetexpress,dc=com",
}, false)
})
t.Run("should sync avatar when enabled", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()