From c73ce0288e3cb9438fcef8fd27cd1c4c9b65af3c Mon Sep 17 00:00:00 2001
From: Christoph Haas <christoph.h@sprinternet.at>
Date: Sat, 18 Jan 2025 17:39:18 +0100
Subject: [PATCH] fix disabling of missing ldap users (#344) and allow deletion
 of all user types

---
 frontend/src/components/UserEditModal.vue |  2 +-
 internal/app/auth/auth.go                 | 12 +++++++++++-
 internal/app/users/user_manager.go        | 21 +++++++++++++++++----
 internal/domain/user.go                   |  6 +-----
 4 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/frontend/src/components/UserEditModal.vue b/frontend/src/components/UserEditModal.vue
index cf2ac0c1..f6e7c224 100644
--- a/frontend/src/components/UserEditModal.vue
+++ b/frontend/src/components/UserEditModal.vue
@@ -165,7 +165,7 @@ async function del() {
     </template>
     <template #footer>
       <div class="flex-fill text-start">
-        <button v-if="props.userId!=='#NEW#'&&formData.Source==='db'" class="btn btn-danger me-1" type="button" @click.prevent="del">{{ $t('general.delete') }}</button>
+        <button v-if="props.userId!=='#NEW#'" class="btn btn-danger me-1" type="button" @click.prevent="del">{{ $t('general.delete') }}</button>
       </div>
       <button class="btn btn-primary me-1" type="button" @click.prevent="save">{{ $t('general.save') }}</button>
       <button class="btn btn-secondary" type="button" @click.prevent="close">{{ $t('general.close') }}</button>
diff --git a/internal/app/auth/auth.go b/internal/app/auth/auth.go
index c032622d..f3098e1b 100644
--- a/internal/app/auth/auth.go
+++ b/internal/app/auth/auth.go
@@ -373,7 +373,7 @@ func (a *Authenticator) processUserInfo(
 	case err != nil:
 		return nil, fmt.Errorf("registration disabled, cannot create missing user: %w", err)
 	default:
-		err = a.updateExternalUser(ctx, user, userInfo)
+		err = a.updateExternalUser(ctx, user, userInfo, source, provider)
 		if err != nil {
 			return nil, fmt.Errorf("failed to update user: %w", err)
 		}
@@ -432,6 +432,8 @@ func (a *Authenticator) updateExternalUser(
 	ctx context.Context,
 	existingUser *domain.User,
 	userInfo *domain.AuthenticatorUserInfo,
+	source domain.UserSource,
+	provider string,
 ) error {
 	if existingUser.IsLocked() || existingUser.IsDisabled() {
 		return nil // user is locked or disabled, do not update
@@ -462,6 +464,14 @@ func (a *Authenticator) updateExternalUser(
 		existingUser.IsAdmin = userInfo.IsAdmin
 		isChanged = true
 	}
+	if existingUser.Source != source {
+		existingUser.Source = source
+		isChanged = true
+	}
+	if existingUser.ProviderName != provider {
+		existingUser.ProviderName = provider
+		isChanged = true
+	}
 
 	if !isChanged {
 		return nil // nothing to update
diff --git a/internal/app/users/user_manager.go b/internal/app/users/user_manager.go
index 41aab3e2..2a905818 100644
--- a/internal/app/users/user_manager.go
+++ b/internal/app/users/user_manager.go
@@ -73,11 +73,16 @@ func (m Manager) NewUser(ctx context.Context, user *domain.User) error {
 		u.Identifier = user.Identifier
 		u.Email = user.Email
 		u.Source = user.Source
+		u.ProviderName = user.ProviderName
 		u.IsAdmin = user.IsAdmin
 		u.Firstname = user.Firstname
 		u.Lastname = user.Lastname
 		u.Phone = user.Phone
 		u.Department = user.Department
+		u.Notes = user.Notes
+		u.ApiToken = user.ApiToken
+		u.ApiTokenCreated = user.ApiTokenCreated
+
 		return u, nil
 	})
 	if err != nil {
@@ -421,13 +426,14 @@ func (m Manager) runLdapSynchronizationService(ctx context.Context) {
 				logrus.Debugf("sync disabled for LDAP server: %s", cfg.ProviderName)
 				return
 			}
+
 			running := true
 			for running {
 				select {
 				case <-ctx.Done():
 					running = false
 					continue
-				case <-time.After(syncInterval * time.Second):
+				case <-time.After(syncInterval):
 					// select blocks until one of the cases evaluate to true
 				}
 
@@ -460,7 +466,7 @@ func (m Manager) synchronizeLdapUsers(ctx context.Context, provider *config.Ldap
 		return err
 	}
 
-	logrus.Tracef("fetched %d raw ldap users...", len(rawUsers))
+	logrus.Tracef("fetched %d raw ldap users from provider %s...", len(rawUsers), provider.ProviderName)
 
 	// Update existing LDAP users
 	err = m.updateLdapUsers(ctx, provider.ProviderName, rawUsers, &provider.FieldMap, provider.ParsedAdminGroupDN)
@@ -497,13 +503,13 @@ func (m Manager) updateLdapUsers(
 			return fmt.Errorf("find error for user id %s: %w", user.Identifier, err)
 		}
 
-		tctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
-		defer cancel()
+		tctx, cancel := context.WithTimeout(ctx, 30*time.Second)
 		tctx = domain.SetUserInfo(tctx, domain.SystemAdminContextUserInfo())
 
 		if existingUser == nil {
 			err := m.NewUser(tctx, user)
 			if err != nil {
+				cancel()
 				return fmt.Errorf("create error for user id %s: %w", user.Identifier, err)
 			}
 		}
@@ -514,6 +520,8 @@ func (m Manager) updateLdapUsers(
 			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
@@ -525,9 +533,12 @@ func (m Manager) updateLdapUsers(
 				return u, nil
 			})
 			if err != nil {
+				cancel()
 				return fmt.Errorf("update error for user id %s: %w", user.Identifier, err)
 			}
 		}
+
+		cancel()
 	}
 
 	return nil
@@ -567,6 +578,8 @@ func (m Manager) disableMissingLdapUsers(
 			continue
 		}
 
+		logrus.Tracef("user %s is missing in ldap provider %s, disabling", user.Identifier, providerName)
+
 		now := time.Now()
 		user.Disabled = &now
 		user.DisabledReason = domain.DisabledReasonLdapMissing
diff --git a/internal/domain/user.go b/internal/domain/user.go
index b64edfb2..9e0c38f8 100644
--- a/internal/domain/user.go
+++ b/internal/domain/user.go
@@ -101,11 +101,7 @@ func (u *User) EditAllowed(new *User) error {
 }
 
 func (u *User) DeleteAllowed() error {
-	if u.Source == UserSourceDatabase {
-		return nil
-	}
-
-	return errors.New("delete only allowed for database source")
+	return nil // all users can be deleted, OAuth and LDAP users might still be recreated
 }
 
 func (u *User) CheckPassword(password string) error {