External-Mirrors/keycloak
2
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-01-10 15:32:05 -03:30
Code Issues Packages Projects Releases Wiki Activity

Add validation of workflow steps also when adding single step to workflow

Browse Source 
Closes #42833

Signed-off-by: vramik <vramik@redhat.com>
This commit is contained in:
vramik 2025-09-23 11:26:41 +02:00 committed by Pedro Igor
parent 5cefc497fd
commit cfec364b17
6 changed files with 199 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
services/src/main/java/org/keycloak/models/workflow/WorkflowsManager.java
View File

@ -91,11 +91,6 @@ public class WorkflowsManager {
for (int i = 0; i < steps.size(); i++) {
WorkflowStep step = steps.get(i);
if (workflow.getId().equals(parentId)) {
// only validate top-level steps, sub-steps are validated as part of the parent step validation
validateStep(workflow, step);
}
// assign priority based on index.
step.setPriority(i + 1);
@ -311,6 +306,7 @@ public class WorkflowsManager {
}
public void updateWorkflow(Workflow workflow, MultivaluedHashMap<String, String> config) {
validateWorkflow(toRepresentation(workflow), config);
ComponentModel component = getWorkflowComponent(workflow.getId());
component.setConfig(config);
getRealm().updateComponent(component);
@ -398,7 +394,7 @@ public class WorkflowsManager {
List<WorkflowStep> steps = new ArrayList<>();
for (WorkflowStepRepresentation stepRep : rep.getSteps()) {
steps.add(toModel(stepRep));
steps.add(toModel(workflow, stepRep));
}
addSteps(workflow, workflow.getId(), steps);
@ -441,16 +437,6 @@ public class WorkflowsManager {
}
}
private WorkflowStep toModel(WorkflowStepRepresentation rep) {
List<WorkflowStep> subSteps = new ArrayList<>();
for (WorkflowStepRepresentation subStep : ofNullable(rep.getSteps()).orElse(List.of())) {
subSteps.add(toModel(subStep));
}
return new WorkflowStep(rep.getUses(), rep.getConfig(), subSteps);
}
public void bind(Workflow workflow, ResourceType type, String resourceId) {
processEvent(List.of(workflow), new AdhocWorkflowEvent(type, resourceId));
}
@ -461,7 +447,11 @@ public class WorkflowsManager {
return type.resolveResource(session, resourceId);
}
private void validateStep(Workflow workflow, WorkflowStep step) throws ModelValidationException {
private void validateStep(Workflow workflow, WorkflowStep step, boolean topLevel) throws ModelValidationException {
if (step.getAfter() < 0) {
throw new ModelValidationException("Step 'after' time condition cannot be negative.");
}
boolean isAggregatedStep = !step.getSteps().isEmpty();
boolean isScheduledWorkflow = workflow.isScheduled();
@ -484,13 +474,15 @@ public class WorkflowsManager {
throw new ModelValidationException("Sub-steps of aggregated step cannot have time conditions.");
}
} else {
if (isScheduledWorkflow) {
if (step.getConfig().getFirst(CONFIG_AFTER) == null || step.getAfter() < 0) {
if (isScheduledWorkflow && topLevel) {
if (step.getConfig().getFirst(CONFIG_AFTER) == null) {
throw new ModelValidationException("All steps of scheduled workflow must have a valid 'after' time condition.");
}
} else { // immediate workflow
} else { // immediate workflow | sub-step of aggregated step
if (step.getConfig().getFirst(CONFIG_AFTER) != null) {
throw new ModelValidationException("Immediate workflow step cannot have a time condition.");
throw new ModelValidationException(topLevel ?
"Immediate workflow step cannot have a time condition." :
"Sub-step of aggregated step cannot have a time conditions.");
}
}
}
@ -586,14 +578,21 @@ public class WorkflowsManager {
}
}
public WorkflowStep toStepModel(WorkflowStepRepresentation rep) {
public WorkflowStep toModel(Workflow workflow, WorkflowStepRepresentation rep) {
return toModel(workflow, rep, true);
}
private WorkflowStep toModel(Workflow workflow, WorkflowStepRepresentation rep, boolean topLevel) {
List<WorkflowStep> subSteps = new ArrayList<>();
for (WorkflowStepRepresentation subStep : ofNullable(rep.getSteps()).orElse(List.of())) {
subSteps.add(toStepModel(subStep));
subSteps.add(toModel(workflow, subStep, false));
}
return new WorkflowStep(rep.getUses(), rep.getConfig(), subSteps);
WorkflowStep step = new WorkflowStep(rep.getUses(), rep.getConfig(), subSteps);
validateStep(workflow, step, topLevel);
return step;
}
public WorkflowConditionProvider getConditionProvider(String providerId, MultivaluedHashMap<String, String> modelConfig) {

3
services/src/main/java/org/keycloak/workflow/admin/resource/WorkflowResource.java
View File

@ -39,6 +39,9 @@ public class WorkflowResource {
}
}
/**
* Update the workflow configuration. The method does not update the workflow steps.
*/
@PUT
public void update(WorkflowRepresentation rep) {
try {

15
services/src/main/java/org/keycloak/workflow/admin/resource/WorkflowStepsResource.java
View File

@ -41,10 +41,12 @@ import org.eclipse.microprofile.openapi.annotations.responses.APIResponse;
import org.eclipse.microprofile.openapi.annotations.responses.APIResponses;
import org.eclipse.microprofile.openapi.annotations.tags.Tag;
import org.keycloak.models.ModelException;
import org.keycloak.models.workflow.WorkflowStep;
import org.keycloak.models.workflow.Workflow;
import org.keycloak.models.workflow.WorkflowsManager;
import org.keycloak.representations.workflows.WorkflowStepRepresentation;
import org.keycloak.services.ErrorResponse;
/**
* Resource for managing steps within a workflow.
@ -105,13 +107,16 @@ public class WorkflowStepsResource {
@Parameter(description = "Position to insert the step at (0-based index). If not specified, step is added at the end.")
@QueryParam("position") Integer position) {
if (stepRep == null) {
throw new BadRequestException("Step representation cannot be null");
throw ErrorResponse.error("Step representation cannot be null", Response.Status.BAD_REQUEST);
}
try {
WorkflowStep step = workflowsManager.toModel(workflow, stepRep);
WorkflowStep addedStep = workflowsManager.addStepToWorkflow(workflow.getId(), step, position);
WorkflowStep step = workflowsManager.toStepModel(stepRep);
WorkflowStep addedStep = workflowsManager.addStepToWorkflow(workflow.getId(), step, position);
return Response.ok(workflowsManager.toRepresentation(addedStep)).build();
return Response.ok(workflowsManager.toRepresentation(addedStep)).build();
} catch (ModelException e) {
throw ErrorResponse.error(e.getMessage(), Response.Status.BAD_REQUEST);
}
}
/**

58
tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/AggregatedStepTest.java
View File

@ -50,7 +50,7 @@ public class AggregatedStepTest {
@Test
public void testCreate() {
managedRealm.admin().workflows().create(WorkflowRepresentation.create()
try (Response response = managedRealm.admin().workflows().create(WorkflowRepresentation.create()
.of(UserCreationTimeWorkflowProviderFactory.ID)
.withSteps(
WorkflowStepRepresentation.create().of(AggregatedStepProviderFactory.ID)
@ -63,32 +63,33 @@ public class AggregatedStepTest {
.of(DisableUserStepProviderFactory.ID)
.build()
).build())
.build()).close();
List<WorkflowRepresentation> workflows = managedRealm.admin().workflows().list();
assertThat(workflows, hasSize(1));
WorkflowRepresentation workflow = workflows.get(0);
assertThat(workflow.getSteps(), hasSize(1));
WorkflowStepRepresentation aggregatedStep = workflow.getSteps().get(0);
assertThat(aggregatedStep.getUses(), is(AggregatedStepProviderFactory.ID));
List<WorkflowStepRepresentation> steps = aggregatedStep.getSteps();
assertThat(steps, hasSize(2));
assertStep(steps, SetUserAttributeStepProviderFactory.ID, a -> {
assertNotNull(a.getConfig());
assertThat(a.getConfig().isEmpty(), is(false));
assertThat(a.getConfig(), hasEntry("priority", List.of("1")));
assertThat(a.getConfig(), hasEntry("message", List.of("message")));
});
assertStep(steps, DisableUserStepProviderFactory.ID, a -> {
assertNotNull(a.getConfig());
assertThat(a.getConfig().isEmpty(), is(false));
assertThat(a.getConfig(), hasEntry("priority", List.of("2")));
});
.build())) {
assertThat(response.getStatus(), is(Status.CREATED.getStatusCode()));
List<WorkflowRepresentation> workflows = managedRealm.admin().workflows().list();
assertThat(workflows, hasSize(1));
WorkflowRepresentation workflow = workflows.get(0);
assertThat(workflow.getSteps(), hasSize(1));
WorkflowStepRepresentation aggregatedStep = workflow.getSteps().get(0);
assertThat(aggregatedStep.getUses(), is(AggregatedStepProviderFactory.ID));
List<WorkflowStepRepresentation> steps = aggregatedStep.getSteps();
assertThat(steps, hasSize(2));
assertStep(steps, SetUserAttributeStepProviderFactory.ID, a -> {
assertNotNull(a.getConfig());
assertThat(a.getConfig().isEmpty(), is(false));
assertThat(a.getConfig(), hasEntry("priority", List.of("1")));
assertThat(a.getConfig(), hasEntry("message", List.of("message")));
});
assertStep(steps, DisableUserStepProviderFactory.ID, a -> {
assertNotNull(a.getConfig());
assertThat(a.getConfig().isEmpty(), is(false));
assertThat(a.getConfig(), hasEntry("priority", List.of("2")));
});
}
}
@Test
public void testCreateAggregatedStepAsSubStep() {
managedRealm.admin().workflows().create(WorkflowRepresentation.create()
try (Response response = managedRealm.admin().workflows().create(WorkflowRepresentation.create()
.of(UserCreationTimeWorkflowProviderFactory.ID)
.withSteps(
WorkflowStepRepresentation.create().of(AggregatedStepProviderFactory.ID)
@ -110,10 +111,11 @@ public class AggregatedStepTest {
.of(DisableUserStepProviderFactory.ID)
.build()
).build())
.build()).close();
List<WorkflowRepresentation> workflows = managedRealm.admin().workflows().list();
assertThat(workflows, hasSize(1));
.build())) {
assertThat(response.getStatus(), is(Status.CREATED.getStatusCode()));
List<WorkflowRepresentation> workflows = managedRealm.admin().workflows().list();
assertThat(workflows, hasSize(1));
}
}
@Test
@ -157,7 +159,7 @@ public class AggregatedStepTest {
).build())
.build())) {
assertThat(response.getStatus(), is(Status.BAD_REQUEST.getStatusCode()));
assertThat(response.readEntity(ErrorRepresentation.class).getErrorMessage(), equalTo("Step provider " + SetUserAttributeStepProviderFactory.ID + " does not support aggregated steps"));
assertThat(response.readEntity(ErrorRepresentation.class).getErrorMessage(), equalTo("Sub-step of aggregated step cannot have a time conditions."));
}
}

69
tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/WorkflowManagementTest.java
View File

@ -28,6 +28,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_SCHEDULED;
import java.time.Duration;
import java.util.Arrays;
@ -629,6 +630,65 @@ public class WorkflowManagementTest {
}
}
@Test
public void testUpdateWorkflowFromImmediateToScheduled() {
// Create an immediate workflow
WorkflowSetRepresentation workflows = WorkflowRepresentation.create()
.of(UserCreationTimeWorkflowProviderFactory.ID)
.immediate()
.withSteps(
WorkflowStepRepresentation.create().of(NotifyUserStepProviderFactory.ID)
.build(),
WorkflowStepRepresentation.create().of(DisableUserStepProviderFactory.ID)
.build()
).build();
WorkflowsResource workflowsResource = managedRealm.admin().workflows();
try (Response response = workflowsResource.create(workflows)) {
assertThat(response.getStatus(), is(Response.Status.CREATED.getStatusCode()));
}
WorkflowRepresentation workflow = workflowsResource.list().get(0);
// Attempt to update the workflow to scheduled
workflow.getConfig().putSingle(CONFIG_SCHEDULED, "true");
try (Response response = workflowsResource.workflow(workflow.getId()).update(workflow)) {
assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode()));
String error = response.readEntity(String.class);
assertThat(error, containsString("Scheduled workflow cannot have steps without time conditions."));
}
}
@Test
public void testUpdateWorkflowFromScheduledToImmediate() {
// Create a scheduled workflow
WorkflowSetRepresentation workflows = WorkflowRepresentation.create()
.of(UserCreationTimeWorkflowProviderFactory.ID)
.withSteps(
WorkflowStepRepresentation.create().of(NotifyUserStepProviderFactory.ID)
.after(Duration.ofDays(1))
.build(),
WorkflowStepRepresentation.create().of(DisableUserStepProviderFactory.ID)
.after(Duration.ofDays(1))
.build()
).build();
WorkflowsResource workflowsResource = managedRealm.admin().workflows();
try (Response response = workflowsResource.create(workflows)) {
assertThat(response.getStatus(), is(Response.Status.CREATED.getStatusCode()));
}
WorkflowRepresentation workflow = workflowsResource.list().get(0);
// Attempt to update the workflow to immediate
workflow.setScheduled(false);
try (Response response = workflowsResource.workflow(workflow.getId()).update(workflow)) {
assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode()));
String error = response.readEntity(String.class);
assertThat(error, containsString("Immediate workflow cannot have steps with time conditions."));
}
}
@Test
public void testCreateScheduledWorkflowWithoutTimeConditions() {
WorkflowSetRepresentation workflows = WorkflowRepresentation.create()
@ -670,8 +730,8 @@ public class WorkflowManagementTest {
}
@Test
public void testFailCreateNegativeTime() {
try (Response response = managedRealm.admin().workflows().create(WorkflowRepresentation.create()
public void testFailCreateWorkflowWithNegativeTime() {
WorkflowSetRepresentation workflows = WorkflowRepresentation.create()
.of(UserCreationTimeWorkflowProviderFactory.ID)
.withSteps(
WorkflowStepRepresentation.create().of(SetUserAttributeStepProviderFactory.ID)
@ -685,9 +745,10 @@ public class WorkflowManagementTest {
.of(DisableUserStepProviderFactory.ID)
.build()
).build())
.build())) {
.build();
try (Response response = managedRealm.admin().workflows().create(workflows)) {
assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode()));
assertThat(response.readEntity(ErrorRepresentation.class).getErrorMessage(), equalTo("Step provider " + SetUserAttributeStepProviderFactory.ID + " does not support aggregated steps"));
assertThat(response.readEntity(ErrorRepresentation.class).getErrorMessage(), equalTo("Step 'after' time condition cannot be negative."));
}
}

68
tests/base/src/test/java/org/keycloak/tests/admin/model/workflow/WorkflowStepManagementTest.java
View File

@ -44,13 +44,23 @@ import org.keycloak.testframework.remote.runonserver.InjectRunOnServer;
import org.keycloak.testframework.remote.runonserver.RunOnServerClient;
import jakarta.ws.rs.core.Response;
import org.keycloak.testframework.util.ApiUtil;
import java.time.Duration;
import java.util.List;
import java.util.Map;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
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;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_SCHEDULED;
@KeycloakIntegrationTest(config = WorkflowsServerConfig.class)
public class WorkflowStepManagementTest {
@ -126,6 +136,64 @@ public class WorkflowStepManagementTest {
assertTrue(foundOurStep, "Our added step should be present in the workflow");
}
@Test
public void testAddStepWithTimeToImmediateWorkflow() {
workflowsResource.create(WorkflowRepresentation.create()
.of(UserCreationTimeWorkflowProviderFactory.ID)
.onEvent(ResourceOperationType.USER_ADD.toString())
.name("immediate-workflow")
.immediate()
.withSteps(
WorkflowStepRepresentation.create().of(NotifyUserStepProviderFactory.ID).build()
).build()).close();
String immediateWorkflowId = workflowsResource.list().stream()
.filter(wf -> "immediate-workflow".equals(wf.getName()))
.findFirst()
.map(WorkflowRepresentation::getId)
.orElse(null);
assertThat(immediateWorkflowId, notNullValue());
WorkflowRepresentation representation = workflowsResource.workflow(immediateWorkflowId).toRepresentation();
assertThat(representation.getConfig().getFirst(CONFIG_SCHEDULED), equalTo("false"));
// Attempt to add a step with 'after' time (should be rejected)
WorkflowStepRepresentation stepRep = new WorkflowStepRepresentation();
stepRep.setUses(DisableUserStepProviderFactory.ID);
stepRep.setConfig("name", "Invalid Step");
stepRep.setConfig("after", String.valueOf(Duration.ofDays(30).toMillis()));
try (Response response = workflowsResource.workflow(immediateWorkflowId).steps().create(stepRep)) {
assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode()));
String errorMessage = response.readEntity(String.class);
assertThat(errorMessage, containsString("Immediate workflow step cannot have a time condition."));
}
// Verify step was not added (should still be only the original setup step)
List<WorkflowStepRepresentation> allSteps = workflowsResource.workflow(immediateWorkflowId).steps().list();
assertThat(allSteps, hasSize(1));
}
@Test
public void testAddStepWithoutTimeToScheduledWorkflow() {
WorkflowRepresentation representation = workflowsResource.workflow(workflowId).toRepresentation();
assertThat(representation.getConfig().getFirst(CONFIG_SCHEDULED), nullValue());//immediate
// Attempt to add a step without 'after' time (should be rejected)
WorkflowStepRepresentation stepRep = WorkflowStepRepresentation.create()
.of(NotifyUserStepProviderFactory.ID)
.build();
try (Response response = workflowsResource.workflow(workflowId).steps().create(stepRep)) {
assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode()));
String errorMessage = response.readEntity(String.class);
assertThat(errorMessage, containsString("All steps of scheduled workflow must have a valid 'after' time condition."));
}
// Verify step was not added (should still be only the original setup step)
List<WorkflowStepRepresentation> allSteps = workflowsResource.workflow(workflowId).steps().list();
assertThat(allSteps, hasSize(1));
}
@Test
public void testRemoveStepFromWorkflow() {
WorkflowResource workflow = workflowsResource.workflow(workflowId);