Repository navigation
Conversation
resolves keycloak#54834 Signed-off-by: Martin Reinhardt <martin@m13t.de>
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved SCIM extension defects allow malformed schema identifiers, omit the required core schema from projected responses, silently ignore custom-URN PATCH updates, and incorrectly exclude unrelated extensions.
4 open issues
What changed in this PR
Adds per-realm SCIM Group extension mappings, allowing existing group metadata—including LDAP-synchronized attributes—to be exposed through custom schemas, similar to user extensions.
Changes:
- Adds mapping validation and support for reading, writing, patching, and filtering Group extension attributes.
- Updates schema discovery, read-only metadata, and extension attribute selection.
- Documents configuration and adds parser and integration tests.
| File | Description |
|---|---|
scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/GroupExtensionTest.java |
Adds integration coverage for Group extensions. |
scim/model/src/test/java/org/keycloak/scim/model/group/GroupAttributeMappingsTest.java |
Tests mapping parsing and validation. |
scim/model/src/main/java/org/keycloak/scim/model/schema/SchemaResourceTypeProvider.java |
Advertises attribute mutability in schema discovery. |
scim/model/src/main/java/org/keycloak/scim/model/group/GroupResourceTypeProviderFactory.java |
Validates mappings on realm attribute updates. |
scim/model/src/main/java/org/keycloak/scim/model/group/GroupResourceTypeProvider.java |
Registers the Group extension schema. |
scim/model/src/main/java/org/keycloak/scim/model/group/GroupExtensionModelSchema.java |
Maps configured attributes between Groups and SCIM extensions. |
scim/model/src/main/java/org/keycloak/scim/model/group/GroupAttributeMappings.java |
Parses and validates per-realm mappings. |
scim/model/pom.xml |
Adds JUnit test support. |
scim/core/src/main/java/org/keycloak/scim/resource/schema/attribute/Attribute.java |
Adds read-only metadata and extension-URN selection. |
scim/core/src/main/java/org/keycloak/scim/resource/group/Group.java |
Removes the core-only schema override. |
docs/documentation/server_admin/topics/scim/managing-schema-extensions.adoc |
Documents Group extension configuration and usage. |
docs/documentation/server_admin/topics/scim/managing-groups.adoc |
Links to Group extension documentation. |
Review effort: Balanced
| private Map<String, Mapping> mappings; | ||
|
|
||
| public Gro 8000 upExtensionModelSchema(KeycloakSession session) { | ||
| super(KEYCLOAK_GROUP_SCHEMA); |
| Map<String, Object> schemaAttributes = (Map<String, Object>) extensions.computeIfAbsent(schema, k -> new HashMap<>()); | ||
|
|
||
| schemaAttributes.put(attribute.getSimpleName(), value); | ||
| group.addSchema(schema); |
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved Group extension bugs silently ignore valid PATCH requests, omit the required core schema from projected responses, conflate extension selections, and accept schema mappings that filters cannot parse.
4 open issues
Review effort: Balanced
ab098bb to
5b7ad9b
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Active issues remain in extension PATCH handling: replacement merges multivalued attributes instead of replacing them, and filtered removal uses case-sensitive comparisons despite caseExact being false.
4 open issues
2 resolved since last review
Review effort: Balanced
| // the path is not necessarily the id of this schema if it serves multiple extension schemas | ||
| String schema = isExtensionSchema(path) ? path : getId(); | ||
|
|
||
| if (!name.startsWith(schema)) { | ||
| name = schema + ":" + name; |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Martin Reinhardt <martin@m13t.de>
Unreported flaky test detectedIf 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.broker.KcSamlSpDescriptorTest#testAttributeConsumingServiceAttributeRoleMapperInSpMetadataWithServiceNameKeycloak CI - FIPS IT (non-strict) |
There was a problem hiding this comment.
🔵 Needs a closer look
Extension-object PATCH replace operations incorrectly merge multivalued attributes, retaining old values for both schema-URN and pathless forms instead of replacing them.
3 open issues
1 resolved since last review
Review effort: Balanced
Closes #54834
Configuration
Groups have no declarative profile, so the mappings are stored in the realm attribute
scimGroupAttributeMappingsas a JSON array of{name, scimAttribute, readOnly, multivalued}. I chose a realm attribute over a new section in the User Profile config, which would mix group settings into the user profile, and over an SPI option, which would apply the same mapping to every realm. A realm attribute can be set per realm through the existing admin API and realm import, without new endpoints or representation changes. Admin Console support is left for a follow-up.Validation
The mappings are validated by a
RealmAttributeUpdateEventlistener in the Groups provider factory, so invalid values are rejected with400 Bad Requestfrom both the admin API and realm import. Besides duplicate mappings (SCIM names are compared case-insensitively, per RFC 7643 §2.1), the validation rejects schemas reserved by RFC 7643/7644 and the Enterprise User extension, and sub-attributes, which are out of scope for the first iteration. It also rejectsexternalId, which the core schema already maps, and names ofGroupEntityfields such asnameordescription: the JPA filter resolves a model attribute name against entity fields before the attribute collection, so mapping these names would make filters compare against the column instead of the group attribute. If an invalid value reaches the database anyway, it is logged and ignored at runtime, and no attributes are exposed.Behavior
Behavior mirrors user extensions: one internal schema (
urn:keycloak:params:scim:schemas:extension:realm:1.0:Group) serves every configured extension URN, and an extension is listed in/Schemasonly when it has at least one mapped attribute. Values sent for read-only attributes are silently ignored, the same as read-only User Profile attributes, rather than rejected withmutability. Read-only attributes are now advertised withmutability: readOnlyin/Schemas, which also required areadOnly()option onAttribute.Related fixes
Group#getSchemas()override. It always returned only the core schema, so extension URNs could never appear in Group responses.Attribute.isPresentnow also matches an extension attribute's own schema URN. Before,attributes/excludedAttributesmatched only the id of the model schema, which differs from the URN when one internal schema serves several extensions. This also fixes excluding custom user extension schemas by URN.Known limitations
A
PUTthat omits an extension attribute leaves its value unchanged, and the create/update responses echo the request. Both match the current behavior for users and groups and are not changed here.Testing
Added unit tests for the mapping parser in
scim/modelandGroupExtensionTest(discovery, read, create/update, patch, filter, read-only, invalid configuration). The fullscim/tests/basesuite passes on the embedded server.🤖 Generated with Claude Code