From f61ad1e8d0c00fea9be68777647e572cb9dc9082 Mon Sep 17 00:00:00 2001 From: Stefan Guilhen Date: Tue, 3 Nov 2026 10:20:12 -0300 Subject: [PATCH] Enforce per-resource view permission on SCIM listings - filter resources by the VIEW scope when the realm does not use FGAPv2, mirroring UsersResource and GroupsResource Closes CVE-2026-116470 Closes #54340 Signed-off-by: Stefan Guilhen (cherry picked from commit 0cbbed9f94d4c6006ae6eff0d5c1479d78bba62b) --- .../topics/scim/accessing-endpoints.adoc | 14 ++- .../topics/scim/filtering-resources.adoc | 28 +++++ .../topics/changes/changes-26_8_3.adoc | 15 +++ .../spi/BaseResourceTypeProvider.java | 55 +++++++- .../scim/resource/spi/SearchOptions.java | 16 +++ .../group/GroupResourceTypeProvider.java | 4 + .../model/user/UserResourceTypeProvider.java | 4 + .../services/ScimResourceTypeResource.java | 6 +- .../tests/scim/tck/AuthorizationTest.java | 117 ++++++++++++++++-- 9 files changed, 244 insertions(+), 15 deletions(-) diff --git a/docs/documentation/server_admin/topics/scim/accessing-endpoints.adoc b/docs/documentation/server_admin/topics/scim/accessing-endpoints.adoc index ef32f79daaf1..f123d750e943 100644 --- a/docs/documentation/server_admin/topics/scim/accessing-endpoints.adoc +++ b/docs/documentation/server_admin/topics/scim/accessing-endpoints.adoc @@ -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` @@ -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` diff --git a/docs/documentation/server_admin/topics/scim/filtering-resources.adoc b/docs/documentation/server_admin/topics/scim/filtering-resources.adoc index a4fd1b114883..16034be7aef8 100644 --- a/docs/documentation/server_admin/topics/scim/filtering-resources.adoc +++ b/docs/documentation/server_admin/topics/scim/filtering-resources.adoc @@ -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] @@ -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 <>. * *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 <>. +* *`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 +<>. == Error responses diff --git a/docs/documentation/upgrading/topics/changes/changes-26_8_3.adoc b/docs/documentation/upgrading/topics/changes/changes-26_8_3.adoc index 925bf17f632c..642159e55ca9 100644 --- a/docs/documentation/upgrading/topics/changes/changes-26_8_3.adoc +++ b/docs/documentation/upgrading/topics/changes/changes-26_8_3.adoc @@ -73,6 +73,21 @@ Updating an existing client scope does not revalidate its stored mappers. Existing deployments continue to start, because mappers that are read from the database at startup are not validated. If you provision realms from an exported JSON file, import the file once after upgrading to find out whether it contains a mapper that is now rejected. +=== 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 diff --git a/scim/core/src/main/java/org/keycloak/scim/resource/spi/BaseResourceTypeProvider.java b/scim/core/src/main/java/org/keycloak/scim/resource/spi/BaseResourceTypeProvider.java index 9aa06f75939e..39fdf5cfd51d 100644 --- a/scim/core/src/main/java/org/keycloak/scim/resource/spi/BaseResourceTypeProvider.java +++ b/scim/core/src/main/java/org/keycloak/scim/resource/spi/BaseResourceTypeProvider.java @@ -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; @@ -75,8 +76,42 @@ public Stream getAll(SearchOptions searchOptions) { throw new ForbiddenException(); } - return getModels(searchOptions) - .map(m -> createResourceTypeInstance(m, searchOptions.getAttributes(), searchOptions.getExcludedAttributes())); + Stream 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 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 @@ -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); } diff --git a/scim/core/src/main/java/org/keycloak/scim/resource/spi/SearchOptions.java b/scim/core/src/main/java/org/keycloak/scim/resource/spi/SearchOptions.java index dfb692185106..4f5a6bd0e4b8 100644 --- a/scim/core/src/main/java/org/keycloak/scim/resource/spi/SearchOptions.java +++ b/scim/core/src/main/java/org/keycloak/scim/resource/spi/SearchOptions.java @@ -23,6 +23,8 @@ public class SearchOptions { private ScimFilterParser.FilterContext filterContext; + private Integer matchedSize; + // Getters and setters public List getAttributes() { @@ -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 getSort() { return Optional.ofNullable(sort).orElse(List.of()); } diff --git a/scim/model/src/main/java/org/keycloak/scim/model/group/GroupResourceTypeProvider.java b/scim/model/src/main/java/org/keycloak/scim/model/group/GroupResourceTypeProvider.java index 77004d6dc1ed..aee15e030921 100644 --- a/scim/model/src/main/java/org/keycloak/scim/model/group/GroupResourceTypeProvider.java +++ b/scim/model/src/main/java/org/keycloak/scim/model/group/GroupResourceTypeProvider.java @@ -158,6 +158,10 @@ protected Stream 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); } diff --git a/scim/model/src/main/java/org/keycloak/scim/model/user/UserResourceTypeProvider.java b/scim/model/src/main/java/org/keycloak/scim/model/user/UserResourceTypeProvider.java index 071b2d753c97..0c1aa96fc3e5 100644 --- a/scim/model/src/main/java/org/keycloak/scim/model/user/UserResourceTypeProvider.java +++ b/scim/model/src/main/java/org/keycloak/scim/model/user/UserResourceTypeProvider.java @@ -231,6 +231,10 @@ protected Stream 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); } diff --git a/scim/services/src/main/java/org/keycloak/scim/services/ScimResourceTypeResource.java b/scim/services/src/main/java/org/keycloak/scim/services/ScimResourceTypeResource.java index 834db95d57ff..b0f37bc4a641 100644 --- a/scim/services/src/main/java/org/keycloak/scim/services/ScimResourceTypeResource.java +++ b/scim/services/src/main/java/org/keycloak/scim/services/ScimResourceTypeResource.java @@ -154,7 +154,11 @@ public Response search(SearchRequest searchRequest) { } List 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 the 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 response = new ListResponse<>(); response.setResources(resources); diff --git a/scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/AuthorizationTest.java b/scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/AuthorizationTest.java index 0665bd54630a..61c072460768 100644 --- a/scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/AuthorizationTest.java +++ b/scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/AuthorizationTest.java @@ -128,13 +128,81 @@ public void testMissingResourceDoesNotLeakExistence() { } @Test - public void testUsersCanQueryIfQueryRoleGranted() { + public void testUsersQueryRoleAloneReturnsNoResources() { + // query-users admits the caller to the listing endpoint, but every resource still requires view + // permission. Without view-users the listing is empty, consistent with the Admin REST API. grantAdminRole(AdminRoles.QUERY_USERS); ListResponse response = noAccessClient.users().search(""); - assertEquals(1, response.getTotalResults()); assertNotNull(response.getResources(), "Resources array should not be null"); - assertEquals(1, response.getResources().size(), "Resources size should match totalResults"); - assertEquals(1, response.getItemsPerPage(), "itemsPerPage should match resources size"); + assertEquals(0, response.getResources().size(), "No user resource may be returned without view permission"); + assertEquals(0, response.getItemsPerPage(), "itemsPerPage should match resources size"); + // Without any fine-grained admin permissions in effect, access is all-or-nothing: the caller cannot view + // any user, on any page, so totalResults must be zero rather than the unfiltered DB count. + assertEquals(0, response.getTotalResults(), "totalResults must be zero when the caller cannot view any resource"); + } + + @Test + public void testUsersQueryRoleAloneDoesNotLeakPersonalData() { + createUser("leak-probe"); + grantAdminRole(AdminRoles.QUERY_USERS); + + // every way of reaching the listing must be covered, not just GET /Users + assertNoUsersVisible(noAccessClient.users().getAll()); + assertNoUsersVisible(noAccessClient.users().search("")); + assertNoUsersVisible(noAccessClient.users().search("userName eq \"leak-probe\"")); + assertNoUsersVisible(noAccessClient.users().getAll("userName sw \"leak\"")); + assertNoUsersVisible(noAccessClient.users().getAll("emails.value eq \"leak-probe@keycloak.org\"")); + + // an attribute projection must not be usable to get around the check either + assertNoUsersVisible(noAccessClient.users().getAll(List.of("userName"), null)); + assertNoUsersVisible(noAccessClient.users().getAll(null, List.of("name", "emails"))); + } + + @Test + public void testGroupsQueryRoleAloneDoesNotLeakGroups() { + createGroup("leak-probe-group"); + grantAdminRole(AdminRoles.QUERY_GROUPS); + + assertNoGroupsVisible(noAccessClient.groups().getAll("")); + assertNoGroupsVisible(noAccessClient.groups().getAll("displayName eq \"leak-probe-group\"")); + assertNoGroupsVisible(noAccessClient.groups().search("displayName sw \"leak\"", null, null)); + } + + @Test + public void testTotalResultsIsZeroAcrossPagesWhenAccessFullyDenied() { + // Without fine-grained admin permissions in effect, the per-resource view check evaluates the same way + // for every resource of this type, so a caller lacking it cannot view any resource, on any page. The + // listing and the count must agree on that: totalResults is zero regardless of which page is requested, + // not an unfiltered DB count. See the SCIM pagination documentation. + createUser("pagination-probe"); + grantAdminRole(AdminRoles.QUERY_USERS); + + ListResponse firstPage = noAccessClient.users().search("", 1, 1); + assertEquals(0, firstPage.getResources().size(), "no resource may be returned regardless of the page"); + assertEquals(0, firstPage.getTotalResults(), "totalResults must be zero when the caller cannot view any resource"); + + ListResponse secondPage = noAccessClient.users().search("", 2, 1); + assertEquals(0, secondPage.getResources().size(), "no resource may be returned regardless of the page"); + assertEquals(0, secondPage.getTotalResults(), "totalResults must not change from one page to the next"); + } + + @Test + public void testUsersVisibleToQueryRoleOnceViewUsersGranted() { + // guards against over-filtering: the per-resource check must not hide resources from callers that + // legitimately hold a realm-wide view permission + createUser("visible-probe"); + grantAdminRole(AdminRoles.QUERY_USERS); + assertEquals(0, noAccessClient.users().getAll().getResources().size()); + + grantAdminRole(AdminRoles.VIEW_USERS); + + ListResponse response = noAccessClient.users().getAll(); + assertEquals(2, response.getTotalResults()); + User probe = response.getResources().stream() + .filter(u -> "visible-probe".equals(u.getUserName())) + .findAny() + .orElseThrow(() -> new AssertionError("visible-probe should be returned to a view-users caller")); + assertNotNull(probe.getEmails(), "a view-users caller must still receive the full representation"); } @Test @@ -251,14 +319,18 @@ public void testSchemaByIdAccessIfQueryUsersRoleGranted() { } @Test - public void testGroupsCanQueryIfQueryRoleGranted() { + public void testGroupsQueryRoleAloneReturnsNoResources() { + // query-groups admits the caller to the listing endpoint, but every resource still requires view + // permission. Without view-users the listing is empty, consistent with the Admin REST API. createGroup(); grantAdminRole(AdminRoles.QUERY_GROUPS); ListResponse response = noAccessClient.groups().getAll(""); - assertEquals(1, response.getTotalResults()); assertNotNull(response.getResources(), "Resources array should not be null"); - assertEquals(1, response.getResources().size(), "Resources size should match totalResults"); - assertEquals(1, response.getItemsPerPage(), "itemsPerPage should match resources size"); + assertEquals(0, response.getResources().size(), "No group resource may be returned without view permission"); + assertEquals(0, response.getItemsPerPage(), "itemsPerPage should match resources size"); + // Without any fine-grained admin permissions in effect, access is all-or-nothing: the caller cannot view + // any group, on any page, so totalResults must be zero rather than the unfiltered DB count. + assertEquals(0, response.getTotalResults(), "totalResults must be zero when the caller cannot view any resource"); } @Test @@ -1475,6 +1547,35 @@ private GroupRepresentation createGroup(String name) { return group; } + private UserRepresentation createUser(String username) { + UserRepresentation user = UserBuilder.create() + .username(username) + .firstName("First") + .lastName("Last") + .email(username + "@keycloak.org") + .enabled(true) + .build(); + try (Response response = realm.admin().users().create(user)) { + user.setId(ApiUtil.getCreatedId(response)); + } + realm.cleanup().add(realm -> realm.users().get(user.getId()).remove()); + return user; + } + + private void assertNoUsersVisible(ListResponse response) { + assertNotNull(response.getResources(), "Resources array should not be null"); + assertEquals(0, response.getResources().size(), "No user resource may be returned without view permission"); + assertEquals(0, response.getItemsPerPage(), "itemsPerPage should match resources size"); + assertEquals(0, response.getTotalResults(), "totalResults must be zero when the caller cannot view any resource"); + } + + private void assertNoGroupsVisible(ListResponse response) { + assertNotNull(response.getResources(), "Resources array should not be null"); + assertEquals(0, response.getResources().size(), "No group resource may be returned without view permission"); + assertEquals(0, response.getItemsPerPage(), "itemsPerPage should match resources size"); + assertEquals(0, response.getTotalResults(), "totalResults must be zero when the caller cannot view any resource"); + } + private ClientResource getAdminPermissionsClient() { String permissionsClientId = realm.admin().clients().findByClientId(Constants.ADMIN_PERMISSIONS_CLIENT_ID).get(0).getId(); return realm.admin().clients().get(permissionsClientId);