diff --git a/core/src/main/java/org/keycloak/representations/workflows/WorkflowConstants.java b/core/src/main/java/org/keycloak/representations/workflows/WorkflowConstants.java index 2410e32b53a..589697d75f8 100644 --- a/core/src/main/java/org/keycloak/representations/workflows/WorkflowConstants.java +++ b/core/src/main/java/org/keycloak/representations/workflows/WorkflowConstants.java @@ -11,8 +11,6 @@ public final class WorkflowConstants { public static final String CONFIG_ON_EVENT = "on"; public static final String CONFIG_RESET_ON = "reset-on"; public static final String CONFIG_NAME = "name"; - public static final String CONFIG_RECURRING = "recurring"; - public static final String CONFIG_SCHEDULED = "scheduled"; public static final String CONFIG_ENABLED = "enabled"; public static final String CONFIG_CONDITIONS = "conditions"; public static final String CONFIG_STEPS = "steps"; diff --git a/core/src/main/java/org/keycloak/representations/workflows/WorkflowRepresentation.java b/core/src/main/java/org/keycloak/representations/workflows/WorkflowRepresentation.java index 71a7664a01e..1ae701346ec 100644 --- a/core/src/main/java/org/keycloak/representations/workflows/WorkflowRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/workflows/WorkflowRepresentation.java @@ -5,7 +5,6 @@ import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_EN import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_IF; import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_NAME; import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_ON_EVENT; -import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_RECURRING; import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_RESET_ON; import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_STATE; import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_STEPS; @@ -28,7 +27,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonPropertyOrder; import org.keycloak.common.util.MultivaluedHashMap; -@JsonPropertyOrder({"id", CONFIG_NAME, CONFIG_USES, CONFIG_ENABLED, CONFIG_ON_EVENT, CONFIG_RESET_ON, CONFIG_RECURRING, CONFIG_IF, CONFIG_STEPS, CONFIG_STATE}) +@JsonPropertyOrder({"id", CONFIG_NAME, CONFIG_USES, CONFIG_ENABLED, CONFIG_ON_EVENT, CONFIG_RESET_ON, CONFIG_IF, CONFIG_STEPS, CONFIG_STATE}) @JsonIgnoreProperties(CONFIG_WITH) public final class WorkflowRepresentation extends AbstractWorkflowComponentRepresentation { @@ -99,14 +98,6 @@ public final class WorkflowRepresentation extends AbstractWorkflowComponentRepre setConfigValue(CONFIG_NAME, name); } - public Boolean getRecurring() { - return getConfigValue(CONFIG_RECURRING, Boolean.class); - } - - public void setRecurring(Boolean recurring) { - setConfigValue(CONFIG_RECURRING, recurring); - } - public Boolean getEnabled() { return getConfigValue(CONFIG_ENABLED, Boolean.class); } @@ -213,11 +204,6 @@ public final class WorkflowRepresentation extends AbstractWorkflowComponentRepre return this; } - public Builder recurring() { - representation.setRecurring(true); - return this; - } - public WorkflowSetRepresentation build() { List workflows = new ArrayList<>(); diff --git a/core/src/test/java/org/keycloak/representations/workflows/WorkflowDefinitionTest.java b/core/src/test/java/org/keycloak/representations/workflows/WorkflowDefinitionTest.java index 18b4d95f02a..1e6f94f87c0 100644 --- a/core/src/test/java/org/keycloak/representations/workflows/WorkflowDefinitionTest.java +++ b/core/src/test/java/org/keycloak/representations/workflows/WorkflowDefinitionTest.java @@ -28,7 +28,6 @@ public class WorkflowDefinitionTest { expected.setOnEventReset("event-reset-1", "event-reset-2"); expected.setSteps(null); expected.setConditions(null); - expected.setRecurring(true); expected.setEnabled(true); expected.setConditions(Arrays.asList( @@ -76,7 +75,6 @@ public class WorkflowDefinitionTest { assertEquals(expected.getOn(), (String) actual.getOn()); assertArrayEquals(((List) expected.getOnEventReset()).toArray(), ((List) actual.getOnEventReset()).toArray()); assertEquals(expected.getName(), actual.getName()); - assertEquals(expected.getRecurring(), actual.getRecurring()); assertEquals(expected.getEnabled(), actual.getEnabled()); List actualConditions = actual.getConditions(); diff --git a/server-spi-private/src/main/java/org/keycloak/models/workflow/DefaultWorkflowExecutionContext.java b/server-spi-private/src/main/java/org/keycloak/models/workflow/DefaultWorkflowExecutionContext.java index c65523544c5..bff7cf339ef 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/workflow/DefaultWorkflowExecutionContext.java +++ b/server-spi-private/src/main/java/org/keycloak/models/workflow/DefaultWorkflowExecutionContext.java @@ -10,6 +10,7 @@ final class DefaultWorkflowExecutionContext implements WorkflowExecutionContext private final String executionId; private final Workflow workflow; private WorkflowStep currentStep; + private boolean restarted; /** * A new execution context for a workflow event. The execution ID is randomly generated. @@ -71,4 +72,18 @@ final class DefaultWorkflowExecutionContext implements WorkflowExecutionContext void setCurrentStep(WorkflowStep step) { this.currentStep = step; } + + boolean restarted() { + return this.restarted; + } + + void restart() { + this.restarted = true; + } + + void resetState() { + this.restarted = false; + this.currentStep = null; + } } + diff --git a/server-spi-private/src/main/java/org/keycloak/models/workflow/Workflow.java b/server-spi-private/src/main/java/org/keycloak/models/workflow/Workflow.java index b59d4a29132..d7936116828 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/workflow/Workflow.java +++ b/server-spi-private/src/main/java/org/keycloak/models/workflow/Workflow.java @@ -20,7 +20,6 @@ package org.keycloak.models.workflow; import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_ENABLED; import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_ERROR; import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_NAME; -import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_RECURRING; import java.util.List; import java.util.Map; @@ -68,10 +67,6 @@ public class Workflow { return config != null && Boolean.parseBoolean(config.getFirstOrDefault(CONFIG_ENABLED, "true")); } - public boolean isRecurring() { - return config != null && Boolean.parseBoolean(config.getFirst(CONFIG_RECURRING)); - } - public Long getNotBefore() { return notBefore; } diff --git a/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowsManager.java b/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowsManager.java index c5bafcc4caf..58b8084b4e5 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowsManager.java +++ b/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowsManager.java @@ -41,14 +41,12 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Stream; import static java.util.Optional.ofNullable; import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_CONDITIONS; import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_ENABLED; import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_NAME; -import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_RECURRING; public class WorkflowsManager { @@ -271,7 +269,7 @@ public class WorkflowsManager { } } else { // process the workflow steps, scheduling or running them as needed - runWorkflowNextSteps(context); + runWorkflow(context); } } } else { @@ -311,81 +309,45 @@ public class WorkflowsManager { continue; } - // run the scheduled step that is due - runWorkflowStep(context); - - // now process the subsequent steps, scheduling or running them as needed - runWorkflowNextSteps(context); + runWorkflow(context); } }); } - public void bind(Workflow workflow, ResourceType type, String resourceId) { - processEvent(Stream.of(workflow), new AdhocWorkflowEvent(type, resourceId)); - } - - public void bindToAllEligibleResources(Workflow workflow) { - if (workflow.isEnabled()) { - WorkflowProvider provider = getWorkflowProvider(workflow); - provider.getEligibleResourcesForInitialStep() - .forEach(resourceId -> processEvent(Stream.of(workflow), new AdhocWorkflowEvent(ResourceType.USERS, resourceId))); - } - } - - private void restartWorkflow(DefaultWorkflowExecutionContext context) { - Workflow workflow = context.getWorkflow(); + private void runWorkflow(DefaultWorkflowExecutionContext context) { + String executionId = context.getExecutionId(); String resourceId = context.getResourceId(); - String executionId = context.getExecutionId(); - context.setCurrentStep(null); - log.debugf("Restarted workflow '%s' for resource %s (execution id: %s)", workflow.getName(), resourceId, executionId); - runWorkflowNextSteps(context); - } - - private void runWorkflowNextSteps(DefaultWorkflowExecutionContext context) { - String executionId = context.getExecutionId(); Workflow workflow = context.getWorkflow(); - Stream steps = getSteps(workflow.getId()); WorkflowStep currentStep = context.getCurrentStep(); if (currentStep != null) { - steps = steps.skip(currentStep.getPriority()); + // we are resuming from a scheduled step - run it and then continue with the rest of the workflow + runWorkflowStep(context); } - - String resourceId = context.getResourceId(); - AtomicBoolean scheduled = new AtomicBoolean(false); - - steps.forEach(step -> { - if (scheduled.get()) { - // if we already scheduled a step, we skip processing the other steps of the workflow - return; - } + List stepsToRun = getSteps(workflow.getId()) + .skip(currentStep != null ? currentStep.getPriority() : 0).toList(); + for (WorkflowStep step : stepsToRun) { if (step.getAfter() > 0) { // If a step has a time defined, schedule it and stop processing the other steps of workflow log.debugf("Scheduling step %s to run in %d ms for resource %s (execution id: %s)", step.getProviderId(), step.getAfter(), resourceId, executionId); workflowStateProvider.scheduleStep(workflow, step, resourceId, executionId); - scheduled.set(true); + return; } else { // Otherwise, run the step right away context.setCurrentStep(step); runWorkflowStep(context); } - }); - - if (scheduled.get()) { - // if we scheduled a step, we stop processing the workflow here + } + if (context.restarted()) { + // last step was a restart, so we restart the workflow from the beginning + restartWorkflow(context); return; } - // if we've reached the end of the workflow, check if it is recurring or if we can mark it as completed - if (workflow.isRecurring()) { - // if the workflow is recurring, restart it - restartWorkflow(context); - } else { - // not recurring, remove the state record - log.debugf("Workflow '%s' completed for resource %s (execution id: %s)", workflow.getName(), resourceId, executionId); - workflowStateProvider.remove(executionId); - } + // not recurring, remove the state record + log.debugf("Workflow '%s' completed for resource %s (execution id: %s)", workflow.getName(), resourceId, executionId); + workflowStateProvider.remove(executionId); } private void runWorkflowStep(DefaultWorkflowExecutionContext context) { @@ -413,6 +375,27 @@ public class WorkflowsManager { }); } + private void restartWorkflow(DefaultWorkflowExecutionContext context) { + Workflow workflow = context.getWorkflow(); + String resourceId = context.getResourceId(); + String executionId = context.getExecutionId(); + context.resetState(); + log.debugf("Restarted workflow '%s' for resource %s (execution id: %s)", workflow.getName(), resourceId, executionId); + runWorkflow(context); + } + + public void bind(Workflow workflow, ResourceType type, String resourceId) { + processEvent(Stream.of(workflow), new AdhocWorkflowEvent(type, resourceId)); + } + + public void bindToAllEligibleResources(Workflow workflow) { + if (workflow.isEnabled()) { + WorkflowProvider provider = getWorkflowProvider(workflow); + provider.getEligibleResourcesForInitialStep() + .forEach(resourceId -> processEvent(Stream.of(workflow), new AdhocWorkflowEvent(ResourceType.USERS, resourceId))); + } + } + /* ======================= Workflows representation <-> model conversions and validations ======================== */ public WorkflowRepresentation toRepresentation(Workflow workflow) { @@ -485,12 +468,25 @@ public class WorkflowsManager { private void validateWorkflow(WorkflowRepresentation rep) { validateEvents(rep.getOnValues()); validateEvents(rep.getOnEventsReset()); - // a recurring workflow must have at least one scheduled step to prevent an infinite loop of immediate executions - if (rep.getConfig() != null && Boolean.parseBoolean(rep.getConfig().getFirstOrDefault(CONFIG_RECURRING, "false"))) { - boolean hasScheduledStep = ofNullable(rep.getSteps()).orElse(List.of()).stream() + + // if a workflow has a restart step, at least one of the previous steps must be scheduled to prevent an infinite loop of immediate executions + List steps = ofNullable(rep.getSteps()).orElse(List.of()); + List restartSteps = steps.stream() + .filter(step -> Objects.equals("restart", step.getUses())) + .toList(); + + if (!restartSteps.isEmpty()) { + if (restartSteps.size() > 1) { + throw new WorkflowInvalidStateException("Workflow can have only one restart step."); + } + WorkflowStepRepresentation restartStep = restartSteps.get(0); + if (steps.indexOf(restartStep) != steps.size() - 1) { + throw new WorkflowInvalidStateException("Workflow restart step must be the last step."); + } + boolean hasScheduledStep = steps.stream() .anyMatch(step -> Integer.parseInt(ofNullable(step.getAfter()).orElse("0")) > 0); if (!hasScheduledStep) { - throw new WorkflowInvalidStateException("A recurring workflow must have at least one step with a time delay."); + throw new WorkflowInvalidStateException("A workflow with a restart step must have at least one step with a time delay."); } } } diff --git a/services/src/main/java/org/keycloak/models/workflow/RestartWorkflowStepProvider.java b/services/src/main/java/org/keycloak/models/workflow/RestartWorkflowStepProvider.java new file mode 100644 index 00000000000..5528aa7359a --- /dev/null +++ b/services/src/main/java/org/keycloak/models/workflow/RestartWorkflowStepProvider.java @@ -0,0 +1,19 @@ +package org.keycloak.models.workflow; + +public class RestartWorkflowStepProvider implements WorkflowStepProvider { + + @Override + public void run(WorkflowExecutionContext context) { + if (context instanceof DefaultWorkflowExecutionContext) { + ((DefaultWorkflowExecutionContext) context).restart(); + } else { + throw new IllegalArgumentException("Context must be DefaultWorkflowExecutionContext"); + } + } + + @Override + public void close() { + // No resources to close + } +} + diff --git a/services/src/main/java/org/keycloak/models/workflow/RestartWorkflowStepProviderFactory.java b/services/src/main/java/org/keycloak/models/workflow/RestartWorkflowStepProviderFactory.java new file mode 100644 index 00000000000..d59768fb7bf --- /dev/null +++ b/services/src/main/java/org/keycloak/models/workflow/RestartWorkflowStepProviderFactory.java @@ -0,0 +1,54 @@ +package org.keycloak.models.workflow; + +import org.keycloak.component.ComponentModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.provider.ProviderConfigProperty; + +import java.util.List; + +public class RestartWorkflowStepProviderFactory implements WorkflowStepProviderFactory { + + public static final String ID = "restart"; + + @Override + public RestartWorkflowStepProvider create(KeycloakSession session, ComponentModel model) { + return new RestartWorkflowStepProvider(); + } + + @Override + public void init(org.keycloak.Config.Scope config) { + // No initialization needed + } + + @Override + public void postInit(org.keycloak.models.KeycloakSessionFactory factory) { + // No post-initialization needed + } + + @Override + public void close() { + // No resources to close + } + + @Override + public String getId() { + return ID; + } + + @Override + public List getConfigProperties() { + return List.of(); + } + + @Override + public ResourceType getType() { + // TODO: need to revisit this once we support more types as this provider should be usable for all resource types. + return ResourceType.USERS; + } + + @Override + public String getHelpText() { + return "Restarts the current workflow"; + } +} + diff --git a/services/src/main/resources/META-INF/services/org.keycloak.models.workflow.WorkflowStepProviderFactory b/services/src/main/resources/META-INF/services/org.keycloak.models.workflow.WorkflowStepProviderFactory index 58216bedbbc..16c0f11a48f 100644 --- a/services/src/main/resources/META-INF/services/org.keycloak.models.workflow.WorkflowStepProviderFactory +++ b/services/src/main/resources/META-INF/services/org.keycloak.models.workflow.WorkflowStepProviderFactory @@ -20,3 +20,4 @@ org.keycloak.models.workflow.NotifyUserStepProviderFactory org.keycloak.models.workflow.DeleteUserStepProviderFactory org.keycloak.models.workflow.SetUserAttributeStepProviderFactory org.keycloak.models.workflow.AddRequiredActionStepProviderFactory +org.keycloak.models.workflow.RestartWorkflowStepProviderFactory diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/RoleWorkflowConditionTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/RoleWorkflowConditionTest.java index d9390e8b1fb..90aa073e5a5 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/RoleWorkflowConditionTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/RoleWorkflowConditionTest.java @@ -23,6 +23,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.workflow.EventBasedWorkflowProviderFactory; import org.keycloak.models.workflow.ResourceOperationType; +import org.keycloak.models.workflow.RestartWorkflowStepProviderFactory; import org.keycloak.models.workflow.WorkflowsManager; import org.keycloak.models.workflow.SetUserAttributeStepProviderFactory; import org.keycloak.models.workflow.conditions.RoleWorkflowConditionFactory; @@ -140,7 +141,6 @@ public class RoleWorkflowConditionTest { WorkflowSetRepresentation expectedWorkflows = WorkflowRepresentation.create() .of(EventBasedWorkflowProviderFactory.ID) .onEvent(ResourceOperationType.USER_ROLE_ADD.name()) - .recurring() .onConditions(WorkflowConditionRepresentation.create() .of(RoleWorkflowConditionFactory.ID) .withConfig(attributes) @@ -150,6 +150,9 @@ public class RoleWorkflowConditionTest { .of(SetUserAttributeStepProviderFactory.ID) .withConfig("notified", "true") .after(Duration.ofDays(5)) + .build(), + WorkflowStepRepresentation.create() + .of(RestartWorkflowStepProviderFactory.ID) .build() ).build(); diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/UserAttributeWorkflowConditionTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/UserAttributeWorkflowConditionTest.java index 9a2d62b2303..9a14684183a 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/UserAttributeWorkflowConditionTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/UserAttributeWorkflowConditionTest.java @@ -22,6 +22,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.workflow.EventBasedWorkflowProviderFactory; import org.keycloak.models.workflow.ResourceOperationType; +import org.keycloak.models.workflow.RestartWorkflowStepProviderFactory; import org.keycloak.models.workflow.WorkflowsManager; import org.keycloak.models.workflow.SetUserAttributeStepProviderFactory; import org.keycloak.models.workflow.conditions.UserAttributeWorkflowConditionFactory; @@ -135,7 +136,6 @@ public class UserAttributeWorkflowConditionTest { WorkflowSetRepresentation expectedWorkflows = WorkflowRepresentation.create() .of(EventBasedWorkflowProviderFactory.ID) .onEvent(ResourceOperationType.USER_ADD.name()) - .recurring() .onConditions(WorkflowConditionRepresentation.create() .of(UserAttributeWorkflowConditionFactory.ID) .withConfig(attributes) @@ -145,6 +145,9 @@ public class UserAttributeWorkflowConditionTest { .of(SetUserAttributeStepProviderFactory.ID) .withConfig("notified", "true") .after(Duration.ofDays(5)) + .build(), + WorkflowStepRepresentation.create() + .of(RestartWorkflowStepProviderFactory.ID) .build() ).build(); diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/WorkflowManagementTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/WorkflowManagementTest.java index ebcac5b6a1e..15169689e71 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/WorkflowManagementTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/WorkflowManagementTest.java @@ -52,6 +52,7 @@ import org.keycloak.models.workflow.DeleteUserStepProviderFactory; import org.keycloak.models.workflow.DisableUserStepProviderFactory; import org.keycloak.models.workflow.EventBasedWorkflowProviderFactory; import org.keycloak.models.workflow.NotifyUserStepProviderFactory; +import org.keycloak.models.workflow.RestartWorkflowStepProviderFactory; import org.keycloak.models.workflow.WorkflowStep; import org.keycloak.models.workflow.ResourceOperationType; import org.keycloak.models.workflow.Workflow; @@ -173,17 +174,19 @@ public class WorkflowManagementTest { workflows.create(WorkflowRepresentation.create() .of(UserCreationTimeWorkflowProviderFactory.ID) .onEvent(ResourceOperationType.USER_ADD.toString()) - .recurring() .withSteps( WorkflowStepRepresentation.create().of(NotifyUserStepProviderFactory.ID) .after(Duration.ofDays(5)) + .build(), + WorkflowStepRepresentation.create().of(RestartWorkflowStepProviderFactory.ID) .build() ).of(EventBasedWorkflowProviderFactory.ID) .onEvent(ResourceOperationType.USER_LOGIN.toString()) - .recurring() .withSteps( WorkflowStepRepresentation.create().of(NotifyUserStepProviderFactory.ID) .after(Duration.ofDays(5)) + .build(), + WorkflowStepRepresentation.create().of(RestartWorkflowStepProviderFactory.ID) .build() ).build()).close(); @@ -567,10 +570,11 @@ public class WorkflowManagementTest { managedRealm.admin().workflows().create(WorkflowRepresentation.create() .of(UserCreationTimeWorkflowProviderFactory.ID) .onEvent(ResourceOperationType.USER_ADD.toString()) - .recurring() .withSteps( WorkflowStepRepresentation.create().of(NotifyUserStepProviderFactory.ID) .after(Duration.ofDays(5)) + .build(), + WorkflowStepRepresentation.create().of(RestartWorkflowStepProviderFactory.ID) .build() ).build()).close();