Skip to content

GET /roles/{role}/users returns user PII without the per-user view fi… - #51192

Merged
pedroigor merged 1 commit into
keycloak:mainfrom
vramik:51142-information-disclosure
Jul 28, 2026
Merged

GET /roles/{role}/users returns user PII without the per-user view fi…#51192
pedroigor merged 1 commit into
keycloak:mainfrom
vramik:51142-information-disclosure

Conversation

@vramik

@vramik vramik commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

…lter with FGAP V1

Closes #51142

Copilot AI review requested due to automatic review settings July 27, 2026 12:03
@vramik
vramik requested a review from a team as a code owner July 27, 2026 12:03

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

Fixes CVE-2026-17059 by filtering realm-role members according to FGAP V1 user-view permissions.

Changes:

  • Applies per-user authorization filtering to role-member results.
  • Adds access metadata and a realm-role regression test.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
RoleContainerResource.java Filters role members by user-view permission.
UserResourceTypeFilteringTest.java Adds realm-role member filtering coverage.

Comment on lines +607 to +608
if (!AdminPermissionsSchema.SCHEMA.isAdminPermissionsEnabled(realm)) {
members = members.filter(usersEvaluator::canView);
}

return members.map(user -> {
UserRepresentation userRep = ModelToRepresentation.toRepresentation(session, user, briefRep);

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.

One might also consider PII disclosure if the returned attributes are not based on the user profile settings. We had similar issues in the past where, in addition to the view check, we had to build the representation via the user profile provider.

Perhaps we should just return (of course, in addition to the view check) the id and username when resolving users associated with a role? And whenever returning users associated with some other realm resource?

@pedroigor pedroigor 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.

It LGTM if we are only fixing for now the missing view check.

I'm OK to have a follow-up on filtering user attributes based on the user profile settings.

…lter with FGAP V1

Closes keycloak#51142

Signed-off-by: vramik <vramik@redhat.com>
Copilot AI review requested due to automatic review settings July 27, 2026 23:04
@pedroigor
pedroigor force-pushed the 51142-information-disclosure branch from d8f7813 to 4518e4d Compare July 27, 2026 23:04

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 2 out of 2 changed files in this pull request and generated 2 comments.

Comment on lines 602 to +609
boolean briefRep = Boolean.TRUE.equals(briefRepresentation);
UserPermissionEvaluator usersEvaluator = auth.users();

return session.users().getRoleMembersStream(realm, role, firstResult, maxResults)
.map((u) -> ModelToRepresentation.toRepresentation(session, u, briefRep));
Stream<UserModel> members = session.users().getRoleMembersStream(realm, role, firstResult, maxResults);

if (!AdminPermissionsSchema.SCHEMA.isAdminPermissionsEnabled(realm)) {
members = members.filter(usersEvaluator::canView);
}
Comment on lines +601 to +604
String myadminId = realm.admin().users().search("myadmin").get(0).getId();
realm.admin().users().get(myadminId).roles().clientLevel(realmMgmtClientId).add(List.of(viewRealmRole));
realm.cleanup().add(r -> r.users().get(r.users().search("myadmin").get(0).getId())
.roles().clientLevel(realmMgmtClientId).remove(List.of(viewRealmRole)));
@pedroigor

Copy link
Copy Markdown
Contributor

Created #51233.

@pedroigor
pedroigor merged commit 29b7324 into keycloak:main Jul 28, 2026
91 checks passed
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.

[CVE-2026-17059] Information disclosure: GET /roles/{role}/users returns user PII without the per-user view filter

3 participants