From f9bd658950635fac4439ded3bd322175be2639d7 Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Mon, 16 Dec 2024 14:30:19 +0100 Subject: [PATCH] keycloak_authentication: Fix priority attribute during execution updates. --- .../9263-kc_authentication-api-priority.yaml | 2 + .../identity/keycloak/keycloak.py | 48 +++++++++++++++++-- plugins/modules/keycloak_authentication.py | 10 ++-- .../modules/test_keycloak_authentication.py | 8 +++- 4 files changed, 58 insertions(+), 10 deletions(-) create mode 100644 changelogs/fragments/9263-kc_authentication-api-priority.yaml diff --git a/changelogs/fragments/9263-kc_authentication-api-priority.yaml b/changelogs/fragments/9263-kc_authentication-api-priority.yaml new file mode 100644 index 00000000000..a943e659add --- /dev/null +++ b/changelogs/fragments/9263-kc_authentication-api-priority.yaml @@ -0,0 +1,2 @@ +security_fixes: + - keycloak_authentication - API calls did not properly set the ``priority`` during update resulting in incorrectly sorted authentication flows. This apparently only affects Keycloak 25 or newer (https://github.com/ansible-collections/community.general/pull/9263). \ No newline at end of file diff --git a/plugins/module_utils/identity/keycloak/keycloak.py b/plugins/module_utils/identity/keycloak/keycloak.py index b2a1892503b..18a0198ebaf 100644 --- a/plugins/module_utils/identity/keycloak/keycloak.py +++ b/plugins/module_utils/identity/keycloak/keycloak.py @@ -91,6 +91,7 @@ URL_AUTHENTICATION_FLOW_EXECUTIONS = "{url}/admin/realms/{realm}/authentication/flows/{flowalias}/executions" URL_AUTHENTICATION_FLOW_EXECUTIONS_EXECUTION = "{url}/admin/realms/{realm}/authentication/flows/{flowalias}/executions/execution" URL_AUTHENTICATION_FLOW_EXECUTIONS_FLOW = "{url}/admin/realms/{realm}/authentication/flows/{flowalias}/executions/flow" +URL_AUTHENTICATION_EXECUTION = "{url}/admin/realms/{realm}/authentication/executions/{id}" URL_AUTHENTICATION_EXECUTION_CONFIG = "{url}/admin/realms/{realm}/authentication/executions/{id}/config" URL_AUTHENTICATION_EXECUTION_RAISE_PRIORITY = "{url}/admin/realms/{realm}/authentication/executions/{id}/raise-priority" URL_AUTHENTICATION_EXECUTION_LOWER_PRIORITY = "{url}/admin/realms/{realm}/authentication/executions/{id}/lower-priority" @@ -2185,14 +2186,14 @@ def create_subflow(self, subflowName, flowAlias, realm='master', flowType='basic :param subflowName: name of the subflow to create :param flowAlias: name of the parent flow - :return: HTTPResponse object on success + :return: ID of the generated subflow """ try: newSubFlow = {} newSubFlow["alias"] = subflowName newSubFlow["provider"] = "registration-page-form" newSubFlow["type"] = flowType - open_url( + response = open_url( URL_AUTHENTICATION_FLOW_EXECUTIONS_FLOW.format( url=self.baseurl, realm=realm, @@ -2202,21 +2203,59 @@ def create_subflow(self, subflowName, flowAlias, realm='master', flowType='basic data=json.dumps(newSubFlow), timeout=self.connection_timeout, validate_certs=self.validate_certs) + return response.headers["Location"].rsplit("/", 1)[-1] except Exception as e: self.fail_open_url(e, msg="Unable to create new subflow %s: %s" % (subflowName, str(e))) + def get_execution(self, executionId, flowAlias, realm='master'): + """ Get a representation of the specified execution + + :param executionId: id of execution to query + :param flowAlias: name of the parent flow + :return: Representation of the execution + """ + try: + execution = json.load( + open_url( + URL_AUTHENTICATION_EXECUTION.format( + url=self.baseurl, + realm=realm, + id=executionId), + method='GET', + http_agent=self.http_agent, headers=self.restheaders, + timeout=self.connection_timeout, + validate_certs=self.validate_certs) + ) + if "authenticationConfig" in execution: + execConfigId = execution["authenticationConfig"] + execConfig = json.load( + open_url( + URL_AUTHENTICATION_CONFIG.format( + url=self.baseurl, + realm=realm, + id=execConfigId), + method='GET', + http_agent=self.http_agent, headers=self.restheaders, + timeout=self.connection_timeout, + validate_certs=self.validate_certs)) + execution["authenticationConfig"] = execConfig + return execution + except Exception as e: + self.fail_open_url(e, msg='Could not get execution %s for authentication flow %s in realm %s: %s' + % (executionId, flowAlias, realm, str(e))) + def create_execution(self, execution, flowAlias, realm='master'): """ Create new execution on the flow :param execution: name of execution to create :param flowAlias: name of the parent flow - :return: HTTPResponse object on success + :return: ID of the generated execution """ try: newExec = {} newExec["provider"] = execution["providerId"] newExec["requirement"] = execution["requirement"] - open_url( + response = open_url( URL_AUTHENTICATION_FLOW_EXECUTIONS_EXECUTION.format( url=self.baseurl, realm=realm, @@ -2226,6 +2265,7 @@ def create_execution(self, execution, flowAlias, realm='master'): data=json.dumps(newExec), timeout=self.connection_timeout, validate_certs=self.validate_certs) + return response.headers["Location"].rsplit("/", 1)[-1] except HTTPError as e: self.fail_open_url(e, msg="Unable to create new execution '%s' %s: %s: %s %s" % (flowAlias, execution["providerId"], repr(e), ";".join([e.url, e.msg, str(e.code), str(e.hdrs)]), str(newExec))) diff --git a/plugins/modules/keycloak_authentication.py b/plugins/modules/keycloak_authentication.py index bc2898d9be8..d922fbdf497 100644 --- a/plugins/modules/keycloak_authentication.py +++ b/plugins/modules/keycloak_authentication.py @@ -287,16 +287,14 @@ def create_or_update_executions(kc, config, realm='master'): # 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) + id_to_update = kc.create_execution(new_exec, flowAlias=flow_alias_parent, realm=realm) exec_found = True exec_index = new_exec_index - id_to_update = kc.get_executions_representation(config, realm=realm)[exec_index]["id"] after += str(new_exec) + '\n' elif new_exec["displayName"] is not None: - kc.create_subflow(new_exec["displayName"], flow_alias_parent, realm=realm, flowType=new_exec["subFlowType"]) + id_to_update = kc.create_subflow(new_exec["displayName"], flow_alias_parent, realm=realm, flowType=new_exec["subFlowType"]) exec_found = True exec_index = new_exec_index - id_to_update = kc.get_executions_representation(config, realm=realm)[exec_index]["id"] after += str(new_exec) + '\n' if exec_found: changed = True @@ -313,6 +311,10 @@ def create_or_update_executions(kc, config, realm='master'): if key not in ("flowAlias", "authenticationConfig", "subFlowType"): updated_exec[key] = new_exec[key] if new_exec["requirement"] is not None: + # Load the existing execution so we can set the priority (API requires priority since keycloak 25) + execution = kc.get_execution(id_to_update, flow_alias_parent, realm=realm) + if "priority" in execution: + updated_exec["priority"] = execution["priority"] kc.update_authentication_executions(flow_alias_parent, updated_exec, realm=realm) diff = exec_index - new_exec_index kc.change_execution_priority(updated_exec["id"], diff, realm=realm) diff --git a/tests/unit/plugins/modules/test_keycloak_authentication.py b/tests/unit/plugins/modules/test_keycloak_authentication.py index aaa1fa9b1b1..d0306b3c537 100644 --- a/tests/unit/plugins/modules/test_keycloak_authentication.py +++ b/tests/unit/plugins/modules/test_keycloak_authentication.py @@ -37,6 +37,9 @@ def patch_keycloak_api(get_authentication_flow_by_alias=None, copy_auth_flow=Non ... """ + def default_get_execution(*args, **kwargs): + return {"priority": 0} + obj = keycloak_authentication.KeycloakAPI with patch.object(obj, 'get_authentication_flow_by_alias', side_effect=get_authentication_flow_by_alias) \ as mock_get_authentication_flow_by_alias: @@ -48,8 +51,9 @@ def patch_keycloak_api(get_authentication_flow_by_alias=None, copy_auth_flow=Non as mock_get_executions_representation: with patch.object(obj, 'delete_authentication_flow_by_id', side_effect=delete_authentication_flow_by_id) \ as mock_delete_authentication_flow_by_id: - yield mock_get_authentication_flow_by_alias, mock_copy_auth_flow, mock_create_empty_auth_flow, \ - mock_get_executions_representation, mock_delete_authentication_flow_by_id + with patch.object(obj, 'get_execution', side_effect=default_get_execution): + yield mock_get_authentication_flow_by_alias, mock_copy_auth_flow, mock_create_empty_auth_flow, \ + mock_get_executions_representation, mock_delete_authentication_flow_by_id def get_response(object_with_future_response, method, get_id_call_count):