Skip to content

Commit

Permalink
ldap-sync: fix creation of only one user per LDAP sync (#375)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
klmmr authored Feb 26, 2025
1 parent 67f076e commit eeb0c87
Showing 1 changed file with 34 additions and 35 deletions.
69 changes: 34 additions & 35 deletions internal/app/users/user_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""

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)
if existingUser.IsDisabled() && !user.IsDisabled() {
m.bus.Publish(app.TopicUserEnabled, *user)
}
}
}

Expand Down

0 comments on commit eeb0c87

Please sign in to comment.