Fix SCIM unfiltered search returning non-imported federated users by sguilhen · Pull Request #51683 · keycloak/keycloak

sguilhen · 2026-08-12T15:33:08Z

Copilot

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Aligns SCIM /Users list semantics to exclude non-imported federated users (e.g., LDAP/custom providers with import disabled), fixing inconsistent results between filtered vs. unfiltered searches and documenting the expected behavior.

Changes:

Update SCIM user listing/counting to query local storage only for unfiltered searches.
Add an integration test covering filtered vs. unfiltered search consistency with non-imported federated users.
Document SCIM /Users behavior and add test dependency needed for the custom federation provider.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
scim/model/src/main/java/org/keycloak/scim/model/user/UserResourceTypeProvider.java	Ensures unfiltered SCIM user search/count uses local storage, excluding non-imported federated users.
scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/FederatedUserTest.java	Adds regression test verifying consistent filtered/unfiltered results and exclusion of non-imported federated users.
scim/tests/base/pom.xml	Adds test-scoped dependency required by the new integration test.
docs/documentation/server_admin/topics/scim/managing-users.adoc	Documents that SCIM `/Users` returns only locally stored users and excludes non-imported federated users.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Pull request overview

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

Suppressed comments (3)

scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/FederatedUserTest.java:62

This comment is no longer true after the change: the unfiltered request now uses local storage and does not include federated-provider results. Update it to describe the expected fixed path.

            // unfiltered SCIM search - goes through UserStorageManager, includes federated providers

scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/FederatedUserTest.java:84

not(hasItems(a, b)) passes when either one of the two users is present; it only rejects a list containing both. Assert each forbidden username separately so a partial leak cannot satisfy this regression test.

            assertThat(unfilteredUserNames, not(hasItems("fed-charlie", "fed-diana")));
            assertThat(filteredUserNames, not(hasItems("fed-charlie", "fed-diana")));

scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/FederatedUserTest.java:46

This describes the pre-fix implementation as the current behavior, but the production change now routes unfiltered searches to local storage. Phrase it as historical regression context so the test documentation remains accurate.

This issue also appears on line 62 of the same file.

     * Currently, unfiltered searches go through UserStorageManager (which queries
     * federated providers), while filtered searches use direct JPA queries against
     * the local UserEntity table - so non-imported federated users only appear in
     * unfiltered results.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

scim/model/src/main/java/org/keycloak/scim/model/user/UserResourceTypeProvider.java:41

This introduces a dependency on UserStoragePrivateUtil (a private/internal utility). That makes SCIM more sensitive to internal refactors in user storage. If there is an existing supported accessor for local user storage in this layer, prefer that; otherwise consider encapsulating the local-only lookup behind a small SCIM-local helper (or extending the SCIM/user SPI to express ‘local-only’ intent) so the private util isn’t referenced directly throughout resource type providers.

import org.keycloak.storage.UserStoragePrivateUtil;

scim/model/src/main/java/org/keycloak/scim/model/user/UserResourceTypeProvider.java:155

This introduces a dependency on UserStoragePrivateUtil (a private/internal utility). That makes SCIM more sensitive to internal refactors in user storage. If there is an existing supported accessor for local user storage in this layer, prefer that; otherwise consider encapsulating the local-only lookup behind a small SCIM-local helper (or extending the SCIM/user SPI to express ‘local-only’ intent) so the private util isn’t referenced directly throughout resource type providers.

            return UserStoragePrivateUtil.userLocalStorage(session).searchForUserStream(realm, Map.of(UserModel.INCLUDE_SERVICE_ACCOUNT, "false"), firstResult, maxResults);

scim/model/src/main/java/org/keycloak/scim/model/user/UserResourceTypeProvider.java:174

This introduces a dependency on UserStoragePrivateUtil (a private/internal utility). That makes SCIM more sensitive to internal refactors in user storage. If there is an existing supported accessor for local user storage in this layer, prefer that; otherwise consider encapsulating the local-only lookup behind a small SCIM-local helper (or extending the SCIM/user SPI to express ‘local-only’ intent) so the private util isn’t referenced directly throughout resource type providers.

            return (long) UserStoragePrivateUtil.userLocalStorage(session).getUsersCount(realm, false);

pedroigor

I guess we also want to cover GET by ID?

Copilot

Pull request overview

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

sguilhen · 2026-08-13T13:21:15Z

I guess we also want to cover GET by ID?

Yes, I've updated the commit.

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-08-13T13:59:42Z

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.federation.ldap.LDAPUserLoginTest#loginLDAPUserAuthenticationSimpleEncryptionStartTLS

Keycloak CI - Java Distribution IT (windows-latest - temurin - 17)

org.openqa.selenium.TimeoutException: 
java.net.SocketTimeoutException: Read timed out
Build info: version: '4.28.1', revision: '73f5ad48a2'
System info: os.name: 'Windows Server 2025', os.arch: 'amd64', os.version: '10.0', java.version: '17.0.20'
Driver info: driver.version: HtmlUnitDriver
...

Report flaky test

Copilot

Pull request overview

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

Suppressed comments (1)

scim/model/src/main/java/org/keycloak/scim/model/user/UserResourceTypeProvider.java:104

Returning the local adapter directly bypasses UserStorageManager validation for imported federated users. For example, a user linked to a removed provider remains readable and updatable here instead of being rejected, and a disabled provider's read-only delegate is skipped; first prove that a local row exists, then resolve it through session.users() to preserve federation validation without exposing non-imported users.

        UserModel model = UserStoragePrivateUtil.userLocalStorage(session).getUserById(realm, id);

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-08-14T15:33:14Z

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.federation.ldap.LDAPUserLoginTest#loginLDAPUserAuthenticationSimpleEncryptionStartTLS

Keycloak CI - Java Distribution IT (windows-latest - temurin - 17)

org.openqa.selenium.TimeoutException: 
java.net.SocketTimeoutException: Read timed out
Build info: version: '4.28.1', revision: '73f5ad48a2'
System info: os.name: 'Windows Server 2025', os.arch: 'amd64', os.version: '10.0', java.version: '17.0.20'
Driver info: driver.version: HtmlUnitDriver
...

Report flaky test

Closes keycloak#51343 Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

Copilot

Pull request overview

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

Suppressed comments (1)

scim/model/src/main/java/org/keycloak/scim/model/user/UserResourceTypeProvider.java:104

Looking up the raw local adapter bypasses UserStorageManager.validateUser, so imported users no longer receive their federation proxy/read-only delegate; SCIM PUT/PATCH can consequently update the local row even when the provider is read-only, and GET can return stale provider data. Reject provider-owned IDs first, then use session.users() for local IDs so imported users retain normal federation validation and decoration.

        UserModel model = UserStoragePrivateUtil.userLocalStorage(session).getUserById(realm, id);

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-08-17T19:40:53Z

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.webauthn.registration.WebAuthnOtherSettingsTest#timeout

Keycloak CI - WebAuthn IT

org.openqa.selenium.WebDriverException: 
unknown error: unhandled inspector error: {"code":-32000,"message":"Node with given id does not belong to the document"}
  (Session info: chrome=151.0.7922.108)
Build info: version: '4.28.1', revision: '73f5ad48a2'
System info: os.name: 'Linux', os.arch: 'amd64', os.version: '6.17.0-1022-azure', java.version: '25.0.4'
...

Report flaky test

Copilot AI balanced review requested due to automatic review settings August 12, 2026 15:33

sguilhen requested a review from a team as a code owner August 12, 2026 15:33

Copilot AI reviewed Aug 12, 2026

View reviewed changes

Copilot started reviewing on behalf of sguilhen August 12, 2026 16:01

View session

Copilot AI review requested due to automatic review settings August 12, 2026 16:24

sguilhen force-pushed the 51343-scim-federated-users branch from a7ac047 to 0e6d75c Compare August 12, 2026 16:24

Copilot started reviewing on behalf of sguilhen August 12, 2026 16:39

View session

Copilot AI reviewed Aug 12, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 12, 2026 17:41

sguilhen force-pushed the 51343-scim-federated-users branch from 0e6d75c to 0709242 Compare August 12, 2026 17:41

Copilot AI reviewed Aug 12, 2026

View reviewed changes

Comment thread scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/FederatedUserTest.java Resolved

Comment thread scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/FederatedUserTest.java Resolved

Copilot started reviewing on behalf of sguilhen August 12, 2026 17:58

View session

pedroigor reviewed Aug 12, 2026

View reviewed changes

stianst added the team/core-iam label Aug 13, 2026

Copilot AI review requested due to automatic review settings August 13, 2026 13:01

sguilhen force-pushed the 51343-scim-federated-users branch from 0709242 to f625976 Compare August 13, 2026 13:01

Copilot started reviewing on behalf of sguilhen August 13, 2026 13:04

View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Comment thread scim/model/src/main/java/org/keycloak/scim/model/user/UserResourceTypeProvider.java Resolved

keycloak-github-bot Bot added the flaky-test label Aug 13, 2026

keycloak-github-bot Bot reviewed Aug 13, 2026

View reviewed changes

pedroigor previously approved these changes Aug 14, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 14, 2026 14:31

sguilhen dismissed pedroigor’s stale review via 0555571 August 14, 2026 14:31

sguilhen force-pushed the 51343-scim-federated-users branch from f625976 to 0555571 Compare August 14, 2026 14:31

Copilot started reviewing on behalf of sguilhen August 14, 2026 14:31

View session

Copilot AI reviewed Aug 14, 2026

View reviewed changes

sguilhen mentioned this pull request Aug 14, 2026

Define SCIM user creation behavior when user storage providers with sync registration are present #51735

Closed

keycloak-github-bot Bot reviewed Aug 14, 2026

View reviewed changes

pedroigor approved these changes Aug 17, 2026

View reviewed changes

pedroigor enabled auto-merge (rebase) August 17, 2026 18:39

sguilhen added 2 commits August 17, 2026 15:40

Fix SCIM unfiltered search returning non-imported federated users

8202627

Closes keycloak#51343 Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

Document that SCIM user searches only return locally stored users

1eacd4d

Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

Copilot AI review requested due to automatic review settings August 17, 2026 18:40

pedroigor force-pushed the 51343-scim-federated-users branch from 0555571 to 1eacd4d Compare August 17, 2026 18:40

Copilot started reviewing on behalf of pedroigor August 17, 2026 18:40

View session

Copilot AI reviewed Aug 17, 2026

View reviewed changes

pedroigor merged commit c9f5f55 into keycloak:main Aug 17, 2026
97 of 98 checks passed

This was referenced Aug 17, 2026

Flaky test: org.keycloak.testsuite.forms.RecoveryAuthnCodesAuthenticatorTest#test09recoveryAuthnCodesWithThresholdConfigured #51169

Closed

Flaky test: org.keycloak.testsuite.broker.KcOidcBrokerTest#loginWithExistingUserWithBruteForceEnabled #51600

Closed

keycloak-github-bot Bot reviewed Aug 17, 2026

View reviewed changes

sguilhen deleted the 51343-scim-federated-users branch September 21, 2026 19:00

Conversation

sguilhen commented Aug 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

pedroigor left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

sguilhen commented Aug 13, 2026

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot Bot commented Aug 13, 2026

Unreported flaky test detected

org.keycloak.testsuite.federation.ldap.LDAPUserLoginTest#loginLDAPUserAuthenticationSimpleEncryptionStartTLS

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot Bot commented Aug 14, 2026

Unreported flaky test detected

org.keycloak.testsuite.federation.ldap.LDAPUserLoginTest#loginLDAPUserAuthenticationSimpleEncryptionStartTLS

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot Bot commented Aug 17, 2026

Unreported flaky test detected

org.keycloak.testsuite.webauthn.registration.WebAuthnOtherSettingsTest#timeout

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants