Skip to content

Commit

Permalink
[PR #5619/f0b3bba0 backport][stable-6] Fix keycloak_client_rolemappin…
Browse files Browse the repository at this point in the history
…g role removal and diff (#5656)

Fix keycloak_client_rolemapping role removal and diff (#5619)

* Keycloak: Fix client rolemapping removal

Keycloak's delete_group_rolemapping API wrapper didn't pass data about
the roles to remove to keycloak, resulting in removal of all roles.

Follow the intended behaviour and delete only the roles listed in the
module invocation.

Signed-off-by: Florian Achleitner <flo@fopen.at>

* Keycloak: Fix client_rolemapping diff

The module's diff output wrongly showed the changed roles list as
'after' state. This is obviously wrong for role removal and also
wrong for role addition, if there are other roles assigned.

Use the result of the API query for 'end_state' for 'diff' as well.

Signed-off-by: Florian Achleitner <flo@fopen.at>

* Keycloak: Calculate client_rolemapping proposed state properly

Signed-off-by: Florian Achleitner <flo@fopen.at>

* Add changelog fragment

Signed-off-by: Florian Achleitner <flo@fopen.at>
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>

* Fix for python2 unit test

Signed-off-by: Florian Achleitner <flo@fopen.at>
Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit f0b3bba)

Co-authored-by: fachleitner <flo@fopen.at>
  • Loading branch information
patchback[bot] and flyingflo authored Dec 5, 2022
1 parent 2fa3659 commit 671f850
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
3 changes: 3 additions & 0 deletions changelogs/fragments/5619-keycloak-improvements.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
bugfixes:
- "keycloak_client_rolemapping - remove only listed mappings with ``state=absent`` (https://github.com/ansible-collections/community.general/pull/5619)."
- "keycloak_client_rolemapping - calculate ``proposed`` and ``after`` return values properly (https://github.com/ansible-collections/community.general/pull/5619)."
2 changes: 1 addition & 1 deletion plugins/module_utils/identity/keycloak/keycloak.py
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ def delete_group_rolemapping(self, gid, cid, role_rep, realm="master"):
"""
available_rolemappings_url = URL_CLIENT_GROUP_ROLEMAPPINGS.format(url=self.baseurl, realm=realm, id=gid, client=cid)
try:
open_url(available_rolemappings_url, method="DELETE", http_agent=self.http_agent, headers=self.restheaders,
open_url(available_rolemappings_url, method="DELETE", http_agent=self.http_agent, headers=self.restheaders, data=json.dumps(role_rep),
validate_certs=self.validate_certs, timeout=self.connection_timeout)
except Exception as e:
self.module.fail_json(msg="Could not delete available rolemappings for client %s in group %s, realm %s: %s"
Expand Down
11 changes: 7 additions & 4 deletions plugins/modules/keycloak_client_rolemapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ def main():
assigned_roles_before = kc.get_client_group_composite_rolemappings(gid, cid, realm=realm)

result['existing'] = assigned_roles_before
result['proposed'] = roles
result['proposed'] = list(assigned_roles_before) if assigned_roles_before else []

update_roles = []
for role_index, role in enumerate(roles, start=0):
Expand All @@ -307,6 +307,7 @@ def main():
'id': role['id'],
'name': role['name'],
})
result['proposed'].append(available_role)
# Fetch roles to remove if state absent
else:
for assigned_role in assigned_roles_before:
Expand All @@ -315,13 +316,15 @@ def main():
'id': role['id'],
'name': role['name'],
})
if assigned_role in result['proposed']: # Handle double removal
result['proposed'].remove(assigned_role)

if len(update_roles):
if state == 'present':
# Assign roles
result['changed'] = True
if module._diff:
result['diff'] = dict(before=assigned_roles_before, after=update_roles)
result['diff'] = dict(before=assigned_roles_before, after=result['proposed'])
if module.check_mode:
module.exit_json(**result)
kc.add_group_rolemapping(gid, cid, update_roles, realm=realm)
Expand All @@ -333,7 +336,7 @@ def main():
# Remove mapping of role
result['changed'] = True
if module._diff:
result['diff'] = dict(before=assigned_roles_before, after=update_roles)
result['diff'] = dict(before=assigned_roles_before, after=result['proposed'])
if module.check_mode:
module.exit_json(**result)
kc.delete_group_rolemapping(gid, cid, update_roles, realm=realm)
Expand All @@ -344,7 +347,7 @@ def main():
# Do nothing
else:
result['changed'] = False
result['msg'] = 'Nothing to do, roles %s are correctly mapped with group %s.' % (roles, group_name)
result['msg'] = 'Nothing to do, roles %s are %s with group %s.' % (roles, 'mapped' if state == 'present' else 'not mapped', group_name)
module.exit_json(**result)


Expand Down

0 comments on commit 671f850

Please sign in to comment.