Fix bug when 2 identical executions in same auth flow (#2904) (#2908)

* 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 <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 0e829e6a23)

Co-authored-by: Gaetan2907 <48204380+Gaetan2907@users.noreply.github.com>

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).

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:

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