Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

keycloak_user_federation: set krbPrincipalAttribute to '' if unset in kc responses #8785

Conversation

fgruenbauer
Copy link
Contributor

@fgruenbauer fgruenbauer commented Aug 21, 2024

SUMMARY

Issue:
The keycloak_user_federation module always detects a change in check mode if the parameter krbPrincipalAttribute is set to ''. The empty string is a valid value:

When this is empty, the LDAP user will be looked based on LDAP username corresponding

Keycloak completely removes the parameter krbPrincipalAttribute if it is set to ''. So subsequent check runs always detect a change. In a normal run the module would always make an update (its the same change check), but compare the before and after responses afterwards, in both of which the parameter is not present. In the check diff this is already fixed by setting '' in the sanitize function if the parameter is not present (see 8320).

Proposed solution:
Normalize the keycloak responses (before and after) by setting krbPrincipalAttribute = '' if the parameter is not present in the response.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

keycloak_user_federation

ADDITIONAL INFORMATION
  1. set krbPrincipalAttribute = '' for the module
  2. do a normal module run to set the parameter
  3. subsequent check runs always detect a change

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added WIP Work in progress bug This issue/PR relates to a bug module module plugins plugin (any type) labels Aug 21, 2024
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-8 Automatically create a backport for the stable-8 branch backport-9 Automatically create a backport for the stable-9 branch labels Aug 21, 2024
@fgruenbauer fgruenbauer reopened this Sep 15, 2024
@ansibullbot ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci CI is older than 7 days, rerun before merging labels Sep 15, 2024
@ansibullbot
Copy link
Collaborator

@fgruenbauer this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! and removed stale_ci CI is older than 7 days, rerun before merging labels Sep 15, 2024
@fgruenbauer fgruenbauer force-pushed the keycloak-user-federation-normalize-krbPrincipalAttribute-in-kc-responses branch from 5964005 to 5ea8a2d Compare September 15, 2024 19:57
@fgruenbauer fgruenbauer marked this pull request as ready for review September 15, 2024 20:00
@ansibullbot ansibullbot removed WIP Work in progress merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Sep 15, 2024
@felixfontein
Copy link
Collaborator

Please remember to add a changelog fragment. Thanks!

@fgruenbauer
Copy link
Contributor Author

Sry - Done.

…cipalAttribute-to-empty-string-if-missing.yaml

Co-authored-by: Felix Fontein <[email protected]>
@felixfontein
Copy link
Collaborator

If nobody objects I'll merge this by the end of the week.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 21, 2024
@felixfontein felixfontein merged commit ac302eb into ansible-collections:main Sep 21, 2024
147 checks passed
Copy link

patchback bot commented Sep 21, 2024

Backport to stable-8: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-8/ac302eb77d82f5ed87cf8b037297c3482622247d/pr-8785

Backported as #8891

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Sep 21, 2024
…t in kc responses (#8785)

* set `krbPrincipalAttribute` to `''` if unset in kc before and after responses

* add changelog fragment

* Update changelogs/fragments/8785-keycloak_user_federation-set-krbPrincipalAttribute-to-empty-string-if-missing.yaml

Co-authored-by: Felix Fontein <[email protected]>

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit ac302eb)
Copy link

patchback bot commented Sep 21, 2024

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/ac302eb77d82f5ed87cf8b037297c3482622247d/pr-8785

Backported as #8892

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein
Copy link
Collaborator

@fgruenbauer thanks for fixing this!

patchback bot pushed a commit that referenced this pull request Sep 21, 2024
…t in kc responses (#8785)

* set `krbPrincipalAttribute` to `''` if unset in kc before and after responses

* add changelog fragment

* Update changelogs/fragments/8785-keycloak_user_federation-set-krbPrincipalAttribute-to-empty-string-if-missing.yaml

Co-authored-by: Felix Fontein <[email protected]>

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit ac302eb)
felixfontein pushed a commit that referenced this pull request Sep 21, 2024
…`krbPrincipalAttribute` to `''` if unset in kc responses (#8891)

keycloak_user_federation: set `krbPrincipalAttribute` to `''` if unset in kc responses (#8785)

* set `krbPrincipalAttribute` to `''` if unset in kc before and after responses

* add changelog fragment

* Update changelogs/fragments/8785-keycloak_user_federation-set-krbPrincipalAttribute-to-empty-string-if-missing.yaml

Co-authored-by: Felix Fontein <[email protected]>

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit ac302eb)

Co-authored-by: fgruenbauer <[email protected]>
felixfontein pushed a commit that referenced this pull request Sep 21, 2024
…`krbPrincipalAttribute` to `''` if unset in kc responses (#8892)

keycloak_user_federation: set `krbPrincipalAttribute` to `''` if unset in kc responses (#8785)

* set `krbPrincipalAttribute` to `''` if unset in kc before and after responses

* add changelog fragment

* Update changelogs/fragments/8785-keycloak_user_federation-set-krbPrincipalAttribute-to-empty-string-if-missing.yaml

Co-authored-by: Felix Fontein <[email protected]>

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit ac302eb)

Co-authored-by: fgruenbauer <[email protected]>
aioue pushed a commit to aioue/community.general that referenced this pull request Oct 1, 2024
…t in kc responses (ansible-collections#8785)

* set `krbPrincipalAttribute` to `''` if unset in kc before and after responses

* add changelog fragment

* Update changelogs/fragments/8785-keycloak_user_federation-set-krbPrincipalAttribute-to-empty-string-if-missing.yaml

Co-authored-by: Felix Fontein <[email protected]>

---------

Co-authored-by: Felix Fontein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8 Automatically create a backport for the stable-8 branch backport-9 Automatically create a backport for the stable-9 branch bug This issue/PR relates to a bug module module plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants