mirror of
https://github.com/keycloak/keycloak.git
synced 2026-01-09 23:12:06 -03:30
Allow ISO-8601 compatible format for the after field in workflow steps
- aligns the format with what is used in the JPA connection provider pool max lifetime for time-based configurations Closes #42913 Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>
This commit is contained in:
parent
5ff2e22f18
commit
da7993896d
@ -0,0 +1,66 @@
|
||||
package org.keycloak.common.util;
|
||||
|
||||
import java.time.Duration;
|
||||
import java.time.format.DateTimeParseException;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
public class DurationConverter {
|
||||
|
||||
private static final String PERIOD = "P";
|
||||
private static final String PERIOD_OF_TIME = "PT";
|
||||
public static final Pattern DIGITS = Pattern.compile("^[-+]?\\d+$");
|
||||
private static final Pattern DIGITS_AND_UNIT = Pattern.compile("^(?:[-+]?\\d+(?:\\.\\d+)?(?i)[hms])+$");
|
||||
private static final Pattern DAYS = Pattern.compile("^[-+]?\\d+(?i)d$");
|
||||
private static final Pattern MILLIS = Pattern.compile("^[-+]?\\d+(?i)ms$");
|
||||
|
||||
/**
|
||||
* If the {@code value} starts with a number, then:
|
||||
* <ul>
|
||||
* <li>If the value is only a number, it is treated as a number of seconds.</li>
|
||||
* <li>If the value is a number followed by {@code ms}, it is treated as a number of milliseconds.</li>
|
||||
* <li>If the value is a number followed by {@code h}, {@code m}, or {@code s}, it is prefixed with {@code PT}
|
||||
* and {@link Duration#parse(CharSequence)} is called.</li>
|
||||
* <li>If the value is a number followed by {@code d}, it is prefixed with {@code P}
|
||||
* and {@link Duration#parse(CharSequence)} is called.</li>
|
||||
* </ul>
|
||||
*
|
||||
* Otherwise, {@link Duration#parse(CharSequence)} is called.
|
||||
*
|
||||
* @param value a string duration
|
||||
* @return the parsed {@link Duration}
|
||||
* @throws IllegalArgumentException in case of parse failure
|
||||
*/
|
||||
public static Duration parseDuration(String value) {
|
||||
if (value == null || value.trim().isEmpty()) {
|
||||
return null;
|
||||
}
|
||||
if (DIGITS.asPredicate().test(value)) {
|
||||
return Duration.ofSeconds(Long.parseLong(value));
|
||||
} else if (MILLIS.asPredicate().test(value)) {
|
||||
return Duration.ofMillis(Long.parseLong(value.substring(0, value.length() - 2)));
|
||||
}
|
||||
|
||||
try {
|
||||
if (DIGITS_AND_UNIT.asPredicate().test(value)) {
|
||||
return Duration.parse(PERIOD_OF_TIME + value);
|
||||
} else if (DAYS.asPredicate().test(value)) {
|
||||
return Duration.parse(PERIOD + value);
|
||||
}
|
||||
|
||||
return Duration.parse(value);
|
||||
} catch (DateTimeParseException e) {
|
||||
throw new IllegalArgumentException(e);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether the given value represents a positive duration.
|
||||
*
|
||||
* @param value a string duration following the same format as in {@link #parseDuration(String)}
|
||||
* @return true if the value represents a positive duration, false otherwise
|
||||
*/
|
||||
public static boolean isPositiveDuration(String value) {
|
||||
Duration duration = parseDuration(value);
|
||||
return duration != null && !duration.isNegative() && !duration.isZero();
|
||||
}
|
||||
}
|
||||
@ -52,8 +52,8 @@ public final class WorkflowStepRepresentation extends AbstractWorkflowComponentR
|
||||
return getConfigValue(CONFIG_AFTER, String.class);
|
||||
}
|
||||
|
||||
public void setAfter(long ms) {
|
||||
setConfig(CONFIG_AFTER, String.valueOf(ms));
|
||||
public void setAfter(String after) {
|
||||
setConfig(CONFIG_AFTER, after);
|
||||
}
|
||||
|
||||
public String getPriority() {
|
||||
@ -86,7 +86,11 @@ public final class WorkflowStepRepresentation extends AbstractWorkflowComponentR
|
||||
}
|
||||
|
||||
public Builder after(Duration duration) {
|
||||
step.setAfter(duration.toMillis());
|
||||
return after(String.valueOf(duration.getSeconds()));
|
||||
}
|
||||
|
||||
public Builder after(String after) {
|
||||
step.setAfter(after);
|
||||
return this;
|
||||
}
|
||||
|
||||
@ -95,15 +99,6 @@ public final class WorkflowStepRepresentation extends AbstractWorkflowComponentR
|
||||
return this;
|
||||
}
|
||||
|
||||
public Builder before(WorkflowStepRepresentation targetStep, Duration timeBeforeTarget) {
|
||||
// Calculate absolute time: targetStep.after - timeBeforeTarget
|
||||
String targetAfter = targetStep.getConfig().get(CONFIG_AFTER).get(0);
|
||||
long targetTime = Long.parseLong(targetAfter);
|
||||
long thisTime = targetTime - timeBeforeTarget.toMillis();
|
||||
step.setAfter(thisTime);
|
||||
return this;
|
||||
}
|
||||
|
||||
public Builder withConfig(String key, String value) {
|
||||
step.setConfig(key, value);
|
||||
return this;
|
||||
|
||||
@ -35,7 +35,7 @@ public interface WorkflowResource {
|
||||
@Path("bind/{type}/{resourceId}")
|
||||
@POST
|
||||
@Consumes(MediaType.APPLICATION_JSON)
|
||||
void bind(@PathParam("type") String type, @PathParam("resourceId") String resourceId, Long milliseconds);
|
||||
void bind(@PathParam("type") String type, @PathParam("resourceId") String resourceId, String notBefore);
|
||||
|
||||
@Path("deactivate/{type}/{resourceId}")
|
||||
@POST
|
||||
|
||||
@ -16,6 +16,7 @@ import java.util.stream.Stream;
|
||||
|
||||
import jakarta.ws.rs.BadRequestException;
|
||||
import org.jboss.logging.Logger;
|
||||
import org.keycloak.common.util.DurationConverter;
|
||||
import org.keycloak.common.util.MultivaluedHashMap;
|
||||
import org.keycloak.component.ComponentFactory;
|
||||
import org.keycloak.component.ComponentModel;
|
||||
@ -233,8 +234,8 @@ public class DefaultWorkflowProvider implements WorkflowProvider {
|
||||
if (isAlreadyScheduledInSession(event, workflow)) {
|
||||
return;
|
||||
}
|
||||
// If the workflow has a notBefore set, schedule the first step with it
|
||||
if (workflow.getNotBefore() != null && workflow.getNotBefore() > 0) {
|
||||
// If the workflow has a positive notBefore set, schedule the first step with it
|
||||
if (DurationConverter.isPositiveDuration(workflow.getNotBefore())) {
|
||||
scheduleWorkflow(event, workflow);
|
||||
} else {
|
||||
DefaultWorkflowExecutionContext context = new DefaultWorkflowExecutionContext(session, workflow, event);
|
||||
@ -321,7 +322,7 @@ public class DefaultWorkflowProvider implements WorkflowProvider {
|
||||
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);
|
||||
.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.");
|
||||
}
|
||||
|
||||
@ -24,12 +24,15 @@ import jakarta.persistence.criteria.CriteriaQuery;
|
||||
import jakarta.persistence.criteria.Predicate;
|
||||
import jakarta.persistence.criteria.Root;
|
||||
import org.jboss.logging.Logger;
|
||||
import org.keycloak.common.util.DurationConverter;
|
||||
import org.keycloak.common.util.Time;
|
||||
import org.keycloak.connections.jpa.JpaConnectionProvider;
|
||||
import org.keycloak.models.KeycloakSession;
|
||||
import org.keycloak.models.RealmModel;
|
||||
import org.keycloak.utils.StringUtil;
|
||||
|
||||
import java.time.Duration;
|
||||
import java.time.Instant;
|
||||
import java.util.List;
|
||||
|
||||
public class JpaWorkflowStateProvider implements WorkflowStateProvider {
|
||||
@ -57,17 +60,24 @@ public class JpaWorkflowStateProvider implements WorkflowStateProvider {
|
||||
@Override
|
||||
public void scheduleStep(Workflow workflow, WorkflowStep step, String resourceId, String executionId) {
|
||||
WorkflowStateEntity entity = em.find(WorkflowStateEntity.class, executionId);
|
||||
Duration duration = DurationConverter.parseDuration(step.getAfter());
|
||||
if (duration == null) {
|
||||
// shouldn't happen as the step duration should have been validated before
|
||||
throw new IllegalArgumentException("Invalid duration (%s) found when scheduling step %s in workflow %s"
|
||||
.formatted(step.getAfter(), step.getProviderId(), workflow.getName()));
|
||||
}
|
||||
|
||||
if (entity == null) {
|
||||
entity = new WorkflowStateEntity();
|
||||
entity.setResourceId(resourceId);
|
||||
entity.setWorkflowId(workflow.getId());
|
||||
entity.setExecutionId(executionId);
|
||||
entity.setScheduledStepId(step.getId());
|
||||
entity.setScheduledStepTimestamp(Time.currentTimeMillis() + step.getAfter());
|
||||
entity.setScheduledStepTimestamp(Instant.now().plus(duration).toEpochMilli());
|
||||
em.persist(entity);
|
||||
} else {
|
||||
entity.setScheduledStepId(step.getId());
|
||||
entity.setScheduledStepTimestamp(Time.currentTimeMillis() + step.getAfter());
|
||||
entity.setScheduledStepTimestamp(Instant.now().plus(duration).toEpochMilli());
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -3,6 +3,7 @@ package org.keycloak.models.workflow;
|
||||
import java.util.List;
|
||||
|
||||
import org.jboss.logging.Logger;
|
||||
import org.keycloak.common.util.DurationConverter;
|
||||
import org.keycloak.models.KeycloakSession;
|
||||
|
||||
class RunWorkflowTask extends WorkflowTransactionalTask {
|
||||
@ -34,7 +35,7 @@ class RunWorkflowTask extends WorkflowTransactionalTask {
|
||||
WorkflowStateProvider stateProvider = session.getProvider(WorkflowStateProvider.class);
|
||||
|
||||
for (WorkflowStep step : stepsToRun) {
|
||||
if (step.getAfter() > 0) {
|
||||
if (DurationConverter.isPositiveDuration(step.getAfter())) {
|
||||
// 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);
|
||||
|
||||
@ -32,7 +32,7 @@ final class ScheduleWorkflowTask extends WorkflowTransactionalTask {
|
||||
WorkflowStep firstStep = workflow.getSteps().findFirst().orElseThrow(() -> new WorkflowInvalidStateException("No steps found for workflow " + workflow.getName()));
|
||||
log.debugf("Scheduling first step '%s' of workflow '%s' for resource %s based on on event %s with notBefore %d",
|
||||
firstStep.getProviderId(), workflow.getName(), event.getResourceId(), event.getOperation(), workflow.getNotBefore());
|
||||
Long originalAfter = firstStep.getAfter();
|
||||
String originalAfter = firstStep.getAfter();
|
||||
try {
|
||||
firstStep.setAfter(workflow.getNotBefore());
|
||||
WorkflowStateProvider stateProvider = session.getProvider(WorkflowStateProvider.class);
|
||||
|
||||
@ -22,16 +22,19 @@ import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_EN
|
||||
import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_ERROR;
|
||||
import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_NAME;
|
||||
|
||||
import java.time.Duration;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.stream.Stream;
|
||||
|
||||
import org.keycloak.common.util.DurationConverter;
|
||||
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;
|
||||
|
||||
public class Workflow {
|
||||
|
||||
@ -39,7 +42,7 @@ public class Workflow {
|
||||
private final KeycloakSession session;
|
||||
private MultivaluedHashMap<String, String> config;
|
||||
private String id;
|
||||
private Long notBefore;
|
||||
private String notBefore;
|
||||
|
||||
public Workflow(KeycloakSession session, ComponentModel c) {
|
||||
this.session = session;
|
||||
@ -72,11 +75,11 @@ public class Workflow {
|
||||
return config != null && Boolean.parseBoolean(config.getFirstOrDefault(CONFIG_ENABLED, "true"));
|
||||
}
|
||||
|
||||
public Long getNotBefore() {
|
||||
public String getNotBefore() {
|
||||
return notBefore;
|
||||
}
|
||||
|
||||
public void setNotBefore(Long notBefore) {
|
||||
public void setNotBefore(String notBefore) {
|
||||
this.notBefore = notBefore;
|
||||
}
|
||||
|
||||
@ -133,22 +136,33 @@ public class Workflow {
|
||||
}
|
||||
|
||||
private WorkflowStep toModel(WorkflowStepRepresentation rep) {
|
||||
WorkflowStep step = new WorkflowStep(rep.getUses(), rep.getConfig());
|
||||
validateStep(step);
|
||||
return step;
|
||||
validateStep(rep);
|
||||
return new WorkflowStep(rep.getUses(), rep.getConfig());
|
||||
}
|
||||
|
||||
private void validateStep(WorkflowStep step) throws ModelValidationException {
|
||||
if (step.getAfter() < 0) {
|
||||
throw new ModelValidationException("Step 'after' time condition cannot be negative.");
|
||||
private void validateStep(WorkflowStepRepresentation step) throws ModelValidationException {
|
||||
|
||||
// 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<WorkflowStepProvider> factory = (WorkflowStepProviderFactory<WorkflowStepProvider>) session
|
||||
.getKeycloakSessionFactory().getProviderFactory(WorkflowStepProvider.class, step.getProviderId());
|
||||
.getKeycloakSessionFactory().getProviderFactory(WorkflowStepProvider.class, step.getUses());
|
||||
|
||||
if (factory == null) {
|
||||
throw new WorkflowInvalidStateException("Step not found: " + step.getProviderId());
|
||||
throw new WorkflowInvalidStateException("Step not found: " + step.getUses());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -81,12 +81,12 @@ public class WorkflowStep implements Comparable<WorkflowStep> {
|
||||
}
|
||||
}
|
||||
|
||||
public void setAfter(Long ms) {
|
||||
setConfig(CONFIG_AFTER, String.valueOf(ms));
|
||||
public void setAfter(String after) {
|
||||
setConfig(CONFIG_AFTER, after);
|
||||
}
|
||||
|
||||
public Long getAfter() {
|
||||
return Long.valueOf(getConfig().getFirstOrDefault(CONFIG_AFTER, "0"));
|
||||
public String getAfter() {
|
||||
return getConfig().getFirst(CONFIG_AFTER);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@ -20,6 +20,7 @@ package org.keycloak.models.workflow;
|
||||
import static org.keycloak.representations.workflows.WorkflowConstants.CONFIG_AFTER;
|
||||
|
||||
import org.jboss.logging.Logger;
|
||||
import org.keycloak.common.util.DurationConverter;
|
||||
import org.keycloak.component.ComponentModel;
|
||||
import org.keycloak.email.EmailException;
|
||||
import org.keycloak.email.EmailTemplateProvider;
|
||||
@ -119,21 +120,21 @@ public class NotifyUserStepProvider implements WorkflowStepProvider {
|
||||
}
|
||||
|
||||
private String getNextStepType() {
|
||||
Map<ComponentModel, Long> nextStepMap = getNextNonNotificationStep();
|
||||
Map<ComponentModel, Duration> nextStepMap = getNextNonNotificationStep();
|
||||
return nextStepMap.isEmpty() ? "unknown-step" : nextStepMap.keySet().iterator().next().getProviderId();
|
||||
}
|
||||
|
||||
private int calculateDaysUntilNextStep() {
|
||||
Map<ComponentModel, Long> nextStepMap = getNextNonNotificationStep();
|
||||
Map<ComponentModel, Duration> nextStepMap = getNextNonNotificationStep();
|
||||
if (nextStepMap.isEmpty()) {
|
||||
return 0;
|
||||
}
|
||||
Long timeToNextStep = nextStepMap.values().iterator().next();
|
||||
return Math.toIntExact(Duration.ofMillis(timeToNextStep).toDays());
|
||||
Duration timeToNextStep = nextStepMap.values().iterator().next();
|
||||
return Math.toIntExact(timeToNextStep.toDays());
|
||||
}
|
||||
|
||||
private Map<ComponentModel, Long> getNextNonNotificationStep() {
|
||||
long timeToNextNonNotificationStep = 0L;
|
||||
private Map<ComponentModel, Duration> getNextNonNotificationStep() {
|
||||
Duration timeToNextNonNotificationStep = Duration.ZERO;
|
||||
|
||||
RealmModel realm = session.getContext().getRealm();
|
||||
ComponentModel workflowModel = realm.getComponent(stepModel.getParentId());
|
||||
@ -150,7 +151,8 @@ public class NotifyUserStepProvider implements WorkflowStepProvider {
|
||||
boolean foundCurrent = false;
|
||||
for (ComponentModel step : steps) {
|
||||
if (foundCurrent) {
|
||||
timeToNextNonNotificationStep += step.get(CONFIG_AFTER, 0L);
|
||||
Duration duration = DurationConverter.parseDuration(step.get(CONFIG_AFTER, "0"));
|
||||
timeToNextNonNotificationStep = timeToNextNonNotificationStep.plus(duration != null ? duration : Duration.ZERO);
|
||||
if (!step.getProviderId().equals("notify-user")) {
|
||||
// we found the next non-notification action, accumulate its time and break
|
||||
return Map.of(step, timeToNextNonNotificationStep);
|
||||
|
||||
@ -61,13 +61,14 @@ public class WorkflowResource {
|
||||
*
|
||||
* @param type the resource type
|
||||
* @param resourceId the resource id
|
||||
* @param notBefore optional notBefore time in milliseconds to schedule the first workflow step,
|
||||
* it overrides the first workflow step time configuration (after).
|
||||
* @param notBefore optional value representing the time to schedule the first workflow step, overriding the first
|
||||
* step time configuration (after). The value is either an integer representing the seconds from now,
|
||||
* an integer followed by 'ms' representing milliseconds from now, or an ISO-8601 date string.
|
||||
*/
|
||||
@POST
|
||||
@Consumes(MediaType.APPLICATION_JSON)
|
||||
@Path("bind/{type}/{resourceId}")
|
||||
public void bind(@PathParam("type") ResourceType type, @PathParam("resourceId") String resourceId, Long notBefore) {
|
||||
public void bind(@PathParam("type") ResourceType type, @PathParam("resourceId") String resourceId, String notBefore) {
|
||||
Object resource = provider.getResourceTypeSelector(type).resolveResource(resourceId);
|
||||
|
||||
if (resource == null) {
|
||||
|
||||
@ -65,7 +65,7 @@ public class AdhocWorkflowTest extends AbstractWorkflowTest {
|
||||
try (Response response = managedRealm.admin().users().create(getUserRepresentation("alice", "Alice", "Wonderland", "alice@wornderland.org"))) {
|
||||
String id = ApiUtil.getCreatedId(response);
|
||||
try {
|
||||
managedRealm.admin().workflows().workflow(workflow.getId()).bind(ResourceType.USERS.name(), id, Duration.ofDays(5).toMillis());
|
||||
managedRealm.admin().workflows().workflow(workflow.getId()).bind(ResourceType.USERS.name(), id, "5D");
|
||||
} catch (Exception e) {
|
||||
assertThat(e, instanceOf(BadRequestException.class));
|
||||
}
|
||||
@ -135,7 +135,7 @@ public class AdhocWorkflowTest extends AbstractWorkflowTest {
|
||||
|
||||
try (Response response = managedRealm.admin().users().create(getUserRepresentation("alice", "Alice", "Wonderland", "alice@wornderland.org"))) {
|
||||
id = ApiUtil.getCreatedId(response);
|
||||
managedRealm.admin().workflows().workflow(workflow.getId()).bind(ResourceType.USERS.name(), id, Duration.ofDays(5).toMillis());
|
||||
managedRealm.admin().workflows().workflow(workflow.getId()).bind(ResourceType.USERS.name(), id, "5D");
|
||||
}
|
||||
|
||||
runScheduledSteps(Duration.ZERO);
|
||||
@ -158,7 +158,8 @@ public class AdhocWorkflowTest extends AbstractWorkflowTest {
|
||||
}
|
||||
}));
|
||||
|
||||
managedRealm.admin().workflows().workflow(workflow.getId()).bind(ResourceType.USERS.name(), id, Duration.ofDays(10).toMillis());
|
||||
// using seconds as the notBefore parameter just to check if this format is also working properly
|
||||
managedRealm.admin().workflows().workflow(workflow.getId()).bind(ResourceType.USERS.name(), id, String.valueOf(Duration.ofDays(10).toSeconds()));
|
||||
|
||||
runScheduledSteps(Duration.ZERO);
|
||||
|
||||
|
||||
@ -257,7 +257,7 @@ public class WorkflowManagementTest extends AbstractWorkflowTest {
|
||||
|
||||
// revert conditions, but change one of the steps
|
||||
workflow.setConditions(null);
|
||||
workflow.getSteps().get(0).setAfter(Duration.ofDays(8).toMillis());
|
||||
workflow.getSteps().get(0).setAfter("8D"); // 8 days
|
||||
try (Response response = workflows.workflow(workflow.getId()).update(workflow)) {
|
||||
assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode()));
|
||||
}
|
||||
@ -679,7 +679,7 @@ public class WorkflowManagementTest extends AbstractWorkflowTest {
|
||||
.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 'after' time condition cannot be negative."));
|
||||
assertThat(response.readEntity(ErrorRepresentation.class).getErrorMessage(), equalTo("Step 'after' configuration cannot be negative."));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user