Hi,
I am looking into Bug #25795: LDAP - When User Group sync is enabled, user wait long time to authenticate / login - Foreman
As I can see it is unecessarily refreshing all the group users.
I figure out a performance tweek, but I would appreciate experienced user eyes and opinion before I proceed with the solution (make tests…)
I suppose, one user can be authentified only by one auth_source.
If that is the case, I think we can do just this: removes necessity to call refresh for all users external groups · ezr-ondrej/foreman@ffc2105 · GitHub
Yes, that is correct. Every user is explicitly assigned 1 auth source which is used during authentication. But user can have multiple external usergroup mappings potentially from different auth sources. So we should make sure we only apply changes from those mappings that are relevant, in other words we’ll use only mappings for user’s auth source.
If I understand the patch correctly, you’re replacing the logic from ExternalUsergroup#refresh with simply adding/removing user who logs in to/from external user group. I think that’s good improvement, few thoughts:
- you’ll need to
next unless auth_source.respond_to?(:users_in_group) since some auth sources don’t support fetching users from groups.
- you may also want to skip this entirely if user’s authsource is not ldap one (there’s internal that uses credetials in DB, there’s external which only relies on REMOTE_USER env variable, there’s hidden for hidden system users, and there’s at least one introduced in some plugin inheriting from hidden)
- we need to make sure the change creates audit record
I think it would be better to continue discussion in a PR 
I have moved the work in ldap auth source, so the checks are not necessary.
I am doing it user based, so I guess it is safe to presume the user can be added to the group just by this external user group?
I have opened PR: https://github.com/theforeman/foreman/pull/6388
1 Like
