Fix removal of custom required actions via admin user API#48485
Open
ozimakov wants to merge 4 commits into
Open
Fix removal of custom required actions via admin user API#48485ozimakov wants to merge 4 commits into
ozimakov wants to merge 4 commits into
Conversation
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 keycloak#48144
Signed-off-by: Oleg Zimakov <oleg@zimakov.net>
|
Thank you very much, looks to me as it should fix the issue #48144 which btw also occurs by using the admin interface |
Author
|
@stianst May I get a review for this one? Thx. |
Signed-off-by: Oleg Zimakov <oleg.zimakov@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes required-action removal during Admin API user updates.
Changes:
- Replaces factory-based removal with clear-and-reapply logic.
- Adds integration tests for custom required-action removal.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
services/.../UserResource.java |
Updates required-action synchronization. |
tests/.../UserRequiredActionsTest.java |
Adds custom-action removal tests. |
… API Required-action values stored on a user are realm aliases, which are not necessarily equal to the provider factory id (they can diverge once an action's alias is renamed). The re-add filter now resolves the realm required-action model by alias first, then validates that its provider id maps to a registered factory. This keeps custom actions whose alias differs from the provider id from being silently dropped on a user update. Update the integration tests to register the dummy action and then rename its alias so it no longer matches the provider factory id. The tests now exercise the alias/provider-id distinction and reproduce the reported failure, and a new test asserts such an aliased action is retained across an update. Signed-off-by: Oleg Zimakov <oleg.zimakov@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DLP6kVvr88Q87AtHhS7b9w
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When updating a user via
PUT /admin/realms/{realm}/users/{id}, custom required actions (registered via theRequiredActionProviderSPI) could not be removed. The old code iterated only over registered provider factory IDs to decide which actions to remove — any action not in that factory set was silently retained.The fix replaces the single-pass "diff against factory IDs" approach with a clear-then-set approach: on update, all existing required actions are removed first, then only the valid requested ones are added back. This correctly handles custom, built-in, and orphaned actions alike.
Two new integration tests verify:
Closes #48144