diff --git a/backend/internal/service/ldap_service.go b/backend/internal/service/ldap_service.go index 1c080eaf..6283538f 100644 --- a/backend/internal/service/ldap_service.go +++ b/backend/internal/service/ldap_service.go @@ -366,17 +366,22 @@ func (s *LdapService) SyncUsers(ctx context.Context, tx *gorm.DB, client *ldap.C } if dbConfig.LdapSoftDeleteUsers.IsTrue() { - err = s.userService.DisableUser(ctx, user.ID, tx) + err = s.userService.disableUserInternal(ctx, user.ID, tx) if err != nil { - log.Printf("Failed to disable user %s: %v", user.Username, err) - continue + return fmt.Errorf("failed to disable user %s: %w", user.Username, err) } + + log.Printf("Disabled user '%s'", user.Username) } else { - err = s.userService.DeleteUser(ctx, user.ID, true) - if err != nil { - log.Printf("Failed to delete user %s: %v", user.Username, err) - continue + err = s.userService.deleteUserInternal(ctx, user.ID, true, tx) + target := &common.LdapUserUpdateError{} + if errors.As(err, &target) { + return fmt.Errorf("failed to delete user %s: LDAP user must be disabled before deletion", user.Username) + } else if err != nil { + return fmt.Errorf("failed to delete user %s: %w", user.Username, err) } + + log.Printf("Deleted user '%s'", user.Username) } } diff --git a/backend/internal/service/user_service.go b/backend/internal/service/user_service.go index b6347d04..e83111c5 100644 --- a/backend/internal/service/user_service.go +++ b/backend/internal/service/user_service.go @@ -175,28 +175,9 @@ func (s *UserService) UpdateProfilePicture(userID string, file io.Reader) error } func (s *UserService) DeleteUser(ctx context.Context, userID string, allowLdapDelete bool) error { - tx := s.db.Begin() - - var user model.User - if err := tx.WithContext(ctx).First(&user, "id = ?", userID).Error; err != nil { - tx.Rollback() - return err - } - - // Only soft-delete if user is LDAP and soft-delete is enabled and not allowing hard delete - if user.LdapID != nil && s.appConfigService.GetDbConfig().LdapSoftDeleteUsers.IsTrue() && !allowLdapDelete { - if !user.Disabled { - tx.Rollback() - return fmt.Errorf("LDAP user must be disabled before deletion") - } - } - - // Otherwise, hard delete (local users or LDAP users when allowed) - if err := s.deleteUserInternal(ctx, userID, allowLdapDelete, tx); err != nil { - tx.Rollback() - return err - } - return tx.Commit().Error + return s.db.Transaction(func(tx *gorm.DB) error { + return s.deleteUserInternal(ctx, userID, allowLdapDelete, tx) + }) } func (s *UserService) deleteUserInternal(ctx context.Context, userID string, allowLdapDelete bool, tx *gorm.DB) error { @@ -355,7 +336,6 @@ func (s *UserService) RequestOneTimeAccessEmailAsAdmin(ctx context.Context, user } return s.requestOneTimeAccessEmailInternal(ctx, userID, "", expiration) - } func (s *UserService) RequestOneTimeAccessEmailAsUnauthenticatedUser(ctx context.Context, userID, redirectPath string) error { @@ -649,8 +629,9 @@ func (s *UserService) ResetProfilePicture(userID string) error { return nil } -func (s *UserService) DisableUser(ctx context.Context, userID string, tx *gorm.DB) error { - return tx.WithContext(ctx). +func (s *UserService) disableUserInternal(ctx context.Context, userID string, tx *gorm.DB) error { + return tx. + WithContext(ctx). Model(&model.User{}). Where("id = ?", userID). Update("disabled", true).