From 7111edd6314f12c5ad1a698e48cb702e0b685651 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Mon, 5 Dec 2022 05:47:14 +0000 Subject: [PATCH] [PR #5619/f0b3bba0 backport][stable-5] Fix keycloak_client_rolemapping role removal and diff (#5655) 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 * 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 * Keycloak: Calculate client_rolemapping proposed state properly Signed-off-by: Florian Achleitner * Add changelog fragment Signed-off-by: Florian Achleitner Co-authored-by: Felix Fontein Co-authored-by: Felix Fontein * Fix for python2 unit test Signed-off-by: Florian Achleitner Co-authored-by: Felix Fontein (cherry picked from commit f0b3bba030055183b864678208e187c24d2ca581) Co-authored-by: fachleitner --- changelogs/fragments/5619-keycloak-improvements.yml | 3 +++ plugins/module_utils/identity/keycloak/keycloak.py | 2 +- .../identity/keycloak/keycloak_client_rolemapping.py | 11 +++++++---- 3 files changed, 11 insertions(+), 5 deletions(-) create mode 100644 changelogs/fragments/5619-keycloak-improvements.yml diff --git a/changelogs/fragments/5619-keycloak-improvements.yml b/changelogs/fragments/5619-keycloak-improvements.yml new file mode 100644 index 0000000000..2e5a739dad --- /dev/null +++ b/changelogs/fragments/5619-keycloak-improvements.yml @@ -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)." diff --git a/plugins/module_utils/identity/keycloak/keycloak.py b/plugins/module_utils/identity/keycloak/keycloak.py index 078925ef71..ce22800334 100644 --- a/plugins/module_utils/identity/keycloak/keycloak.py +++ b/plugins/module_utils/identity/keycloak/keycloak.py @@ -606,7 +606,7 @@ class KeycloakAPI(object): """ 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" diff --git a/plugins/modules/identity/keycloak/keycloak_client_rolemapping.py b/plugins/modules/identity/keycloak/keycloak_client_rolemapping.py index 4f1f9b0d0f..f0da97ef59 100644 --- a/plugins/modules/identity/keycloak/keycloak_client_rolemapping.py +++ b/plugins/modules/identity/keycloak/keycloak_client_rolemapping.py @@ -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): @@ -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: @@ -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) @@ -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) @@ -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)