From e7a253b4c9dfc733623473758e8527bbb3cd4cfc Mon Sep 17 00:00:00 2001 From: thomasbargetz Date: Wed, 18 Mar 2026 06:49:24 +0100 Subject: [PATCH] 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 --- plugins/modules/keycloak_authentication_v2.py | 34 ++++++++-- .../keycloak_authentication_v2/tasks/main.yml | 18 +++-- ...lient_flow_override_flow_modification.yml} | 58 +++------------- ...vider_flow_overrides_flow_modification.yml | 66 +++++++++++++++++++ ...used_default_browser_flow_modification.yml | 52 +++++++++++++++ 5 files changed, 167 insertions(+), 61 deletions(-) rename tests/integration/targets/keycloak_authentication_v2/tasks/tests/{test_used_flow_modification.yml => test_client_flow_override_flow_modification.yml} (59%) create mode 100644 tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_identity_provider_flow_overrides_flow_modification.yml create mode 100644 tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_used_default_browser_flow_modification.yml diff --git a/plugins/modules/keycloak_authentication_v2.py b/plugins/modules/keycloak_authentication_v2.py index 79aaedad01..a51a6000e8 100644 --- a/plugins/modules/keycloak_authentication_v2.py +++ b/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, diff --git a/tests/integration/targets/keycloak_authentication_v2/tasks/main.yml b/tests/integration/targets/keycloak_authentication_v2/tasks/main.yml index ed4b01ff3b..bfadb829e1 100644 --- a/tests/integration/targets/keycloak_authentication_v2/tasks/main.yml +++ b/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 \ No newline at end of file diff --git a/tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_used_flow_modification.yml b/tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_client_flow_override_flow_modification.yml similarity index 59% rename from tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_used_flow_modification.yml rename to tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_client_flow_override_flow_modification.yml index 1bd0d916e8..a44895e881 100644 --- a/tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_used_flow_modification.yml +++ b/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 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 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 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 - 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 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 although it is bound as realm browser and client override flow +- name: Modify the 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 after the revert +- name: Assert the 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 diff --git a/tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_identity_provider_flow_overrides_flow_modification.yml b/tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_identity_provider_flow_overrides_flow_modification.yml new file mode 100644 index 0000000000..9fcf875bf7 --- /dev/null +++ b/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 + ansible.builtin.include_tasks: + file: ../actions/flow_v1.yml + +- name: Create an identity-provider with first and post login flow override + 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 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 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" \ No newline at end of file diff --git a/tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_used_default_browser_flow_modification.yml b/tests/integration/targets/keycloak_authentication_v2/tasks/tests/test_used_default_browser_flow_modification.yml new file mode 100644 index 0000000000..c09750cc93 --- /dev/null +++ b/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 + ansible.builtin.include_tasks: + file: ../actions/flow_v1.yml + +- name: Assign the 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 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 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 + 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" \ No newline at end of file