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

Allow for defining Flow priorities declaratively #296

Open
svenstaro opened this issue May 23, 2020 · 7 comments · May be fixed by #970
Open

Allow for defining Flow priorities declaratively #296

svenstaro opened this issue May 23, 2020 · 7 comments · May be fixed by #970

Comments

@svenstaro
Copy link

Hey, great plugin. We use in Arch Linux for basically all of our SSO infra.

However, we ran into the limitation that we can't declaratively define the priorities of executions and subflows inside of a flow. This is kind of annoying. I know that you can hack this using depends_on but frankly that's not really the Terraform spirit. It would be great if you could add some logic to allow for declaratively defining the Flow priorities.

@tomrutsaert
Copy link
Contributor

Hey,

I agree it is annoying and I agree using depends_on is hack and against the terraform spirit.
The issue is:
Keycloak admin API seems to be build with 1 purpose and that is to support the admin ui.
So the Keycloak admin API sets the priority in the order you are creating the subflows/authenticators.
There is no field for priority during creation.
When changing the order/priority for an existing flow there is only a lowerPriority and raisePriority admin API endpoint. So again you can not directly set the priority of a sublow/authenticator.

You can make a solution that works with nested objects and that fetches the whole flow and compares everything and checks if everything is where it should be.
But I encountered some other problems in doing that (I can not recall exactly what comment in pr 138)
So eventually I made the point that the depends_on solution is good enough solution, especially if you take into account that the flow/authenticator mechanism configuration may get a complete overhaul in Keycloak X

Another solution would be to make PR towards the Keycloak project to allow to set the priority of subflow/authenticator directly. (Internally Keycloak is using an integer to sort the subflows/authenticators if i am not mistaken, so it is really a pity)

As you already know: there is more info about this in the comments of PR #138

@svenstaro
Copy link
Author

Thank you for the detailed explanation. That's also what I figured. What I'm thinking is: you can still kind of acceptably pretend to allow for absolute ordering by allowing the user to input absolute number, sorting the items of the same flow level by those numbers and then issuing the API calls in order. You could even allow for reordering using that method. Granted, it would take a little longer as you'd be issuing manual relative requests (which really sucks from keycloak tbh) but I think it would be workable.

@phisch1991
Copy link

I noticed that the authentication executions have a priority attribute, at least according to the API documentation (https://www.keycloak.org/docs-api/15.0/rest-api/index.html#_authenticationexecutionexportrepresentation). Would this help to fix the issue? We would also appreciate to use the priority within our authentication flows.

@tomrutsaert
Copy link
Contributor

AFAIK the priority has always been there. Furthermore keycloak uses it internally to order them and you get the priority returned when getting a flow. But you can not set it via POST or PUT. (I think it is stripped out of the input) You can only influence the priority by calling raise or lower priority. The admin API is only build for supporting the admin UI, and I think that is the main culprit
It has been a while that I looked at it, but AFAIK this has not changed.

@oysteinhauan
Copy link
Contributor

This is super annoying. We are heavily reliant on custom auth flows that frequently change due to different requirements.
We have made a workaround for this by always checking the plan command (outputted as json) for any changes in the auth flows.
If we spot any new executions or subflows, all resources containing the relevant parent_flow_alias will be replaced. This is all done automatically in a pipeline. PM me if you are in the same situation and need some examples.

It's a hack of course, but with so many realms, we have no choice. The order in the modules must match the order in Keycloak.

@kherock
Copy link
Contributor

kherock commented Dec 20, 2023

With Terraform 1.2 and the replace_triggered_by lifecycle argument, this now has a slightly improved workaround:

-  depends_on = [
-    keycloak_authentication_execution.browser_sso_cookie
-  ]
+ lifecycle {
+    replace_triggered_by = [
+      keycloak_authentication_subflow.browser_sso_cookie
+    ]
+  }

This will instruct Terraform to re-create a resource after one of the dependent executions are re-created.

If this provider exposed the underlying priority attribute tracked by Keycloak, this would also allow Terraform to detect changes in priority and re-create them in the correct order. e.g.

 lifecycle {
    replace_triggered_by = [
      keycloak_authentication_subflow.browser_sso_cookie.priority
    ]
  }

@gim-
Copy link

gim- commented May 30, 2024

Good news everyone, keycloak/keycloak#27751 got finally merged 🎉 There are no blockers to get this implemented now for Keycloak 25 (when it gets released ofc) and up.

gim- added a commit to gim-/terraform-provider-keycloak that referenced this issue Jun 1, 2024
gim- added a commit to gim-/terraform-provider-keycloak that referenced this issue Jun 1, 2024
gim- added a commit to gim-/terraform-provider-keycloak that referenced this issue Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants