Hi,

thank you for the patch. I like the changes but since this is a change of behavior I think an option to return to the old behavior would be useful.

We regular have questions about e.g. updating sudo rules more often. Also we currently try to refresh HBAC rules whenever there is a new access control check requested. Both often depend on group memberships and are tested by opening a new session, often while there is another session still running. While it would be possible to tell SSSD to update the cached initgroups data more often by tuning entry_cache_user_timeout this might have the negative effect that now all user data is updated more often. So being able to return to the old behavior seems to be better.

Additionally with such an option it would be possible to allow that the cached data is always used for every PAM operation. E.g. an option pam_id_use_cache can have the values never (old behavior), if_user_session_exists (your patch and new default) and always.

bye,
Sumit

Hi Sumit,
can you think of a case where the old behavior is desirable ?
initgroups does not update sudo or hbac rules.

I am really not a fan of adding more knobs here, I would actually love to start deprecating options in sssd, the code is drowning and the test matrix can't possibly cover all cases already.

One thing I thought we may do is to run a initgroups call in the background, when a cached one is done (Ie not waiting on it to finish but still triggering it). Would that achieve your goal and avoid the need for an option ?

Hi Sumit,
can you think of a case where the old behavior is desirable ?
initgroups does not update sudo or hbac rules.

No, but the hbac rules are updated during every login attempt during the access control step and since the group memberships of a user are typically part of the rules it would be good to have the current group memberships as well so that the evaluation of the rules lead to the expected result.

My point is just that it is currently expected behavior that the group memberships are updated for every new login independent if you are already logged in or not and people might rely on this. E.g. you ask helpdesk to be added to a new group and then check with 'su - yourusername' if you are are now a member of the group.

I am really not a fan of adding more knobs here, I would actually love to start deprecating options in sssd, the code is drowning and the test matrix can't possibly cover all cases already.

I think at least in this case it might be acceptable because the option will only tell the cache_req in which order backend and cache should be checked, which is already covered by existing tests.

One thing I thought we may do is to run a initgroups call in the background, when a cached one is done (Ie not waiting on it to finish but still triggering it). Would that achieve your goal and avoid the need for an option ?

But this would mean that only a second login attempt will the updated group memberships. Btw, Jakub recently improved and fixed the background refresh where one of the steps was that initgroups is handled properly as well, which would make sure that the cached entries do not expire.

bye,
Sumit

I think that if update of groups is desired at every login that we should have it done asynchronously when a session is active.
The problem is that it currently blocks the login leading to unacceptable user experience for interactive sessions.
I also think that should be done as a separate improvement though.

If you insist (then say so :) I can add an option to control this behavior that defaults to this behavior being enabled, but I am really not too happy to add more and more options. I would rather have heuristics that work well and do not need options to tweak if at all possible,

Hi,

Yes, please add such an option or let me know if I should add it. Since it is a change of behavior I still think we need a way to switch back to the old behavior although the new one might be better for most use-cases.

bye,
Sumit

FYI, previously I backported these patches to RHEL 8.0 (SSSD 2.0) and all was good, now I did it for RHEL 8.1 (SSSD 2.2) and these patches seem not sufficient anymore. Either something else is forcing a lookup of some kind in the authentication path, or something is not working in detecting an active session.

Hi,

I have added the config option with additional patches and created a PR which includes the patches here at https://github.com/SSSD/sssd/pull/1005.

I close the PR here.

bye,
Sumit