8000 Enforce per-resource view permission on SCIM listings by sguilhen · Pull Request #54790 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,11 @@ The following table summarizes the permissions required for each type of SCIM op
| Read-only access to individual user resources.

| Search users
| `query-users`
| Ability to list and filter user resources.
| `query-users` and `view-users`
| `query-users` grants access to the listing and search endpoints. It does not grant read access to the resources
those endpoints match: every user is returned only if the client can also view it. Grant `view-users` as well, or
a fine-grained `view` permission on the users the client is allowed to see. A client holding `query-users` alone
receives an empty `Resources` array.

| Create, update, delete groups
| `manage-users`
Expand All @@ -79,8 +82,11 @@ The following table summarizes the permissions required for each type of SCIM op
| Read-only access to individual group resources.

| Search groups
| `query-groups`
| Ability to list and filter group resources.
| `query-groups` and `view-users`
| `query-groups` grants access to the listing and search endpoints. It does not grant read access to the resources
those endpoints match: every group is returned only if the client can also view it. Grant `view-users` as well, or
a fine-grained `view` permission on the groups the client is allowed to see. A client holding `query-groups` alone
receives an empty `Resources` array.

| Access discovery endpoints
| `query-users` or `query-groups`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,30 @@ The response includes pagination metadata:
* `itemsPerPage`: The number of resources in the current page.
* `startIndex`: The 1-based index of the first resource in the current page.

[NOTE]
====
How `totalResults` relates to the resources the caller is allowed to see depends on the realm's fine-grained admin
permissions configuration:

With fine-grained admin permissions v2 enabled for the realm, the listing and the count are restricted by the same
authorization filter in the underlying database query, so `totalResults` reflects exactly what the caller is
allowed to see.

With the deprecated fine-grained admin permissions v1, a caller can be allowed to view some resources but not
others. `totalResults` counts the resources matching the query, before the per-resource view check, so it can be
larger than the number of resources actually returned, and should be treated as an upper bound. A page can also be
shorter than the requested `count`, or even empty, simply because resources were removed from it, while later pages
still contain resources the caller is allowed to see. Neither a short page nor an empty page is a reliable signal
that no further resources exist. Keep incrementing `startIndex` by `count` until `startIndex` exceeds `totalResults`
rather than stopping at the first short or empty page.

Without any fine-grained admin permissions, access is controlled by role-based access control alone, and the
per-resource view check evaluates the same way for every resource of a given type: either the caller holds a
realm-wide view role (such as `view-users`) and can see every resource, or it does not and can see none. A client
holding only `query-users` or `query-groups` therefore receives an empty `Resources` array and a `totalResults` of
`0`, consistently across every page.
====

To retrieve the next page, increment `startIndex` by `count`:

[source,bash]
Expand Down Expand Up @@ -305,6 +329,10 @@ date/time or boolean attributes, and incompatible combinations are rejected. See
where the attribute has no value. Use `not (attribute pr)` to find them. See <<Attributes without a value>>.
* *Membership filters carry extra restrictions.* The `groups.value` and `members.value` paths behave differently under
fine-grained admin permissions and with LDAP `LDAP_ONLY` group mappers. See <<Filtering by group membership>>.
* *`totalResults` is only exact when fine-grained admin permissions v2 is enabled.* With the deprecated v1, it is an
upper bound that can exceed the number of resources actually returned. Without any fine-grained admin permissions,
it is `0` whenever the caller lacks a realm-wide view role, even though the query matched resources. See
<<Pagination>>.

== Error responses

Expand Down
22 changes: 22 additions & 0 deletions docs/documentation/upgrading/topics/changes/changes-26_8_3.adoc
Original file line number Diff line number Diff line change
@@ -1,3 +1,25 @@
// ------------------------ Breaking changes ------------------------ //
== Breaking changes

Breaking changes are identified as those that might require changes for existing users to their configurations or applications.
In minor or patch releases, {project_name} will only introduce breaking changes to fix bugs.

=== SCIM listings respect per-resource view permissions (CVE-2026-116470)
// https://github.com/keycloak/keycloak/issues/54340

Listing and searching resources through the SCIM API now returns only the resources the caller is allowed to view.
Previously, the `query-users` and `query-groups` roles were checked when entering `GET /realms/\{realm}/scim/v2/Users`, `GET /realms/\{realm}/scim/v2/Groups`, and their `.search` counterparts, but the resources those endpoints matched were returned in full without a per-resource view check.
A service account holding only `query-users` could therefore read every user in the realm, including email addresses, names, and custom attributes, which the Admin REST API would not have disclosed to it.

The query roles now behave as they do in the Admin REST API: they grant access to the listing and search endpoints, not read access to the resources those endpoints match.
After upgrading, a client holding only `query-users` or `query-groups` receives an empty `Resources` array.
Grant `view-users` in addition, or configure a fine-grained `view` permission on the users and groups the client is allowed to see.
When fine-grained admin permissions v2 are enabled for the realm, listings were already narrowed to the permitted resources and are unaffected.

With fine-grained admin permissions v2, the listing and the count are restricted by the same authorization filter in the database query, so `totalResults` stays exact.
With the deprecated fine-grained admin permissions v1, a caller can be allowed to view some resources but not others, so `totalResults` counts the resources matching the query, before the per-resource view check is applied, and should be treated as an upper bound; a page can also be shorter than the requested `count`, or even empty, while later pages still contain resources the caller is allowed to see, so neither a short page nor an empty page means that no further resources exist, and clients should keep incrementing `startIndex` by `count` until `startIndex` exceeds `totalResults`.
Without any fine-grained admin permissions, the per-resource view check evaluates the same way for every resource of a given type, so access is all-or-nothing: a client holding only a query role and no realm-wide view role gets a `totalResults` of `0`, consistently across every page, rather than the unfiltered database count.

// ------------------------ Notable changes ------------------------ //
== Notable changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.stream.Stream;

import org.keycloak.authorization.fgap.AdminPermissionsSchema;
import org.keycloak.common.Profile;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.Model;
import org.keycloak.scim.protocol.ForbiddenException;
Expand Down Expand Up @@ -75,8 +76,42 @@ public Stream<R> getAll(SearchOptions searchOptions) {
throw new ForbiddenException();
}

return getModels(searchOptions)
.map(m -> createResourceTypeInstance(m, searchOptions.getAttributes(), searchOptions.getExcludedAttributes()));
Stream<M> models = getModels(searchOptions);

// The query scope only allows a caller to reach this endpoint, it does not entitle them to read the
// resources it matches. When fine-grained admin permissions v2 are enabled for the realm, the queries
// behind getModels are already narrowed at the SQL level by AdminPermissionsSchema#applyAuthorizationFilters,
// which is gated on that same realm flag. In every other configuration, including the deprecated v1
// feature, nothing narrows the result set, so the per-resource VIEW check is the only enforcement point.
// This mirrors UsersResource#toRepresentation and GroupsResource#getGroups in the Admin REST API.
// Resources are filtered before being converted so that representations are never built for resources the
// caller cannot see.
if (!AdminPermissionsSchema.SCHEMA.isAdminPermissionsEnabled(session.getContext().getRealm())
&& !hasPermission(getRealmResourceType(), AdminPermissionsSchema.VIEW)) {
// The page was already narrowed by the query, so this filter removes resources from a page that has
// been paginated. Record how many resources the query matched before filtering: the count of total
// results is derived from the size of the page the query produced, and a page shortened by this filter
// must not be mistaken for the last page.
List<M> page = models.toList();

searchOptions.setMatchedSize(page.size());

if (isViewFullyDenied()) {
// Plain RBAC: the VIEW check above evaluates the same way for every resource of this type, so
// lacking it here means the caller cannot view any resource of this type, on any page. count()
// independently recognizes this and reports zero instead of falling through to a DB count that
// does not account for permissions at all.
models = Stream.empty();
} else {
// Deprecated fine-grained admin permissions v1: a per-resource policy can grant view access to a
// subset of resources, so some, none, or all resources on this page may pass. Because this is not
// narrowed at the SQL level, count() falls back to an unfiltered DB count, making totalResults an
// upper bound rather than an exact count.
models = page.stream().filter(m -> hasPermission(m, getRealmResourceType(), AdminPermissionsSchema.VIEW));
}
}

return models.map(m -> createResourceTypeInstance(m, searchOptions.getAttributes(), searchOptions.getExcludedAttributes()));
}

@Override
Expand Down Expand Up @@ -116,6 +151,22 @@ protected boolean canQuery() {
|| hasPermission(getRealmResourceType(), AdminPermissionsSchema.VIEW);
}

/**
* Whether the caller cannot view any resource of this type, on any page, because neither fine-grained admin
* permissions v2 nor the deprecated v1 are in effect for the realm and the caller lacks the blanket VIEW
* permission. Without any fine-grained admin permissions, the per-resource VIEW check evaluates the same way
* for every resource of this type, so this is an all-or-nothing decision. Implementations of {@code count()}
* must honor this by reporting zero rather than falling back to a DB count that does not account for
* permissions at all.
*
* @return {@code true} when the caller is denied view access to every resource of this type
*/
protected boolean isViewFullyDenied() {
return !AdminPermissionsSchema.SCHEMA.isAdminPermissionsEnabled(session.getContext().getRealm())
&& !hasPermission(getRealmResourceType(), AdminPermissionsSchema.VIEW)
&& !Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ);
}

public boolean hasPermission(R representation, String scope) {
return hasPermission(Optional.ofNullable(representation).map(this::getModelId).map(this::getModel).orElse(null), getRealmResourceType(), scope);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class SearchOptions {

private ScimFilterParser.FilterContext filterContext;

private Integer matchedSize;

// Getters and setters

public List<String> getAttributes() {
Expand Down Expand Up @@ -73,6 +75,20 @@ public void setCount(Integer count) {
this.count = count;
}

/**
* The number of resources the query matched for this page, before any post-query authorization filtering was
* applied, or {@code null} when the page was not filtered after the query ran.
*
* @return the unfiltered size of the page the query produced
*/
public Integer getMatchedSize() {
return matchedSize;
}

public void setMatchedSize(Integer matchedSize) {
this.matchedSize = matchedSize;
}

public List<SortField> getSort() {
return Optional.ofNullable(sort).orElse(List.of());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ protected Stream<GroupModel> getModels(SearchOptions searchRequest) {

@Override
public Long count(SearchOptions searchRequest, int resourceSize) {
if (isViewFullyDenied()) {
return 0L;
}

if (resourceSize < searchRequest.getCount() && (resourceSize > 0 || searchRequest.getStartIndex() == 1)) {
return (long) (searchRequest.getStartIndex() - 1 + resourceSize);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,10 @@ protected Stream<UserModel> getModels(SearchOptions searchRequest) {

@Override
public Long count(SearchOptions searchRequest, int resourceSize) {
if (isViewFullyDenied()) {
return 0L;
}

if (resourceSize < searchRequest.getCount() && (resourceSize > 0 || searchRequest.getStartIndex() == 1)) {
return (long) (searchRequest.getStartIndex() - 1 + resourceSize);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,11 @@ public Response search(SearchRequest searchRequest) {
}

List<R> resources = stream.toList();
Long totalResults = resourceTypeProvider.count(searchOptions, resources.size());
// count() uses a page shorter than the requested count as the signal that the last page was reached, so
// it must be given the size of the page the query produced. When getAll removed resources th 6B17 e caller
// cannot view, that is no longer the size of the page being returned.
int matchedSize = searchOptions.getMatchedSize() != null ? searchOptions.getMatchedSize() : resources.size();
Long totalResults = resourceTypeProvider.count(searchOptions, matchedSize);
ListResponse<R> response = new ListResponse<>();

response.setResources(resources);
Expand Down
Loading
Loading
0