From cd350082f78b595027e4f3b3444bafb1fa728ae6 Mon Sep 17 00:00:00 2001 From: Stefan Guilhen Date: Fri, 21 Nov 2025 23:26:38 -0300 Subject: [PATCH] Ensure workflow is only restarted on events that match the activation condition Closes #44399 Signed-off-by: Stefan Guilhen --- .../workflow/DefaultWorkflowProvider.java | 2 +- .../models/workflow/EventBasedWorkflow.java | 4 +- .../UserSessionRefreshTimeWorkflowTest.java | 88 +++++++++++++++++++ 3 files changed, 91 insertions(+), 3 deletions(-) diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/DefaultWorkflowProvider.java b/model/jpa/src/main/java/org/keycloak/models/workflow/DefaultWorkflowProvider.java index 76b6ab7db75..b6bafd67c82 100644 --- a/model/jpa/src/main/java/org/keycloak/models/workflow/DefaultWorkflowProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/DefaultWorkflowProvider.java @@ -261,7 +261,7 @@ public class DefaultWorkflowProvider implements WorkflowProvider { // workflow is active for the resource, check if the provider wants to reset or deactivate it based on the event String executionId = scheduledStep.executionId(); String resourceId = scheduledStep.resourceId(); - if (provider.reset(context)) { + if (provider.restart(context)) { new DefaultWorkflowExecutionContext(session, workflow, event, scheduledStep).restart(); } else if (provider.deactivate(context)) { log.debugf("Workflow '%s' cancelled for resource %s (execution id: %s)", workflow.getName(), resourceId, executionId); diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/EventBasedWorkflow.java b/model/jpa/src/main/java/org/keycloak/models/workflow/EventBasedWorkflow.java index 6eaa4714113..cf26ac59c35 100644 --- a/model/jpa/src/main/java/org/keycloak/models/workflow/EventBasedWorkflow.java +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/EventBasedWorkflow.java @@ -47,12 +47,12 @@ final class EventBasedWorkflow { return false; } - boolean reset(WorkflowExecutionContext executionContext) throws WorkflowInvalidStateException { + boolean restart(WorkflowExecutionContext executionContext) throws WorkflowInvalidStateException { WorkflowEvent event = executionContext.getEvent(); if (event == null) { return false; } - return supports(event.getResourceType()) && isCancelIfRunning() && validateResourceConditions(executionContext); + return isCancelIfRunning() && activate(executionContext); } public boolean validateResourceConditions(WorkflowExecutionContext context) { diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/UserSessionRefreshTimeWorkflowTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/UserSessionRefreshTimeWorkflowTest.java index 4ba599229b5..aebdee49732 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/UserSessionRefreshTimeWorkflowTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/UserSessionRefreshTimeWorkflowTest.java @@ -17,15 +17,19 @@ package org.keycloak.tests.admin.model.workflow; +import java.io.IOException; import java.time.Duration; import java.util.List; import jakarta.mail.internet.MimeMessage; +import jakarta.ws.rs.core.Response; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.workflow.DisableUserStepProviderFactory; import org.keycloak.models.workflow.NotifyUserStepProviderFactory; +import org.keycloak.models.workflow.SetUserAttributeStepProviderFactory; +import org.keycloak.models.workflow.WorkflowStateProvider; import org.keycloak.representations.workflows.WorkflowRepresentation; import org.keycloak.representations.workflows.WorkflowStepRepresentation; import org.keycloak.testframework.annotations.InjectUser; @@ -33,9 +37,11 @@ import org.keycloak.testframework.annotations.KeycloakIntegrationTest; import org.keycloak.testframework.injection.LifeCycle; import org.keycloak.testframework.mail.MailServer; import org.keycloak.testframework.mail.annotations.InjectMailServer; +import org.keycloak.testframework.realm.GroupConfigBuilder; import org.keycloak.testframework.realm.ManagedUser; import org.keycloak.testframework.realm.UserConfig; import org.keycloak.testframework.realm.UserConfigBuilder; +import org.keycloak.testframework.util.ApiUtil; import org.junit.jupiter.api.Test; @@ -45,6 +51,10 @@ import static org.keycloak.tests.admin.model.workflow.WorkflowManagementTest.fin import static org.keycloak.tests.admin.model.workflow.WorkflowManagementTest.findEmailsByRecipient; import static org.keycloak.tests.admin.model.workflow.WorkflowManagementTest.verifyEmailContent; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -59,6 +69,84 @@ public class UserSessionRefreshTimeWorkflowTest extends AbstractWorkflowTest { @InjectMailServer private MailServer mailServer; + @Test + public void testWorkflowIsRestartedOnSameEvent() throws IOException { + // create a workflow that can restarted on the same event - i.e. has concurrency setting to cancel if running + managedRealm.admin().workflows().create(WorkflowRepresentation.withName("myworkflow") + .onEvent(USER_LOGGED_IN.toString()) + .concurrency().cancelIfRunning() // this setting enables restarting the workflow + .withSteps( + WorkflowStepRepresentation.create() + .of(SetUserAttributeStepProviderFactory.ID) + .withConfig("attribute", "attr1") + .after(Duration.ofDays(1)) + .build(), + WorkflowStepRepresentation.create().of(DisableUserStepProviderFactory.ID) + .after(Duration.ofDays(5)) + .build() + ).build()).close(); + + // login with alice - this will attach the workflow to the user and schedule the first step + oauth.openLoginForm(); + String userId = userAlice.getId(); + String username = userAlice.getUsername(); + loginPage.fillLogin(username, userAlice.getPassword()); + loginPage.submit(); + assertTrue(driver.getPageSource() != null && driver.getPageSource().contains("Happy days")); + + // store the first step id for later comparison + String firstStepId = runOnServer.fetch(session-> { + WorkflowStateProvider provider = session.getProvider(WorkflowStateProvider.class); + List< WorkflowStateProvider.ScheduledStep> steps = provider.getScheduledStepsByResource(userId); + assertThat(steps, hasSize(1)); + return steps.get(0).stepId(); + }, String.class); + + // run the first schedule task - workflow should now be waiting to run the second step + runScheduledSteps(Duration.ofDays(2)); + String secondStepId = runOnServer.fetch(session -> { + RealmModel realm = session.getContext().getRealm(); + UserModel user = session.users().getUserByUsername(realm, username); + // first step should have run and the attribute should be set + assertThat(user.getFirstAttribute("attribute"), is("attr1")); + assertTrue(user.isEnabled()); + + WorkflowStateProvider provider = session.getProvider(WorkflowStateProvider.class); + List< WorkflowStateProvider.ScheduledStep> steps = provider.getScheduledStepsByResource(userId); + assertThat(steps, hasSize(1)); + return steps.get(0).stepId(); + }, String.class); + assertThat(secondStepId, is(not(firstStepId))); + + String groupId; + // trigger an unrelated event - like user joining a group. The workflow must not be restarted + try (Response response = managedRealm.admin().groups().add(GroupConfigBuilder.create() + .name("generic-group").build())) { + groupId = ApiUtil.getCreatedId(response); + } + managedRealm.admin().users().get(userAlice.getId()).joinGroup(groupId); + + runOnServer.run(session -> { + WorkflowStateProvider provider = session.getProvider(WorkflowStateProvider.class); + List< WorkflowStateProvider.ScheduledStep> steps = provider.getScheduledStepsByResource(userId); + // step id must remain the same as before + assertThat(steps, hasSize(1)); + assertThat(steps.get(0).stepId(), is(secondStepId)); + }); + + // now trigger the same event again that can restart the workflow + oauth.openLoginForm(); + + // workflow should be restarted and the first step should be scheduled again + runOnServer.run(session -> { + WorkflowStateProvider provider = session.getProvider(WorkflowStateProvider.class); + List< WorkflowStateProvider.ScheduledStep> steps = provider.getScheduledStepsByResource(userId); + // step id must be the first one now as the workflow was restarted + assertThat(steps, hasSize(1)); + assertThat(steps.get(0).stepId(), is(firstStepId)); + }); + } + @Test public void testDisabledUserAfterInactivityPeriod() { managedRealm.admin().workflows().create(WorkflowRepresentation.withName("myworkflow")