From 95d5d7f9192ea20446b3b8c9971ffedfff0fa25c Mon Sep 17 00:00:00 2001 From: Oleg Zimakov Date: Sun, 26 Apr 2026 18:41:11 -0700 Subject: [PATCH] Fix removal of custom required actions via admin user API When updating a user via PUT /admin/realms/{realm}/users/{id}, custom required actions (registered via RequiredActionProvider SPI) could not be removed. The old code only iterated over registered provider factory IDs to decide which actions to remove, so any action not in that set was silently retained. Replace the single-pass "diff against factory IDs" approach with a clear-then-set approach: on update, remove all existing required actions first, then add back only the valid requested ones. This correctly handles custom, built-in, and orphaned actions alike. Closes #48144 Signed-off-by: Oleg Zimakov --- .../resources/admin/UserResource.java | 21 ++--- .../admin/user/UserRequiredActionsTest.java | 87 ++++++++++++++++++- 2 files changed, 94 insertions(+), 14 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java index b186c3aac15..dbc4428d9eb 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java @@ -94,7 +94,6 @@ import org.keycloak.organization.utils.Organizations; import org.keycloak.policy.PasswordPolicyNotMetException; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.utils.RedirectUtils; -import org.keycloak.provider.ProviderFactory; import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.ErrorRepresentation; import org.keycloak.representations.idm.FederatedIdentityRepresentation; @@ -305,18 +304,14 @@ public class UserResource { List reqActions = rep.getRequiredActions(); if (reqActions != null) { - session.getKeycloakSessionFactory() - .getProviderFactoriesStream(RequiredActionProvider.class) - .map(ProviderFactory::getId) - .distinct() - .sorted() - .forEach(action -> { - if (reqActions.contains(action)) { - user.addRequiredAction(action); - } else if (removeMissingRequiredActions) { - user.removeRequiredAction(action); - } - }); + if (removeMissingRequiredActions) { + user.getRequiredActionsStream().toList().forEach(user::removeRequiredAction); + } + + reqActions.stream() + .filter(action -> session.getKeycloakSessionFactory() + .getProviderFactory(RequiredActionProvider.class, action) != null) + .forEach(user::addRequiredAction); } List credentials = rep.getCredentials(); diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/user/UserRequiredActionsTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/user/UserRequiredActionsTest.java index c5137619c16..21dcba1cc9b 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/user/UserRequiredActionsTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/user/UserRequiredActionsTest.java @@ -1,13 +1,19 @@ package org.keycloak.tests.admin.user; +import java.util.List; + import org.keycloak.admin.client.resource.UserResource; import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; import org.keycloak.models.UserModel; import org.keycloak.representations.idm.RequiredActionProviderRepresentation; +import org.keycloak.representations.idm.RequiredActionProviderSimpleRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testframework.annotations.KeycloakIntegrationTest; import org.keycloak.testframework.events.AdminEventAssertion; +import org.keycloak.testframework.server.KeycloakServerConfig; +import org.keycloak.testframework.server.KeycloakServerConfigBuilder; +import org.keycloak.tests.providers.actions.DummyRequiredActionFactory; import org.keycloak.tests.suites.DatabaseTest; import org.keycloak.tests.utils.admin.AdminEventPaths; @@ -17,7 +23,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; -@KeycloakIntegrationTest +@KeycloakIntegrationTest(config = UserRequiredActionsTest.CustomProvidersServerConfig.class) public class UserRequiredActionsTest extends AbstractUserTest { @Test @@ -77,4 +83,83 @@ public class UserRequiredActionsTest extends AbstractUserTest { managedRealm.admin().flows().updateRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD.toString(), updatePasswordReqAction); AdminEventAssertion.assertEvent(adminEvents.poll(), OperationType.UPDATE, AdminEventPaths.authRequiredActionPath(UserModel.RequiredAction.UPDATE_PASSWORD.toString()), updatePasswordReqAction, ResourceType.REQUIRED_ACTION); } + + @Test + @DatabaseTest + public void removeCustomRequiredAction() { + registerDummyRequiredAction(); + + String id = createUser(); + UserResource user = managedRealm.admin().users().get(id); + + // Add the custom required action to the user + UserRepresentation userRep = user.toRepresentation(); + userRep.getRequiredActions().add(DummyRequiredActionFactory.PROVIDER_ID); + updateUser(user, userRep); + + userRep = user.toRepresentation(); + assertEquals(1, userRep.getRequiredActions().size()); + assertEquals(DummyRequiredActionFactory.PROVIDER_ID, userRep.getRequiredActions().get(0)); + + // Remove the custom required action by sending an empty list + userRep.getRequiredActions().clear(); + updateUser(user, userRep); + + userRep = user.toRepresentation(); + assertTrue(userRep.getRequiredActions().isEmpty(), + "Custom required action should be removed but was still present: " + userRep.getRequiredActions()); + } + + @Test + @DatabaseTest + public void removeCustomRequiredActionKeepBuiltIn() { + registerDummyRequiredAction(); + + String id = createUser(); + UserResource user = managedRealm.admin().users().get(id); + + // Add both a built-in and custom required action + UserRepresentation userRep = user.toRepresentation(); + userRep.getRequiredActions().add(UserModel.RequiredAction.UPDATE_PASSWORD.toString()); + userRep.getRequiredActions().add(DummyRequiredActionFactory.PROVIDER_ID); + updateUser(user, userRep); + + userRep = user.toRepresentation(); + assertEquals(2, userRep.getRequiredActions().size()); + + // Remove only the custom action, keep the built-in one + userRep.setRequiredActions(List.of(UserModel.RequiredAction.UPDATE_PASSWORD.toString())); + updateUser(user, userRep); + + userRep = user.toRepresentation(); + assertEquals(1, userRep.getRequiredActions().size()); + assertEquals(UserModel.RequiredAction.UPDATE_PASSWORD.toString(), userRep.getRequiredActions().get(0)); + } + + private void registerDummyRequiredAction() { + RequiredActionProviderSimpleRepresentation action = managedRealm.admin().flows().getUnregisteredRequiredActions() + .stream() + .filter(a -> a.getProviderId().equals(DummyRequiredActionFactory.PROVIDER_ID)) + .findFirst() + .orElseThrow(() -> new AssertionError("Dummy required action not found")); + managedRealm.admin().flows().registerRequiredAction(action); + AdminEventAssertion.assertEvent(adminEvents.poll(), OperationType.CREATE, + AdminEventPaths.authMgmtBasePath() + "/register-required-action", action, ResourceType.REQUIRED_ACTION); + + managedRealm.cleanup().add(r -> { + try { + r.flows().removeRequiredAction(DummyRequiredActionFactory.PROVIDER_ID); + } catch (jakarta.ws.rs.NotFoundException ignored) { + } + }); + } + + public static class CustomProvidersServerConfig implements KeycloakServerConfig { + + @Override + public KeycloakServerConfigBuilder configure(KeycloakServerConfigBuilder config) { + return config.dependency("org.keycloak.tests", "keycloak-tests-custom-providers"); + } + + } }