From a3e3feb726a5bd9ee9e835451d02030b01908dae Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Wed, 6 May 2026 11:01:29 -0400 Subject: [PATCH] fix: initial projection implementation (#48460) closes: #48734 Signed-off-by: Steve Hawkins --- js/libs/keycloak-admin-client/openapi.yaml | 11 ++ .../keycloak/admin/api/client/ClientsApi.java | 9 +- .../models/mapper/BaseClientModelMapper.java | 127 ++++++++++-------- .../models/mapper/ClientModelMapper.java | 3 +- .../mapper/ClientModelMapperFactory.java | 9 -- .../models/mapper/ClientModelMapperSpi.java | 30 ----- .../models/mapper/ClientModelMappers.java | 26 ++++ .../models/mapper/OIDCClientModelMapper.java | 57 ++++---- .../mapper/OIDCClientModelMapperFactory.java | 33 ----- .../models/mapper/RepModelMapper.java | 15 ++- .../models/mapper/SAMLClientModelMapper.java | 73 ++++------ .../mapper/SAMLClientModelMapperFactory.java | 51 ------- .../admin/api/client/DefaultClientsApi.java | 11 +- .../services/client/ClientService.java | 25 ++-- .../services/client/DefaultClientService.java | 32 +++-- ...oak.models.mapper.ClientModelMapperFactory | 2 - .../services/org.keycloak.provider.Spi | 1 - .../admin/client/v2/ClientApiV2Test.java | 24 ++++ .../mapper/OIDCClientModelMapperTest.java | 40 +++--- .../mapper/SAMLClientModelMapperTest.java | 40 +++--- 20 files changed, 281 insertions(+), 338 deletions(-) delete mode 100644 rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMapperFactory.java delete mode 100644 rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMapperSpi.java create mode 100644 rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMappers.java delete mode 100644 rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/OIDCClientModelMapperFactory.java delete mode 100644 rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/SAMLClientModelMapperFactory.java delete mode 100644 rest/admin-v2/services/src/main/resources/META-INF/services/org.keycloak.models.mapper.ClientModelMapperFactory delete mode 100755 rest/admin-v2/services/src/main/resources/META-INF/services/org.keycloak.provider.Spi diff --git a/js/libs/keycloak-admin-client/openapi.yaml b/js/libs/keycloak-admin-client/openapi.yaml index 25f1116d683..1b599075664 100644 --- a/js/libs/keycloak-admin-client/openapi.yaml +++ b/js/libs/keycloak-admin-client/openapi.yaml @@ -179,6 +179,17 @@ paths: operationId: getClients tags: - Clients (v2) + parameters: + - description: "Set of fields to include in the response. Must be top-level\ + \ fields on one of the client types. If omitted or empty, all fields will\ + \ be populated." + name: fields + in: query + schema: + type: array + uniqueItems: true + items: + type: string responses: "200": description: OK diff --git a/rest/admin-v2/api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java b/rest/admin-v2/api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java index fd1ad7914c9..93f882af3ee 100644 --- a/rest/admin-v2/api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java +++ b/rest/admin-v2/api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java @@ -1,5 +1,6 @@ package org.keycloak.admin.api.client; +import java.util.Set; import java.util.stream.Stream; import jakarta.validation.Valid; @@ -9,6 +10,7 @@ import jakarta.ws.rs.POST; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; import jakarta.ws.rs.Produces; +import jakarta.ws.rs.QueryParam; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; @@ -20,6 +22,7 @@ import org.eclipse.microprofile.openapi.annotations.enums.SchemaType; import org.eclipse.microprofile.openapi.annotations.extensions.Extension; import org.eclipse.microprofile.openapi.annotations.media.Content; import org.eclipse.microprofile.openapi.annotations.media.Schema; +import org.eclipse.microprofile.openapi.annotations.parameters.Parameter; import org.eclipse.microprofile.openapi.annotations.responses.APIResponse; import org.eclipse.microprofile.openapi.annotations.responses.APIResponses; import org.eclipse.microprofile.openapi.annotations.tags.Tag; @@ -35,7 +38,11 @@ public interface ClientsApi { @APIResponses(value = { @APIResponse(responseCode = "200", content = @Content(schema = @Schema(type = SchemaType.ARRAY, implementation = BaseClientRepresentation.class))) }) - Stream getClients(); + Stream getClients(@Parameter(description = "Set of fields to include in the response. Must be top-level fields on one of the client types. If omitted or empty, all fields will be populated.") @QueryParam("fields") Set fields); + + default Stream getClients() { + return getClients(Set.of()); + } @POST @Consumes(MediaType.APPLICATION_JSON) diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/BaseClientModelMapper.java b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/BaseClientModelMapper.java index 4f9b4e039ba..9b332445c5d 100644 --- a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/BaseClientModelMapper.java +++ b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/BaseClientModelMapper.java @@ -1,81 +1,96 @@ package org.keycloak.models.mapper; -import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; +import java.util.function.BiConsumer; +import java.util.function.Function; import java.util.stream.Collectors; import org.keycloak.models.ClientModel; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.representations.admin.v2.BaseClientRepresentation; -/** - * @author Vaclav Muzikar - */ public abstract class BaseClientModelMapper implements ClientModelMapper { - protected final KeycloakSession session; - - public BaseClientModelMapper(KeycloakSession session) { - this.session = session; + + public static class MappedField { + + Function repGetter; + BiConsumer repSetter; + Function modelGetter; + BiConsumer modelSetter; + + void fromModel(ClientModel model, T rep) { + if (repSetter != null && modelGetter != null) { + repSetter.accept(rep, modelGetter.apply(model)); + } + } + + void toModel(T rep, ClientModel model) { + if (hasGetter() && modelSetter != null) { + // TODO: exception handling to make things clearer when things fail + modelSetter.accept(model, getValue(rep)); + } + } + + public boolean hasGetter() { + return repGetter != null; + } + + public V getValue(T rep) { + if (repGetter != null) { + return (V) repGetter.apply(rep); + } + return null; + } } - + + final Map> fields = new LinkedHashMap>(); + + protected void addMapping(String name, Function repGetter, BiConsumer repSetter, Function modelGetter, BiConsumer modelSetter) { + MappedField prop = new MappedField<>(); + prop.repGetter = repGetter; + prop.repSetter = repSetter; + prop.modelGetter = modelGetter; + prop.modelSetter = modelSetter; + this.fields.put(name, prop); + } + + public BaseClientModelMapper() { + this.addMapping("protocol", BaseClientRepresentation::getProtocol, null, ClientModel::getProtocol, ClientModel::setProtocol); + this.addMapping("uuid", BaseClientRepresentation::getUuid, BaseClientRepresentation::setUuid, ClientModel::getId, null); + this.addMapping("enabled", BaseClientRepresentation::getEnabled, BaseClientRepresentation::setEnabled, ClientModel::isEnabled, (model, enabled) -> model.setEnabled(Boolean.TRUE.equals(enabled))); + this.addMapping("clientId", BaseClientRepresentation::getClientId, BaseClientRepresentation::setClientId, ClientModel::getClientId, ClientModel::setClientId); + this.addMapping("description", BaseClientRepresentation::getDescription, BaseClientRepresentation::setDescription, ClientModel::getDescription, ClientModel::setDescription); + this.addMapping("displayName", BaseClientRepresentation::getDisplayName, BaseClientRepresentation::setDisplayName, ClientModel::getName, ClientModel::setName); + this.addMapping("appUrl", BaseClientRepresentation::getAppUrl, BaseClientRepresentation::setAppUrl, ClientModel::getBaseUrl, ClientModel::setBaseUrl); + // TODO: consider built-in logic for copying collections + this.addMapping("redirectUris", BaseClientRepresentation::getRedirectUris, BaseClientRepresentation::setRedirectUris, model -> new LinkedHashSet<>(model.getRedirectUris()), (model, uris) -> model.setRedirectUris(new LinkedHashSet<>(uris))); + this.addMapping("roles", BaseClientRepresentation::getRoles, BaseClientRepresentation::setRoles, model -> model.getRolesStream().map(RoleModel::getName).collect(Collectors.toSet()), null); + } + @Override - public BaseClientRepresentation fromModel(ClientModel model) { + public BaseClientRepresentation fromModel(ClientModel model, Set includeFields) { // We don't want reps to depend on any unnecessary fields deps, hence no generated builder. T rep = createClientRepresentation(); - - rep.setUuid(model.getId()); - rep.setEnabled(model.isEnabled()); - rep.setClientId(model.getClientId()); - rep.setDescription(model.getDescription()); - rep.setDisplayName(model.getName()); - rep.setAppUrl(model.getBaseUrl()); - rep.setRedirectUris(new HashSet<>(model.getRedirectUris())); - rep.setRoles(model.getRolesStream().map(RoleModel::getName).collect(Collectors.toSet())); - - fromModelSpecific(model, rep); + + var stream = fields.entrySet().stream(); + if (includeFields != null && !includeFields.isEmpty()) { + stream = stream.filter(e -> includeFields.contains(e.getKey())); + } + stream.forEach(e -> e.getValue().fromModel(model, rep)); return rep; } @Override @SuppressWarnings("unchecked") - public ClientModel toModel(BaseClientRepresentation rep, ClientModel existingModel) { - if (existingModel == null) { - existingModel = createClientModel(rep); - } - - existingModel.setProtocol(rep.getProtocol()); - existingModel.setEnabled(Boolean.TRUE.equals(rep.getEnabled())); - existingModel.setClientId(rep.getClientId()); - existingModel.setDescription(rep.getDescription()); - existingModel.setName(rep.getDisplayName()); - existingModel.setBaseUrl(rep.getAppUrl()); - existingModel.setRedirectUris(new HashSet<>(rep.getRedirectUris())); - // Roles are not handled here - - toModelSpecific((T) rep, existingModel); - - return existingModel; - } - - protected ClientModel createClientModel(BaseClientRepresentation rep) { - RealmModel realm = session.getContext().getRealm(); - - // dummy add/remove to obtain a detached model - var model = realm.addClient(rep.getClientId()); - realm.removeClient(model.getId()); - return model; + public void toModel(BaseClientRepresentation rep, ClientModel existingModel) { + fields.values().forEach(m -> m.toModel(rep, existingModel)); } protected abstract T createClientRepresentation(); - protected abstract void fromModelSpecific(ClientModel model, T rep); - - protected abstract void toModelSpecific(T rep, ClientModel model); - - @Override - public void close() { - } } diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMapper.java b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMapper.java index 54234581b4a..71de58c6f7b 100644 --- a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMapper.java +++ b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMapper.java @@ -1,11 +1,10 @@ package org.keycloak.models.mapper; import org.keycloak.models.ClientModel; -import org.keycloak.provider.Provider; import org.keycloak.representations.admin.v2.BaseClientRepresentation; /** * @author Vaclav Muzikar */ -public interface ClientModelMapper extends Provider, RepModelMapper { +public interface ClientModelMapper extends RepModelMapper { } diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMapperFactory.java b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMapperFactory.java deleted file mode 100644 index 612d8c003f5..00000000000 --- a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMapperFactory.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.keycloak.models.mapper; - -import org.keycloak.provider.ProviderFactory; - -/** - * @author Vaclav Muzikar - */ -public interface ClientModelMapperFactory extends ProviderFactory { -} diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMapperSpi.java b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMapperSpi.java deleted file mode 100644 index 3f1504ffbad..00000000000 --- a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMapperSpi.java +++ /dev/null @@ -1,30 +0,0 @@ -package org.keycloak.models.mapper; - -import org.keycloak.provider.Provider; -import org.keycloak.provider.ProviderFactory; -import org.keycloak.provider.Spi; - -/** - * @author Vaclav Muzikar - */ -public class ClientModelMapperSpi implements Spi { - @Override - public boolean isInternal() { - return true; - } - - @Override - public String getName() { - return "client-model-mapper"; - } - - @Override - public Class getProviderClass() { - return ClientModelMapper.class; - } - - @Override - public Class> getProviderFactoryClass() { - return ClientModelMapperFactory.class; - } -} diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMappers.java b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMappers.java new file mode 100644 index 00000000000..1ff032821f2 --- /dev/null +++ b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/ClientModelMappers.java @@ -0,0 +1,26 @@ +package org.keycloak.models.mapper; +import java.util.Map; +import java.util.Optional; + +import org.keycloak.representations.admin.v2.OIDCClientRepresentation; +import org.keycloak.representations.admin.v2.SAMLClientRepresentation; + +public class ClientModelMappers { + + private final Map> mappers; + + public ClientModelMappers() { + // TODO: this may be done via discovery later + mappers = Map.of(OIDCClientRepresentation.PROTOCOL, new OIDCClientModelMapper(), + SAMLClientRepresentation.PROTOCOL, new SAMLClientModelMapper()); + } + + public boolean isKnownField(String name) { + return mappers.values().stream().anyMatch(f -> f.fields.containsKey(name)); + } + + public Optional> getMapper(String protocol) { + return Optional.ofNullable(mappers.get(protocol)); + } + +} diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/OIDCClientModelMapper.java b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/OIDCClientModelMapper.java index 4cf555dee06..c5dc4dbf788 100644 --- a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/OIDCClientModelMapper.java +++ b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/OIDCClientModelMapper.java @@ -2,58 +2,51 @@ package org.keycloak.models.mapper; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Set; import java.util.stream.Collectors; import org.keycloak.models.ClientModel; -import org.keycloak.models.KeycloakSession; import org.keycloak.models.RoleModel; import org.keycloak.representations.admin.v2.OIDCClientRepresentation; +import org.keycloak.representations.admin.v2.OIDCClientRepresentation.Auth; +import org.keycloak.utils.KeycloakSessionUtil; /** * @author Vaclav Muzikar */ public class OIDCClientModelMapper extends BaseClientModelMapper { - public OIDCClientModelMapper(KeycloakSession session) { - super(session); - } - @Override protected OIDCClientRepresentation createClientRepresentation() { return new OIDCClientRepresentation(); } - - @Override - protected void fromModelSpecific(ClientModel model, OIDCClientRepresentation rep) { - rep.setLoginFlows(createLoginFlows(model)); - - if (!model.isPublicClient()) { - OIDCClientRepresentation.Auth auth = new OIDCClientRepresentation.Auth(); - auth.setMethod(model.getClientAuthenticatorType()); - auth.setSecret(model.getSecret()); - rep.setAuth(auth); - // TODO: auth.certificate - } - - rep.setWebOrigins(new HashSet<>(model.getWebOrigins())); - rep.setServiceAccountRoles(getServiceAccountRoles(model)); + + public OIDCClientModelMapper() { + addMapping("loginFlows", OIDCClientRepresentation::getLoginFlows, OIDCClientRepresentation::setLoginFlows, model -> createLoginFlows(model), (model, flows) -> setModelFromFlows(flows, model)); + addMapping("auth", OIDCClientRepresentation::getAuth, OIDCClientRepresentation::setAuth, model -> getAuth(model), (model, auth) -> setAuth(model, auth)); + addMapping("webOrigins", OIDCClientRepresentation::getWebOrigins, OIDCClientRepresentation::setWebOrigins, model -> new LinkedHashSet<>(model.getWebOrigins()), (model, webOrigins) -> model.setWebOrigins(new LinkedHashSet<>(webOrigins))); + addMapping("serviceAccountRoles", OIDCClientRepresentation::getServiceAccountRoles, OIDCClientRepresentation::setServiceAccountRoles, model -> getServiceAccountRoles(model), null); } - @Override - protected void toModelSpecific(OIDCClientRepresentation rep, ClientModel model) { - if (rep.getAuth() != null) { + private OIDCClientRepresentation.Auth getAuth(ClientModel model) { + OIDCClientRepresentation.Auth auth = null; + if (!model.isPublicClient()) { + auth = new OIDCClientRepresentation.Auth(); + auth.setMethod(model.getClientAuthenticatorType()); + auth.setSecret(model.getSecret()); + // TODO: auth.certificate + } + return auth; + } + + private void setAuth(ClientModel model, Auth auth) { + if (auth != null) { model.setPublicClient(false); - model.setClientAuthenticatorType(rep.getAuth().getMethod()); - model.setSecret(rep.getAuth().getSecret()); + model.setClientAuthenticatorType(auth.getMethod()); + model.setSecret(auth.getSecret()); } else { model.setPublicClient(true); } - - setModelFromFlows(rep.getLoginFlows(), model); - - model.setWebOrigins(new HashSet<>(rep.getWebOrigins())); - - // Service account roles are not handled here } private Set createLoginFlows(ClientModel model) { @@ -82,7 +75,7 @@ public class OIDCClientModelMapper extends BaseClientModelMapper getServiceAccountRoles(ClientModel client) { if (client.isServiceAccountsEnabled()) { - var serviceAccount = session.users().getServiceAccount(client); + var serviceAccount = KeycloakSessionUtil.getKeycloakSession().users().getServiceAccount(client); if (serviceAccount != null) { return serviceAccount.getRoleMappingsStream() .map(RoleModel::getName) diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/OIDCClientModelMapperFactory.java b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/OIDCClientModelMapperFactory.java deleted file mode 100644 index 5b91e53ec93..00000000000 --- a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/OIDCClientModelMapperFactory.java +++ /dev/null @@ -1,33 +0,0 @@ -package org.keycloak.models.mapper; - -import org.keycloak.Config; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.KeycloakSessionFactory; -import org.keycloak.representations.admin.v2.OIDCClientRepresentation; - -/** - * @author Vaclav Muzikar - */ -public class OIDCClientModelMapperFactory implements ClientModelMapperFactory { - @Override - public ClientModelMapper create(KeycloakSession session) { - return new OIDCClientModelMapper(session); - } - - @Override - public String getId() { - return OIDCClientRepresentation.PROTOCOL; - } - - @Override - public void init(Config.Scope config) { - } - - @Override - public void postInit(KeycloakSessionFactory factory) { - } - - @Override - public void close() { - } -} diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/RepModelMapper.java b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/RepModelMapper.java index 01130c4ecf9..8dadce53fd4 100644 --- a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/RepModelMapper.java +++ b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/RepModelMapper.java @@ -1,14 +1,17 @@ package org.keycloak.models.mapper; +import java.util.Set; + /** * @author Vaclav Muzikar */ public interface RepModelMapper { - T fromModel(U model); - - default U toModel(T rep) { - return toModel(rep, null); + + default T fromModel(U model) { + return fromModel(model, null); } - - U toModel(T rep, U existingModel); + + T fromModel(U model, Set includeFields); + + void toModel(T rep, U existingModel); } diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/SAMLClientModelMapper.java b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/SAMLClientModelMapper.java index 9e7fe7ab532..7eaf84b0c01 100644 --- a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/SAMLClientModelMapper.java +++ b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/SAMLClientModelMapper.java @@ -17,8 +17,10 @@ package org.keycloak.models.mapper; +import java.util.function.BiConsumer; +import java.util.function.Function; + import org.keycloak.models.ClientModel; -import org.keycloak.models.KeycloakSession; import org.keycloak.representations.admin.v2.SAMLClientRepresentation; /** @@ -39,63 +41,40 @@ public class SAMLClientModelMapper extends BaseClientModelMapper repGetter, BiConsumer repSetter) { + this.addMapping(name, repGetter, repSetter, model -> getBooleanAttribute(model, attribute), (model, value) -> setBooleanAttributeIfNotNull(model, attribute, value)); } - - @Override - protected void toModelSpecific(SAMLClientRepresentation rep, ClientModel model) { - model.setProtocol(SAMLClientRepresentation.PROTOCOL); - + + protected void addAttributeMapping(String name, String attribute, Function repGetter, BiConsumer repSetter) { + this.addMapping(name, repGetter, repSetter, model -> model.getAttribute(attribute), (model, value) -> setAttributeIfNotNull(model, attribute, value)); + } + + public SAMLClientModelMapper() { // Name ID settings - setAttributeIfNotNull(model, SAML_NAME_ID_FORMAT, rep.getNameIdFormat()); - setBooleanAttributeIfNotNull(model, SAML_FORCE_NAME_ID_FORMAT, rep.getForceNameIdFormat()); - + addAttributeMapping("nameIdFormat", SAML_NAME_ID_FORMAT, SAMLClientRepresentation::getNameIdFormat, SAMLClientRepresentation::setNameIdFormat); + addBooleanAttributeMapping("forceNameIdFormat", SAML_FORCE_NAME_ID_FORMAT, SAMLClientRepresentation::getForceNameIdFormat, SAMLClientRepresentation::setForceNameIdFormat); + // Signature settings - setBooleanAttributeIfNotNull(model, SAML_AUTHN_STATEMENT, rep.getIncludeAuthnStatement()); - setBooleanAttributeIfNotNull(model, SAML_SERVER_SIGNATURE, rep.getSignDocuments()); - setBooleanAttributeIfNotNull(model, SAML_ASSERTION_SIGNATURE, rep.getSignAssertions()); - setBooleanAttributeIfNotNull(model, SAML_CLIENT_SIGNATURE, rep.getClientSignatureRequired()); - setAttributeIfNotNull(model, SAML_SIGNATURE_ALGORITHM, rep.getSignatureAlgorithm()); - setAttributeIfNotNull(model, SAML_SIGNATURE_CANONICALIZATION, rep.getSignatureCanonicalizationMethod()); - setAttributeIfNotNull(model, SAML_SIGNING_CERTIFICATE, rep.getSigningCertificate()); + addBooleanAttributeMapping("includeAuthnStatement", SAML_AUTHN_STATEMENT, SAMLClientRepresentation::getIncludeAuthnStatement, SAMLClientRepresentation::setIncludeAuthnStatement); + addBooleanAttributeMapping("signDocuments", SAML_SERVER_SIGNATURE, SAMLClientRepresentation::getSignDocuments, SAMLClientRepresentation::setSignDocuments); + addBooleanAttributeMapping("signAssertions", SAML_ASSERTION_SIGNATURE, SAMLClientRepresentation::getSignAssertions, SAMLClientRepresentation::setSignAssertions); + addBooleanAttributeMapping("clientSignatureRequired", SAML_CLIENT_SIGNATURE, SAMLClientRepresentation::getClientSignatureRequired, SAMLClientRepresentation::setClientSignatureRequired); + addAttributeMapping("signatureAlgorithm", SAML_SIGNATURE_ALGORITHM, SAMLClientRepresentation::getSignatureAlgorithm, SAMLClientRepresentation::setSignatureAlgorithm); + addAttributeMapping("signatureCanonicalizationMethod", SAML_SIGNATURE_CANONICALIZATION, SAMLClientRepresentation::getSignatureCanonicalizationMethod, SAMLClientRepresentation::setSignatureCanonicalizationMethod); + addAttributeMapping("signingCertificate", SAML_SIGNING_CERTIFICATE, SAMLClientRepresentation::getSigningCertificate, SAMLClientRepresentation::setSigningCertificate); // Binding and logout settings - setBooleanAttributeIfNotNull(model, SAML_FORCE_POST_BINDING, rep.getForcePostBinding()); - if (rep.getFrontChannelLogout() != null) { - model.setFrontchannelLogout(rep.getFrontChannelLogout()); - } + addBooleanAttributeMapping("forcePostBinding", SAML_FORCE_POST_BINDING, SAMLClientRepresentation::getForcePostBinding, SAMLClientRepresentation::setForcePostBinding); + // TODO: mapping from 3 value to 2 value boolean can be confusing from a patching perspective + addMapping("frontChannelLogout", SAMLClientRepresentation::getFrontChannelLogout, SAMLClientRepresentation::setFrontChannelLogout, ClientModel::isFrontchannelLogout, (model, logout) -> model.setFrontchannelLogout(Boolean.TRUE.equals(logout))); // ECP flow - setBooleanAttributeIfNotNull(model, SAML_ALLOW_ECP_FLOW, rep.getAllowEcpFlow()); + addBooleanAttributeMapping("allowEcpFlow", SAML_ALLOW_ECP_FLOW, SAMLClientRepresentation::getAllowEcpFlow, SAMLClientRepresentation::setAllowEcpFlow); } private Boolean getBooleanAttribute(ClientModel model, String key) { diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/SAMLClientModelMapperFactory.java b/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/SAMLClientModelMapperFactory.java deleted file mode 100644 index 85f3aa42f78..00000000000 --- a/rest/admin-v2/services/src/main/java/org/keycloak/models/mapper/SAMLClientModelMapperFactory.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Copyright 2025 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.keycloak.models.mapper; - -import org.keycloak.Config; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.KeycloakSessionFactory; -import org.keycloak.representations.admin.v2.SAMLClientRepresentation; - -/** - * Factory for creating SAMLClientModelMapper instances. - */ -public class SAMLClientModelMapperFactory implements ClientModelMapperFactory { - - @Override - public ClientModelMapper create(KeycloakSession session) { - return new SAMLClientModelMapper(session); - } - - @Override - public String getId() { - return SAMLClientRepresentation.PROTOCOL; - } - - @Override - public void init(Config.Scope config) { - } - - @Override - public void postInit(KeycloakSessionFactory factory) { - } - - @Override - public void close() { - } -} diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/rest/admin/api/client/DefaultClientsApi.java b/rest/admin-v2/services/src/main/java/org/keycloak/rest/admin/api/client/DefaultClientsApi.java index add0f889af0..1cda721e4c8 100644 --- a/rest/admin-v2/services/src/main/java/org/keycloak/rest/admin/api/client/DefaultClientsApi.java +++ b/rest/admin-v2/services/src/main/java/org/keycloak/rest/admin/api/client/DefaultClientsApi.java @@ -1,11 +1,11 @@ package org.keycloak.rest.admin.api.client; +import java.util.Set; import java.util.stream.Stream; import jakarta.annotation.Nonnull; import jakarta.validation.Valid; import jakarta.ws.rs.ForbiddenException; -import jakarta.ws.rs.GET; import jakarta.ws.rs.POST; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; @@ -17,11 +17,13 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.representations.admin.v2.BaseClientRepresentation; import org.keycloak.services.client.ClientService; +import org.keycloak.services.client.ClientService.ClientProjectionOptions; import org.keycloak.services.client.DefaultClientService; import org.keycloak.services.resources.admin.RealmAdminResource; import org.keycloak.services.resources.admin.fgap.AdminPermissionEvaluator; public class DefaultClientsApi implements ClientsApi { + private final KeycloakSession session; private final AdminPermissionEvaluator permissions; private final RealmModel realm; @@ -41,11 +43,10 @@ public class DefaultClientsApi implements ClientsApi { this.realmAdminResource = realmAdminResource; this.clientService = new DefaultClientService(session, realm, permissions, realmAdminResource); } - - @GET + @Override - public Stream getClients() { - return clientService.getClients(realm); + public Stream getClients(Set fields) { + return clientService.getClients(realm, new ClientProjectionOptions(fields), null, null); } @POST diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/services/client/ClientService.java b/rest/admin-v2/services/src/main/java/org/keycloak/services/client/ClientService.java index f12e281fd7a..a7e029260a1 100644 --- a/rest/admin-v2/services/src/main/java/org/keycloak/services/client/ClientService.java +++ b/rest/admin-v2/services/src/main/java/org/keycloak/services/client/ClientService.java @@ -1,7 +1,10 @@ package org.keycloak.services.client; import java.io.InputStream; +import java.util.Collections; +import java.util.LinkedHashSet; import java.util.Optional; +import java.util.Set; import java.util.stream.Stream; import org.keycloak.models.RealmModel; @@ -17,7 +20,17 @@ public interface ClientService extends Service { } class ClientProjectionOptions { - // TODO + private final LinkedHashSet fields = new LinkedHashSet<>(); + + public ClientProjectionOptions(Set fields) { + if (fields != null) { + this.fields.addAll(fields); + } + } + + public Set getFields() { + return Collections.unmodifiableSet(fields); + } } class ClientSortAndSliceOptions { @@ -29,15 +42,7 @@ public interface ClientService extends Service { record CreateOrUpdateResult(BaseClientRepresentation representation, boolean created) {} - default Optional getClient(RealmModel realm, String clientId) throws ServiceException { - return getClient(realm, clientId, null); - } - - Optional getClient(RealmModel realm, String clientId, ClientProjectionOptions projectionOptions) throws ServiceException; - - default Stream getClients(RealmModel realm) { - return getClients(realm, null, null, null); - } + Optional getClient(RealmModel realm, String clientId) throws ServiceException; Stream getClients(RealmModel realm, ClientProjectionOptions projectionOptions, ClientSearchOptions searchOptions, ClientSortAndSliceOptions sortAndSliceOptions); diff --git a/rest/admin-v2/services/src/main/java/org/keycloak/services/client/DefaultClientService.java b/rest/admin-v2/services/src/main/java/org/keycloak/services/client/DefaultClientService.java index 357950b9c0d..693e4073fdd 100644 --- a/rest/admin-v2/services/src/main/java/org/keycloak/services/client/DefaultClientService.java +++ b/rest/admin-v2/services/src/main/java/org/keycloak/services/client/DefaultClientService.java @@ -27,6 +27,7 @@ import org.keycloak.models.ModelValidationException; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.mapper.ClientModelMapper; +import org.keycloak.models.mapper.ClientModelMappers; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.representations.admin.v2.BaseClientRepresentation; @@ -75,6 +76,7 @@ import static org.keycloak.utils.StringUtil.isBlank; */ public class DefaultClientService implements ClientService { private static final ObjectMapper MAPPER = new ObjectMapperResolver().getContext(null); + private static final ClientModelMappers MAPPERS = new ClientModelMappers(); private final KeycloakSession session; private final AdminPermissionEvaluator permissions; @@ -97,8 +99,7 @@ public class DefaultClientService implements ClientService { @Override public Optional getClient(@Nonnull RealmModel realm, - @Nonnull String clientId, - ClientProjectionOptions projectionOptions) throws ServiceException { + @Nonnull String clientId) throws ServiceException { ClientModel client = realm.getClientByClientId(clientId); if (client == null) { return Optional.empty(); @@ -119,6 +120,17 @@ public class DefaultClientService implements ClientService { ClientSearchOptions searchOptions, ClientSortAndSliceOptions sortAndSliceOptions) { permissions.clients().requireList(); + + // TODO: this check is weak + // a stronger check is whether the remaining fields have repSetters + // however this highlights an issue we may hit with polymorphism a field may + // be projectable in one subtype, but fixed in another + + projectionOptions.getFields().forEach(s -> { + if (!MAPPERS.isKnownField(s)) { + throw new ServiceException("%s is an unknown field".formatted(s), Response.Status.BAD_REQUEST); + } + }); // When FGAP is enabled, authorization filtering is applied at the JPA layer (via PartialEvaluator predicates), so we trust the DB results. // When disabled, we fall back to in-memory filtering by VIEW_CLIENTS role. @@ -127,7 +139,7 @@ public class DefaultClientService implements ClientService { return realm.getClientsStream() .filter(client -> canView || permissions.clients().canView(client)) .filter(client -> client.getProtocol() != null) - .map(client -> getMapper(client.getProtocol()).fromModel(client)) + .map(client -> getMapper(client.getProtocol()).fromModel(client, projectionOptions.getFields())) .filter(java.util.Objects::nonNull); } catch (ModelException e) { throw new ServiceException(e.getMessage(), Response.Status.BAD_REQUEST); @@ -251,7 +263,7 @@ public class DefaultClientService implements ClientService { generateClientSecretIfNeeded(client, model); // Update model - model = mapper.toModel(client, model); + mapper.toModel(client, model); // Validate the fully populated model ValidationUtil.validateClient(session, model, false, r -> { @@ -326,10 +338,12 @@ public class DefaultClientService implements ClientService { */ private ClientRepresentation getProposedOldRepresentation(RealmModel realm, BaseClientRepresentation client, ClientModelMapper mapper) { String tempId = "__temp__" + client.getClientId() + "__" + System.nanoTime(); - ClientModel tempModel = mapper.toModel(client, realm.addClient(tempId)); + ClientModel tempModel = realm.addClient(tempId); + String clientId = client.getClientId(); + mapper.toModel(client, tempModel); try { var proposedRepresentation = ModelToRepresentation.toRepresentation(tempModel, session); - proposedRepresentation.setClientId(client.getClientId()); + proposedRepresentation.setClientId(clientId); return proposedRepresentation; } finally { realm.removeClient(tempModel.getId()); @@ -450,8 +464,8 @@ public class DefaultClientService implements ClientService { } } - protected ClientModelMapper getMapper(String protocol) { - return Optional.ofNullable(session.getProvider(ClientModelMapper.class, protocol)) - .orElseThrow(() -> new ServiceException("Mapper not found, unsupported client protocol: " + protocol, Response.Status.BAD_REQUEST)); + public ClientModelMapper getMapper(String protocol) { + return MAPPERS.getMapper(protocol).orElseThrow(() -> new ServiceException("Mapper not found, unsupported client protocol: " + protocol, + Response.Status.BAD_REQUEST)); } } diff --git a/rest/admin-v2/services/src/main/resources/META-INF/services/org.keycloak.models.mapper.ClientModelMapperFactory b/rest/admin-v2/services/src/main/resources/META-INF/services/org.keycloak.models.mapper.ClientModelMapperFactory deleted file mode 100644 index cb49bb1add5..00000000000 --- a/rest/admin-v2/services/src/main/resources/META-INF/services/org.keycloak.models.mapper.ClientModelMapperFactory +++ /dev/null @@ -1,2 +0,0 @@ -org.keycloak.models.mapper.OIDCClientModelMapperFactory -org.keycloak.models.mapper.SAMLClientModelMapperFactory \ No newline at end of file diff --git a/rest/admin-v2/services/src/main/resources/META-INF/services/org.keycloak.provider.Spi b/rest/admin-v2/services/src/main/resources/META-INF/services/org.keycloak.provider.Spi deleted file mode 100755 index 84d24dcaeab..00000000000 --- a/rest/admin-v2/services/src/main/resources/META-INF/services/org.keycloak.provider.Spi +++ /dev/null @@ -1 +0,0 @@ -org.keycloak.models.mapper.ClientModelMapperSpi diff --git a/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/client/v2/ClientApiV2Test.java b/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/client/v2/ClientApiV2Test.java index 87131497d1f..1f491ff19ea 100644 --- a/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/client/v2/ClientApiV2Test.java +++ b/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/client/v2/ClientApiV2Test.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Set; import java.util.stream.Stream; +import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.MediaType; @@ -306,6 +307,7 @@ public class ClientApiV2Test extends AbstractClientApiV2Test{ .orElse(null); assertThat("OIDC client should be in the list", foundOidc, is(notNullValue())); + assertThat(foundOidc.getDescription(), notNullValue()); assertThat(foundOidc.getLoginFlows(), is(Set.of(OIDCClientRepresentation.Flow.STANDARD, OIDCClientRepresentation.Flow.DIRECT_GRANT))); assertThat(foundOidc.getWebOrigins(), is(Set.of("http://localhost:3000", "http://localhost:4000"))); @@ -317,6 +319,7 @@ public class ClientApiV2Test extends AbstractClientApiV2Test{ .orElse(null); assertThat("SAML client should be in the list", foundSaml, is(notNullValue())); + assertThat(foundSaml.getDescription(), notNullValue()); assertThat(foundSaml.getNameIdFormat(), is("email")); assertThat(foundSaml.getSignDocuments(), is(true)); assertThat(foundSaml.getSignAssertions(), is(true)); @@ -340,6 +343,27 @@ public class ClientApiV2Test extends AbstractClientApiV2Test{ assertThat(samlClient.getSignAssertions(), is(true)); assertThat(samlClient.getForcePostBinding(), is(true)); assertThat(samlClient.getFrontChannelLogout(), is(false)); + + // test projecting only id and protocol + try (Stream baseClientRepresentationStream = getClientsApi().getClients(Set.of("clientId", "protocol"))) { + List clients = baseClientRepresentationStream.toList(); + for (BaseClientRepresentation client : clients) { + BaseClientRepresentation toCompare = null; + if (client.getProtocol().equals(OIDCClientRepresentation.PROTOCOL)) { + toCompare = new OIDCClientRepresentation(); + } else { + toCompare = new SAMLClientRepresentation(); + } + toCompare.setClientId(client.getClientId()); + assertThat(client, Matchers.samePropertyValuesAs(toCompare)); + } + } + } + + @Test + public void invalidFieldProjection() { + BadRequestException e = assertThrows(BadRequestException.class, () -> getClientsApi().getClients(Set.of("unknown!"))); + assertEquals("{\"error\":\"unknown! is an unknown field\"}", e.getResponse().readEntity(String.class)); } @Test diff --git a/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/mapper/OIDCClientModelMapperTest.java b/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/mapper/OIDCClientModelMapperTest.java index 796d16a5e6c..e0b966827ab 100644 --- a/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/mapper/OIDCClientModelMapperTest.java +++ b/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/mapper/OIDCClientModelMapperTest.java @@ -25,7 +25,6 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.UserModel; -import org.keycloak.models.mapper.ClientModelMapper; import org.keycloak.models.mapper.OIDCClientModelMapper; import org.keycloak.representations.admin.v2.BaseClientRepresentation; import org.keycloak.representations.admin.v2.OIDCClientRepresentation; @@ -71,7 +70,7 @@ public class OIDCClientModelMapperTest { clientModel.setServiceAccountsEnabled(false); clientModel.setWebOrigins(Set.of()); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); BaseClientRepresentation rep = mapper.fromModel(clientModel); assertThat(rep, instanceOf(OIDCClientRepresentation.class)); @@ -97,7 +96,7 @@ public class OIDCClientModelMapperTest { setupBasicClientModel(clientModel); RoleModel clientRole = clientModel.addRole("client-role"); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); BaseClientRepresentation rep = mapper.fromModel(clientModel); assertThat(rep.getRoles(), contains("client-role")); @@ -120,7 +119,7 @@ public class OIDCClientModelMapperTest { // Note: serviceAccountsEnabled is not set here as it requires a service account user to exist // for the mapper to work correctly. The SERVICE_ACCOUNT flow is tested separately. - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); OIDCClientRepresentation rep = (OIDCClientRepresentation) mapper.fromModel(clientModel); assertThat(rep.getLoginFlows(), containsInAnyOrder( @@ -143,7 +142,7 @@ public class OIDCClientModelMapperTest { clientModel.setClientAuthenticatorType("client-secret"); clientModel.setSecret("my-secret"); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); OIDCClientRepresentation rep = (OIDCClientRepresentation) mapper.fromModel(clientModel); assertThat(rep.getAuth(), notNullValue()); @@ -164,7 +163,7 @@ public class OIDCClientModelMapperTest { setupBasicClientModel(clientModel); clientModel.setPublicClient(true); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); OIDCClientRepresentation rep = (OIDCClientRepresentation) mapper.fromModel(clientModel); assertThat(rep.getAuth(), nullValue()); @@ -183,7 +182,7 @@ public class OIDCClientModelMapperTest { setupBasicClientModel(clientModel); clientModel.setWebOrigins(Set.of("http://localhost:3000", "http://localhost:4000")); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); OIDCClientRepresentation rep = (OIDCClientRepresentation) mapper.fromModel(clientModel); assertThat(rep.getWebOrigins(), containsInAnyOrder("http://localhost:3000", "http://localhost:4000")); @@ -218,7 +217,7 @@ public class OIDCClientModelMapperTest { serviceAccount.grantRole(role1); serviceAccount.grantRole(role2); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); OIDCClientRepresentation rep = (OIDCClientRepresentation) mapper.fromModel(clientModel); assertThat(rep.getServiceAccountRoles(), hasItems("test-role-1", "test-role-2")); @@ -239,7 +238,7 @@ public class OIDCClientModelMapperTest { setupBasicClientModel(clientModel); clientModel.setServiceAccountsEnabled(false); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); OIDCClientRepresentation rep = (OIDCClientRepresentation) mapper.fromModel(clientModel); assertThat(rep.getServiceAccountRoles(), empty()); @@ -265,7 +264,7 @@ public class OIDCClientModelMapperTest { rep.setWebOrigins(Set.of("http://example.com")); rep.setLoginFlows(Set.of()); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); assertThat(clientModel.isEnabled(), is(true)); @@ -280,6 +279,10 @@ public class OIDCClientModelMapperTest { } } + private OIDCClientModelMapper getModelMapper(KeycloakSession session) { + return new OIDCClientModelMapper(); + } + @TestOnServer public void toModel_setsLoginFlows(KeycloakSession session) { RealmModel realm = session.realms().getRealmByName("master"); @@ -296,7 +299,7 @@ public class OIDCClientModelMapperTest { rep.setRedirectUris(Set.of()); rep.setWebOrigins(Set.of()); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); assertThat(clientModel.isStandardFlowEnabled(), is(true)); @@ -326,7 +329,7 @@ public class OIDCClientModelMapperTest { rep.setRedirectUris(Set.of()); rep.setWebOrigins(Set.of()); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); assertThat(clientModel.isStandardFlowEnabled(), is(false)); @@ -356,7 +359,7 @@ public class OIDCClientModelMapperTest { auth.setSecret("jwt-secret"); rep.setAuth(auth); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); assertThat(clientModel.isPublicClient(), is(false)); @@ -385,7 +388,7 @@ public class OIDCClientModelMapperTest { rep.setLoginFlows(Set.of()); rep.setAuth(null); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); assertThat(clientModel.isPublicClient(), is(true)); @@ -411,7 +414,7 @@ public class OIDCClientModelMapperTest { rep.setWebOrigins(Set.of()); rep.setLoginFlows(Set.of()); - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); + OIDCClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); assertThat(clientModel.isEnabled(), is(false)); @@ -420,13 +423,6 @@ public class OIDCClientModelMapperTest { } } - @TestOnServer - public void close_doesNotThrow(KeycloakSession session) { - OIDCClientModelMapper mapper = (OIDCClientModelMapper) session.getProvider(ClientModelMapper.class, OIDCClientRepresentation.PROTOCOL); - // Just verify close doesn't throw any exception - mapper.close(); - } - private void setupBasicClientModel(ClientModel clientModel) { clientModel.setEnabled(true); clientModel.setDescription("Test description"); diff --git a/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/mapper/SAMLClientModelMapperTest.java b/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/mapper/SAMLClientModelMapperTest.java index 07be13d824a..4bfe3ea96a5 100644 --- a/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/mapper/SAMLClientModelMapperTest.java +++ b/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/mapper/SAMLClientModelMapperTest.java @@ -24,7 +24,6 @@ import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; -import org.keycloak.models.mapper.ClientModelMapper; import org.keycloak.models.mapper.SAMLClientModelMapper; import org.keycloak.representations.admin.v2.BaseClientRepresentation; import org.keycloak.representations.admin.v2.SAMLClientRepresentation; @@ -74,7 +73,7 @@ public class SAMLClientModelMapperTest { clientModel.setBaseUrl("http://localhost:8080/saml"); clientModel.setRedirectUris(Set.of("http://localhost:8080/saml/callback")); - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); BaseClientRepresentation rep = mapper.fromModel(clientModel); assertThat(rep, instanceOf(SAMLClientRepresentation.class)); @@ -90,6 +89,10 @@ public class SAMLClientModelMapperTest { } } + private SAMLClientModelMapper getModelMapper(KeycloakSession session) { + return new SAMLClientModelMapper(); + } + @TestOnServer public void fromModel_mapsRoles(KeycloakSession session) { RealmModel realm = session.realms().getRealmByName("master"); @@ -100,7 +103,7 @@ public class SAMLClientModelMapperTest { setupBasicSamlClientModel(clientModel); RoleModel clientRole = clientModel.addRole("saml-client-role"); - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); BaseClientRepresentation rep = mapper.fromModel(clientModel); assertThat(rep.getRoles(), contains("saml-client-role")); @@ -120,7 +123,7 @@ public class SAMLClientModelMapperTest { clientModel.setAttribute(SAML_NAME_ID_FORMAT, "username"); clientModel.setAttribute(SAML_FORCE_NAME_ID_FORMAT, "true"); - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); SAMLClientRepresentation rep = (SAMLClientRepresentation) mapper.fromModel(clientModel); assertThat(rep.getNameIdFormat(), is("username")); @@ -146,7 +149,7 @@ public class SAMLClientModelMapperTest { clientModel.setAttribute(SAML_SIGNATURE_CANONICALIZATION, "http://www.w3.org/2001/10/xml-exc-c14n#"); clientModel.setAttribute(SAML_SIGNING_CERTIFICATE, "MIICertificate"); - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); SAMLClientRepresentation rep = (SAMLClientRepresentation) mapper.fromModel(clientModel); assertThat(rep.getIncludeAuthnStatement(), is(true)); @@ -172,7 +175,7 @@ public class SAMLClientModelMapperTest { clientModel.setAttribute(SAML_FORCE_POST_BINDING, "true"); clientModel.setFrontchannelLogout(true); - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); SAMLClientRepresentation rep = (SAMLClientRepresentation) mapper.fromModel(clientModel); assertThat(rep.getForcePostBinding(), is(true)); @@ -192,7 +195,7 @@ public class SAMLClientModelMapperTest { setupBasicSamlClientModel(clientModel); clientModel.setAttribute(SAML_ALLOW_ECP_FLOW, "true"); - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); SAMLClientRepresentation rep = (SAMLClientRepresentation) mapper.fromModel(clientModel); assertThat(rep.getAllowEcpFlow(), is(true)); @@ -211,7 +214,7 @@ public class SAMLClientModelMapperTest { setupBasicSamlClientModel(clientModel); // Don't set any SAML-specific attributes - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); SAMLClientRepresentation rep = (SAMLClientRepresentation) mapper.fromModel(clientModel); assertThat(rep.getNameIdFormat(), nullValue()); @@ -242,7 +245,7 @@ public class SAMLClientModelMapperTest { rep.setAppUrl("http://example.com/saml"); rep.setRedirectUris(Set.of("http://example.com/saml/callback")); - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); assertThat(clientModel.isEnabled(), is(true)); @@ -271,7 +274,7 @@ public class SAMLClientModelMapperTest { rep.setNameIdFormat("email"); rep.setForceNameIdFormat(true); - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); assertThat(clientModel.getAttribute(SAML_NAME_ID_FORMAT), is("email")); @@ -300,7 +303,7 @@ public class SAMLClientModelMapperTest { rep.setSignatureCanonicalizationMethod("http://www.w3.org/2001/10/xml-exc-c14n#WithComments"); rep.setSigningCertificate("MIINewCertificate"); - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); assertThat(clientModel.getAttribute(SAML_AUTHN_STATEMENT), is("true")); @@ -329,7 +332,7 @@ public class SAMLClientModelMapperTest { rep.setForcePostBinding(true); rep.setFrontChannelLogout(true); - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); assertThat(clientModel.getAttribute(SAML_FORCE_POST_BINDING), is("true")); @@ -352,7 +355,7 @@ public class SAMLClientModelMapperTest { rep.setRedirectUris(Set.of()); rep.setAllowEcpFlow(true); - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); assertThat(clientModel.getAttribute(SAML_ALLOW_ECP_FLOW), is("true")); @@ -378,7 +381,7 @@ public class SAMLClientModelMapperTest { rep.setRedirectUris(Set.of()); // Leave SAML-specific fields null - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); // Existing attributes should remain unchanged when rep values are null @@ -409,7 +412,7 @@ public class SAMLClientModelMapperTest { rep.setFrontChannelLogout(false); rep.setAllowEcpFlow(false); - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); + SAMLClientModelMapper mapper = getModelMapper(session); mapper.toModel(rep, clientModel); assertThat(clientModel.isEnabled(), is(false)); @@ -426,13 +429,6 @@ public class SAMLClientModelMapperTest { } } - @TestOnServer - public void close_doesNotThrow(KeycloakSession session) { - SAMLClientModelMapper mapper = (SAMLClientModelMapper) session.getProvider(ClientModelMapper.class, SAMLClientRepresentation.PROTOCOL); - // Just verify close doesn't throw any exception - mapper.close(); - } - private void setupBasicSamlClientModel(ClientModel clientModel) { clientModel.setProtocol(SAMLClientRepresentation.PROTOCOL); clientModel.setEnabled(true);