Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keycloak admin cli creating/updating authention executions not respecting the priority value specified #20747

Closed
2 tasks done
tianqiwang opened this issue Jun 2, 2023 · 6 comments · Fixed by #27751
Closed
2 tasks done

Comments

@tianqiwang
Copy link

Before reporting an issue

  • I have searched existing issues
  • I have reproduced the issue with the latest release

Area

admin/cli

Describe the bug

When using the Keycloak admin-cli to create/update authentication flows, it appears that the priority value is not being honored. When running kcadm.sh update authentication/flows/{authenticationFlowID}/executions -f executionConfig.json (where executionConfig.json is a json file that contains the properties of the execution, including its priority), the other properties of the authentication execution do get properly updated, but the priority value does not get updated.

Version

19.0.3

Expected behavior

Running kcadm.sh update authentication/flows/{authenticationFlowID}/executions -f executionConfig.json for different executions under the same parent flow will update the executions and order them properly based on the priority specified in the json file.

Actual behavior

Running kcadm.sh update authentication/flows/{authenticationFlowID}/executions -f executionConfig.json for different executions under the same parent flow keeps the priority in the order in which they were originally created.

How to Reproduce?

Create an authentication flow with 2 authenticationExecutions as its children.

Create configuration json files for each of the executions with "priority": 1 for the authenticationExecution that was created first and "priority": 0 for the authenticationExecution that was created second.

Run kcadm.sh update authentication/flows/{authenticationFlowID}/executions -f executionConfig.json for both execution config files.

Verify the order of the executions using the Keycloak admin console.

Anything else?

No response

@tianqiwang tianqiwang added kind/bug Categorizes a PR related to a bug status/triage labels Jun 2, 2023
@andre-nascimento6791 andre-nascimento6791 added this to the Backlog milestone Jun 6, 2023
@andre-nascimento6791 andre-nascimento6791 self-assigned this Jun 6, 2023
@andre-nascimento6791
Copy link
Contributor

andre-nascimento6791 commented Jun 6, 2023

Thank you for the report, @tianqiwang !

Please, could you add here a sample content of the executionConfig.json file aforementioned?

I did a quick check on the Endpoint used above (authentication/flows/{authenticationFlowID}/executions), in version 21.1.1 Admin REST API docs, and noticed that on the expected payload object of type AuthenticationExecutionInfoRepresentation.java there is no priority property on it.

So, In my view, further investigation is needed to ensure how the order of priority between Authentication Flow Execution steps is handled when updating via that Endpoint.

Besides that, I'd like to ask @keycloak/core Team: Any ideas about this?

Thanks in advance.

@mposolda
Copy link
Contributor

@andre-nascimento6791 I think your analysis is correct.
I can add that there are separate admin REST API endpoints for increase/decrease priority - they are used for instance when you use admin console and update the executions order via an UI. So it can be better to use those endpoints. Considering that, I would say that this issue is really very low priority and I am not sure if we even need to fix it in the codebase. Maybe better is to document the particular REST endpoint authentication/flows/{authenticationFlowID}/executions that priority is not affected when using it.

@leventyalcin
Copy link

leventyalcin commented Nov 21, 2023

@mposolda I do understand there is increase and decrease endpoints on the API. However, I still believe that /admin/realms/{realm}/authentication/flows/{flowAlias}/executions endpoint should accept and respect priority.

For the exact same reason [1], Terraform provider for Keycloak can't also manage executions too. The only thing they can provide [2] currently is using depends_on for creating those in particular order. Although you can create those in an order, problem doesn't end there. If anyone accidentally drags and drops order without noticing it or on purpose for example, Terraform can't understand the change in that and the IaC loses its meaning.

If that has to be done by ...../lower-priority and ......./raise-priority endpoint, it would get nasty.

@keycloak-github-bot
Copy link

Due to the amount of issues reported by the community we are not able to prioritise resolving this issue at the moment.

If you are affected by this issue, upvote it by adding a 👍 to the description. We would also welcome a contribution to fix the issue.

@dasniko
Copy link
Contributor

dasniko commented May 17, 2024

Hi,
I stumbled upon this issue as I have the exact same issue and would more than welcome the consideration of the priority value given through the payload!

Currently, since the creation of this issue, there is also a linked PR (#27751) which fixes this. This PR has also tests, which makes this a very valuable and "complete" PR, IMHO. 👍👍👍

So, what else is needed to drive this issue and get the PR merged?
How can we gain ground?

@b509
Copy link

b509 commented May 21, 2024

Agreed, this is a small change but brings the API a further step towards being able to configure keycloak straight-foward using config-as-code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants