SCIM: PATCH operations list has no size limit — missing maxOperations… by martin-kanis · Pull Request #50872 · keycloak/keycloak

martin-kanis · 2026-07-14T13:11:03Z

… enforcement

Closes #50475

Copilot

Pull request overview

Adds a fixed limit for SCIM PATCH operations to mitigate unbounded request processing.

Changes:

Limits PATCH requests to 100 operations.
Adds an integration test for exceeding the limit.

Reviewed changes

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

File	Description
`AbstractScimResourceTypeProvider.java`	Enforces the PATCH operation limit.
`UserTest.java`	Tests rejection above the limit.

sguilhen · 2026-07-15T14:19:22Z


 public abstract class AbstractScimResourceTypeProvider<M extends Model, R extends ResourceTypeRepresentation> implements ScimResourceTypeProvider<R> {

+    public static final int MAX_PATCH_OPERATIONS = 100;


@martin-kanis I think this is a valid point: the /ServiceProviderConfig endpoint will advertise maxOperations: 0 (meaning "no limit" or "not supported" depending on interpretation) while the server actually enforces 100. These should be consistent. We do something very similar for the filter's maxResults config. The value that is enforced is returned in the ServiceProviderConfig even if filtering is disabled.

After further reviewing, I think this is not a valid suggestion. Mixing bulk parameters with patch in the provider config is semantically wrong. There's nothing in the RFC that suggests patch should advertise its max operations constraint via the standard ServiceProviderConfig.

sguilhen · 2026-07-15T14:21:53Z

+        if (operations.size() > MAX_PATCH_OPERATIONS) {
+            throw new ModelValidationException(
+                    "PATCH request exceeds maximum allowed number of %d operations".formatted(MAX_PATCH_OPERATIONS));


The issue does mention bounding member/value arrays, so it's a legitimate gap in the PR. However, I would say this is a separate concern from the top-level operation count and could be addressed in a follow-up. Raising it as a suggestion for a separate PR is fair, blocking this PR on it seems a bit excessive.

sguilhen · 2026-07-15T14:25:55Z

+            throw new ModelValidationException(
+                    "PATCH request exceeds maximum allowed number of %d operations".formatted(MAX_PATCH_OPERATIONS));


Seems valid to me - The issue explicitly requests scimType=tooMany per RFC 7644 section 3.12. This is a real spec-compliance gap. We should handle the exception differently for this case. The test should also assert the scimType value, not just the status code and message substring.

Copilot

Pull request overview

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

sguilhen · 2026-07-16T18:24:36Z

+    public static class PatchSupport extends Supported {
+        @JsonProperty("maxOperations")
+        private Integer maxOperations = 0;


@martin-kanis I was originally thinking about using the maxOperations from bulk instead of adding it to patch in the service provider config, following Copilot's initial review. However, thinking about it more carefully, it seems to me that there are issues with the original suggestion and also with this approach you've taken:

bulk.maxOperations is specifically about bulk endpoint operations, not PATCH operations. Using it for PATCH would be semantically wrong (approach copilot initially suggested)

adding a non-standard maxOperations field under patch in the ServiceProviderConfig response risks breaking strict SCIM clients that model the patch object per the RFC schema (e.g., Jackson with FAIL_ON_UNKNOWN_PROPERTIES = true would throw on the unknown attribute).

So all of this to say that I now believe it is safer to enforce the limit server-side without advertising it in the config at all, as you originally did. So I'm basically asking you to revert back to the previous approach. Sorry about this, I thought it would be good to allow clients to discover the maximum patch size via service provider config without considering clients might have problems parsing the config if we use non-standard parameters.

No problem. I reverted it to the previous version and I added some notes to the documentation and to javadoc.

martin-kanis · 2026-07-16T10:25:36Z

@sguilhen Thanks for the review and suggestions. I updated the PR with your proposals (I hope, I got them right) except the one that can be done as follow-up.

Copilot

Pull request overview

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

sguilhen · 2026-07-17T13:27:25Z

+        if (operations.size() > MAX_PATCH_OPERATIONS) {
+            throw new ScimPatchException(
+                    "PATCH request exceeds maximum allowed number of %d operations".formatted(MAX_PATCH_OPERATIONS));


Not an issue, every patch-capable provider extends the same abstract class and future providers (if any) will follow the same pattern

sguilhen

Thanks @martin-kanis! LGTM

… enforcement Closes keycloak#50475 Signed-off-by: Martin Kanis <mkanis@ibm.com>

Copilot

Pull request overview

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

keycloak-github-bot · 2026-07-20T13:03:32Z

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.BrowserFlowTest#testUserWithOneAdditionalFactorOtpSuccess

Keycloak CI - Base IT (5)

org.opentest4j.AssertionFailedError: 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

org.keycloak.testsuite.oauth.ClientAuthPostMethodTest#testPostAuthenticationNotAllowedWhenBasicRequested

Keycloak CI - Base IT (6)

java.lang.NullPointerException: Cannot read field "features" because the return value of "org.keycloak.common.Profile.getInstance()" is null
	at org.keycloak.common.Profile.isFeatureEnabled(Profile.java:534)
	at org.keycloak.protocol.oidc.OIDCClientSecretConfigWrapper.<init>(OIDCClientSecretConfigWrapper.java:41)
	at org.keycloak.protocol.oidc.OIDCClientSecretConfigWrapper.fromClientRepresentation(OIDCClientSecretConfigWrapper.java:49)
	at org.keycloak.testsuite.oauth.ClientAuthPostMethodTest.testPostAuthenticationNotAllowedWhenBasicRequested(ClientAuthPostMethodTest.java:141)
...

Report flaky test

org.keycloak.testsuite.oauth.ClientAuthPostMethodTest#testBasicAuthenticationNotAllowedWhenPostRequested

Keycloak CI - Base IT (6)

java.lang.NullPointerException: Cannot read field "features" because the return value of "org.keycloak.common.Profile.getInstance()" is null
	at org.keycloak.common.Profile.isFeatureEnabled(Profile.java:534)
	at org.keycloak.protocol.oidc.OIDCClientSecretConfigWrapper.<init>(OIDCClientSecretConfigWrapper.java:41)
	at org.keycloak.protocol.oidc.OIDCClientSecretConfigWrapper.fromClientRepresentation(OIDCClientSecretConfigWrapper.java:49)
	at org.keycloak.testsuite.oauth.ClientAuthPostMethodTest.testBasicAuthenticationNotAllowedWhenPostRequested(ClientAuthPostMethodTest.java:112)
...

Report flaky test

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-07-20T15:42:05Z

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.BrowserFlowTest#testUserWithOneAdditionalFactorOtpSuccess

Keycloak CI - Base IT (5)

org.opentest4j.AssertionFailedError: 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

org.keycloak.testsuite.oauth.ClientAuthPostMethodTest#testPostAuthenticationNotAllowedWhenBasicRequested

Keycloak CI - Base IT (6)

java.lang.NullPointerException: Cannot read field "features" because the return value of "org.keycloak.common.Profile.getInstance()" is null
	at org.keycloak.common.Profile.isFeatureEnabled(Profile.java:534)
	at org.keycloak.protocol.oidc.OIDCClientSecretConfigWrapper.<init>(OIDCClientSecretConfigWrapper.java:41)
	at org.keycloak.protocol.oidc.OIDCClientSecretConfigWrapper.fromClientRepresentation(OIDCClientSecretConfigWrapper.java:49)
	at org.keycloak.testsuite.oauth.ClientAuthPostMethodTest.testPostAuthenticationNotAllowedWhenBasicRequested(ClientAuthPostMethodTest.java:141)
...

Report flaky test

org.keycloak.testsuite.oauth.ClientAuthPostMethodTest#testBasicAuthenticationNotAllowedWhenPostRequested

Keycloak CI - Base IT (6)

java.lang.NullPointerException: Cannot read field "features" because the return value of "org.keycloak.common.Profile.getInstance()" is null
	at org.keycloak.common.Profile.isFeatureEnabled(Profile.java:534)
	at org.keycloak.protocol.oidc.OIDCClientSecretConfigWrapper.<init>(OIDCClientSecretConfigWrapper.java:41)
	at org.keycloak.protocol.oidc.OIDCClientSecretConfigWrapper.fromClientRepresentation(OIDCClientSecretConfigWrapper.java:49)
	at org.keycloak.testsuite.oauth.ClientAuthPostMethodTest.testBasicAuthenticationNotAllowedWhenPostRequested(ClientAuthPostMethodTest.java:112)
...

Report flaky test

martin-kanis requested a review from a team as a code owner July 14, 2026 13:11

Copilot AI review requested due to automatic review settings July 14, 2026 13:11

keycloak-github-bot Bot added the team/core-iam label Jul 14, 2026

Copilot started reviewing on behalf of martin-kanis July 14, 2026 13:11 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

martin-kanis requested a review from sguilhen July 15, 2026 11:33

martin-kanis force-pushed the issue-50475 branch from 31c2d89 to 6cd8d67 Compare July 16, 2026 09:19

Copilot AI review requested due to automatic review settings July 16, 2026 09:19

Copilot started reviewing on behalf of martin-kanis July 16, 2026 09:19 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

sguilhen suggested changes Jul 16, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 17, 2026 10:05

martin-kanis force-pushed the issue-50475 branch from 6cd8d67 to 23b99a1 Compare July 17, 2026 10:05

Copilot started reviewing on behalf of martin-kanis July 17, 2026 10:07 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

sguilhen approved these changes Jul 17, 2026

View reviewed changes

SCIM: PATCH operations list has no size limit — missing maxOperations…

bc3c390

… enforcement Closes keycloak#50475 Signed-off-by: Martin Kanis <mkanis@ibm.com>

Copilot AI review requested due to automatic review settings July 20, 2026 11:55

martin-kanis force-pushed the issue-50475 branch from 23b99a1 to bc3c390 Compare July 20, 2026 11:55

Copilot started reviewing on behalf of martin-kanis July 20, 2026 11:55 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

keycloak-github-bot Bot added the flaky-test label Jul 20, 2026

keycloak-github-bot Bot reviewed Jul 20, 2026

View reviewed changes

ahus1 approved these changes Jul 21, 2026

View reviewed changes

ahus1 merged commit bef2a5e into keycloak:main Jul 21, 2026
146 of 149 checks passed

martin-kanis mentioned this pull request Jul 21, 2026

Flaky test: org.keycloak.testsuite.forms.BrowserFlowTest#testUserWithOneAdditionalFactorOtpSuccess #51064

Closed

This was referenced Jul 21, 2026

Flaky test: org.keycloak.testsuite.oauth.ClientAuthPostMethodTest#testPostAuthenticationNotAllowedWhenBasicRequested #51065

Closed

Flaky test: org.keycloak.testsuite.oauth.ClientAuthPostMethodTest#testBasicAuthenticationNotAllowedWhenPostRequested #51067

Closed


		public abstract class AbstractScimResourceTypeProvider<M extends Model, R extends ResourceTypeRepresentation> implements ScimResourceTypeProvider<R> {

		public static final int MAX_PATCH_OPERATIONS = 100;

		throw new ModelValidationException(
		"PATCH request exceeds maximum allowed number of %d operations".formatted(MAX_PATCH_OPERATIONS));

Uh oh!

Conversation

martin-kanis commented Jul 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

sguilhen Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

sguilhen Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

sguilhen Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

sguilhen Jul 15, 2026

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!

sguilhen Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martin-kanis Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

martin-kanis commented Jul 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

sguilhen Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

sguilhen 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!

keycloak-github-bot Bot commented Jul 20, 2026

Unreported flaky test detected

org.keycloak.testsuite.forms.BrowserFlowTest#testUserWithOneAdditionalFactorOtpSuccess

org.keycloak.testsuite.oauth.ClientAuthPostMethodTest#testPostAuthenticationNotAllowedWhenBasicRequested

org.keycloak.testsuite.oauth.ClientAuthPostMethodTest#testBasicAuthenticationNotAllowedWhenPostRequested

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot Bot commented Jul 20, 2026

Unreported flaky test detected

org.keycloak.testsuite.forms.BrowserFlowTest#testUserWithOneAdditionalFactorOtpSuccess

org.keycloak.testsuite.oauth.ClientAuthPostMethodTest#testPostAuthenticationNotAllowedWhenBasicRequested

org.keycloak.testsuite.oauth.ClientAuthPostMethodTest#testBasicAuthenticationNotAllowedWhenPostRequested

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sguilhen Jul 16, 2026 •

edited

Loading