From 209b8353893f2e4abb0798b2ad7c57e4e3715e99 Mon Sep 17 00:00:00 2001
From: klmmr <35450576+klmmr@users.noreply.github.com>
Date: Sat, 22 Feb 2025 10:24:48 +0100
Subject: [PATCH] ldap-sync: fix creation of only one user per LDAP sync

Before this fix, a too early `return` statement terminated the
`updateLdapUsers()` function, whenever one not already existing user was
created. Therefore, in each LDAP sync a maximum of one new user could be
created (i.e., it took x LDAP sync cycles until x new LDAP users are
registered in wg-portal). Depending on the LDAP `sync_interval` this can
take a long time and produces unecessary long waiting times until users
are available in wg-portal.

Removing the early return statement, and move the remainder of the
function into an `else` statement, so that all new users can be
added in a single LDAP sync.

Also adding a debug statement to better trace the behavior.

Signed-off-by: klmmr <35450576+klmmr@users.noreply.github.com>
---
 internal/app/users/user_manager.go | 69 +++++++++++++++---------------
 1 file changed, 34 insertions(+), 35 deletions(-)

diff --git a/internal/app/users/user_manager.go b/internal/app/users/user_manager.go
index 7fdc090..03c0358 100644
--- a/internal/app/users/user_manager.go
+++ b/internal/app/users/user_manager.go
@@ -506,50 +506,49 @@ func (m Manager) updateLdapUsers(
 		tctx, cancel := context.WithTimeout(ctx, 30*time.Second)
 		tctx = domain.SetUserInfo(tctx, domain.SystemAdminContextUserInfo())
 
-		// create new user
 		if existingUser == nil {
+			// create new user
+			logrus.Tracef("creating new user %s from provider %s...", user.Identifier, provider.ProviderName)
+			
 			err := m.NewUser(tctx, user)
 			if err != nil {
 				cancel()
 				return fmt.Errorf("create error for user id %s: %w", user.Identifier, err)
 			}
-
-			cancel()
-			return nil
-		}
-
-		// update existing user
-		if provider.AutoReEnable && existingUser.DisabledReason == domain.DisabledReasonLdapMissing {
-			user.Disabled = nil
-			user.DisabledReason = ""
 		} else {
-			user.Disabled = existingUser.Disabled
-			user.DisabledReason = existingUser.DisabledReason
-		}
-		if existingUser.Source == domain.UserSourceLdap && userChangedInLdap(existingUser, user) {
-			err := m.users.SaveUser(tctx, user.Identifier, func(u *domain.User) (*domain.User, error) {
-				u.UpdatedAt = time.Now()
-				u.UpdatedBy = domain.CtxSystemLdapSyncer
-				u.Source = user.Source
-				u.ProviderName = user.ProviderName
-				u.Email = user.Email
-				u.Firstname = user.Firstname
-				u.Lastname = user.Lastname
-				u.Phone = user.Phone
-				u.Department = user.Department
-				u.IsAdmin = user.IsAdmin
-				u.Disabled = nil
-				u.DisabledReason = ""
-
-				return u, nil
-			})
-			if err != nil {
-				cancel()
-				return fmt.Errorf("update error for user id %s: %w", user.Identifier, err)
+			// update existing user
+			if provider.AutoReEnable && existingUser.DisabledReason == domain.DisabledReasonLdapMissing {
+				user.Disabled = nil
+				user.DisabledReason = ""
+			} else {
+				user.Disabled = existingUser.Disabled
+				user.DisabledReason = existingUser.DisabledReason
 			}
+			if existingUser.Source == domain.UserSourceLdap && userChangedInLdap(existingUser, user) {
+				err := m.users.SaveUser(tctx, user.Identifier, func(u *domain.User) (*domain.User, error) {
+					u.UpdatedAt = time.Now()
+					u.UpdatedBy = domain.CtxSystemLdapSyncer
+					u.Source = user.Source
+					u.ProviderName = user.ProviderName
+					u.Email = user.Email
+					u.Firstname = user.Firstname
+					u.Lastname = user.Lastname
+					u.Phone = user.Phone
+					u.Department = user.Department
+					u.IsAdmin = user.IsAdmin
+					u.Disabled = nil
+					u.DisabledReason = ""
 
-			if existingUser.IsDisabled() && !user.IsDisabled() {
-				m.bus.Publish(app.TopicUserEnabled, *user)
+					return u, nil
+				})
+				if err != nil {
+					cancel()
+					return fmt.Errorf("update error for user id %s: %w", user.Identifier, err)
+				}
+
+				if existingUser.IsDisabled() && !user.IsDisabled() {
+					m.bus.Publish(app.TopicUserEnabled, *user)
+				}
 			}
 		}