fix: smaller scope of an initial refactoring to bridge scim and admin v2 - #51456
Open
shawkins wants to merge 1 commit into
Open
fix: smaller scope of an initial refactoring to bridge scim and admin v2#51456shawkins wants to merge 1 commit into
shawkins wants to merge 1 commit into
Conversation
closes: keycloak#51326 Signed-off-by: Steve Hawkins <shawkins@redhat.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors Admin API v2 client queries to reuse SCIM schema metadata while relaxing SCIM representation type constraints.
Changes:
- Generalizes SCIM schema and attribute generics beyond
ResourceTypeRepresentation. - Replaces the dedicated client JPA query schema/provider with protocol-specific client schemas.
- Uses the new schemas for JPA filtering, sorting, projection, and response population.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
server-spi/src/main/java/org/keycloak/models/ClientModel.java |
Makes clients implement the common model contract. |
scim/core/src/main/java/org/keycloak/scim/resource/schema/ModelSchema.java |
Relaxes representation typing. |
scim/core/src/main/java/org/keycloak/scim/resource/schema/attribute/ComplexAttributeSetter.java |
Generalizes complex representation setters. |
scim/core/src/main/java/org/keycloak/scim/resource/schema/attribute/AttributeMapper.java |
Generalizes attribute mapping. |
scim/core/src/main/java/org/keycloak/scim/resource/schema/attribute/Attribute.java |
Generalizes attribute metadata and builders. |
rest/admin-v2/services/src/main/java/org/keycloak/services/client/ScimBackedClientService.java |
Uses protocol schemas for client list responses and projections. |
rest/admin-v2/services/src/main/java/org/keycloak/services/client/scim/SAMLClientModelSchema.java |
Adds the SAML client schema singleton. |
rest/admin-v2/services/src/main/java/org/keycloak/services/client/scim/OIDCClientModelSchema.java |
Adds the OIDC client schema singleton. |
rest/admin-v2/services/src/main/java/org/keycloak/services/client/scim/ClientQueryRepresentation.java |
Removes the query-only placeholder representation. |
rest/admin-v2/services/src/main/java/org/keycloak/services/client/scim/ClientJpaQuerySchema.java |
Removes the dedicated JPA query schema. |
rest/admin-v2/services/src/main/java/org/keycloak/services/client/scim/ClientJpaQueryProvider.java |
Removes the query-only SCIM provider adapter. |
rest/admin-v2/services/src/main/java/org/keycloak/services/client/scim/ClientJpaQueryExecutor.java |
Resolves JPA metadata through client schemas. |
rest/admin-v2/services/src/main/java/org/keycloak/services/client/scim/BaseClientModelSchema.java |
Defines shared client query attributes and representation population. |
Suppressed comments (1)
rest/admin-v2/services/src/main/java/org/keycloak/services/client/ScimBackedClientService.java:130
- Projection validation now rejects existing API fields such as
uuid,redirectUris,roles,loginFlows, and SAML settings whenever the JPA path is used, even though the delegate accepts them. Until the schemas cover all representation fields, validation must continue to use the complete mapper metadata (or force these requests through the delegate).
if (SCHEMAS.values().stream().noneMatch(s -> s.getAttributes().containsKey(field))) {
Comment on lines
+85
to
+89
| .<BaseClientRepresentation>map(client -> { | ||
| BaseClientModelSchema<?> schema = SCHEMAS.get(client.getProtocol()); | ||
| if (schema == null) return null; | ||
| return populateFromSchema(schema, client, includeList); | ||
| }) |
Contributor
Author
There was a problem hiding this comment.
This is the expected initial state.
Member
There was a problem hiding this comment.
IIUC it is not an issue as there is a fallback when field is not JPA_FIELDS?
Member
There was a problem hiding this comment.
Actually, never mind, I need to re-read it properly.
Member
There was a problem hiding this comment.
Right, the issue is that if it is a JPA_FIELDS, we only return one of these JPA fields and not things like uuid, direct uris etc. Ok, got it.
Contributor
Author
There was a problem hiding this comment.
In a subsequent PR or commit all (or nearly all) the fields will be added to the schema. The JPA_FIELDS tracking will instead become "searchable fields"
| * object from the RESTful layer, to a {@link Model} and vice versa. | ||
| */ | ||
| public interface ModelSchema<M extends Model, R extends ResourceTypeRepresentation> { | ||
| public interface ModelSchema<M extends Model, R> { |
| * @see ModelSchema | ||
| */ | ||
| public class Attribute<M extends Model, R extends ResourceTypeRepresentation> { | ||
| public class Attribute<M extends Model, R> { |
| * @see Attribute | ||
| */ | ||
| public class AttributeMapper<M extends Model, R extends ResourceTypeRepresentation> { | ||
| public class AttributeMapper<M extends Model, R> { |
michalvavrik
approved these changes
Aug 6, 2026
michalvavrik
left a comment
Member
There was a problem hiding this comment.
I think there are scenarios that are not handled properly for now (like when you query combination of JPA and non-JPA fields or when you expect also non-JPA fields in response), but that seems to be expected and changes seems fine to me.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It seems better to get the changes in incrementally. As described here #51326 (comment) there would be several more phases of refactoring beyond this.
This first PR is to mainly eliminate the need for ClientJPAQuerySchema, and to make the usage of the schema logic more consistent - full removal of our mapping logic, and any refinements of the Attribute metadata, will come in subsequent PRs.
It also relaxes the type restriction of the scim logic so that we don't have to use ResourceTypeRepresentation as a base type.
Alternatively if it seems preferable, I can keep layering additional commits on top of this PR.
closes: #51326