From 0e829e6a23548ee948bfccc456498554832a56af Mon Sep 17 00:00:00 2001 From: Gaetan2907 <48204380+Gaetan2907@users.noreply.github.com> Date: Wed, 30 Jun 2021 15:01:17 +0200 Subject: [PATCH] Fix bug when 2 identical executions in same auth flow (#2904) * Fix bug when 2 identical executions in same auth flow * Add changelog fragment * Fix unit tests * Update changelogs/fragments/2904-fix-bug-when-2-identical-executions-in-same-auth-flow.yml Co-authored-by: Felix Fontein Co-authored-by: Felix Fontein --- ...when-2-identical-executions-in-same-auth-flow.yml | 3 +++ .../identity/keycloak/keycloak_authentication.py | 12 ++++++------ .../keycloak/test_keycloak_authentication.py | 6 +++--- 3 files changed, 12 insertions(+), 9 deletions(-) create mode 100644 changelogs/fragments/2904-fix-bug-when-2-identical-executions-in-same-auth-flow.yml diff --git a/changelogs/fragments/2904-fix-bug-when-2-identical-executions-in-same-auth-flow.yml b/changelogs/fragments/2904-fix-bug-when-2-identical-executions-in-same-auth-flow.yml new file mode 100644 index 0000000000..21fde3eb58 --- /dev/null +++ b/changelogs/fragments/2904-fix-bug-when-2-identical-executions-in-same-auth-flow.yml @@ -0,0 +1,3 @@ +bugfixes: + - keycloak_authentication - fix bug when two identical executions are in the same authentication flow + (https://github.com/ansible-collections/community.general/pull/2904). diff --git a/plugins/modules/identity/keycloak/keycloak_authentication.py b/plugins/modules/identity/keycloak/keycloak_authentication.py index 98b6378dac..9fd04eb70b 100644 --- a/plugins/modules/identity/keycloak/keycloak_authentication.py +++ b/plugins/modules/identity/keycloak/keycloak_authentication.py @@ -200,11 +200,11 @@ def create_or_update_executions(kc, config, realm='master'): try: changed = False if "authenticationExecutions" in config: + # Get existing executions on the Keycloak server for this alias + existing_executions = kc.get_executions_representation(config, realm=realm) for new_exec_index, new_exec in enumerate(config["authenticationExecutions"], start=0): if new_exec["index"] is not None: new_exec_index = new_exec["index"] - # Get existing executions on the Keycloak server for this alias - existing_executions = kc.get_executions_representation(config, realm=realm) exec_found = False # Get flowalias parent if given if new_exec["flowAlias"] is not None: @@ -222,6 +222,9 @@ def create_or_update_executions(kc, config, realm='master'): # Compare the executions to see if it need changes if not is_struct_included(new_exec, existing_executions[exec_index], exclude_key) or exec_index != new_exec_index: changed = True + id_to_update = existing_executions[exec_index]["id"] + # Remove exec from list in case 2 exec with same name + existing_executions[exec_index].clear() elif new_exec["providerId"] is not None: kc.create_execution(new_exec, flowAlias=flow_alias_parent, realm=realm) changed = True @@ -229,13 +232,10 @@ def create_or_update_executions(kc, config, realm='master'): kc.create_subflow(new_exec["displayName"], flow_alias_parent, realm=realm) changed = True if changed: - # Get existing executions on the Keycloak server for this alias - existing_executions = kc.get_executions_representation(config, realm=realm) - exec_index = find_exec_in_executions(new_exec, existing_executions) if exec_index != -1: # Update the existing execution updated_exec = { - "id": existing_executions[exec_index]["id"] + "id": id_to_update } # add the execution configuration if new_exec["authenticationConfig"] is not None: diff --git a/tests/unit/plugins/modules/identity/keycloak/test_keycloak_authentication.py b/tests/unit/plugins/modules/identity/keycloak/test_keycloak_authentication.py index 91e34eea7b..db0168aa83 100644 --- a/tests/unit/plugins/modules/identity/keycloak/test_keycloak_authentication.py +++ b/tests/unit/plugins/modules/identity/keycloak/test_keycloak_authentication.py @@ -343,7 +343,7 @@ class TestKeycloakAuthentication(ModuleTestCase): self.assertEqual(len(mock_get_authentication_flow_by_alias.mock_calls), 1) self.assertEqual(len(mock_copy_auth_flow.mock_calls), 0) self.assertEqual(len(mock_create_empty_auth_flow.mock_calls), 1) - self.assertEqual(len(mock_get_executions_representation.mock_calls), 3) + self.assertEqual(len(mock_get_executions_representation.mock_calls), 2) self.assertEqual(len(mock_delete_authentication_flow_by_id.mock_calls), 0) # Verify that the module's changed status matches what is expected @@ -434,7 +434,7 @@ class TestKeycloakAuthentication(ModuleTestCase): self.assertEqual(len(mock_get_authentication_flow_by_alias.mock_calls), 1) self.assertEqual(len(mock_copy_auth_flow.mock_calls), 0) self.assertEqual(len(mock_create_empty_auth_flow.mock_calls), 0) - self.assertEqual(len(mock_get_executions_representation.mock_calls), 3) + self.assertEqual(len(mock_get_executions_representation.mock_calls), 2) self.assertEqual(len(mock_delete_authentication_flow_by_id.mock_calls), 0) # Verify that the module's changed status matches what is expected @@ -611,7 +611,7 @@ class TestKeycloakAuthentication(ModuleTestCase): self.assertEqual(len(mock_get_authentication_flow_by_alias.mock_calls), 1) self.assertEqual(len(mock_copy_auth_flow.mock_calls), 0) self.assertEqual(len(mock_create_empty_auth_flow.mock_calls), 1) - self.assertEqual(len(mock_get_executions_representation.mock_calls), 3) + self.assertEqual(len(mock_get_executions_representation.mock_calls), 2) self.assertEqual(len(mock_delete_authentication_flow_by_id.mock_calls), 1) # Verify that the module's changed status matches what is expected