[PR #11601/e7a253b4 backport][stable-12] keycloak_authentication_v2: covers idp flow overrides in safe swap (fix 11582) (#11617)

keycloak_authentication_v2: covers idp flow overrides in safe swap (fix 11582) (#11601)

* 11582 keycloak_authentication_v2 covers idp flow overrides in safe swap

* 11583 update documentation and comments

(cherry picked from commit e7a253b4c9)

Co-authored-by: thomasbargetz <thomas.bargetz@gmail.com>

plugins/modules/keycloak_authentication_v2.py

@@ -16,8 +16,8 @@ description:
   - Rather than modifying an existing flow in place, the module re-creates the flow using the B(Safe Swap) mechanism described below.
   - B(Safe Swap mechanism) - When an authentication flow needs to be updated, the module never modifies the existing flow in place.
      Instead it follows a multi-step swap procedure to ensure the flow is never left in an intermediate or unsafe state during the update.
-     This is especially important when the flow is actively bound to a realm binding or a client override,
-     because a partially-updated flow could inadvertently allow unauthorised access.
+     This is especially important when the flow is actively bound to a realm binding, a client override, or as an identity-provider
+     login-flow or post-flow, because a partially-updated flow could inadvertently allow unauthorised access.
   - The B(Safe Swap mechanism) is as follows. 1. A new flow is created under a temporary name (the original alias plus a configurable suffix,
      for example C(myflow_tmp_for_swap)).
      2. All executions and their configurations are added to the new temporary flow. 3. If the existing flow is currently bound to a realm or a client,
@@ -639,8 +639,8 @@ def existing_auth_to_diff_repr(kc: KeycloakAPI, realm: str, existing_auth: dict)
 
 
 def is_auth_flow_in_use(kc: KeycloakAPI, realm: str, existing_auth: dict) -> bool:
-    """Determine whether the given flow is currently bound to a realm binding or a client
-    authentication flow override.
+    """Determine whether the given flow is currently bound to a realm binding, a client
+    authentication flow override or as an identity-provider login-flow or post-flow.
 
     :param kc: a KeycloakAPI instance.
     :param realm: the realm to inspect.
@@ -673,6 +673,12 @@ def is_auth_flow_in_use(kc: KeycloakAPI, realm: str, existing_auth: dict) -> boo
         if overrides.get("direct_grant") == flow_id:
             return True
 
+    for identity_provider in kc.get_identity_providers(realm):
+        first_broker_login_flow_alias = identity_provider.get("firstBrokerLoginFlowAlias")
+        post_broker_login_flow_alias = identity_provider.get("postBrokerLoginFlowAlias")
+        if first_broker_login_flow_alias == flow_alias or post_broker_login_flow_alias == flow_alias:
+            return True
+
     return False
 
 
@@ -684,8 +690,8 @@ def rebind_auth_flow_bindings(
     to_id: str,
     to_alias: str,
 ) -> None:
-    """Re-point all realm bindings and client overrides that reference the source flow to the
-    target flow.
+    """Re-point all realm bindings, client flow overrides and identity-provider login-flows or post-flows
+    that reference the source flow to the target flow.
 
     This is the critical step in the Safe Swap procedure that transfers live bindings from the
     old flow to the newly-created temporary flow without any gap in coverage.
@@ -733,6 +739,22 @@ def rebind_auth_flow_bindings(
         if client_changed:
             kc.update_client(id=client["id"], clientrep=client, realm=realm)
 
+    for identity_provider in kc.get_identity_providers(realm):
+        first_broker_login_flow_alias = identity_provider.get("firstBrokerLoginFlowAlias")
+        post_broker_login_flow_alias = identity_provider.get("postBrokerLoginFlowAlias")
+        identity_provider_changed = False
+
+        if first_broker_login_flow_alias == from_alias:
+            identity_provider["firstBrokerLoginFlowAlias"] = to_alias
+            identity_provider_changed = True
+
+        if post_broker_login_flow_alias == from_alias:
+            identity_provider["postBrokerLoginFlowAlias"] = to_alias
+            identity_provider_changed = True
+
+        if identity_provider_changed:
+            kc.update_identity_provider(idprep=identity_provider, realm=realm)
+
 
 def delete_tmp_swap_flow_if_exists(
     kc: KeycloakAPI,

tests/integration/targets/keycloak_authentication_v2/tasks/main.yml

@@ -9,18 +9,26 @@
   register: result
   until: result is success
 
-- name: Executing Flow Creation tests
+- name: Executing flow creation tests
   ansible.builtin.include_tasks:
     file: tests/test_flow_creation.yml
 
-- name: Executing unused Flow Modification tests
+- name: Executing unused flow modification tests
   ansible.builtin.include_tasks:
     file: tests/test_unused_flow_modification.yml
 
-- name: Executing used Flow Modification tests
+- name: Executing used default browser flow modification tests
   ansible.builtin.include_tasks:
-    file: tests/test_used_flow_modification.yml
+    file: tests/test_used_default_browser_flow_modification.yml
 
-- name: Executing Flow Deletion tests
+- name: Executing client flow override flow modification tests
+  ansible.builtin.include_tasks:
+    file: tests/test_client_flow_override_flow_modification.yml
+
+- name: Executing identity provider first and post flow override flow modification tests
+  ansible.builtin.include_tasks:
+    file: tests/test_identity_provider_flow_overrides_flow_modification.yml
+
+- name: Executing flow deletion tests
   ansible.builtin.include_tasks:
     file: tests/test_flow_deletion.yml

tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_client_flow_override_flow_modification.yml

@@ -9,48 +9,6 @@
   ansible.builtin.include_tasks:
     file: ../actions/flow_v1.yml
 
-- name: Assign the flow <Integration Test Flow> as default browser flow
-  community.general.keycloak_realm:
-    auth_keycloak_url: "{{ url }}"
-    auth_realm: "{{ admin_realm }}"
-    auth_username: "{{ admin_user }}"
-    auth_password: "{{ admin_password }}"
-    id: "{{ realm }}"
-    realm: "{{ realm }}"
-    state: present
-    browser_flow: Integration Test Flow
-
-- name: Modify the flow <Integration Test Flow> although it is bound as realm browser flow
-  ansible.builtin.include_tasks:
-    file: ../actions/flow_v2.yml
-
-- name: Assert the flow modification result
-  ansible.builtin.assert:
-    that:
-      - flow_v2_result is changed
-      - flow_v2_result.diff is defined
-    msg: "Expected changes and a diff but got: changed={{ flow_v2_result.changed }}, diff={{ flow_v2_result.get('diff') }}"
-
-- name: Assert the flow <Integration Test Flow> after the modifications
-  ansible.builtin.include_tasks:
-    file: ../assertions/assertion_flow_v2.yml
-
-- name: Fetch realm data to validate if the browser flow is still set to <Integration Test Flow>
-  community.general.keycloak_realm:
-    auth_keycloak_url: "{{ url }}"
-    auth_realm: "{{ admin_realm }}"
-    auth_username: "{{ admin_user }}"
-    auth_password: "{{ admin_password }}"
-    id: "{{ realm }}"
-    realm: "{{ realm }}"
-    state: present
-  register: realm_result
-
-- name: Assert realm browser flow binding
-  ansible.builtin.assert:
-    that:
-      - realm_result.end_state.browserFlow == "Integration Test Flow"
-
 - name: Create a client with browser override <Integration Test Flow>
   community.general.keycloak_client:
     auth_keycloak_url: "{{ url }}"
@@ -63,20 +21,20 @@
     authentication_flow_binding_overrides:
       browser_name: "Integration Test Flow"
 
-- name: Revert the flow modifications of <Integration Test Flow> although it is bound as realm browser and client override flow
+- name: Modify the flow <Integration Test Flow> although it is bound as client override flow
   ansible.builtin.include_tasks:
-    file: ../actions/flow_v1.yml
+    file: ../actions/flow_v2.yml
 
 - name: Assert the flow modification result
   ansible.builtin.assert:
     that:
-      - flow_v1_result is changed
-      - flow_v1_result.diff is defined
-    msg: "Expected changes and a diff but got: changed={{ flow_v1_result.changed }}, diff={{ flow_v1_result.get('diff') }}"
+      - flow_v2_result is changed
+      - flow_v2_result.diff is defined
+    msg: "Expected changes and a diff but got: changed={{ flow_v2_result.changed }}, diff={{ flow_v2_result.get('diff') }}"
 
-- name: Assert the flow <Integration Test Flow> after the revert
+- name: Assert the flow <Integration Test Flow> after the modifications
   ansible.builtin.include_tasks:
-    file: ../assertions/assertion_flow_v1.yml
+    file: ../assertions/assertion_flow_v2.yml
 
 - name: Fetch client data
   community.general.keycloak_client:
@@ -93,7 +51,7 @@
   ansible.builtin.include_tasks:
     file: ../actions/fetch_access_token.yml
 
-- name: Fetch authentication flow
+- name: Fetch authentication flow (browser override={{ client_result.end_state.authenticationFlowBindingOverrides.browser }})
   ansible.builtin.uri:
     url: "{{ url }}/admin/realms/{{ realm }}/authentication/flows/{{ client_result.end_state.authenticationFlowBindingOverrides.browser }}"
     method: GET

tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_identity_provider_flow_overrides_flow_modification.yml

@@ -0,0 +1,66 @@
+# Copyright (c) Ansible Project
+# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
+# SPDX-License-Identifier: GPL-3.0-or-later
+- name: Setup Test
+  ansible.builtin.include_tasks:
+    file: test_setup.yml
+
+- name: Create the new flow <Integration Test Flow>
+  ansible.builtin.include_tasks:
+    file: ../actions/flow_v1.yml
+
+- name: Create an identity-provider with first and post login flow override <Integration Test Flow>
+  community.general.keycloak_identity_provider:
+    auth_keycloak_url: "{{ url }}"
+    auth_realm: "{{ admin_realm }}"
+    auth_username: "{{ admin_user }}"
+    auth_password: "{{ admin_password }}"
+    realm: "{{ realm }}"
+    alias: oidc-idp-integration-test
+    state: present
+    display_name: OpenID Connect IdP - Integration Test
+    enabled: true
+    provider_id: oidc
+    first_broker_login_flow_alias: Integration Test Flow
+    post_broker_login_flow_alias: Integration Test Flow
+    config:
+      issuer: https://idp.example.com
+      authorizationUrl: https://idp.example.com/auth
+      tokenUrl: https://idp.example.com/token
+      userInfoUrl: https://idp.example.com/userinfo
+      clientAuthMethod: client_secret_post
+      clientId: my-client
+      clientSecret: secret
+      syncMode: FORCE
+
+- name: Modify the flow <Integration Test Flow> although it is bound as identity-provider first and post login flow
+  ansible.builtin.include_tasks:
+    file: ../actions/flow_v2.yml
+
+- name: Assert the flow modification result
+  ansible.builtin.assert:
+    that:
+      - flow_v2_result is changed
+      - flow_v2_result.diff is defined
+    msg: "Expected changes and a diff but got: changed={{ flow_v2_result.changed }}, diff={{ flow_v2_result.get('diff') }}"
+
+- name: Assert the flow <Integration Test Flow> after the modifications
+  ansible.builtin.include_tasks:
+    file: ../assertions/assertion_flow_v2.yml
+
+- name: Fetch identity-provider data
+  community.general.keycloak_identity_provider:
+    auth_keycloak_url: "{{ url }}"
+    auth_realm: "{{ admin_realm }}"
+    auth_username: "{{ admin_user }}"
+    auth_password: "{{ admin_password }}"
+    realm: "{{ realm }}"
+    alias: oidc-idp-integration-test
+    state: present
+  register: identity_provider_result
+
+- name: Assert identity-provider first and post flow binding
+  ansible.builtin.assert:
+    that:
+      - identity_provider_result.end_state.firstBrokerLoginFlowAlias == "Integration Test Flow"
+      - identity_provider_result.end_state.postBrokerLoginFlowAlias == "Integration Test Flow"

tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_used_default_browser_flow_modification.yml

@@ -0,0 +1,52 @@
+# Copyright (c) Ansible Project
+# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
+# SPDX-License-Identifier: GPL-3.0-or-later
+- name: Setup Test
+  ansible.builtin.include_tasks:
+    file: test_setup.yml
+
+- name: Create the new flow <Integration Test Flow>
+  ansible.builtin.include_tasks:
+    file: ../actions/flow_v1.yml
+
+- name: Assign the flow <Integration Test Flow> as default browser flow
+  community.general.keycloak_realm:
+    auth_keycloak_url: "{{ url }}"
+    auth_realm: "{{ admin_realm }}"
+    auth_username: "{{ admin_user }}"
+    auth_password: "{{ admin_password }}"
+    id: "{{ realm }}"
+    realm: "{{ realm }}"
+    state: present
+    browser_flow: Integration Test Flow
+
+- name: Modify the flow <Integration Test Flow> although it is bound as realm browser flow
+  ansible.builtin.include_tasks:
+    file: ../actions/flow_v2.yml
+
+- name: Assert the flow modification result
+  ansible.builtin.assert:
+    that:
+      - flow_v2_result is changed
+      - flow_v2_result.diff is defined
+    msg: "Expected changes and a diff but got: changed={{ flow_v2_result.changed }}, diff={{ flow_v2_result.get('diff') }}"
+
+- name: Assert the flow <Integration Test Flow> after the modifications
+  ansible.builtin.include_tasks:
+    file: ../assertions/assertion_flow_v2.yml
+
+- name: Fetch realm data to validate if the browser flow is still set to <Integration Test Flow>
+  community.general.keycloak_realm:
+    auth_keycloak_url: "{{ url }}"
+    auth_realm: "{{ admin_realm }}"
+    auth_username: "{{ admin_user }}"
+    auth_password: "{{ admin_password }}"
+    id: "{{ realm }}"
+    realm: "{{ realm }}"
+    state: present
+  register: realm_result
+
+- name: Assert realm browser flow binding
+  ansible.builtin.assert:
+    that:
+      - realm_result.end_state.browserFlow == "Integration Test Flow"