diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/DefaultWorkflowExecutionContext.java b/model/jpa/src/main/java/org/keycloak/models/workflow/DefaultWorkflowExecutionContext.java index 57d5fa0c0f6..fe351d0eac9 100644 --- a/model/jpa/src/main/java/org/keycloak/models/workflow/DefaultWorkflowExecutionContext.java +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/DefaultWorkflowExecutionContext.java @@ -72,6 +72,11 @@ final class DefaultWorkflowExecutionContext implements WorkflowExecutionContext return resourceId; } + @Override + public WorkflowEvent getEvent() { + return event; + } + String getExecutionId() { return this.executionId; } @@ -80,10 +85,6 @@ final class DefaultWorkflowExecutionContext implements WorkflowExecutionContext return workflow; } - WorkflowEvent getEvent() { - return event; - } - WorkflowStep getCurrentStep() { return currentStep; } 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 7c0aef7fcc8..76b6ab7db75 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 @@ -24,15 +24,11 @@ import org.keycloak.models.workflow.WorkflowStateProvider.ScheduledStep; import org.keycloak.representations.workflows.WorkflowConstants; import org.keycloak.representations.workflows.WorkflowRepresentation; import org.keycloak.representations.workflows.WorkflowStepRepresentation; -import org.keycloak.utils.StringUtil; import org.jboss.logging.Logger; import static java.util.Optional.ofNullable; -import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_ENABLED; -import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_NAME; - public class DefaultWorkflowProvider implements WorkflowProvider { private static final Logger log = Logger.getLogger(DefaultWorkflowProvider.class); @@ -62,24 +58,41 @@ public class DefaultWorkflowProvider implements WorkflowProvider { @Override public void updateWorkflow(Workflow workflow, WorkflowRepresentation representation) { + // first step - ensure the updated workflow is valid + WorkflowValidator.validateWorkflow(session, representation); - WorkflowRepresentation currentRep = toRepresentation(workflow); + // check if there are scheduled steps for this workflow - if there aren't, we can update freely + if (!stateProvider.hasScheduledSteps(workflow.getId())) { + // simply delete and re-create the workflow, ensuring the id remains the same + removeWorkflow(workflow); + representation.setId(workflow.getId()); + toModel(representation); + } else { + // if there are scheduled steps, we don't allow to update the workflow's 'on' config + WorkflowRepresentation currentRepresentation = toRepresentation(workflow); + if (!Objects.equals(currentRepresentation.getOn(), representation.getOn())) { + throw new ModelValidationException("Cannot update 'on' configuration when there are scheduled resources for the workflow."); + } - // we compare the representation, removing first the entries we allow updating. If anything else changes, we throw a validation exception - String currentName = currentRep.getName(); currentRep.getConfig().remove(CONFIG_NAME); - String newName = representation.getName(); representation.getConfig().remove(CONFIG_NAME); - Boolean currentEnabled = currentRep.getEnabled(); currentRep.getConfig().remove(CONFIG_ENABLED); - Boolean newEnabled = representation.getEnabled(); representation.getConfig().remove(CONFIG_ENABLED); + // we also need to guarantee the steps remain the same - that is, in the same order with the same 'uses' property. + // each step can have its config updated, but the steps themselves cannot be changed. + List currentSteps = currentRepresentation.getSteps(); + List newSteps = ofNullable(representation.getSteps()).orElse(List.of()); + if (currentSteps.size() != newSteps.size()) { + throw new ModelValidationException("Cannot change the number or order of steps when there are scheduled resources for the workflow."); + } + for (int i = 0; i < currentSteps.size(); i++) { + WorkflowStepRepresentation currentStep = currentSteps.get(i); + WorkflowStepRepresentation newStep = newSteps.get(i); + if (!Objects.equals(currentStep.getUses(), newStep.getUses())) { + throw new ModelValidationException("Cannot change the number or order of steps when there are scheduled resources for the workflow."); + } + // set the id of the step to match the existing one, so we can update the config + newStep.setId(currentStep.getId()); + } - if (!currentRep.equals(representation)) { - throw new ModelValidationException("Workflow update can only change 'name' and 'enabled' config entries."); - } - - if (!Objects.equals(currentName, newName) || !Objects.equals(currentEnabled, newEnabled)) { - // only update component if something changed - representation.setName(newName); - representation.setEnabled(newEnabled); - this.updateWorkflowConfig(workflow, representation.getConfig()); + // finally, update the workflow's config along with the steps' configs + workflow.updateConfig(representation.getConfig(), newSteps); } } @@ -116,17 +129,23 @@ public class DefaultWorkflowProvider implements WorkflowProvider { return; } for (ScheduledStep scheduled : stateProvider.getDueScheduledSteps(workflow)) { + // check if the resource is still passes the workflow's resource conditions DefaultWorkflowExecutionContext context = new DefaultWorkflowExecutionContext(session, workflow, scheduled); - WorkflowStep step = context.getCurrentStep(); - - if (step == null) { - log.warnf("Could not find step %s in workflow %s for resource %s. Removing the workflow state.", - scheduled.stepId(), scheduled.workflowId(), scheduled.resourceId()); + EventBasedWorkflow provider = new EventBasedWorkflow(session, getWorkflowComponent(workflow.getId())); + if (!provider.validateResourceConditions(context)) { + log.debugf("Resource %s is no longer eligible for workflow %s. Cancelling execution of the workflow.", + scheduled.resourceId(), scheduled.workflowId()); stateProvider.remove(scheduled.executionId()); - continue; + } else { + WorkflowStep step = context.getCurrentStep(); + if (step == null) { + log.warnf("Could not find step %s in workflow %s for resource %s. Cancelling execution of the workflow.", + scheduled.stepId(), scheduled.workflowId(), scheduled.resourceId()); + stateProvider.remove(scheduled.executionId()); + } else { + runWorkflow(context); + } } - - runWorkflow(context); } }); } @@ -159,17 +178,15 @@ public class DefaultWorkflowProvider implements WorkflowProvider { @Override public Workflow toModel(WorkflowRepresentation rep) { - validateWorkflow(rep); + WorkflowValidator.validateWorkflow(session, rep); MultivaluedHashMap config = ofNullable(rep.getConfig()).orElse(new MultivaluedHashMap<>()); if (rep.isCancelIfRunning()) { config.putSingle(WorkflowConstants.CONFIG_CANCEL_IF_RUNNING, "true"); } - Workflow workflow = addWorkflow(new Workflow(session, config)); - + Workflow workflow = addWorkflow(new Workflow(session, rep.getId(), config)); workflow.addSteps(rep.getSteps()); - return workflow; } @@ -181,12 +198,6 @@ public class DefaultWorkflowProvider implements WorkflowProvider { return getStepProviderFactory(step).create(session, realm.getComponent(step.getId())); } - private void updateWorkflowConfig(Workflow workflow, MultivaluedHashMap config) { - ComponentModel component = getWorkflowComponent(workflow.getId()); - component.setConfig(config); - realm.updateComponent(component); - } - private ComponentModel getWorkflowComponent(String id) { ComponentModel component = realm.getComponent(id); @@ -230,18 +241,18 @@ public class DefaultWorkflowProvider implements WorkflowProvider { try { ScheduledStep scheduledStep = scheduledSteps.get(workflow.getId()); + DefaultWorkflowExecutionContext context = new DefaultWorkflowExecutionContext(session, workflow, event); // if workflow is not active for the resource, check if the provider allows activating based on the event if (scheduledStep == null) { - if (provider.activateOnEvent(event)) { + if (provider.activate(context)) { if (isAlreadyScheduledInSession(event, workflow)) { return; } // If the workflow has a positive notBefore set, schedule the first step with it if (DurationConverter.isPositiveDuration(workflow.getNotBefore())) { - scheduleWorkflow(event, workflow); + scheduleWorkflow(context); } else { - DefaultWorkflowExecutionContext context = new DefaultWorkflowExecutionContext(session, workflow, event); // process the workflow steps, scheduling or running them as needed runWorkflow(context); } @@ -250,9 +261,9 @@ 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.resetOnEvent(event)) { + if (provider.reset(context)) { new DefaultWorkflowExecutionContext(session, workflow, event, scheduledStep).restart(); - } else if (provider.deactivateOnEvent(event)) { + } else if (provider.deactivate(context)) { log.debugf("Workflow '%s' cancelled for resource %s (execution id: %s)", workflow.getName(), resourceId, executionId); stateProvider.remove(executionId); } @@ -260,7 +271,7 @@ public class DefaultWorkflowProvider implements WorkflowProvider { } catch (WorkflowInvalidStateException e) { workflow.setEnabled(false); workflow.setError(e.getMessage()); - updateWorkflowConfig(workflow, workflow.getConfig()); + workflow.updateConfig(workflow.getConfig(), null); log.warnf("Workflow %s was disabled due to: %s", workflow.getId(), e.getMessage()); } }); @@ -286,8 +297,8 @@ public class DefaultWorkflowProvider implements WorkflowProvider { return isAlreadyScheduled; } - private void scheduleWorkflow(WorkflowEvent event, Workflow workflow) { - executor.runTask(session, new ScheduleWorkflowTask(new DefaultWorkflowExecutionContext(session, workflow, event))); + private void scheduleWorkflow(WorkflowExecutionContext context) { + executor.runTask(session, new ScheduleWorkflowTask((DefaultWorkflowExecutionContext) context)); } private void runWorkflow(DefaultWorkflowExecutionContext context) { @@ -298,49 +309,10 @@ public class DefaultWorkflowProvider implements WorkflowProvider { return new WorkflowStepRepresentation(step.getId(), step.getProviderId(), step.getConfig()); } - private void validateWorkflow(WorkflowRepresentation rep) { - validateField(rep, "name", rep.getName()); - //TODO: validate event and resource conditions (`on` and `if` properties) using the providers with a custom evaluator that calls validate on - // each condition provider used in the expression. - - // 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()); - - if (steps.isEmpty()) { - return; - } - - steps.forEach(step -> validateField(step, "uses", step.getUses())); - - 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 -> DurationConverter.isPositiveDuration(step.getAfter())); - if (!hasScheduledStep) { - throw new WorkflowInvalidStateException("A workflow with a restart step must have at least one step with a time delay."); - } - } - } - - private void validateField(Object obj, String fieldName, String value) { - if (StringUtil.isBlank(value)) { - throw new ModelValidationException("%s field '%s' cannot be null or empty.".formatted(obj.getClass().getCanonicalName(), fieldName)); - } - } - private Workflow addWorkflow(Workflow workflow) { ComponentModel model = new ComponentModel(); + model.setId(workflow.getId()); model.setParentId(realm.getId()); model.setProviderId(DefaultWorkflowProviderFactory.ID); model.setProviderType(WorkflowProvider.class.getName()); 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 87b11e53e44..6eaa4714113 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 @@ -1,7 +1,5 @@ package org.keycloak.models.workflow; -import java.util.List; - import org.keycloak.component.ComponentModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.workflow.conditions.ExpressionWorkflowConditionProvider; @@ -28,45 +26,50 @@ final class EventBasedWorkflow { return ResourceType.USERS.equals(type); } - boolean activateOnEvent(WorkflowEvent event) { - if (!supports(event.getResourceType())) { + /** + * Evaluates the specified context to determine whether the workflow should be activated or not. Activation will happen + * if the context's event matches the configured activation events and the resource conditions evaluate to true. + * + * @param executionContext + * @return + * @throws WorkflowInvalidStateException + */ + boolean activate(WorkflowExecutionContext executionContext) throws WorkflowInvalidStateException { + WorkflowEvent event = executionContext.getEvent(); + if (event == null) { return false; } - return isActivationEvent(event) && evaluateConditions(event); + return supports(event.getResourceType()) && activateOnEvent(event) && validateResourceConditions(executionContext); } - boolean deactivateOnEvent(WorkflowEvent event) { + boolean deactivate(WorkflowExecutionContext executionContext) throws WorkflowInvalidStateException { // TODO: rework this once we support concurrency/restart-if-running and concurrency/cancel-if-running to use expressions just like activation conditions - if (!supports(event.getResourceType())) { - return false; - } - - List events = model.getConfig().getOrDefault(CONFIG_ON_EVENT, List.of()); - - for (String activationEvent : events) { - ResourceOperationType a = ResourceOperationType.valueOf(activationEvent.toUpperCase()); - - if (a.isDeactivationEvent(event.getEvent().getClass())) { - return !evaluateConditions(event); - } - } - return false; } - boolean resetOnEvent(WorkflowEvent event) { - return isCancelIfRunning() && evaluateConditions(event); + boolean reset(WorkflowExecutionContext executionContext) throws WorkflowInvalidStateException { + WorkflowEvent event = executionContext.getEvent(); + if (event == null) { + return false; + } + return supports(event.getResourceType()) && isCancelIfRunning() && validateResourceConditions(executionContext); } - private boolean evaluateConditions(WorkflowEvent event) { + public boolean validateResourceConditions(WorkflowExecutionContext context) { String conditions = getModel().getConfig().getFirst(CONFIG_CONDITIONS); if (StringUtil.isBlank(conditions)) { return true; } - return new ExpressionWorkflowConditionProvider(getSession(), conditions).evaluate(event); + return new ExpressionWorkflowConditionProvider(getSession(), conditions).evaluate(context); } - private boolean isActivationEvent(WorkflowEvent event) { + /** + * Determins whether the workflow should be activated based on the given event or not. + * + * @param event a reference to the workflow event. + * @return {@code true} if the workflow should be activated, {@code false} otherwise. + */ + private boolean activateOnEvent(WorkflowEvent event) { // AD_HOC is a special case that always triggers the workflow regardless of the configured activation events if (ResourceOperationType.AD_HOC.equals(event.getOperation())) { return true; diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/JpaWorkflowStateProvider.java b/model/jpa/src/main/java/org/keycloak/models/workflow/JpaWorkflowStateProvider.java index e7ef10bf686..2600aff36d5 100644 --- a/model/jpa/src/main/java/org/keycloak/models/workflow/JpaWorkflowStateProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/JpaWorkflowStateProvider.java @@ -22,6 +22,7 @@ import java.time.Instant; import java.util.List; import jakarta.persistence.EntityManager; +import jakarta.persistence.TypedQuery; import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaDelete; import jakarta.persistence.criteria.CriteriaQuery; @@ -199,6 +200,22 @@ public class JpaWorkflowStateProvider implements WorkflowStateProvider { } } + @Override + public boolean hasScheduledSteps(String workflowId) { + CriteriaBuilder cb = em.getCriteriaBuilder(); + CriteriaQuery criteriaQuery = cb.createQuery(Long.class); + Root stateRoot = criteriaQuery.from(WorkflowStateEntity.class); + + criteriaQuery.select(cb.count(stateRoot)); + criteriaQuery.where(cb.equal(stateRoot.get("workflowId"), workflowId)); + + TypedQuery query = em.createQuery(criteriaQuery); + query.setMaxResults(1); + + Long count = query.getSingleResult(); + return count > 0; + } + @Override public void close() { } diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/WorkflowValidator.java b/model/jpa/src/main/java/org/keycloak/models/workflow/WorkflowValidator.java new file mode 100644 index 00000000000..a00c9304195 --- /dev/null +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/WorkflowValidator.java @@ -0,0 +1,110 @@ +package org.keycloak.models.workflow; + +import java.time.Duration; +import java.util.List; +import java.util.Objects; + +import org.keycloak.common.util.DurationConverter; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.workflow.conditions.expression.BooleanConditionParser; +import org.keycloak.models.workflow.conditions.expression.ConditionNameCollector; +import org.keycloak.models.workflow.conditions.expression.EvaluatorUtils; +import org.keycloak.representations.workflows.WorkflowRepresentation; +import org.keycloak.representations.workflows.WorkflowStepRepresentation; +import org.keycloak.utils.StringUtil; + +import static java.util.Optional.ofNullable; + +public class WorkflowValidator { + + public static void validateWorkflow(KeycloakSession session, WorkflowRepresentation rep) throws WorkflowInvalidStateException { + validateField(rep, "name", rep.getName()); + //TODO: validate event and resource conditions (`on` and `if` properties) using the providers with a custom evaluator that calls validate on + // each condition provider used in the expression once we have the event condition providers implemented + if (StringUtil.isNotBlank(rep.getOn())) { + validateConditionExpression(session, rep.getOn(), "on"); + } + if (StringUtil.isNotBlank(rep.getConditions())) { + validateConditionExpression(session, rep.getConditions(), "if"); + } + + // 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()); + if (steps.isEmpty()) { + return; + } + steps.forEach(step -> validateStep(session, step)); + + 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 -> DurationConverter.isPositiveDuration(step.getAfter())); + if (!hasScheduledStep) { + throw new WorkflowInvalidStateException("A workflow with a restart step must have at least one step with a time delay."); + } + } + } + + private static void validateStep(KeycloakSession session, WorkflowStepRepresentation step) throws WorkflowInvalidStateException { + + // validate the step rep has 'uses' defined + if (StringUtil.isBlank(step.getUses())) { + throw new WorkflowInvalidStateException("Step 'uses' cannot be null or empty."); + } + + // validate the after time, if present + try { + Duration duration = DurationConverter.parseDuration(step.getAfter()); + if (duration != null && duration.isNegative()) { // duration can only be null if the config is not set + throw new WorkflowInvalidStateException("Step 'after' configuration cannot be negative."); + } + } catch (IllegalArgumentException e) { + throw new WorkflowInvalidStateException("Step 'after' configuration is not valid: " + step.getAfter()); + } + + // verify the step does have valid provider + WorkflowStepProviderFactory factory = (WorkflowStepProviderFactory) session + .getKeycloakSessionFactory().getProviderFactory(WorkflowStepProvider.class, step.getUses()); + + if (factory == null) { + throw new WorkflowInvalidStateException("Could not find step provider: " + step.getUses()); + } + } + + private static void validateConditionExpression(KeycloakSession session, String expression, String fieldName) throws WorkflowInvalidStateException { + BooleanConditionParser.EvaluatorContext context = EvaluatorUtils.createEvaluatorContext(expression); + ConditionNameCollector collector = new ConditionNameCollector(); + collector.visit(context); + + // check if there are providers for the conditions used in the expression + if ("on".equals(fieldName)) { + // check if we can get a ResourceOperationType for the events in the expression + for (String name : collector.getConditionNames()) { + try { + ResourceOperationType.valueOf(name.replace("-", "_").toUpperCase()); + } catch (IllegalArgumentException iae) { + throw new WorkflowInvalidStateException("Could not find event: " + name); + } + } + } else if ("if".equals(fieldName)) { + // try to get an instance of the provider -> method throws a WorkflowInvalidStateException if provider is not found + collector.getConditionNames().forEach(name -> Workflows.getConditionProvider(session, name, expression)); + } + } + + private static void validateField(Object obj, String fieldName, String value) throws WorkflowInvalidStateException { + if (StringUtil.isBlank(value)) { + throw new WorkflowInvalidStateException("%s field '%s' cannot be null or empty.".formatted(obj.getClass().getCanonicalName(), fieldName)); + } + } +} diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/ExpressionWorkflowConditionProvider.java b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/ExpressionWorkflowConditionProvider.java index ac0f938c8e7..29802074cae 100644 --- a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/ExpressionWorkflowConditionProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/ExpressionWorkflowConditionProvider.java @@ -7,7 +7,7 @@ import jakarta.persistence.criteria.Root; import org.keycloak.models.KeycloakSession; import org.keycloak.models.workflow.WorkflowConditionProvider; -import org.keycloak.models.workflow.WorkflowEvent; +import org.keycloak.models.workflow.WorkflowExecutionContext; import org.keycloak.models.workflow.conditions.expression.BooleanConditionParser.EvaluatorContext; import org.keycloak.models.workflow.conditions.expression.ConditionEvaluator; import org.keycloak.models.workflow.conditions.expression.EvaluatorUtils; @@ -25,9 +25,9 @@ public class ExpressionWorkflowConditionProvider implements WorkflowConditionPro } @Override - public boolean evaluate(WorkflowEvent event) { + public boolean evaluate(WorkflowExecutionContext context) { validate(); - ConditionEvaluator evaluator = new ConditionEvaluator(session, event); + ConditionEvaluator evaluator = new ConditionEvaluator(session, context); return evaluator.visit(this.evaluatorContext); } diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/GroupMembershipWorkflowConditionProvider.java b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/GroupMembershipWorkflowConditionProvider.java index 33d03a3088f..197bdd957d0 100644 --- a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/GroupMembershipWorkflowConditionProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/GroupMembershipWorkflowConditionProvider.java @@ -5,9 +5,8 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.utils.KeycloakModelUtils; -import org.keycloak.models.workflow.ResourceType; import org.keycloak.models.workflow.WorkflowConditionProvider; -import org.keycloak.models.workflow.WorkflowEvent; +import org.keycloak.models.workflow.WorkflowExecutionContext; import org.keycloak.models.workflow.WorkflowInvalidStateException; import org.keycloak.utils.StringUtil; @@ -22,16 +21,11 @@ public class GroupMembershipWorkflowConditionProvider implements WorkflowConditi } @Override - public boolean evaluate(WorkflowEvent event) { - if (!ResourceType.USERS.equals(event.getResourceType())) { - return false; - } - + public boolean evaluate(WorkflowExecutionContext context) { validate(); - String userId = event.getResourceId(); RealmModel realm = session.getContext().getRealm(); - UserModel user = session.users().getUserById(realm, userId); + UserModel user = session.users().getUserById(realm, context.getResourceId()); if (user == null) { return false; } diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/IdentityProviderWorkflowConditionProvider.java b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/IdentityProviderWorkflowConditionProvider.java index bc23c26ee7f..1e984c65458 100644 --- a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/IdentityProviderWorkflowConditionProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/IdentityProviderWorkflowConditionProvider.java @@ -13,9 +13,8 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.jpa.entities.FederatedIdentityEntity; -import org.keycloak.models.workflow.ResourceType; import org.keycloak.models.workflow.WorkflowConditionProvider; -import org.keycloak.models.workflow.WorkflowEvent; +import org.keycloak.models.workflow.WorkflowExecutionContext; import org.keycloak.models.workflow.WorkflowInvalidStateException; import org.keycloak.utils.StringUtil; @@ -30,16 +29,11 @@ public class IdentityProviderWorkflowConditionProvider implements WorkflowCondit } @Override - public boolean evaluate(WorkflowEvent event) { - if (!ResourceType.USERS.equals(event.getResourceType())) { - return false; - } - + public boolean evaluate(WorkflowExecutionContext context) { validate(); - String userId = event.getResourceId(); RealmModel realm = session.getContext().getRealm(); - UserModel user = session.users().getUserById(realm, userId); + UserModel user = session.users().getUserById(realm, context.getResourceId()); if (user == null) { return false; } diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/RoleWorkflowConditionProvider.java b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/RoleWorkflowConditionProvider.java index e3fa2cc1ac2..abe423b7405 100644 --- a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/RoleWorkflowConditionProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/RoleWorkflowConditionProvider.java @@ -9,9 +9,8 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.UserModel; import org.keycloak.models.utils.RoleUtils; -import org.keycloak.models.workflow.ResourceType; import org.keycloak.models.workflow.WorkflowConditionProvider; -import org.keycloak.models.workflow.WorkflowEvent; +import org.keycloak.models.workflow.WorkflowExecutionContext; import org.keycloak.models.workflow.WorkflowInvalidStateException; import org.keycloak.utils.StringUtil; @@ -26,16 +25,11 @@ public class RoleWorkflowConditionProvider implements WorkflowConditionProvider } @Override - public boolean evaluate(WorkflowEvent event) { - if (!ResourceType.USERS.equals(event.getResourceType())) { - return false; - } - + public boolean evaluate(WorkflowExecutionContext context) { validate(); - String userId = event.getResourceId(); RealmModel realm = session.getContext().getRealm(); - UserModel user = session.users().getUserById(realm, userId); + UserModel user = session.users().getUserById(realm, context.getResourceId()); if (user == null) { return false; diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/UserAttributeWorkflowConditionProvider.java b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/UserAttributeWorkflowConditionProvider.java index 88026db8c47..b1a56482a21 100644 --- a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/UserAttributeWorkflowConditionProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/UserAttributeWorkflowConditionProvider.java @@ -7,9 +7,8 @@ import java.util.Properties; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; -import org.keycloak.models.workflow.ResourceType; import org.keycloak.models.workflow.WorkflowConditionProvider; -import org.keycloak.models.workflow.WorkflowEvent; +import org.keycloak.models.workflow.WorkflowExecutionContext; import org.keycloak.models.workflow.WorkflowInvalidStateException; import static org.keycloak.common.util.CollectionUtil.collectionEquals; @@ -25,16 +24,11 @@ public class UserAttributeWorkflowConditionProvider implements WorkflowCondition } @Override - public boolean evaluate(WorkflowEvent event) { - if (!ResourceType.USERS.equals(event.getResourceType())) { - return false; - } - + public boolean evaluate(WorkflowExecutionContext context) { validate(); - String userId = event.getResourceId(); RealmModel realm = session.getContext().getRealm(); - UserModel user = session.users().getUserById(realm, userId); + UserModel user = session.users().getUserById(realm, context.getResourceId()); if (user == null) { return false; diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/AbstractBooleanEvaluator.java b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/AbstractBooleanEvaluator.java new file mode 100644 index 00000000000..488bc96e7c2 --- /dev/null +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/AbstractBooleanEvaluator.java @@ -0,0 +1,68 @@ +package org.keycloak.models.workflow.conditions.expression; + +public abstract class AbstractBooleanEvaluator extends BooleanConditionParserBaseVisitor { + + @Override + public Boolean visitEvaluator(BooleanConditionParser.EvaluatorContext ctx) { + return visit(ctx.expression()); + } + + @Override + public Boolean visitExpression(BooleanConditionParser.ExpressionContext ctx) { + if (ctx.expression() != null && ctx.OR() != null) { + return visit(ctx.expression()) || visit(ctx.andExpression()); + } + return visit(ctx.andExpression()); + } + + @Override + public Boolean visitAndExpression(BooleanConditionParser.AndExpressionContext ctx) { + if (ctx.andExpression() != null && ctx.AND() != null) { + return visit(ctx.andExpression()) && visit(ctx.notExpression()); + } + return visit(ctx.notExpression()); + } + + @Override + public Boolean visitNotExpression(BooleanConditionParser.NotExpressionContext ctx) { + if (ctx.NOT() != null) { + return !visit(ctx.notExpression()); + } + return visit(ctx.atom()); + } + + @Override + public Boolean visitAtom(BooleanConditionParser.AtomContext ctx) { + if (ctx.conditionCall() != null) { + return visit(ctx.conditionCall()); + } + return visit(ctx.expression()); + } + + @Override + public abstract Boolean visitConditionCall(BooleanConditionParser.ConditionCallContext ctx); + + protected String extractParameter(BooleanConditionParser.ParameterContext paramCtx) { + // Case 1: No parentheses were used (e.g., "user-logged-in") + // Case 2: Empty parentheses were used (e.g., "user-logged-in()") + if (paramCtx == null || paramCtx.ParameterText() == null) { + return null; + } + + // Case 3: A parameter was provided (e.g., "has-role(param)") + String rawText = paramCtx.ParameterText().getText(); + return unEscapeParameter(rawText); + } + + /** + * The grammar defines escapes as '\)' and '\\'. + * @param rawText The raw text from the ParameterText token. + * @return A clean, un-escaped string. + */ + private String unEscapeParameter(String rawText) { + // This handles both \) -> ) and \\ -> \ + // Note: replaceAll uses regex, so we must double-escape the backslashes + return rawText.replace("\\)", ")") + .replace("\\\\", "\\"); + } +} diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/ConditionEvaluator.java b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/ConditionEvaluator.java index c76f153f9fa..6980389728f 100644 --- a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/ConditionEvaluator.java +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/ConditionEvaluator.java @@ -2,85 +2,25 @@ package org.keycloak.models.workflow.conditions.expression; import org.keycloak.models.KeycloakSession; import org.keycloak.models.workflow.WorkflowConditionProvider; -import org.keycloak.models.workflow.WorkflowEvent; +import org.keycloak.models.workflow.WorkflowExecutionContext; import static org.keycloak.models.workflow.Workflows.getConditionProvider; -public class ConditionEvaluator extends BooleanConditionParserBaseVisitor { +public class ConditionEvaluator extends AbstractBooleanEvaluator { protected final KeycloakSession session; - protected final WorkflowEvent event; + protected final WorkflowExecutionContext context; - public ConditionEvaluator(KeycloakSession session, WorkflowEvent event) { + public ConditionEvaluator(KeycloakSession session, WorkflowExecutionContext context) { this.session = session; - this.event = event; - } - - @Override - public Boolean visitEvaluator(BooleanConditionParser.EvaluatorContext ctx) { - return visit(ctx.expression()); - } - - @Override - public Boolean visitExpression(BooleanConditionParser.ExpressionContext ctx) { - if (ctx.expression() != null && ctx.OR() != null) { - return visit(ctx.expression()) || visit(ctx.andExpression()); - } - return visit(ctx.andExpression()); - } - - @Override - public Boolean visitAndExpression(BooleanConditionParser.AndExpressionContext ctx) { - if (ctx.andExpression() != null && ctx.AND() != null) { - return visit(ctx.andExpression()) && visit(ctx.notExpression()); - } - return visit(ctx.notExpression()); - } - - @Override - public Boolean visitNotExpression(BooleanConditionParser.NotExpressionContext ctx) { - if (ctx.NOT() != null) { - return !visit(ctx.notExpression()); - } - return visit(ctx.atom()); - } - - @Override - public Boolean visitAtom(BooleanConditionParser.AtomContext ctx) { - if (ctx.conditionCall() != null) { - return visit(ctx.conditionCall()); - } - return visit(ctx.expression()); + this.context = context; } @Override public Boolean visitConditionCall(BooleanConditionParser.ConditionCallContext ctx) { String conditionName = ctx.Identifier().getText(); - WorkflowConditionProvider conditionProvider = getConditionProvider(session, conditionName, extractParameter(ctx.parameter())); - return conditionProvider.evaluate(event); + WorkflowConditionProvider conditionProvider = getConditionProvider(session, conditionName, super.extractParameter(ctx.parameter())); + return conditionProvider.evaluate(context); } - protected String extractParameter(BooleanConditionParser.ParameterContext paramCtx) { - // Case 1: No parentheses were used (e.g., "user-logged-in") - // Case 2: Empty parentheses were used (e.g., "user-logged-in()") - if (paramCtx == null || paramCtx.ParameterText() == null) { - return null; - } - - // Case 3: A parameter was provided (e.g., "has-role(param)") - String rawText = paramCtx.ParameterText().getText(); - return unEscapeParameter(rawText); - } - - /** - * The grammar defines escapes as '\)' and '\\'. - * @param rawText The raw text from the ParameterText token. - * @return A clean, un-escaped string. - */ - private String unEscapeParameter(String rawText) { - // This handles both \) -> ) and \\ -> \ - // Note: replaceAll uses regex, so we must double-escape the backslashes - return rawText.replace("\\)", ")") - .replace("\\\\", "\\"); - } } diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/ConditionNameCollector.java b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/ConditionNameCollector.java new file mode 100644 index 00000000000..bc9a5cc9936 --- /dev/null +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/ConditionNameCollector.java @@ -0,0 +1,76 @@ +package org.keycloak.models.workflow.conditions.expression; + +import java.util.ArrayList; +import java.util.List; + +/** + * This visitor traverses the entire parse tree and collects the names of all conditionCalls. + */ +public class ConditionNameCollector extends BooleanConditionParserBaseVisitor { + + // 1. A list to store the names we find. + private final List conditionNames = new ArrayList<>(); + + /** + * Returns the list of all collected condition call names. + */ + public List getConditionNames() { + return conditionNames; + } + + // --- Traversal Methods --- + // These methods are necessary to ensure we visit every node in the tree. + + @Override + public Void visitEvaluator(BooleanConditionParser.EvaluatorContext ctx) { + return visit(ctx.expression()); + } + + @Override + public Void visitExpression(BooleanConditionParser.ExpressionContext ctx) { + // Visit both sides of the 'OR' + if (ctx.expression() != null) { + visit(ctx.expression()); + } + return visit(ctx.andExpression()); + } + + @Override + public Void visitAndExpression(BooleanConditionParser.AndExpressionContext ctx) { + // Visit both sides of the 'AND' + if (ctx.andExpression() != null) { + visit(ctx.andExpression()); + } + return visit(ctx.notExpression()); + } + + @Override + public Void visitNotExpression(BooleanConditionParser.NotExpressionContext ctx) { + // Visit the inner expression of the 'NOT' + if (ctx.notExpression() != null) { + return visit(ctx.notExpression()); + } + return visit(ctx.atom()); + } + + @Override + public Void visitAtom(BooleanConditionParser.AtomContext ctx) { + // This is the key: decide whether to visit a conditionCall + // or a nested expression. + if (ctx.conditionCall() != null) { + return visit(ctx.conditionCall()); + } + return visit(ctx.expression()); + } + + // --- The Collector Method --- + + @Override + public Void visitConditionCall(BooleanConditionParser.ConditionCallContext ctx) { + String conditionName = ctx.Identifier().getText(); + conditionNames.add(conditionName); + + // We don't need to visit children (like 'parameter') + return null; + } +} diff --git a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/EventEvaluator.java b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/EventEvaluator.java index 55551719b9a..6b33ffcfe8f 100644 --- a/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/EventEvaluator.java +++ b/model/jpa/src/main/java/org/keycloak/models/workflow/conditions/expression/EventEvaluator.java @@ -4,10 +4,12 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.workflow.ResourceOperationType; import org.keycloak.models.workflow.WorkflowEvent; -public class EventEvaluator extends ConditionEvaluator { +public class EventEvaluator extends AbstractBooleanEvaluator { + + private final WorkflowEvent event; public EventEvaluator(KeycloakSession session, WorkflowEvent event) { - super(session, event); + this.event = event; } @Override @@ -15,6 +17,6 @@ public class EventEvaluator extends ConditionEvaluator { String name = ctx.Identifier().getText(); ResourceOperationType operation = ResourceOperationType.valueOf(name.replace("-", "_").toUpperCase()); String param = super.extractParameter(ctx.parameter()); - return operation.test(super.event, param); + return operation.test(event, param); } } diff --git a/server-spi-private/src/main/java/org/keycloak/models/workflow/ResourceType.java b/server-spi-private/src/main/java/org/keycloak/models/workflow/ResourceType.java index 30874da8b28..6b1a0b286c7 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/workflow/ResourceType.java +++ b/server-spi-private/src/main/java/org/keycloak/models/workflow/ResourceType.java @@ -59,7 +59,7 @@ public enum ResourceType { && this.supportedAdminOperationTypes.contains(event.getOperationType())) { ResourceOperationType resourceOperationType = toOperationType(event.getOperationType()); - if (resourceOperationType != null) { + if (resourceOperationType != null && event.getResourceId() != null) { return new WorkflowEvent(this, resourceOperationType, event.getResourceId(), event); } } 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 84d1211edbe..223a80dd085 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 @@ -17,19 +17,19 @@ package org.keycloak.models.workflow; -import java.time.Duration; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.stream.Stream; -import org.keycloak.common.util.DurationConverter; +import jakarta.ws.rs.BadRequestException; + import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.component.ComponentModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.ModelValidationException; import org.keycloak.models.RealmModel; import org.keycloak.representations.workflows.WorkflowStepRepresentation; -import org.keycloak.utils.StringUtil; import static java.util.Optional.ofNullable; @@ -39,10 +39,10 @@ import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_NA public class Workflow { + private final String id; private final RealmModel realm; private final KeycloakSession session; private MultivaluedHashMap config; - private String id; private String notBefore; public Workflow(KeycloakSession session, ComponentModel c) { @@ -52,9 +52,10 @@ public class Workflow { this.config = c.getConfig(); } - public Workflow(KeycloakSession session, Map> config) { + public Workflow(KeycloakSession session, String id, Map> config) { this.session = session; this.realm = session.getContext().getRealm(); + this.id = id; MultivaluedHashMap c = new MultivaluedHashMap<>(); config.forEach(c::addAll); this.config = c; @@ -98,6 +99,21 @@ public class Workflow { config.putSingle(CONFIG_ERROR, message); } + public void updateConfig(MultivaluedHashMap config, List steps) { + ComponentModel component = getWorkflowComponent(this.id, WorkflowProvider.class.getName()); + component.setConfig(config); + realm.updateComponent(component); + + // check if there are steps to be updated as well + if (steps != null) { + steps.forEach(step -> { + ComponentModel stepComponent = getWorkflowComponent(step.getId(), WorkflowStepProvider.class.getName()); + stepComponent.setConfig(step.getConfig()); + realm.updateComponent(stepComponent); + }); + } + } + public Stream getSteps() { return realm.getComponentsStream(getId(), WorkflowStepProvider.class.getName()) .map(WorkflowStep::new).sorted(); @@ -137,33 +153,15 @@ public class Workflow { } private WorkflowStep toModel(WorkflowStepRepresentation rep) { - validateStep(rep); return new WorkflowStep(rep.getUses(), rep.getConfig()); } - private void validateStep(WorkflowStepRepresentation step) throws ModelValidationException { + private ComponentModel getWorkflowComponent(String id, String providerType) { + ComponentModel component = realm.getComponent(id); - // validate the step rep has 'uses' defined - if (StringUtil.isBlank(step.getUses())) { - throw new ModelValidationException("Step 'uses' cannot be null or empty."); - } - - // validate the after time, if present - try { - Duration duration = DurationConverter.parseDuration(step.getAfter()); - if (duration != null && duration.isNegative()) { // duration can only be null if the config is not set - throw new ModelValidationException("Step 'after' configuration cannot be negative."); - } - } catch (IllegalArgumentException e) { - throw new ModelValidationException("Step 'after' configuration is not valid: " + step.getAfter()); - } - - // verify the step does have valid provider - WorkflowStepProviderFactory factory = (WorkflowStepProviderFactory) session - .getKeycloakSessionFactory().getProviderFactory(WorkflowStepProvider.class, step.getUses()); - - if (factory == null) { - throw new WorkflowInvalidStateException("Step not found: " + step.getUses()); + if (component == null || !Objects.equals(providerType, component.getProviderType())) { + throw new BadRequestException("Not a valid resource workflow: " + id); } + return component; } } diff --git a/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowConditionProvider.java b/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowConditionProvider.java index b300c5f37d8..a9208df26f5 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowConditionProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowConditionProvider.java @@ -9,7 +9,7 @@ import org.keycloak.provider.Provider; public interface WorkflowConditionProvider extends Provider { - boolean evaluate(WorkflowEvent event); + boolean evaluate(WorkflowExecutionContext context); default Predicate toPredicate(CriteriaBuilder cb, CriteriaQuery query, Root resourceRoot) { return null; diff --git a/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowExecutionContext.java b/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowExecutionContext.java index 535c5f1c7d7..5d7442e4915 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowExecutionContext.java +++ b/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowExecutionContext.java @@ -11,4 +11,12 @@ public interface WorkflowExecutionContext { * @return the id of the resource */ String getResourceId(); + + /** + * Returns the workflow event that activated the current workflow execution. Can be null if the execution is being + * resumed from a scheduled step. + * + * @return the event bound to the current execution. + */ + WorkflowEvent getEvent(); } diff --git a/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowStateProvider.java b/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowStateProvider.java index 2396c8d8391..50d74bcc26f 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowStateProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/models/workflow/WorkflowStateProvider.java @@ -57,6 +57,14 @@ public interface WorkflowStateProvider extends Provider { */ void removeAll(); + /** + * Checks whether there are any scheduled steps for the given {@code workflowId}. + * + * @param workflowId the id of the workflow. + * @return {@code true} if there are scheduled steps, {@code false} otherwise. + */ + boolean hasScheduledSteps(String workflowId); + void scheduleStep(Workflow workflow, WorkflowStep step, String resourceId, String executionId); ScheduledStep getScheduledStep(String workflowId, String resourceId); diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/ExpressionConditionWorkflowTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/ExpressionConditionWorkflowTest.java index ffac4d185f6..153f4cb9f6c 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/ExpressionConditionWorkflowTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/ExpressionConditionWorkflowTest.java @@ -27,6 +27,7 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -109,16 +110,10 @@ public class ExpressionConditionWorkflowTest extends AbstractWorkflowTest { checkWorkflowRunsForUser("hdent", false); managedRealm.admin().workflows().workflow(workflowId).delete().close(); - // a malformed expression should cause the condition to evaluate to false and the step should not run for all users + // a malformed expression should cause the condition to evaluate to false and the workflow should not be created expression = ")(has-role(tester) AND OR has-user-attribute(key, value1,value2)"; - workflowId = createWorkflow(expression); - - checkWorkflowRunsForUser("bwayne", false); - checkWorkflowRunsForUser("lfox", false); - checkWorkflowRunsForUser("jgordon", false); - checkWorkflowRunsForUser("hdent", false); - managedRealm.admin().workflows().workflow(workflowId).delete().close(); - + workflowId = createWorkflow(expression, false); + assertThat(workflowId, nullValue()); } public void checkWorkflowRunsForUser(String username, boolean shouldHaveAttribute) { @@ -174,6 +169,10 @@ public class ExpressionConditionWorkflowTest extends AbstractWorkflowTest { } private String createWorkflow(String expression) { + return this.createWorkflow(expression, true); + } + + private String createWorkflow(String expression, boolean creationExpectedToSucceed) { WorkflowRepresentation expectedWorkflow = WorkflowRepresentation.withName("myworkflow") .onEvent("user-logged-in(test-app)") .onCondition(expression) @@ -188,7 +187,12 @@ public class ExpressionConditionWorkflowTest extends AbstractWorkflowTest { WorkflowsResource workflows = managedRealm.admin().workflows(); try (Response response = workflows.create(expectedWorkflow)) { - assertThat(response.getStatus(), is(Response.Status.CREATED.getStatusCode())); + if (creationExpectedToSucceed) { + assertThat(response.getStatus(), is(Response.Status.CREATED.getStatusCode())); + } else { + assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode())); + return null; + } return ApiUtil.getCreatedId(response); } } 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 db768ace2e5..a9f56cf03fa 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 @@ -42,6 +42,7 @@ import org.keycloak.models.workflow.DeleteUserStepProviderFactory; import org.keycloak.models.workflow.DisableUserStepProviderFactory; import org.keycloak.models.workflow.NotifyUserStepProviderFactory; import org.keycloak.models.workflow.ResourceOperationType; +import org.keycloak.models.workflow.ResourceType; import org.keycloak.models.workflow.RestartWorkflowStepProviderFactory; import org.keycloak.models.workflow.SetUserAttributeStepProviderFactory; import org.keycloak.models.workflow.Workflow; @@ -50,15 +51,20 @@ import org.keycloak.models.workflow.WorkflowStateProvider; import org.keycloak.models.workflow.WorkflowStateProvider.ScheduledStep; import org.keycloak.models.workflow.WorkflowStep; import org.keycloak.models.workflow.conditions.IdentityProviderWorkflowConditionFactory; +import org.keycloak.models.workflow.conditions.RoleWorkflowConditionFactory; import org.keycloak.representations.idm.ErrorRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.workflows.WorkflowRepresentation; import org.keycloak.representations.workflows.WorkflowStepRepresentation; import org.keycloak.testframework.annotations.InjectAdminClient; import org.keycloak.testframework.annotations.InjectKeycloakUrls; +import org.keycloak.testframework.annotations.InjectUser; 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.ManagedUser; +import org.keycloak.testframework.realm.UserConfig; import org.keycloak.testframework.realm.UserConfigBuilder; import org.keycloak.testframework.remote.providers.runonserver.RunOnServer; import org.keycloak.testframework.server.KeycloakUrls; @@ -77,7 +83,9 @@ import org.junit.jupiter.api.Test; import static org.keycloak.models.workflow.ResourceOperationType.USER_ADDED; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; @@ -90,6 +98,9 @@ import static org.junit.jupiter.api.Assertions.assertTrue; @KeycloakIntegrationTest(config = WorkflowsBlockingServerConfig.class) public class WorkflowManagementTest extends AbstractWorkflowTest { + @InjectUser(ref = "alice", config = DefaultUserConfig.class, lifecycle = LifeCycle.METHOD, realmRef = DEFAULT_REALM_NAME) + private ManagedUser userAlice; + @InjectMailServer private MailServer mailServer; @@ -119,9 +130,9 @@ public class WorkflowManagementTest extends AbstractWorkflowTest { } List actualWorkflows = workflows.list(); - assertThat(actualWorkflows, Matchers.hasSize(1)); + assertThat(actualWorkflows, hasSize(1)); - assertThat(actualWorkflows.get(0).getSteps(), Matchers.hasSize(2)); + assertThat(actualWorkflows.get(0).getSteps(), hasSize(2)); assertThat(actualWorkflows.get(0).getSteps().get(0).getUses(), is(NotifyUserStepProviderFactory.ID)); assertThat(actualWorkflows.get(0).getSteps().get(1).getUses(), is(DisableUserStepProviderFactory.ID)); assertThat(actualWorkflows.get(0).getState(), is(nullValue())); @@ -198,11 +209,11 @@ public class WorkflowManagementTest extends AbstractWorkflowTest { managedRealm.admin().users().create(UserConfigBuilder.create().username("testuser").email("testuser@example.com").build()).close(); List actualWorkflows = workflows.list(); - assertThat(actualWorkflows, Matchers.hasSize(2)); + assertThat(actualWorkflows, hasSize(2)); workflows.workflow(workflowId).delete().close(); actualWorkflows = workflows.list(); - assertThat(actualWorkflows, Matchers.hasSize(1)); + assertThat(actualWorkflows, hasSize(1)); runOnServer.run((RunOnServer) session -> { WorkflowProvider provider = session.getProvider(WorkflowProvider.class); @@ -216,7 +227,7 @@ public class WorkflowManagementTest extends AbstractWorkflowTest { } @Test - public void testUpdate() { + public void testUpdateWorkflowWithNoScheduledSteps() { WorkflowRepresentation workflowRep = WorkflowRepresentation.withName("test-workflow") .withSteps( WorkflowStepRepresentation.create().of(NotifyUserStepProviderFactory.ID) @@ -236,37 +247,151 @@ public class WorkflowManagementTest extends AbstractWorkflowTest { } List actualWorkflows = workflows.list(); - assertThat(actualWorkflows, Matchers.hasSize(1)); + assertThat(actualWorkflows, hasSize(1)); WorkflowRepresentation workflow = actualWorkflows.get(0); assertThat(workflow.getName(), is("test-workflow")); + // while the workflow has no scheduled steps - i.e. no resource is currently going through the workflow - we can update any property workflow.setName("changed"); - managedRealm.admin().workflows().workflow(workflowId).update(workflow).close(); - actualWorkflows = workflows.list(); - workflow = actualWorkflows.get(0); - assertThat(workflow.getName(), is("changed")); - - // now let's try to update another property that we can't update - String previousOn = workflow.getOn(); - workflow.setOn(ResourceOperationType.USER_LOGGED_IN.toString()); - try (Response response = workflows.workflow(workflowId).update(workflow)) { - assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode())); - } - - // restore previous value, but change the conditions - workflow.setOn(previousOn); workflow.setConditions(IdentityProviderWorkflowConditionFactory.ID + "(someidp)"); - try (Response response = workflows.workflow(workflowId).update(workflow)) { + workflow.setOn("user-logged-in"); + + managedRealm.admin().workflows().workflow(workflow.getId()).update(workflow).close(); + workflow = workflows.workflow(workflow.getId()).toRepresentation(); + assertThat(workflow.getName(), is("changed")); + assertThat(workflow.getOn(), is("user-logged-in")); + assertThat(workflow.getConditions(), is(IdentityProviderWorkflowConditionFactory.ID + "(someidp)")); + + // even adding or removing steps should be allowed + WorkflowStepRepresentation newStep = WorkflowStepRepresentation.create().of(DeleteUserStepProviderFactory.ID) + .after(Duration.ofDays(10)) + .build(); + workflow.getSteps().remove(1); // remove the disable step + workflow.getSteps().get(0).getConfig().putSingle("custom_message", "Your account will be disabled"); // change the notify step config + workflow.getSteps().add(newStep); // add a new delete step + + managedRealm.admin().workflows().workflow(workflow.getId()).update(workflow).close(); + workflow = workflows.workflow(workflow.getId()).toRepresentation(); + assertThat(workflow.getSteps(), hasSize(2)); + assertThat(workflow.getSteps().get(0).getUses(), is(NotifyUserStepProviderFactory.ID)); + assertThat(workflow.getSteps().get(0).getConfig().getFirst("custom_message"), is("Your account will be disabled")); + assertThat(workflow.getSteps().get(1).getUses(), is(DeleteUserStepProviderFactory.ID)); + } + + @Test + public void testUpdateWorkflowWithScheduledSteps() { + WorkflowRepresentation expectedWorkflows = WorkflowRepresentation.withName("test-workflow") + .withSteps( + WorkflowStepRepresentation.create().of(NotifyUserStepProviderFactory.ID) + .after(Duration.ofDays(5)) + .build(), + WorkflowStepRepresentation.create().of(DisableUserStepProviderFactory.ID) + .after(Duration.ofDays(10)) + .build() + ).build(); + + WorkflowsResource workflows = managedRealm.admin().workflows(); + + String workflowId; + try (Response response = workflows.create(expectedWorkflows)) { + assertThat(response.getStatus(), is(Response.Status.CREATED.getStatusCode())); + workflowId = ApiUtil.getCreatedId(response); + } + + // bind the workflow to a resource, so it schedules the first step + managedRealm.admin().workflows().workflow(workflowId).activate(ResourceType.USERS.name(), userAlice.getId()); + + // when a scheduled step exists, we cannot change the 'on' event, nor the number or order of steps. Individual step config can still be updated, except for the 'uses'. + WorkflowRepresentation workflow = managedRealm.admin().workflows().workflow(workflowId).toRepresentation(); + workflow.setName("changed"); + workflow.setConditions(IdentityProviderWorkflowConditionFactory.ID + "(someidp)"); + workflow.getSteps().get(0).getConfig().putSingle("custom_message", "Your account will be disabled"); // modify one of the steps config + + managedRealm.admin().workflows().workflow(workflow.getId()).update(workflow).close(); + workflow = workflows.workflow(workflow.getId()).toRepresentation(); + assertThat(workflow.getName(), is("changed")); + assertThat(workflow.getConditions(), is(IdentityProviderWorkflowConditionFactory.ID + "(someidp)")); + assertThat(workflow.getSteps().get(0).getConfig().getFirst("custom_message"), is("Your account will be disabled")); + + // now let's try to update the 'on' event - should fail + workflow.setOn("user-logged-in"); + try (Response response = workflows.workflow(workflow.getId()).update(workflow)) { assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode())); } - // revert conditions, but change one of the steps - workflow.setConditions(null); - workflow.getSteps().get(0).setAfter("8D"); - try (Response response = workflows.workflow(workflowId).update(workflow)) { + // restore the 'on' value, but try removing a step + workflow.setOn(null); + WorkflowStepRepresentation removedStep = workflow.getSteps().remove(1); // remove disable step + try (Response response = workflows.workflow(workflow.getId()).update(workflow)) { assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode())); } + // restore the step, but invert the order of the steps + workflow.getSteps().add(0, removedStep); + try (Response response = workflows.workflow(workflow.getId()).update(workflow)) { + assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode())); + } + + // restore the original order, but try changing the 'uses' of one step (i.e. replace it with something else) + workflow.getSteps().remove(0); // this will put notify back as the first step. + WorkflowStepRepresentation newStep = WorkflowStepRepresentation.create().of(DeleteUserStepProviderFactory.ID) + .after(Duration.ofDays(10)) + .build(); + workflow.getSteps().add(newStep); // we've added a delete step in the place of the disable step, with same config + try (Response response = workflows.workflow(workflow.getId()).update(workflow)) { + assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode())); + } + } + + @Test + public void testUpdateWorkflowConditionsCancelsExecutionForAffectedResources() { + WorkflowRepresentation expectedWorkflows = WorkflowRepresentation.withName("test-workflow") + .withSteps( + WorkflowStepRepresentation.create().of(DisableUserStepProviderFactory.ID) + .after(Duration.ofDays(5)) + .build() + ).build(); + + WorkflowsResource workflows = managedRealm.admin().workflows(); + + String workflowId; + try (Response response = workflows.create(expectedWorkflows)) { + assertThat(response.getStatus(), is(Response.Status.CREATED.getStatusCode())); + workflowId = ApiUtil.getCreatedId(response); + } + + // bind the workflow to a resource, so it schedules the first step + managedRealm.admin().workflows().workflow(workflowId).activate(ResourceType.USERS.name(), userAlice.getId()); + + // check step has been scheduled for the user + runOnServer.run((RunOnServer) session -> { + RealmModel realm = session.getContext().getRealm(); + UserModel user = session.users().getUserByUsername(realm, "alice"); + + WorkflowStateProvider stateProvider = session.getProvider(WorkflowStateProvider.class); + List scheduledSteps = stateProvider.getScheduledStepsByResource(user.getId()); + assertThat("A step should have been scheduled for the user " + user.getUsername(), scheduledSteps, hasSize(1)); + }); + + // now update the workflow to add a condition that will make the user no longer eligible + WorkflowRepresentation workflow = managedRealm.admin().workflows().workflow(workflowId).toRepresentation(); + workflow.setConditions(RoleWorkflowConditionFactory.ID + "(realm-management/realm-admin)"); + managedRealm.admin().workflows().workflow(workflowId).update(workflow).close(); + + // simulate running the step - user should no longer be eligible, so the step should be cancelled + runScheduledSteps(Duration.ofDays(6)); + + // check the user is still enabled and no scheduled steps exist + runOnServer.run((RunOnServer) session -> { + RealmModel realm = session.getContext().getRealm(); + UserModel user = session.users().getUserByUsername(realm, "alice"); + assertThat(user.isEnabled(), is(true)); + + WorkflowStateProvider stateProvider = session.getProvider(WorkflowStateProvider.class); + List scheduledSteps = stateProvider.getScheduledStepsByResource(user.getId()); + assertThat(scheduledSteps, empty()); + }); + } @Test @@ -285,27 +410,27 @@ public class WorkflowManagementTest extends AbstractWorkflowTest { // use the API to search for workflows by name, both partial and exact matches WorkflowsResource workflows = managedRealm.admin().workflows(); - List representations = workflows.list("alpha", false, null, null); - assertThat(representations, Matchers.hasSize(1)); + List representations = workflows.list("alpha", false, null, null); + assertThat(representations, hasSize(1)); assertThat(representations.get(0).getName(), is("alpha-workflow")); - representations = workflows.list("workflow", false, null, null); - assertThat(representations, Matchers.hasSize(4)); - representations = workflows.list("beta-workflow", true, null, null); - assertThat(representations, Matchers.hasSize(1)); + representations = workflows.list("workflow", false, null, null); + assertThat(representations, hasSize(4)); + representations = workflows.list("beta-workflow", true, null, null); + assertThat(representations, hasSize(1)); assertThat(representations.get(0).getName(), is("beta-workflow")); - representations = workflows.list("nonexistent", false, null, null); - assertThat(representations, Matchers.hasSize(0)); + representations = workflows.list("nonexistent", false, null, null); + assertThat(representations, hasSize(0)); // test pagination parameters - representations = workflows.list(null, null, 1, 2); - assertThat(representations, Matchers.hasSize(2)); + representations = workflows.list(null, null, 1, 2); + assertThat(representations, hasSize(2)); // returned workflows should be ordered by name assertThat(representations.get(0).getName(), is("beta-workflow")); assertThat(representations.get(1).getName(), is("delta-workflow")); - representations = workflows.list("gamma", false, 0, 10); - assertThat(representations, Matchers.hasSize(1)); + representations = workflows.list("gamma", false, 0, 10); + assertThat(representations, hasSize(1)); assertThat(representations.get(0).getName(), is("gamma-workflow")); } @@ -329,7 +454,7 @@ public class WorkflowManagementTest extends AbstractWorkflowTest { runOnServer.run((RunOnServer) session -> { RealmModel realm = session.getContext().getRealm(); WorkflowProvider provider = session.getProvider(WorkflowProvider.class); - UserModel user = session.users().getUserByUsername(realm,"testuser"); + UserModel user = session.users().getUserByUsername(realm, "testuser"); List registeredWorkflows = provider.getWorkflows().toList(); assertEquals(1, registeredWorkflows.size()); @@ -351,7 +476,7 @@ public class WorkflowManagementTest extends AbstractWorkflowTest { runOnServer.run((RunOnServer) session -> { RealmModel realm = session.getContext().getRealm(); WorkflowProvider provider = session.getProvider(WorkflowProvider.class); - UserModel user = session.users().getUserByUsername(realm,"testuser"); + UserModel user = session.users().getUserByUsername(realm, "testuser"); try { user = session.users().getUserById(realm, user.getId()); @@ -373,7 +498,7 @@ public class WorkflowManagementTest extends AbstractWorkflowTest { // Verify that the first step (notify) was executed by checking email was sent MimeMessage testUserMessage = findEmailByRecipient(mailServer, "testuser@example.com"); assertNotNull(testUserMessage, "The first step (notify) should have sent an email."); - + mailServer.runCleanup(); } @@ -529,7 +654,7 @@ public class WorkflowManagementTest extends AbstractWorkflowTest { WorkflowsResource workflows = managedRealm.admin().workflows(); List actualWorkflows = workflows.list(); - assertThat(actualWorkflows, Matchers.hasSize(1)); + assertThat(actualWorkflows, hasSize(1)); WorkflowRepresentation workflow = actualWorkflows.get(0); assertThat(workflow.getName(), is("test-workflow")); @@ -947,7 +1072,7 @@ public class WorkflowManagementTest extends AbstractWorkflowTest { } public static void verifyEmailContent(MimeMessage message, String expectedRecipient, String subjectContains, - String... contentContains) { + String... contentContains) { try { assertEquals(expectedRecipient, MailUtils.getRecipient(message)); assertThat(message.getSubject(), Matchers.containsString(subjectContains)); @@ -968,4 +1093,16 @@ public class WorkflowManagementTest extends AbstractWorkflowTest { Assertions.fail("Failed to read email message: " + e.getMessage()); } } + + private static class DefaultUserConfig implements UserConfig { + + @Override + public UserConfigBuilder configure(UserConfigBuilder user) { + user.username("alice"); + user.password("alice"); + user.name("alice", "alice"); + user.email("master-admin@email.org"); + return user; + } + } }