Skip to content

fix(admin-ui-ext): include representation in role mapping delete events - #51462

Open
alehhu wants to merge 3 commits into
keycloak:mainfrom
alehhu:fix/issue-51459
Open

fix(admin-ui-ext): include representation in role mapping delete events#51462
alehhu wants to merge 3 commits into
keycloak:mainfrom
alehhu:fix/issue-51459

Conversation

@alehhu

@alehhu alehhu commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Currently, when unassigning a realm role via the Admin Console, the generated REALM_ROLE_MAPPING admin event has a null representation. This happens because the Admin UI extension RoleMappingDeleteResource was simply reporting success without capturing and providing the deleted role representations to the event builder.

This PR fixes it by collecting the RoleRepresentation list inside deleteRoleMappings and passing it to adminEvent.representation(...), which restores parity with the classic REST API endpoints.

Closes #51459

Closes keycloak#51459

Signed-off-by: alehhu <159355663+alehhu@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 5, 2026 15:05
@alehhu
alehhu requested review from a team as code owners August 5, 2026 15:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Populates admin-event representations when role mappings are deleted through the Admin UI extension.

Changes:

  • Collects brief representations of deleted roles.
  • Adds representations to user, group, client, client-scope, and composite-role deletion events.

adminEvent.operation(OperationType.DELETE)
.resourcePath(session.getContext().getUri())
.resource(ResourceType.REALM_ROLE_MAPPING)
.representation(reps)
@michalvavrik
michalvavrik requested a review from edewit August 5, 2026 19:41
Copilot AI review requested due to automatic review settings August 6, 2026 07:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/RoleMappingDeleteResource.java:142

  • The event type here is selected from each removed composite, but the modified resource is the parent role. A realm parent containing a client-role composite will now emit CLIENT_ROLE (and mixed composites emit multiple events), whereas RoleResource.deleteComposites and RealmRolesCRUDTest.java:129-148 emit one REALM_ROLE event based on the parent; determine the type from parentRole and keep this deletion in one event.
        deleteRoleMappings(roles, compositeRole -> parentRole.removeCompositeRole(compositeRole), ResourceType.REALM_ROLE, ResourceType.CLIENT_ROLE);

@alehhu

alehhu commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

I pushed a new commit to improve how the admin events are generated:
initially, this PR fixed the missing representation issue, but it was lumping all deleted roles into a single admin event since the Admin UI bulk-deletes them in one request. This would have caused client roles to show up in the audit log under REALM_ROLE_MAPPING, which isn't ideal for consistency.

To fix this, I refactored the deleteRoleMappings helper. It now groups the roles being deleted by their actual type (realm vs client) and emits separate, semantically correct events. This way we fix the missing representations while keeping the UI audit logs 100% consistent with the ones generated by the standard REST API.

@alehhu
alehhu requested review from a team as code owners August 8, 2026 16:06
alehhu added 2 commits August 8, 2026 18:09
Signed-off-by: alehhu <159355663+alehhu@users.noreply.github.com>
Signed-off-by: alehhu <159355663+alehhu@users.noreply.github.com>
@keycloak-github-bot

Copy link
Copy Markdown

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.forms.BruteForceTest#testExceedMaxTemporaryLockouts

Keycloak CI - Base IT (5)

org.opentest4j.AssertionFailedError: Expected error event ==> expected: <true> but was: <false>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
...

Report flaky test

@keycloak-github-bot keycloak-github-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unreported flaky test detected, please review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Admin Console emits representation=null when unassigning a realm role in Keycloak 26.6.3

2 participants