8000 Workflows: don't throw at runtime when a role condition references a missing role by Inf1mum · Pull Request #54779 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Workflows: don't throw at runtime when a role condition references a missing role #54779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Inf1mum wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Workflows: don't throw at runtime when a role condition references a missing role #54779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e03bae
Don't throw workflowConditionRoleNotFound at runtime for missing role…
Inf1mum Nov 6, 2026
2104b61
Fix import order in workflow condition tests
Inf1mum Nov 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions ...ava/org/keycloak/models/workflow/conditions/GroupMembershipWorkflowConditionProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,18 @@ public ResourceType getSupportedResourceType() {

@Override
public boolean evaluate(WorkflowExecutionContext context) {
validate();

RealmModel realm = session.getContext().getRealm();
UserModel user = session.users().getUserById(realm, context.getResourceId());
if (user == null) {
return false;
}

GroupModel group = KeycloakModelUtils.findGroupByPath(session, realm, expectedGroup);
return user.isMemberOf(group);
return group != null && user.isMemberOf(group);
}

@Override
public Predicate toPredicate(CriteriaBuilder cb, CriteriaQuery<String> query, Root<?> path) {
validate();

GroupModel group = KeycloakModelUtils.findGroupByPath(session, session.getContext().getRealm(), expectedGroup);
if (group == null) {
return cb.disjunction(); // always false
Expand Down
2 changes: 0 additions & 2 deletions ...va/org/keycloak/models/workflow/conditions/IdentityProviderWorkflowConditionProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ public ResourceType getSupportedResourceType() {

@Override
public boolean evaluate(WorkflowExecutionContext context) {
validate();

RealmModel realm = session.getContext().getRealm();
UserModel user = session.users().getUserById(realm, context.getResourceId());
if (user == null) {
Expand Down
4 changes: 0 additions & 4 deletions .../src/main/java/org/keycloak/models/workflow/conditions/RoleWorkflowConditionProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ public ResourceType getSupportedResourceType() {

@Override
public boolean evaluate(WorkflowExecutionContext context) {
validate();

RealmModel realm = session.getContext().getRealm();
UserModel user = session.users().getUserById(realm, context.getResourceId());

Expand All @@ -55,8 +53,6 @@ public boolean evaluate(WorkflowExecutionContext context) {

@Override
public Predicate toPredicate(CriteriaBuilder cb, CriteriaQuery<String> query, Root<?> path) {
validate();

RoleModel role = getRole(expectedRole, session.getContext().getRealm());
if (role == null) {
return cb.disjunction(); // always false
Expand Down
46 changes: 46 additions & 0 deletions .../base/src/test/java/org/keycloak/tests/workflow/condition/GroupWorkflowConditionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.keycloak.models.UserModel;
import org.keycloak.models.workflow.DisableUserStepProviderFactory;
import org.keycloak.models.workflow.NotifyUserStepProviderFactory;
import org.keycloak.models.workflow.ResourceType;
import org.keycloak.models.workflow.SetUserAttributeStepProviderFactory;
import org.keycloak.models.workflow.Workflow;
import org.keycloak.models.workflow.WorkflowProvider;
Expand All @@ -34,10 +35,12 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
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.assertNotNull;

@KeycloakIntegrationTest(config = WorkflowsBlockingServerConfig.class)
public class GroupWorkflowConditionTest extends AbstractWorkflowTest {
Expand Down Expand Up @@ -143,4 +146,47 @@ public void testActivateWorkflowForEligibleResources() {
});
}

@Test
public void testConditionForNonExistingGroup() {
// create a workflow with a condition on a group that does not exist - the workflow is accepted
// as the group existence is not checked when creating or updating a workflow
managedRealm.admin().workflows().create(WorkflowRepresentation.withName("missing-group-workflow")
.schedule(WorkflowScheduleRepresentation.create().after("1s").build())
.onCondition(GroupMembershipWorkflowConditionFactory.ID + "(missing-group)")
.withSteps(
WorkflowStepRepresentation.create().of(SetUserAttributeStepProviderFactory.ID)
.withConfig("attribute", "attr1")
.build()
).build()).close();

List<WorkflowRepresentation> workflows = managedRealm.admin().workflows().list();
assertThat(workflows, hasSize(1));

String userId;
try (Response response = managedRealm.admin().users().create(UserBuilder.create()
.username("user-missing-group").email("user-missing-group@example.com").build())) {
userId = ApiUtil.getCreatedId(response);
}
assertThat(userId, notNullValue());

runOnServer.run((RunOnServer) session -> {
WorkflowProvider provider = session.getProvider(WorkflowProvider.class);
Workflow workflow = provider.getWorkflows().findFirst().orElse(null);
assertNotNull(workflow);

// resolving the eligible resources must not throw even though the group no longer exists
provider.activateForAllEligibleResources(workflow);

// activating the workflow for a user must not throw; the condition is simply not matched
RealmModel realm = session.getContext().getRealm();
UserModel user = session.users().getUserByUsername(realm, "user-missing-group");
assertNotNull(user);
provider.activate(workflow, ResourceType.USERS, user.getId());

// the user must not be activated for the workflow as the group does not exist
WorkflowStateProvider stateProvider = session.getProvider(WorkflowStateProvider.class);
assertThat(stateProvider.getScheduledStepsByWorkflow(workflow.getId()).toList(), is(empty()));
});
}

}
48 changes: 48 additions & 0 deletions ...s/base/src/test/java/org/keycloak/tests/workflow/condition/RoleWorkflowConditionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.keycloak.models.UserModel;
import org.keycloak.models.workflow.DisableUserStepProviderFactory;
import org.keycloak.models.workflow.NotifyUserStepProviderFactory;
import org.keycloak.models.workflow.ResourceType;
import org.keycloak.models.workflow.RestartWorkflowStepProviderFactory;
import org.keycloak.models.workflow.SetUserAttributeStepProviderFactory;
import org.keycloak.models.workflow.Workflow;
Expand Down Expand Up @@ -40,6 +41,7 @@
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertFalse;
Expand Down Expand Up @@ -120,6 +122,52 @@ public void testActivateWorkflowForEligibleResources() {
});
}

@Test
public void testConditionForNonExistingRole() {
// create a workflow with a condition on a role that does not exist - the workflow is accepted
// as the role existence is not checked when creating or updating a workflow
managedRealm.admin().workflows().create(WorkflowRepresentation.withName("missing-role-workflow")
.schedule(WorkflowScheduleRepresentation.create().after("1s").build())
.onCondition(RoleWorkflowConditionFactory.ID + "(missing-role)")
.withSteps(
WorkflowStepRepresentation.create().of(SetUserAttributeStepProviderFactory.ID)
.withConfig("notified", "true")
.after(Duration.ofDays(5))
.build(),
WorkflowStepRepresentation.create().of(RestartWorkflowStepProviderFactory.ID)
.build()
).build()).close();

List<WorkflowRepresentation> workflows = managedRealm.admin().workflows().list();
assertThat(workflows, hasSize(1));

try (Response response = managedRealm.admin().users().create(UserBuilder.create()
.username("user-missing-role")
.email("user-missing-role@example.com")
.build())) {
assertThat(response.getStatus(), is(Status.CREATED.getStatusCode()));
}

runOnServer.run((RunOnServer) session -> {
WorkflowProvider provider = session.getProvider(WorkflowProvider.class);
Workflow workflow = provider.getWorkflows().findFirst().orElse(null);
assertNotNull(workflow);

// resolving the eligible resources must not throw even though the role no longer exists
provider.activateForAllEligibleResources(workflow);

// activating the workflow for a user must not throw; the condition is simply not matched
RealmModel realm = session.getContext().getRealm();
UserModel user = session.users().getUserByUsername(realm, "user-missing-role");
assertNotNull(user);
provider.activate(workflow, ResourceType.USERS, user.getId());

// the user must not be activated for the workflow as the role does not exist
WorkflowStateProvider stateProvider = session.getProvider(WorkflowStateProvider.class);
assertThat(stateProvider.getScheduledStepsByWorkflow(workflow.getId()).toList(), is(empty()));
});
}

private void assertUserRoles(String username, boolean shouldExist, String... roles) {
assertUserRoles(username, shouldExist, List.of(roles));
}
Expand Down
Loading
0