Skip to content

Commit

Permalink
keycloak_authentication: Fix priority attribute during execution upda…
Browse files Browse the repository at this point in the history
…tes.
  • Loading branch information
apollo13 committed Dec 16, 2024
1 parent 7dd7cbd commit f9bd658
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 10 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/9263-kc_authentication-api-priority.yaml
Original file line number Diff line number Diff line change
@@ -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).
48 changes: 44 additions & 4 deletions plugins/module_utils/identity/keycloak/keycloak.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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)))
Expand Down
10 changes: 6 additions & 4 deletions plugins/modules/keycloak_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions tests/unit/plugins/modules/test_keycloak_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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):
Expand Down

0 comments on commit f9bd658

Please sign in to comment.