From f7c8a893289aa1699889c0f15a1cbbe937b30a7f Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Tue, 22 Feb 2022 09:08:42 +0000 Subject: [PATCH] Fixes for keycloak_user_federation (#4212) (#4251) * keycloak: fix creating a user federation w/ idempotent id Creating a user federation while specifying an id (that doesn't exist yet) will fail with a 404. This commits fix this behavior. * keycloak: fix user federation mapper duplication This commit fixes a bug where mappers are duplicated instead of configured when creating a user federation. When creating a user federation, some mappers are autogenerated by keycloak. This commit lets the keycloak_user_federation module recompute mappers final values after the user federation is created so that the module can try to merge them by their name. * add missing fragment for pr #4212 (cherry picked from commit c1485b885d8250214a1a8f46c1c301b60b0b9830) Co-authored-by: Jules Lamur --- ...212-fixes-for-keycloak-user-federation.yml | 8 +++++++ .../keycloak/keycloak_user_federation.py | 23 ++++++++++++++----- .../keycloak/test_keycloak_user_federation.py | 4 ++-- 3 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/4212-fixes-for-keycloak-user-federation.yml diff --git a/changelogs/fragments/4212-fixes-for-keycloak-user-federation.yml b/changelogs/fragments/4212-fixes-for-keycloak-user-federation.yml new file mode 100644 index 0000000000..033add7a90 --- /dev/null +++ b/changelogs/fragments/4212-fixes-for-keycloak-user-federation.yml @@ -0,0 +1,8 @@ +--- +bugfixes: + - keycloak_user_federation - creating a user federation while specifying an + ID (that does not exist yet) no longer fail with a 404 Not Found + (https://github.com/ansible-collections/community.general/pull/4212). + - keycloak_user_federation - mappers auto-created by keycloak are matched and + merged by their name and no longer create duplicated entries + (https://github.com/ansible-collections/community.general/pull/4212). diff --git a/plugins/modules/identity/keycloak/keycloak_user_federation.py b/plugins/modules/identity/keycloak/keycloak_user_federation.py index 4298334924..871837a5f3 100644 --- a/plugins/modules/identity/keycloak/keycloak_user_federation.py +++ b/plugins/modules/identity/keycloak/keycloak_user_federation.py @@ -828,7 +828,7 @@ def main(): before_comp = dict() # if user federation exists, get associated mappers - if cid is not None: + if cid is not None and before_comp: before_comp['mappers'] = sorted(kc.get_components(urlencode(dict(parent=cid)), realm), key=lambda x: x.get('name')) # build a changeset @@ -904,12 +904,23 @@ def main(): after_comp = kc.create_component(updated_comp, realm) for mapper in updated_mappers: - if mapper.get('id') is not None: - kc.update_component(mapper, realm) + found = kc.get_components(urlencode(dict(parent=cid, name=mapper['name'])), realm) + if len(found) > 1: + module.fail_json(msg='Found multiple mappers with name `{name}`. Cannot continue.'.format(name=mapper['name'])) + if len(found) == 1: + old_mapper = found[0] else: - if mapper.get('parentId') is None: - mapper['parentId'] = after_comp['id'] - mapper = kc.create_component(mapper, realm) + old_mapper = {} + + new_mapper = old_mapper.copy() + new_mapper.update(mapper) + + if new_mapper.get('id') is not None: + kc.update_component(new_mapper, realm) + else: + if new_mapper.get('parentId') is None: + new_mapper['parentId'] = after_comp['id'] + mapper = kc.create_component(new_mapper, realm) after_comp['mappers'] = updated_mappers result['end_state'] = sanitize(after_comp) diff --git a/tests/unit/plugins/modules/identity/keycloak/test_keycloak_user_federation.py b/tests/unit/plugins/modules/identity/keycloak/test_keycloak_user_federation.py index 674efeb237..a37ea1bb11 100644 --- a/tests/unit/plugins/modules/identity/keycloak/test_keycloak_user_federation.py +++ b/tests/unit/plugins/modules/identity/keycloak/test_keycloak_user_federation.py @@ -342,7 +342,7 @@ class TestKeycloakUserFederation(ModuleTestCase): ] } return_value_components_get = [ - [] + [], [] ] return_value_component_create = [ { @@ -457,7 +457,7 @@ class TestKeycloakUserFederation(ModuleTestCase): with self.assertRaises(AnsibleExitJson) as exec_info: self.module.main() - self.assertEqual(len(mock_get_components.mock_calls), 1) + self.assertEqual(len(mock_get_components.mock_calls), 2) self.assertEqual(len(mock_get_component.mock_calls), 0) self.assertEqual(len(mock_create_component.mock_calls), 2) self.assertEqual(len(mock_update_component.mock_calls), 0)