Add Support for Custom User Attributes in SCIM Provisioning#81
Conversation
- Introduced SCIM_IDENTITY_PROVIDER_ALIAS. - Updated kc-test.json to include unmanagedAttributePolicy and custom attributes. - Enhanced MetadataController to handle custom attributes based on identity provider mappers. - Updated tests to validate provisioning and patching of custom attributes.
|
|
Hi @nicolamacoir , could you take a look to this PR and share feedback please? |
nicolamacoir
left a comment
nicolamacoir
left a comment
There was a problem hiding this comment.
Nice work on this one — clean feature addition, and I like that it's fully opt-in. Overall this looks solid and the test coverage is good. Just the duplicate env var and the swapped descriptions are worth fixing before merge IMO.
|
|
Is this ready to merge? |
nicolamacoir
left a comment
nicolamacoir
left a comment
There was a problem hiding this comment.
Reviewed the change. Useful feature and the design (use IdP mappers as the extension surface for SCIM custom attributes) is reasonable. Two blocking issues I'd like to discuss before merge, plus a few smaller ones.
1. Duplicate scimPath between USER_PROFILE and IDP_MAPPER crashes every endpoint that calls getUserAttributes
Collectors.toMap has no merge function — duplicate keys throw IllegalStateException. The new code at MetadataController.java:314-336 only filters IDP_MAPPER candidates against the 5 built-in Keycloak attribute names (username, email, firstName, lastName, enabled), not against the existing USER_PROFILE custom attributes. The test realm (kc-test.json) already defines three of those (displayName, externalId, preferredLanguage), and any of them is a realistic IdP-mapper target in an Entra ID setup. The first SCIM call after such a config bricks the endpoint:
java.lang.IllegalStateException: Duplicate key displayName ...
at j.u.s.Collectors.duplicateKeyException
at j.u.s.Collectors$$Lambda...
at MetadataController.getUserAttributeMappingList ...
Suggest collecting USER_PROFILE attribute names into the dedup set as well — or passing a merge function to toMap and logging the conflict.
2. NPE on PATCH remove for any IDP_MAPPER attribute
The writer is (user, value) -> user.setAttribute(attribute, List.of(value)). List.of(null) throws NPE on Java 9+. SCIM PATCH op=remove lands in UsersController.applyPatchValue which calls ua.write(existing, null) → unhandled 500. Same defect class flagged in the discussion threads on #101 / #108 for USER_PROFILE attributes; this PR adds new instances of the pattern. Fix: when value == null, call user.removeAttribute(attribute) instead.
3. Silent no-op when scim.identity.provider.alias points at a nonexistent IdP
IdentityProviderStorageProvider.getMappersByAliasStream(alias) returns an empty stream for a typo'd or stale alias — no exception, no log. The admin sees the feature silently produce nothing. RealmScimConfig.validateConfig doesn't catch this case either (it only requires the alias when SCIM_LINK_IDP=true). Suggest a WARN log when UnmanagedAttributePolicy=ENABLED and the alias is set but the stream yields zero mappers.
4. Custom + IDP_MAPPER attributes registered under the core User URN
All custom and IDP_MAPPER attributes are appended to urn:ietf:params:scim:schemas:core:2.0:User. RFC 7643 §3.3 treats the core User schema as fixed; vendor-specific attributes should live under a separate extension schema URN registered via ResourceType.schemaExtensions. Entra ID and Okta tolerate this in practice but strict SCIM consumers will reject or ignore unknown attributes inside the core schema. Worth a forward-compatible path even if not changed in this PR.
Smaller stuff
- The reader
user -> user.getFirstAttribute(attribute)silently drops additional values for multi-valued Keycloak attributes. Fine for SCIM single-string fields; worth a comment so it's not surprising later. USER_ATTRIBUTEis imported fromorg.keycloak.broker.oidc.mappers.UserAttributeMapper— couples a generic feature to the OIDC broker module. The literal string"user.attribute"matches SAML mappers by coincidence; recommend a local constant + a comment listing the supported mapper types. Mappers that use a different config key are silently skipped with no log — at least DEBUG-log skips.- IDP_MAPPER
scimPathequals the raw Keycloak attribute name. An attribute calledjob_titleshows up in the SCIM schema asjob_title, not the camelCase that Entra/Okta expect. Either document the naming expectation or allow an explicitscimPathoverride on the mapper config. - README (
### Configuration on Realm levelblock) adds"scim.external.shared.secret.hash.algorithm": "string"but no corresponding implementation exists in this PR. Either drop the docs line or wire it throughRealmScimConfig. - README Azure Portal section has duplicate / mangled step numbering and a stray
### Microsoft Entra ID SCIM Configurationheader injected mid-section (thefix: cleanup after wrong mergecommit didn't quite finish). Worth tidying up before merge. ScimConfig.javachange is whitespace-only;RealmScimConfig.javais constant reordering; several test files expand wildcardimport staticto explicit imports. None of those belong in a feature PR — squashing them out would make the diff easier to review.
… support for custom attributes
|
3 participants
This PR enables provisioning of custom user attributes (e.g., job, department, employeeId) from external identity providers like Azure Entra ID into Keycloak via SCIM.
What Changed
Previously, the SCIM server only exposed built-in attributes (userName, email, name.givenName, name.familyName, active).
This PR extends SCIM to support custom attributes by:
How It Works
When a SCIM provisioning request is received, the server:
Documentation
Full configuration guide added to README.md with step-by-step instructions for both Keycloak and Azure Entra ID setup.