Add client support to workflows #44209

Hathoute · 2025-11-14T01:08:40Z

Hathoute · 2025-11-14T01:09:50Z

model/jpa/src/main/java/org/keycloak/models/workflow/DefaultWorkflowProvider.java

        List<WorkflowStepRepresentation> steps = ofNullable(rep.getSteps()).orElse(List.of());

        if (steps.isEmpty()) {
+            // Do we really want to allow workflows with no steps?


Does it make sense to support workflows with no steps? Why not throw a validation error?

We should probably reject this, doesn't make any sense IMO.

Hathoute · 2025-11-14T01:10:58Z

server-spi-private/src/main/java/org/keycloak/models/workflow/Workflow.java

+        // - Should we allow workflows to have more than one supported type ?
+        // - allowedTypes should probably be saved in config, since we do not allow changing steps,
+        // so that we don't recompute it.
+        if (allowedTypes.isEmpty()) {


Could we discuss the comments above? What do you think?

I think we need to persist the resource type of the workflow, and enforce that all conditions and steps operate on the same type.

So if we persist this as part of the workflow, every step needs to support the same type (including the restart step that supports all types and can be used in any workflow).

Hathoute · 2025-11-18T09:10:36Z

Hey @sguilhen , I've rebased the branch on main. Would you please have a look at the comments I've made above?

sguilhen · 2025-11-18T17:53:41Z

Hey @Hathoute, Pedro and I will try to take a look this week - we're wrapping up a sprint, but we plan on checking this right after that. Thanks a lot for the work you've put into it, will get back to you asap.

Signed-off-by: Hathoute <[email protected]>

sguilhen · 2025-11-25T19:45:32Z

services/src/main/java/org/keycloak/models/workflow/client/DisableClientStepProvider.java

+        ClientModel client = session.clients().getClientById(realm, context.getResourceId());
+
+        if (client != null && client.isEnabled()) {
+            log.debugv("Disabling user {0} ({1})", client.getName(), client.getId());


Should be "Disabling client" instead of "Disabling user".

sguilhen · 2025-11-25T19:55:25Z

model/jpa/src/main/java/org/keycloak/models/workflow/JpaWorkflowStateProvider.java

        } else {
            entity.setScheduledStepId(step.getId());
-            entity.setScheduledStepTimestamp(Instant.now().plus(duration).toEpochMilli());
+            entity.setScheduledStepTimestamp(Time.currentTimeMillis() + duration.toMillis());


Just curious - any particular reason for this change?

I've seen that Workflows use the Time utils, so I just replaced it here.

sguilhen · 2025-11-25T19:55:43Z

quarkus/tests/junit5/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java

-        if (debug) {
+        if (true) {
            builder.environment().put("DEBUG_SUSPEND", "y");
+            builder.environment().put("DEBUG_PORT", "5005");


The changes here need probably to be reverted.

Correct, I forgot to exclude this change

sguilhen · 2025-11-25T20:27:34Z

server-spi-private/src/main/java/org/keycloak/models/workflow/ResourceType.java

+            org.keycloak.events.admin.ResourceType.CLIENT,
+            List.of(OperationType.CREATE),
+            List.of(EventType.CLIENT_LOGIN, EventType.CLIENT_REGISTER),
+            (session, id) -> session.users().getUserById(session.getContext().getRealm(), id)


This function is used in the ResourceTypeSelector.resolveResource impl that you moved to the AbstractResourceTypeWorkflowProvider, and it needs to resolve to the actual resource - in this case it should be client, not user.

sguilhen · 2025-11-25T20:32:41Z

server-spi-private/src/main/java/org/keycloak/models/workflow/ResourceOperationType.java

+    USER_ROLE_REMOVED(ResourceType.USERS, List.of(RoleModel.RoleRevokedEvent.class), roleMembershipPredicate()),
+
+    CLIENT_ADDED(ResourceType.CLIENTS, List.of(OperationType.CREATE, ClientCreationEvent.class)),
+    CLIENT_LOGGED_IN(ResourceType.CLIENTS, List.of(EventType.CLIENT_LOGIN)),


I wonder about this - the CLIENT_LOGIN event is, if I recall correctly, only issued in the Client Credentials Grant scenario. However, when trying to identify whether a client is not being used, we should probably also check the user LOGIN event and get the client from the that event as well. In other words, do we want to track only explicit client logins, or do we want to track if the client is being used as part of a user login?

@pedroigor What do you think?

You're right, it would be bad to disable a client used for authentication flow. Maybe have a dedicated ResourceOperationType ?

tbh I haven't reached a good conclusion yet on how to best handle this ;-)

We could perhaps make CLIENT_LOGGED_IN also accept the regular LOGIN event type and get the resourceId using that event's clientId. In this case, a single LOGIN event would result in two distinct workflow events - USER_LOGGED_IN and CLIENT_LOGGED_IN. But it mixes CLIENT_LOGIN with LOGIN and I'm not sure this is wanted all the time. Perhaps it is if the intention is to disable clients not used for any authentication in a period of time.

If we mix the two events, then we should perhaps have some conditions in place to allow admins to differentiate the clients that should be affected by the workflow. For example, you might want a particular policy for service accounts (AFAIK they only trigger CLIENT_LOGIN events, not regular LOGIN events), and a different one for the other clients involved in user authentication flows (would trigger only regular LOGIN events), but have them both activated on CLIENT_LOGGED_IN. What do you think?

@vramik @pedroigor Any opinions on this?

Would it help to introduce a separate event USER_CLIENT_LOGGED_IN? Reading it as the client involved during a user logged in event.

Hathoute · 2025-11-26T12:26:04Z

@sguilhen I actually have another WIP branch in fork where I have implemented what we discussed previously. I'll push everything here (with your latest comments). Do you prefer it being in a new commit or squashed into one?

sguilhen · 2025-11-26T13:04:15Z

@sguilhen I actually have another WIP branch in fork where I have implemented what we discussed previously. I'll push everything here (with your latest comments). Do you prefer it being in a new commit or squashed into one?

I think having separate commits is better to review. We can always squash when merging.

Signed-off-by: Hathoute <[email protected]>

Signed-off-by: Hathoute <[email protected]> Signed-off-by: Hamza HATHOUTE <[email protected]>

Hathoute · 2025-11-27T12:10:43Z

server-spi-private/src/main/java/org/keycloak/models/workflow/Workflow.java

+            throw new ModelValidationException("Steps provided should support a single type, actual: " + formattedTypes);
+        }
+
+        ResourceType supported = allowedTypes.stream().findFirst().orElseThrow();


TBH I am not a fan of detecting the resource type of the workflow this way. I'd rather let the user choose it in a dropdown and then visually display the conditions/steps that are compatible (thinking about a create workflow UI)

@sguilhen what do you think?

Hathoute requested a review from a team as a code owner November 14, 2025 01:08

Hathoute commented Nov 14, 2025

View reviewed changes

Hathoute marked this pull request as draft November 14, 2025 01:13

Hathoute marked this pull request as ready for review November 16, 2025 23:11

Hathoute force-pushed the 43870-disable-dormant-clients branch 4 times, most recently from 4ca9edc to 9647ee2 Compare November 18, 2025 07:17

Hathoute requested a review from a team as a code owner November 18, 2025 07:17

keycloak-github-bot bot added the team/cloud-native label Nov 18, 2025

Hathoute force-pushed the 43870-disable-dormant-clients branch from 9647ee2 to 5fc0824 Compare November 18, 2025 09:09

feat: support client resources in workflows

282dd93

Signed-off-by: Hathoute <[email protected]>

sguilhen suggested changes Nov 25, 2025

View reviewed changes

sguilhen requested a review from pedroigor November 25, 2025 20:43

feat: ensure conditions are compatible with workflow type

ab3ff64

Signed-off-by: Hathoute <[email protected]>

Hathoute force-pushed the 43870-disable-dormant-clients branch from 5fc0824 to 3ed45c9 Compare November 27, 2025 00:59

fix: review fixes

5b14728

Signed-off-by: Hathoute <[email protected]> Signed-off-by: Hamza HATHOUTE <[email protected]>

Hathoute force-pushed the 43870-disable-dormant-clients branch from 3ed45c9 to 5b14728 Compare November 27, 2025 08:25

Hathoute commented Nov 27, 2025

View reviewed changes

Add client support to workflows #44209

Are you sure you want to change the base?

Add client support to workflows #44209

Uh oh!

Conversation

Hathoute commented Nov 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hathoute Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hathoute commented Nov 18, 2025

Uh oh!

sguilhen commented Nov 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pedroigor Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hathoute commented Nov 26, 2025

Uh oh!

sguilhen commented Nov 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Hathoute Nov 14, 2025 •

edited

Loading

pedroigor Nov 26, 2025 •

edited

Loading