From b3d8a56364b1ddb9d564de1b1e901febc6918f5a Mon Sep 17 00:00:00 2001 From: kolaente Date: Wed, 25 Feb 2026 09:59:14 +0100 Subject: [PATCH] fix: use caller's session in LDAP syncUserGroups to avoid nested transactions syncUserGroups created its own db.NewSession() internally while being called from AuthenticateUserInLDAP which already has an active session with writes. In SQLite shared-cache mode this causes a lock conflict. Pass the caller's session through instead, and add s.Commit() before db.AssertExists calls in LDAP tests. --- pkg/modules/auth/ldap/ldap.go | 17 ++--------------- pkg/modules/auth/ldap/ldap_test.go | 3 +++ 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/pkg/modules/auth/ldap/ldap.go b/pkg/modules/auth/ldap/ldap.go index e8ad32a82..527adebe3 100644 --- a/pkg/modules/auth/ldap/ldap.go +++ b/pkg/modules/auth/ldap/ldap.go @@ -24,7 +24,6 @@ import ( "strings" "code.vikunja.io/api/pkg/config" - "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/auth" @@ -251,7 +250,7 @@ func AuthenticateUserInLDAP(s *xorm.Session, username, password string, syncGrou return } - err = syncUserGroups(l, u, userdn) + err = syncUserGroups(s, l, u, userdn) return u, err } @@ -310,10 +309,7 @@ func getOrCreateLdapUser(s *xorm.Session, entry *ldap.Entry) (u *user.User, err return } -func syncUserGroups(l *ldap.Conn, u *user.User, userdn string) (err error) { - s := db.NewSession() - defer s.Close() - +func syncUserGroups(s *xorm.Session, l *ldap.Conn, u *user.User, userdn string) (err error) { searchRequest := ldap.NewSearchRequest( config.AuthLdapBaseDN.GetString(), ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, @@ -354,14 +350,5 @@ func syncUserGroups(l *ldap.Conn, u *user.User, userdn string) (err error) { } err = models.SyncExternalTeamsForUser(s, u, teams, user.IssuerLDAP, "LDAP") - if err != nil { - return - } - - err = s.Commit() - if err != nil { - _ = s.Rollback() - } - return } diff --git a/pkg/modules/auth/ldap/ldap_test.go b/pkg/modules/auth/ldap/ldap_test.go index ca30ebff7..d2926fa98 100644 --- a/pkg/modules/auth/ldap/ldap_test.go +++ b/pkg/modules/auth/ldap/ldap_test.go @@ -46,6 +46,7 @@ func TestLdapLogin(t *testing.T) { require.NoError(t, err) assert.Equal(t, "professor", user.Username) + require.NoError(t, s.Commit()) db.AssertExists(t, "users", map[string]interface{}{ "username": "professor", "issuer": "ldap", @@ -86,6 +87,7 @@ func TestLdapLogin(t *testing.T) { require.NoError(t, err) assert.Equal(t, "professor", user.Username) + require.NoError(t, s.Commit()) db.AssertExists(t, "users", map[string]interface{}{ "username": "professor", "issuer": "ldap", @@ -111,6 +113,7 @@ func TestLdapLogin(t *testing.T) { require.NoError(t, err) assert.Equal(t, "professor", user.Username) + require.NoError(t, s.Commit()) db.AssertExists(t, "users", map[string]interface{}{ "username": "professor", "issuer": "ldap",