idp: Implement initgroups sync

[pdf]

Other backends treat initgroups results as authoritative rather than additive. This change brings the idp backend into line with this behaviour so that group members are correctly removed from the cache when they are removed from the directory.

Also cleans up the logging to make it obvious when EEXIST errors are no-ops rather than errors, to avoid confusion.

Fixes #9117 #8979

[sumit-bose]

Hi,

thank you very much for fixing the issue and for the tests. I started with the tests because, as you might have recognized, we currently have some issues with the CI test.

But your tests currently fail for two different reasons. The first is that the number of :step: and :expectedresults: must match, so please add something like "User is removed successfully from the group " or similar as second step.

The second reason if an issue in the test framework. So far the remove_member() was never used for keycloak and as a result it didn't work. SSSD/sssd-test-framework#270 has a fix in case you want to run the tests locally.

I will look at the code next.

bye,
Sumit

[pdf]

Hi @sumit-bose I believe that the comments I left about the test failures are related to existing issues with the Actions configuration / existing tests - I don't think that there's anything in this PR that would produce additional test failures, beyond the expected failures in the existing codebase?.

The current state of the sudo tests I believe has been around for a year or more (I didn't try to trace git blame beyond that) - I can look at submitting a PR to fix the heredoc in those tests, but unless I'm misinterpreting things - I think pre-existing CI issues should be fixed independently of this PR.

On the Keycloak issue, I don't have a lab to test that, but if you have some suggestions for additional test cases I'd be happy to write them up - my own manual validation was only performed on Entra ID.

[pdf]

Apologies, I see what you were referring to, sorry I was confused as this was the same problem as some of the previously existing tests.

[spoore1]

Overall, the tests look good to me. Just a small suggested docstring change to one of the steps.

Also, I ran them with this update and the fix in sssd-test-framework PR #270:

==================== short test summary info ====================
PASSED tests/test_idp.py::test_idp__group_member_removal[true] (keycloak)
PASSED tests/test_idp.py::test_idp__group_member_removal[false] (keycloak)
PASSED tests/test_idp.py::test_idp__user_group_removal[true] (keycloak)
PASSED tests/test_idp.py::test_idp__user_group_removal[false] (keycloak)
==================== 4 passed, 963 deselected in 195.57s (0:03:15) ====

[spoore1]

Can this be reworded to more directly reflect what lookup is being done? To me (and maybe it's just me) it reads like the step requires running a group lookup (e.g. getent group). Maybe rewording with something like:

"Look up the user's info to populate the cache with group memberships"