Enforce SCIM attribute mutability on PATCH operations by sguilhen · Pull Request #51580 · keycloak/keycloak

sguilhen · 2026-08-10T13:23:09Z

Closes #51155

Copilot

Pull request overview

Enforces SCIM immutable attribute handling during PATCH requests.

Changes:

Rejects PATCH mutations with HTTP 400 and scimType=mutability.
Adds mutability exception handling.
Adds Group and User integration tests.

Reviewed changes

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

Show a summary per file

File	Description
`scim/core/.../AbstractModelSchema.java`	Enforces immutable attributes during PATCH.
`scim/core/.../ScimMutabilityException.java`	Defines the mutability exception.
`scim/services/.../Error.java`	Maps mutability errors to SCIM responses.
`scim/tests/.../GroupTest.java`	Tests immutable Group attributes.
`scim/tests/.../UserTest.java`	Tests immutable User metadata.

💡 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 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

scim/core/src/main/java/org/keycloak/scim/resource/schema/AbstractModelSchema.java:363

The error message uses attribute.getName(), which may be ambiguous for nested attributes (e.g., clients patching meta.created may only see created). Consider including the full SCIM path being patched (or otherwise a fully-qualified attribute path) in the exception message to make client-side debugging and TCK diagnostics more actionable.

                throw new ScimMutabilityException(
                        "Attribute '" + attribute.getName() + "' is immutable");

scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/GroupTest.java:176

These repeated try/catch blocks assert the same response shape multiple times. To reduce duplication and make future mutability tests easier to extend, consider extracting a small helper (e.g., assertMutabilityError(Runnable patchCall, @Nullable String expectedDetailContains)) used by replace/add/remove cases.

        // PATCH replace on immutable externalId should fail
        try {
            client.groups().patch(group.getId(), PatchRequest.create()
                    .replace("externalId", "new-value")
                    .build());
            fail("should fail because externalId is immutable");
        } catch (ScimClientException sce) {
            ErrorResponse error = sce.getError();
            assertNotNull(error);
            assertEquals(400, error.getStatusInt());
            assertEquals("mutability", error.getScimType());
            assertTrue(error.getDetail().contains("externalId"));
        }

        // PATCH add on immutable externalId should fail
        try {
            client.groups().patch(group.getId(), PatchRequest.create()
                    .add("externalId", "new-value")
                    .build());
            fail("should fail because externalId is immutable");
        } catch (ScimClientException sce) {
            ErrorResponse error = sce.getError();
            assertNotNull(error);
            assertEquals(400, error.getStatusInt());
            assertEquals("mutability", error.getScimType());
        }

        // PATCH remove on immutable externalId should fail
        try {
            client.groups().patch(group.getId(), PatchRequest.create()
                    .remove("externalId")
                    .build());
            fail("should fail because externalId is immutable");
        } catch (ScimClientException sce) {
            ErrorResponse error = sce.getError();
            assertNotNull(error);
            assertEquals(400, error.getStatusInt());
            assertEquals("mutability", error.getScimType());
        }

Copilot

Pull request overview

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

Suppressed comments (1)

scim/core/src/main/java/org/keycloak/scim/resource/schema/AbstractModelSchema.java:360

The presence check cannot detect either immutable meta.created value: GroupCoreModelSchema.getAttributeValue() has no createdTimestamp case, and the user schema delegates it to profile attributes, which do not expose UserModel.getCreatedTimestamp(). As a result, PATCH remove/add/replace sees null, returns/no-ops instead of throwing, and the new testPatchImmutableMetaCreated will fail; make both schema getters return the model creation timestamp and cover the group path too.

            String modelAttrName = attribute.getModelAttributeName();

            if (modelAttrName == null || getAttributeValue(model, modelAttrName) != null) {

Copilot

Pull request overview

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

Suppressed comments (1)

scim/core/src/main/java/org/keycloak/scim/resource/schema/AbstractModelSchema.java:359

GroupCoreModelSchema.getAttributeValue() returns null for its createdTimestamp mapping, so PATCHing the already-set Group meta.created bypasses this check instead of returning scimType=mutability. Add a createdTimestamp case backed by GroupModel.getCreatedTimestamp() and cover Group meta.created, which is one of the attributes reported in #51155.

            String modelAttrName = attribute.getModelAttributeName();
            if (modelAttrName == null || getAttributeValue(model, modelAttrName) != null) {

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

Copilot

Pull request overview

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

sguilhen requested a review from a team as a code owner August 10, 2026 13:23

Copilot AI balanced review requested due to automatic review settings August 10, 2026 13:23

keycloak-github-bot Bot added the team/core-iam label Aug 10, 2026

sguilhen requested a review from martin-kanis August 10, 2026 13:24

Copilot started reviewing on behalf of sguilhen August 10, 2026 13:27 View session

Copilot AI reviewed Aug 10, 2026

View reviewed changes

Comment thread scim/core/src/main/java/org/keycloak/scim/resource/schema/AbstractModelSchema.java

Copilot AI review requested due to automatic review settings August 10, 2026 15:18

sguilhen force-pushed the 51155-scim-patch-immutable-mutability branch from 1735885 to 4eb5095 Compare August 10, 2026 15:18

Copilot AI reviewed Aug 10, 2026

View reviewed changes

Comment thread scim/core/src/main/java/org/keycloak/scim/resource/schema/AbstractModelSchema.java

Copilot started reviewing on behalf of sguilhen August 10, 2026 16:11 View session

Copilot AI review requested due to automatic review settings August 10, 2026 17:29

sguilhen force-pushed the 51155-scim-patch-immutable-mutability branch from 4eb5095 to b5219c4 Compare August 10, 2026 17:29

Copilot started reviewing on behalf of sguilhen August 10, 2026 17:39 View session

Copilot AI reviewed Aug 10, 2026

View reviewed changes

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

keycloak-github-bot Bot mentioned this pull request Aug 10, 2026

Flaky test: org.keycloak.testsuite.webauthn.registration.WebAuthnOtherSettingsTest#apiInvalidStateErrorMessage #51175

Open

Copilot AI review requested due to automatic review settings August 10, 2026 19:16

sguilhen force-pushed the 51155-scim-patch-immutable-mutability branch from b5219c4 to 0dc8129 Compare August 10, 2026 19:16

Copilot started reviewing on behalf of sguilhen August 10, 2026 19:17 View session

Copilot AI reviewed Aug 10, 2026

View reviewed changes

Enforce SCIM attribute mutability on PATCH operations

e2db7e6

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

Copilot AI review requested due to automatic review settings August 11, 2026 02:09

sguilhen force-pushed the 51155-scim-patch-immutable-mutability branch from 0dc8129 to e2db7e6 Compare August 11, 2026 02:09

Copilot started reviewing on behalf of sguilhen August 11, 2026 02:09 View session

Copilot AI reviewed Aug 11, 2026

View reviewed changes

pedroigor approved these changes Aug 11, 2026

View reviewed changes

pedroigor merged commit e13aab9 into keycloak:main Aug 11, 2026
150 of 151 checks passed

Navigation Menu

Uh oh!

Enforce SCIM attribute mutability on PATCH operations - #51580

Enforce SCIM attribute mutability on PATCH operations#51580
pedroigor merged 1 commit into
keycloak:mainfrom
sguilhen:51155-scim-patch-immutable-mutability

sguilhen commented Aug 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

sguilhen commented Aug 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants