diff --git a/server-spi-private/src/main/java/org/keycloak/client/clienttype/ClientType.java b/server-spi-private/src/main/java/org/keycloak/client/clienttype/ClientType.java index afd9f021868..9f5a2e903f0 100644 --- a/server-spi-private/src/main/java/org/keycloak/client/clienttype/ClientType.java +++ b/server-spi-private/src/main/java/org/keycloak/client/clienttype/ClientType.java @@ -19,7 +19,9 @@ package org.keycloak.client.clienttype; import org.keycloak.models.ClientModel; -import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.ClientTypeRepresentation; + +import java.util.Map; /** * TODO:client-types javadocs @@ -41,10 +43,8 @@ public interface ClientType { T getDefaultValue(String optionName, Class optionType); - // Augment at the client type - // Augment particular client on creation of client (TODO:client-types Should it be clientModel or clientRepresentation? Or something else?) - void onCreate(ClientRepresentation newClient) throws ClientTypeException; + Map getConfiguration(); - // Augment particular client on update of client (TODO:client-types Should it be clientModel or clientRepresentation? Or something else?) - void onUpdate(ClientModel currentClient, ClientRepresentation clientToUpdate) throws ClientTypeException; + // Augment at the client type + ClientModel augment(ClientModel client); } \ No newline at end of file diff --git a/server-spi-private/src/main/java/org/keycloak/models/delegate/ClientModelLazyDelegate.java b/server-spi-private/src/main/java/org/keycloak/models/delegate/ClientModelLazyDelegate.java index 2ffa10b097f..50fa25a299a 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/delegate/ClientModelLazyDelegate.java +++ b/server-spi-private/src/main/java/org/keycloak/models/delegate/ClientModelLazyDelegate.java @@ -23,7 +23,7 @@ import org.keycloak.models.ProtocolMapperModel; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; -import java.util.List; + import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicMarkableReference; diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index 2f2b7f07237..254e91e63a1 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -18,17 +18,23 @@ package org.keycloak.models.utils; import java.io.IOException; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.ListIterator; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; +import java.util.Optional; import java.util.Set; +import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.Supplier; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.jboss.logging.Logger; import org.keycloak.Config; @@ -50,8 +56,12 @@ import org.keycloak.broker.provider.IdentityProvider; import org.keycloak.broker.provider.IdentityProviderFactory; import org.keycloak.broker.social.SocialIdentityProvider; +import org.keycloak.client.clienttype.ClientType; +import org.keycloak.client.clienttype.ClientTypeException; +import org.keycloak.client.clienttype.ClientTypeManager; import org.keycloak.common.Profile; import org.keycloak.common.util.MultivaluedHashMap; +import org.keycloak.common.util.ObjectUtil; import org.keycloak.common.util.UriUtils; import org.keycloak.component.ComponentModel; import org.keycloak.credential.CredentialModel; @@ -317,18 +327,14 @@ public static ClientModel createClient(KeycloakSession session, RealmModel realm logger.debugv("Create client: {0}", resourceRep.getClientId()); ClientModel client = resourceRep.getId() != null ? realm.addClient(resourceRep.getId(), resourceRep.getClientId()) : realm.addClient(resourceRep.getClientId()); - if (resourceRep.getName() != null) client.setName(resourceRep.getName()); - if (resourceRep.getDescription() != null) client.setDescription(resourceRep.getDescription()); - if (resourceRep.getType() != null) client.setType(resourceRep.getType()); - if (resourceRep.isEnabled() != null) client.setEnabled(resourceRep.isEnabled()); - if (resourceRep.isAlwaysDisplayInConsole() != null) client.setAlwaysDisplayInConsole(resourceRep.isAlwaysDisplayInConsole()); - client.setManagementUrl(resourceRep.getAdminUrl()); - if (resourceRep.isSurrogateAuthRequired() != null) - client.setSurrogateAuthRequired(resourceRep.isSurrogateAuthRequired()); - if (resourceRep.getRootUrl() != null) client.setRootUrl(resourceRep.getRootUrl()); - if (resourceRep.getBaseUrl() != null) client.setBaseUrl(resourceRep.getBaseUrl()); - if (resourceRep.isBearerOnly() != null) client.setBearerOnly(resourceRep.isBearerOnly()); - if (resourceRep.isConsentRequired() != null) client.setConsentRequired(resourceRep.isConsentRequired()); + + if (Profile.isFeatureEnabled(Profile.Feature.CLIENT_TYPES) && resourceRep.getType() != null) { + ClientTypeManager mgr = session.getProvider(ClientTypeManager.class); + ClientType clientType = mgr.getClientType(realm, resourceRep.getType()); + client = clientType.augment(client); + } + + updateClientProperties(client, resourceRep, true); // Backwards compatibility only if (resourceRep.isDirectGrantsOnly() != null) { @@ -337,61 +343,9 @@ public static ClientModel createClient(KeycloakSession session, RealmModel realm client.setDirectAccessGrantsEnabled(resourceRep.isDirectGrantsOnly()); } - if (resourceRep.isStandardFlowEnabled() != null) - client.setStandardFlowEnabled(resourceRep.isStandardFlowEnabled()); - if (resourceRep.isImplicitFlowEnabled() != null) - client.setImplicitFlowEnabled(resourceRep.isImplicitFlowEnabled()); - if (resourceRep.isDirectAccessGrantsEnabled() != null) - client.setDirectAccessGrantsEnabled(resourceRep.isDirectAccessGrantsEnabled()); - if (resourceRep.isServiceAccountsEnabled() != null) - client.setServiceAccountsEnabled(resourceRep.isServiceAccountsEnabled()); - - if (resourceRep.isPublicClient() != null) client.setPublicClient(resourceRep.isPublicClient()); - if (resourceRep.isFrontchannelLogout() != null) - client.setFrontchannelLogout(resourceRep.isFrontchannelLogout()); - - // set defaults to openid-connect if no protocol specified - if (resourceRep.getProtocol() != null) { - client.setProtocol(resourceRep.getProtocol()); - } else { - client.setProtocol(OIDC); - } - if (resourceRep.getNodeReRegistrationTimeout() != null) { - client.setNodeReRegistrationTimeout(resourceRep.getNodeReRegistrationTimeout()); - } else { - client.setNodeReRegistrationTimeout(-1); - } - - if (resourceRep.getNotBefore() != null) { - client.setNotBefore(resourceRep.getNotBefore()); - } - - if (resourceRep.getClientAuthenticatorType() != null) { - client.setClientAuthenticatorType(resourceRep.getClientAuthenticatorType()); - } else { - client.setClientAuthenticatorType(KeycloakModelUtils.getDefaultClientAuthenticatorType()); - } - - // adding secret if the client isn't public nor bearer only - if (Objects.nonNull(resourceRep.getSecret())) { - client.setSecret(resourceRep.getSecret()); - } else { - if (client.isPublicClient() || client.isBearerOnly()) { - client.setSecret(null); - } else { - KeycloakModelUtils.generateSecret(client); - } - } - - if (resourceRep.getAttributes() != null) { - for (Map.Entry entry : resourceRep.getAttributes().entrySet()) { - client.setAttribute(entry.getKey(), entry.getValue()); - } - } - if ("saml".equals(resourceRep.getProtocol()) && (resourceRep.getAttributes() == null - || !resourceRep.getAttributes().containsKey("saml.artifact.binding.identifier"))) { + || !resourceRep.getAttributes().containsKey("saml.artifact.binding.identifier"))) { client.setAttribute("saml.artifact.binding.identifier", computeArtifactBindingIdentifierString(resourceRep.getClientId())); } @@ -413,35 +367,6 @@ public static ClientModel createClient(KeycloakSession session, RealmModel realm } } - - if (resourceRep.getRedirectUris() != null) { - for (String redirectUri : resourceRep.getRedirectUris()) { - client.addRedirectUri(redirectUri); - } - } - if (resourceRep.getWebOrigins() != null) { - for (String webOrigin : resourceRep.getWebOrigins()) { - logger.debugv("Client: {0} webOrigin: {1}", resourceRep.getClientId(), webOrigin); - client.addWebOrigin(webOrigin); - } - } else { - // add origins from redirect uris - if (resourceRep.getRedirectUris() != null) { - Set origins = new HashSet(); - for (String redirectUri : resourceRep.getRedirectUris()) { - logger.debugv("add redirect-uri to origin: {0}", redirectUri); - if (redirectUri.startsWith("http")) { - String origin = UriUtils.getOrigin(redirectUri); - logger.debugv("adding default client origin: {0}", origin); - origins.add(origin); - } - } - if (origins.size() > 0) { - client.setWebOrigins(origins); - } - } - } - if (resourceRep.getRegisteredNodes() != null) { for (Map.Entry entry : resourceRep.getRegisteredNodes().entrySet()) { client.registerNode(entry.getKey(), entry.getValue()); @@ -457,7 +382,6 @@ public static ClientModel createClient(KeycloakSession session, RealmModel realm } MigrationUtils.updateProtocolMappers(client); - } if (resourceRep.getClientTemplate() != null) { @@ -467,12 +391,6 @@ public static ClientModel createClient(KeycloakSession session, RealmModel realm updateClientScopes(resourceRep, client); - if (resourceRep.isFullScopeAllowed() != null) { - client.setFullScopeAllowed(resourceRep.isFullScopeAllowed()); - } else { - client.setFullScopeAllowed(!client.isConsentRequired()); - } - client.updateClient(); resourceRep.setId(client.getId()); @@ -489,45 +407,26 @@ private static void addClientScopeToClient(RealmModel realm, ClientModel client, } public static void updateClient(ClientRepresentation rep, ClientModel resource, KeycloakSession session) { - String newClientId = rep.getClientId(); - String previousClientId = resource.getClientId(); - if (newClientId != null) resource.setClientId(newClientId); - if (rep.getName() != null) resource.setName(rep.getName()); - if (rep.getDescription() != null) resource.setDescription(rep.getDescription()); - if (rep.getType() != null) resource.setType(rep.getType()); - if (rep.isEnabled() != null) resource.setEnabled(rep.isEnabled()); - if (rep.isAlwaysDisplayInConsole() != null) resource.setAlwaysDisplayInConsole(rep.isAlwaysDisplayInConsole()); - if (rep.isBearerOnly() != null) resource.setBearerOnly(rep.isBearerOnly()); - if (rep.isConsentRequired() != null) resource.setConsentRequired(rep.isConsentRequired()); - if (rep.isStandardFlowEnabled() != null) resource.setStandardFlowEnabled(rep.isStandardFlowEnabled()); - if (rep.isImplicitFlowEnabled() != null) resource.setImplicitFlowEnabled(rep.isImplicitFlowEnabled()); - if (rep.isDirectAccessGrantsEnabled() != null) - resource.setDirectAccessGrantsEnabled(rep.isDirectAccessGrantsEnabled()); - if (rep.isServiceAccountsEnabled() != null) resource.setServiceAccountsEnabled(rep.isServiceAccountsEnabled()); - if (rep.isPublicClient() != null) resource.setPublicClient(rep.isPublicClient()); - if (rep.isFullScopeAllowed() != null) resource.setFullScopeAllowed(rep.isFullScopeAllowed()); - if (rep.isFrontchannelLogout() != null) resource.setFrontchannelLogout(rep.isFrontchannelLogout()); - if (rep.getRootUrl() != null) resource.setRootUrl(rep.getRootUrl()); - if (rep.getAdminUrl() != null) resource.setManagementUrl(rep.getAdminUrl()); - if (rep.getBaseUrl() != null) resource.setBaseUrl(rep.getBaseUrl()); - if (rep.isSurrogateAuthRequired() != null) resource.setSurrogateAuthRequired(rep.isSurrogateAuthRequired()); - if (rep.getNodeReRegistrationTimeout() != null) - resource.setNodeReRegistrationTimeout(rep.getNodeReRegistrationTimeout()); - if (rep.getClientAuthenticatorType() != null) - resource.setClientAuthenticatorType(rep.getClientAuthenticatorType()); - if (rep.getProtocol() != null) resource.setProtocol(rep.getProtocol()); - if (rep.getAttributes() != null) { - for (Map.Entry entry : rep.getAttributes().entrySet()) { - resource.setAttribute(entry.getKey(), entry.getValue()); + if (Profile.isFeatureEnabled(Profile.Feature.CLIENT_TYPES)) { + if (!ObjectUtil.isEqualOrBothNull(rep.getType(), rep.getType())) { + throw new ClientTypeException("Not supported to change client type"); } - } - if (rep.getAttributes() != null) { - for (Map.Entry entry : removeEmptyString(rep.getAttributes()).entrySet()) { - resource.setAttribute(entry.getKey(), entry.getValue()); + if (rep.getType() != null) { + RealmModel realm = session.getContext().getRealm(); + ClientTypeManager mgr = session.getProvider(ClientTypeManager.class); + ClientType clientType = mgr.getClientType(realm, rep.getType()); + resource = clientType.augment(resource); } } + String newClientId = rep.getClientId(); + String previousClientId = resource.getClientId(); + + if (newClientId != null) resource.setClientId(newClientId); + + updateClientProperties(resource, rep, false); + if ("saml".equals(rep.getProtocol()) && (rep.getAttributes() == null || !rep.getAttributes().containsKey("saml.artifact.binding.identifier"))) { @@ -548,46 +447,20 @@ public static void updateClient(ClientRepresentation rep, ClientModel resource, } } - if (rep.getNotBefore() != null) { - resource.setNotBefore(rep.getNotBefore()); - } - - List redirectUris = rep.getRedirectUris(); - if (redirectUris != null) { - resource.setRedirectUris(new HashSet<>(redirectUris)); - } - - List webOrigins = rep.getWebOrigins(); - if (webOrigins != null) { - resource.setWebOrigins(new HashSet<>(webOrigins)); - } - if (rep.getRegisteredNodes() != null) { for (Map.Entry entry : rep.getRegisteredNodes().entrySet()) { resource.registerNode(entry.getKey(), entry.getValue()); } } - if (resource.isPublicClient() || resource.isBearerOnly()) { - resource.setSecret(null); - } else { - String currentSecret = resource.getSecret(); - String newSecret = rep.getSecret(); - - if (newSecret == null && currentSecret == null) { - KeycloakModelUtils.generateSecret(resource); - } else if (newSecret != null) { - resource.setSecret(newSecret); - } - } - resource.updateClient(); if (!Objects.equals(newClientId, previousClientId)) { + ClientModel finalResource = resource; ClientModel.ClientIdChangeEvent event = new ClientModel.ClientIdChangeEvent() { @Override public ClientModel getUpdatedClient() { - return resource; + return finalResource; } @Override @@ -609,6 +482,126 @@ public KeycloakSession getKeycloakSession() { } } + /** + * Update client properties and process any {@link ClientTypeException} validation errors combining into one to be thrown. + * + * @param client {@link ClientModel} to update + * @param rep {@link ClientRepresentation} to apply updates. + * @param isNew Whether the client is new or not (needed because some getters cannot return null). + */ + private static void updateClientProperties(ClientModel client, ClientRepresentation rep, boolean isNew) { + List> clientPropertyUpdates = new LinkedList>() {{ + /** + * Values from the ClientRepresentation take precedence. + * Then values from the ClientModel (these may be augmented from the client type configuration). + * Otherwise, we can choose some sane default. + */ + add(updatePropertyAction(client::setName, rep::getName, client::getName)); + add(updatePropertyAction(client::setDescription, rep::getDescription, client::getDescription)); + add(updatePropertyAction(client::setType, rep::getType, client::getType)); + add(updatePropertyAction(client::setEnabled, rep::isEnabled, client::isEnabled)); + add(updatePropertyAction(client::setAlwaysDisplayInConsole, rep::isAlwaysDisplayInConsole, client::isAlwaysDisplayInConsole)); + add(updatePropertyAction(client::setManagementUrl, rep::getAdminUrl, client::getManagementUrl)); + add(updatePropertyAction(client::setSurrogateAuthRequired, rep::isSurrogateAuthRequired, client::isSurrogateAuthRequired)); + add(updatePropertyAction(client::setRootUrl, rep::getRootUrl, client::getRootUrl)); + add(updatePropertyAction(client::setBaseUrl, rep::getBaseUrl, client::getBaseUrl)); + add(updatePropertyAction(client::setBearerOnly, rep::isBearerOnly, client::isBearerOnly)); + add(updatePropertyAction(client::setConsentRequired, rep::isConsentRequired, client::isConsentRequired)); + add(updatePropertyAction(client::setStandardFlowEnabled, rep::isStandardFlowEnabled, client::isStandardFlowEnabled)); + add(updatePropertyAction(client::setImplicitFlowEnabled, rep::isImplicitFlowEnabled, client::isImplicitFlowEnabled)); + add(updatePropertyAction(client::setDirectAccessGrantsEnabled, rep::isDirectAccessGrantsEnabled, client::isDirectAccessGrantsEnabled)); + add(updatePropertyAction(client::setServiceAccountsEnabled, rep::isServiceAccountsEnabled, client::isServiceAccountsEnabled)); + add(updatePropertyAction(client::setPublicClient, rep::isPublicClient, client::isPublicClient)); + add(updatePropertyAction(client::setFrontchannelLogout, rep::isFrontchannelLogout, client::isFrontchannelLogout)); + add(updatePropertyAction(client::setNotBefore, rep::getNotBefore, client::getNotBefore)); + // Fields with defaults if not initially provided + add(updatePropertyAction(client::setProtocol, rep::getProtocol, client::getProtocol, () -> OIDC)); + add(updatePropertyAction(client::setNodeReRegistrationTimeout, rep::getNodeReRegistrationTimeout, () -> defaultNodeReRegistrationTimeout(client, isNew))); + add(updatePropertyAction(client::setClientAuthenticatorType, rep::getClientAuthenticatorType, client::getClientAuthenticatorType, KeycloakModelUtils::getDefaultClientAuthenticatorType)); + add(updatePropertyAction(client::setFullScopeAllowed, rep::isFullScopeAllowed, () -> defaultFullScopeAllowed(client, isNew))); + // Client Secret + add(updatePropertyAction(client::setSecret, () -> determineNewSecret(client, rep))); + // Redirect uris / Web origins + add(updatePropertyAction(client::setRedirectUris, () -> collectionToSet(rep.getRedirectUris()), client::getRedirectUris)); + add(updatePropertyAction(client::setWebOrigins, () -> collectionToSet(rep.getWebOrigins()), () -> defaultWebOrigins(client))); + }}; + + // Extended client attributes + if (rep.getAttributes() != null) { + for (Map.Entry entry : rep.getAttributes().entrySet()) { + clientPropertyUpdates.add( + updatePropertyAction(val -> client.setAttribute(entry.getKey(), val), entry::getValue)); + } + } + + List propertyUpdateExceptions = clientPropertyUpdates + .stream() + .map(Supplier::get) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + + if (propertyUpdateExceptions.size() > 0) { + throw new ClientTypeException( + "Cannot change property of client as it is not allowed by the specified client type.", + propertyUpdateExceptions.stream().map(ClientTypeException::getParameters).flatMap(Stream::of).toArray()); + } + } + + private static Boolean defaultFullScopeAllowed(ClientModel client, boolean isNew) { + // If the client is newly created and the value is set to true, the value must be augmented through a + // client type configuration, so do not update. Yet if new and false, the field MAY be controlled through a + // client type. We can only attempt to set the sane default based on consent required. + return isNew && !client.isFullScopeAllowed() ? !client.isConsentRequired() : client.isFullScopeAllowed(); + } + + private static Integer defaultNodeReRegistrationTimeout(ClientModel client, boolean isNew) { + // If the client is newly created and the value is 0, the client MAY not be augmented with a client type. + if (isNew && Objects.equals(client.getNodeReRegistrationTimeout(), 0)) { + // Return the sane default. + return -1; + } + // Update client request without an overriding value or is new and augmented with a client type, + // do not attempt to update. + return client.getNodeReRegistrationTimeout(); + } + + private static String determineNewSecret(ClientModel client, ClientRepresentation rep) { + if (Boolean.TRUE.equals(rep.isPublicClient()) || Boolean.TRUE.equals(rep.isBearerOnly())) { + // Clear out the secret with null + return null; + } + + // adding secret if the client isn't public nor bearer only + String currentSecret = client.getSecret(); + String newSecret = rep.getSecret(); + + if (newSecret == null && currentSecret == null) { + return KeycloakModelUtils.generateSecret(client); + } else if (newSecret != null) { + return newSecret; + } + // Do not change current secret. + return currentSecret; + } + + private static Set defaultWebOrigins(ClientModel client) { + Set webOrigins = client.getWebOrigins(); + if (webOrigins != null && !webOrigins.isEmpty()) { + return webOrigins; + } + + Set redirectUris = client.getRedirectUris(); + if (redirectUris == null || redirectUris.isEmpty()) { + return new HashSet<>(); + } + + return client.getRedirectUris() + .stream() + .filter(uri -> uri.startsWith("http")) + .map(UriUtils::getOrigin) + .collect(Collectors.toSet()); + } + public static void updateClientProtocolMappers(ClientRepresentation rep, ClientModel resource) { if (rep.getProtocolMappers() != null) { @@ -663,6 +656,45 @@ public static void updateClientScopes(ClientRepresentation resourceRep, ClientMo } } + /** + * Create Supplier to update property, if not null. + * Captures {@link ClientTypeException} if thrown by the setter. + * + * @param modelSetter setter to call. + * @param representationGetter getter supplying the property update. + * @return {@link Supplier} resulting whether a {@link ClientTypeException} was thrown. + * @param Type of property. + */ + private static Supplier updateProperty(Consumer modelSetter, Supplier representationGetter) { + return () -> { + try { + T value = representationGetter.get(); + modelSetter.accept(value); + } catch (ClientTypeException cte) { + return cte; + } + return null; + }; + } + + /** + * Create an update property action, passing the first non-null value supplied to the setter, in argument order. + * + * @param modelSetter {@link Consumer} The setter to call. + * @param getters {@link Supplier} to query for the first non-null value. + * @return {@link Supplier} with results of operation. + * @param Type of property. + */ + private static Supplier updatePropertyAction(Consumer modelSetter, Supplier... getters) { + Stream firstNonNullSupplied = Stream.of(getters) + .map(Supplier::get) + .map(Optional::ofNullable) + .filter(Optional::isPresent) + .map(Optional::get); + return updateProperty(modelSetter, () -> firstNonNullSupplied.findFirst().orElse(null)); + } + + private static String generateProtocolNameKey(String protocol, String name) { return String.format("%s%%%s", protocol, name); } @@ -1607,4 +1639,10 @@ public static ResourceServer createResourceServer(ClientModel client, KeycloakSe return toModel(representation, authorization, client); } + + private static Set collectionToSet(Collection collection) { + return Optional.ofNullable(collection) + .map(HashSet::new) + .orElse(null); + } } diff --git a/services/src/main/java/org/keycloak/services/clienttype/client/TypeAwareClientModelDelegate.java b/services/src/main/java/org/keycloak/services/clienttype/client/TypeAwareClientModelDelegate.java index aa76430d43d..f514858c8b7 100644 --- a/services/src/main/java/org/keycloak/services/clienttype/client/TypeAwareClientModelDelegate.java +++ b/services/src/main/java/org/keycloak/services/clienttype/client/TypeAwareClientModelDelegate.java @@ -18,14 +18,15 @@ package org.keycloak.services.clienttype.client; -import java.util.function.Consumer; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; import java.util.function.Supplier; +import java.util.stream.Collectors; -import org.keycloak.common.util.ObjectUtil; import org.keycloak.models.ClientModel; import org.keycloak.models.delegate.ClientModelLazyDelegate; import org.keycloak.client.clienttype.ClientType; -import org.keycloak.client.clienttype.ClientTypeException; /** * Delegates to client-type and underlying delegate @@ -47,45 +48,219 @@ public TypeAwareClientModelDelegate(ClientType clientType, Supplier @Override public boolean isStandardFlowEnabled() { - return getBooleanProperty("standardFlowEnabled", super::isStandardFlowEnabled); + return TypedClientSimpleAttribute.STANDARD_FLOW_ENABLED + .getClientAttribute(clientType, super::isStandardFlowEnabled, Boolean.class); } @Override public void setStandardFlowEnabled(boolean standardFlowEnabled) { - setBooleanProperty("standardFlowEnabled", standardFlowEnabled, super::setStandardFlowEnabled); + TypedClientSimpleAttribute.STANDARD_FLOW_ENABLED + .setClientAttribute(clientType, standardFlowEnabled, super::setStandardFlowEnabled, Boolean.class); } + @Override + public boolean isBearerOnly() { + return TypedClientSimpleAttribute.BEARER_ONLY + .getClientAttribute(clientType, super::isBearerOnly, Boolean.class); + } - protected boolean getBooleanProperty(String propertyName, Supplier clientGetter) { - // Check if clientType supports the feature. If not, simply return false - if (!clientType.isApplicable(propertyName)) { - return false; - } + @Override + public void setBearerOnly(boolean bearerOnly) { + TypedClientSimpleAttribute.BEARER_ONLY + .setClientAttribute(clientType, bearerOnly, super::setBearerOnly, Boolean.class); + } + + @Override + public boolean isConsentRequired() { + return TypedClientSimpleAttribute.CONSENT_REQUIRED + .getClientAttribute(clientType, super::isConsentRequired, Boolean.class); + } + + @Override + public void setConsentRequired(boolean consentRequired) { + TypedClientSimpleAttribute.CONSENT_REQUIRED + .setClientAttribute(clientType, consentRequired, super::setConsentRequired, Boolean.class); + } + + @Override + public boolean isDirectAccessGrantsEnabled() { + return TypedClientSimpleAttribute.DIRECT_ACCESS_GRANTS_ENABLED + .getClientAttribute(clientType, super::isDirectAccessGrantsEnabled, Boolean.class); + } + + @Override + public void setDirectAccessGrantsEnabled(boolean directAccessGrantsEnabled) { + TypedClientSimpleAttribute.DIRECT_ACCESS_GRANTS_ENABLED + .setClientAttribute(clientType, directAccessGrantsEnabled, super::setDirectAccessGrantsEnabled, Boolean.class); + } + + @Override + public boolean isAlwaysDisplayInConsole() { + return TypedClientSimpleAttribute.ALWAYS_DISPLAY_IN_CONSOLE + .getClientAttribute(clientType, super::isAlwaysDisplayInConsole, Boolean.class); + } + + @Override + public void setAlwaysDisplayInConsole(boolean alwaysDisplayInConsole) { + TypedClientSimpleAttribute.ALWAYS_DISPLAY_IN_CONSOLE + .setClientAttribute(clientType, alwaysDisplayInConsole, super::setAlwaysDisplayInConsole, Boolean.class); + } + + @Override + public boolean isFrontchannelLogout() { + return TypedClientSimpleAttribute.FRONTCHANNEL_LOGOUT + .getClientAttribute(clientType, super::isFrontchannelLogout, Boolean.class); + } + + @Override + public void setFrontchannelLogout(boolean frontchannelLogout) { + TypedClientSimpleAttribute.FRONTCHANNEL_LOGOUT + .setClientAttribute(clientType, frontchannelLogout, super::setFrontchannelLogout, Boolean.class); + } + + @Override + public boolean isImplicitFlowEnabled() { + return TypedClientSimpleAttribute.IMPLICIT_FLOW_ENABLED + .getClientAttribute(clientType, super::isImplicitFlowEnabled, Boolean.class); + } + + @Override + public void setImplicitFlowEnabled(boolean implicitFlowEnabled) { + TypedClientSimpleAttribute.IMPLICIT_FLOW_ENABLED + .setClientAttribute(clientType, implicitFlowEnabled, super::setImplicitFlowEnabled, Boolean.class); + } + + @Override + public boolean isServiceAccountsEnabled() { + return TypedClientSimpleAttribute.SERVICE_ACCOUNTS_ENABLED + .getClientAttribute(clientType, super::isServiceAccountsEnabled, Boolean.class); + } + + @Override + public void setServiceAccountsEnabled(boolean flag) { + TypedClientSimpleAttribute.SERVICE_ACCOUNTS_ENABLED + .setClientAttribute(clientType, flag, super::setServiceAccountsEnabled, Boolean.class); + } + + @Override + public String getProtocol() { + return TypedClientSimpleAttribute.PROTOCOL + .getClientAttribute(clientType, super::getProtocol, String.class); + } + + @Override + public void setProtocol(String protocol) { + TypedClientSimpleAttribute.PROTOCOL + .setClientAttribute(clientType, protocol, super::setProtocol, String.class); + } + + @Override + public boolean isPublicClient() { + return TypedClientSimpleAttribute.PUBLIC_CLIENT + .getClientAttribute(clientType, super::isPublicClient, Boolean.class); + } + + @Override + public void setPublicClient(boolean flag) { + TypedClientSimpleAttribute.PUBLIC_CLIENT + .setClientAttribute(clientType, flag, super::setPublicClient, Boolean.class); + } + + @Override + public Set getWebOrigins() { + return TypedClientSimpleAttribute.WEB_ORIGINS + .getClientAttribute(clientType, super::getWebOrigins, Set.class); + } + + @Override + public void setWebOrigins(Set webOrigins) { + TypedClientSimpleAttribute.WEB_ORIGINS + .setClientAttribute(clientType, webOrigins, super::setWebOrigins, Set.class); + } + + @Override + public void addWebOrigin(String webOrigin) { + TypedClientSimpleAttribute.WEB_ORIGINS + .setClientAttribute(clientType, webOrigin, super::addWebOrigin, String.class); + } + + @Override + public void removeWebOrigin(String webOrigin) { + TypedClientSimpleAttribute.WEB_ORIGINS + .setClientAttribute(clientType, null, (val) -> super.removeWebOrigin(webOrigin), String.class); + } - // Check if this is read-only. If yes, then we just directly delegate to return stuff from the clientType rather than from client - if (clientType.isReadOnly(propertyName)) { - return clientType.getDefaultValue(propertyName, Boolean.class); + @Override + public Set getRedirectUris() { + return TypedClientSimpleAttribute.REDIRECT_URIS + .getClientAttribute(clientType, super::getRedirectUris, Set.class); + } + + @Override + public void setRedirectUris(Set redirectUris) { + TypedClientSimpleAttribute.REDIRECT_URIS + .setClientAttribute(clientType, redirectUris, super::setRedirectUris, Set.class); + } + + @Override + public void addRedirectUri(String redirectUri) { + TypedClientSimpleAttribute.REDIRECT_URIS + .setClientAttribute(clientType, redirectUri, super::addRedirectUri, String.class); + } + + @Override + public void removeRedirectUri(String redirectUri) { + TypedClientSimpleAttribute.REDIRECT_URIS + .setClientAttribute(clientType, null, (val) -> super.removeRedirectUri(redirectUri), String.class); + } + + @Override + public void setAttribute(String name, String value) { + TypedClientExtendedAttribute attribute = TypedClientExtendedAttribute.getAttributesByName().get(name); + if (attribute != null) { + attribute.setClientAttribute(clientType, value, (newValue) -> super.setAttribute(name, newValue), String.class); + } else { + super.setAttribute(name, value); } + } - // Delegate to clientGetter - return clientGetter.get(); + @Override + public void removeAttribute(String name) { + TypedClientExtendedAttribute attribute = TypedClientExtendedAttribute.getAttributesByName().get(name); + if (attribute != null) { + attribute.setClientAttribute(clientType, null, (val) -> super.removeAttribute(name), String.class); + } else { + super.removeAttribute(name); + } } - protected void setBooleanProperty(String propertyName, Boolean newValue, Consumer clientSetter) { - // Check if clientType supports the feature. If not, return directly - if (!clientType.isApplicable(propertyName)) { - return; + @Override + public String getAttribute(String name) { + TypedClientExtendedAttribute attribute = TypedClientExtendedAttribute.getAttributesByName().get(name); + if (attribute != null) { + return attribute.getClientAttribute(clientType, () -> super.getAttribute(name), String.class); + } else { + return super.getAttribute(name); } + } + + @Override + public Map getAttributes() { + // Start with attributes set on the delegate. + Map attributes = new HashMap<>(super.getAttributes()); + + // Get extended client type attributes and values from the client type configuration. + Set extendedClientTypeAttributes = + clientType.getConfiguration().entrySet().stream() + .map(Map.Entry::getKey) + .filter(entry -> TypedClientExtendedAttribute.getAttributesByName().containsKey(entry)) + .collect(Collectors.toSet()); - // Check if this is read-only. If yes and there is an attempt to change some stuff, then throw an exception - if (clientType.isReadOnly(propertyName)) { - Boolean oldVal = clientType.getDefaultValue(propertyName, Boolean.class); - if (!ObjectUtil.isEqualOrBothNull(oldVal, newValue)) { - throw new ClientTypeException("Property " + propertyName + " of client " + getClientId() + " is read-only due to client type " + clientType.getName()); - } + // Augment client type attributes on top of attributes on the delegate. + for (String entry : extendedClientTypeAttributes) { + attributes.put(entry, getAttribute(entry)); } - // Call clientSetter - clientSetter.accept(newValue); + return attributes; } } \ No newline at end of file diff --git a/services/src/main/java/org/keycloak/services/clienttype/client/TypedClientSimpleAttribute.java b/services/src/main/java/org/keycloak/services/clienttype/client/TypedClientSimpleAttribute.java new file mode 100644 index 00000000000..14d615c23b2 --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clienttype/client/TypedClientSimpleAttribute.java @@ -0,0 +1,140 @@ +package org.keycloak.services.clienttype.client; + +import org.jboss.logging.Logger; +import org.keycloak.client.clienttype.ClientType; +import org.keycloak.client.clienttype.ClientTypeException; +import org.keycloak.common.util.ObjectUtil; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.function.Consumer; +import java.util.function.Supplier; + +enum TypedClientSimpleAttribute implements TypedClientAttribute { + // Top Level client attributes + STANDARD_FLOW_ENABLED("standardFlowEnabled", false), + BEARER_ONLY("bearerOnly", false), + CONSENT_REQUIRED("consentRequired", false), + DIRECT_ACCESS_GRANTS_ENABLED("directAccessGrantsEnabled", false), + ALWAYS_DISPLAY_IN_CONSOLE("alwaysDisplayInConsole", false), + FRONTCHANNEL_LOGOUT("frontchannelLogout", false), + IMPLICIT_FLOW_ENABLED("implicitFlowEnabled", false), + PROTOCOL("protocol", null), + PUBLIC_CLIENT("publicClient", false), + REDIRECT_URIS("redirectUris", Set.of()), + SERVICE_ACCOUNTS_ENABLED("serviceAccountsEnabled", false), + WEB_ORIGINS("webOrigins", Set.of()), + ; + + private final String propertyName; + private final Object nonApplicableValue; + + TypedClientSimpleAttribute(String propertyName, Object nonApplicableValue) { + this.propertyName = propertyName; + this.nonApplicableValue = nonApplicableValue; + } + + @Override + public String getPropertyName() { + return propertyName; + } + + @Override + public Object getNonApplicableValue() { + return nonApplicableValue; + } +} + +enum TypedClientExtendedAttribute implements TypedClientAttribute { + // Extended Client Type attributes defined as client attribute entities. + DEVICE_AUTHORIZATION_GRANT_ENABLED("oauth2.device.authorization.grant.enabled", "false"), + CIBA_GRANT_ENABLED("oidc.ciba.grant.enabled", "false"), + LOGIN_THEME("login_theme", null), + LOGO_URI("logoUri", null), + POLICY_URI("policyUri", null); + + private static final Map attributesByName = new HashMap<>(); + + static { + Arrays.stream(TypedClientExtendedAttribute.values()) + .forEach(attribute -> attributesByName.put(attribute.getPropertyName(), attribute)); + } + + private final String propertyName; + private final Object nonApplicableValue; + + TypedClientExtendedAttribute(String propertyName, Object nonApplicableValue) { + this.propertyName = propertyName; + this.nonApplicableValue = nonApplicableValue; + } + + @Override + public String getPropertyName() { + return propertyName; + } + + @Override + public Object getNonApplicableValue() { + return nonApplicableValue; + } + + public static Map getAttributesByName() { + return attributesByName; + } +} + +interface TypedClientAttribute { + Logger logger = Logger.getLogger(TypedClientAttribute.class); + + default T getClientAttribute(ClientType clientType, Supplier clientGetter, Class tClass) { + String propertyName = getPropertyName(); + Object nonApplicableValue = getNonApplicableValue(); + + // Check if clientType supports the feature. + if (!clientType.isApplicable(propertyName)) { + try { + return tClass.cast(nonApplicableValue); + } catch (ClassCastException e) { + logger.error("Could not apply client type property %s: %s", propertyName, e); + throw e; + } + } + + // Check if this is read-only. If yes, then we just directly delegate to return stuff from the clientType rather than from client + if (clientType.isReadOnly(propertyName)) { + return clientType.getDefaultValue(propertyName, tClass); + } + + // Delegate to clientGetter + return clientGetter.get(); + } + + default void setClientAttribute(ClientType clientType, T newValue, Consumer clientSetter, Class tClass) { + String propertyName = getPropertyName(); + Object nonApplicableValue = getNonApplicableValue(); + // Check if clientType supports the feature. If not, return directly + if (!clientType.isApplicable(propertyName) && !Objects.equals(nonApplicableValue, newValue)) { + logger.warnf("Property %s is not-applicable to client type %s and can not be modified.", propertyName, clientType.getName()); + return; + } + + // Check if this is read-only. If yes and there is an attempt to change some stuff, then throw an exception + if (clientType.isReadOnly(propertyName)) { + T oldVal = clientType.getDefaultValue(propertyName, tClass); + if (!ObjectUtil.isEqualOrBothNull(oldVal, newValue)) { + throw new ClientTypeException( + "Property " + propertyName + " is read-only due to client type " + clientType.getName(), + propertyName); + } + } + + // Delegate to clientSetter + clientSetter.accept(newValue); + } + + String getPropertyName(); + Object getNonApplicableValue(); +} \ No newline at end of file diff --git a/services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientType.java b/services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientType.java index 31c45f49d44..4deeea9c45a 100644 --- a/services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientType.java +++ b/services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientType.java @@ -18,40 +18,22 @@ package org.keycloak.services.clienttype.impl; -import jakarta.ws.rs.core.Response; -import org.jboss.logging.Logger; import org.keycloak.client.clienttype.ClientType; -import org.keycloak.client.clienttype.ClientTypeException; import org.keycloak.models.ClientModel; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.utils.ModelToRepresentation; -import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ClientTypeRepresentation; +import org.keycloak.services.clienttype.client.TypeAwareClientModelDelegate; -import java.beans.PropertyDescriptor; -import java.lang.reflect.Method; -import java.util.HashMap; -import java.util.List; import java.util.Map; -import java.util.Objects; -import java.util.stream.Collectors; /** * @author Marek Posolda */ public class DefaultClientType implements ClientType { - private static final Logger logger = Logger.getLogger(DefaultClientType.class); - - private final KeycloakSession session; private final ClientTypeRepresentation clientType; - private final Map clientRepresentationProperties; - - public DefaultClientType(KeycloakSession session, ClientTypeRepresentation clientType, Map clientRepresentationProperties) { - this.session = session; + public DefaultClientType(ClientTypeRepresentation clientType) { this.clientType = clientType; - this.clientRepresentationProperties = clientRepresentationProperties; } @Override @@ -83,103 +65,12 @@ public T getDefaultValue(String optionName, Class optionType) { } @Override - public void onCreate(ClientRepresentation createdClient) throws ClientTypeException { - // Create empty client augmented with the applicable default client type values. - ClientRepresentation defaultClientRep = augmentClient(new ClientRepresentation()); - - validateClientRequest(createdClient, defaultClientRep); - - augmentClient(createdClient); + public Map getConfiguration() { + return clientType.getConfig(); } @Override - public void onUpdate(ClientModel currentClient, ClientRepresentation newClient) throws ClientTypeException { - ClientRepresentation currentRep = ModelToRepresentation.toRepresentation(currentClient, session); - validateClientRequest(newClient, currentRep); - } - - protected void validateClientRequest(ClientRepresentation newClient, ClientRepresentation currentClient) throws ClientTypeException { - List validationErrors = clientType.getConfig().entrySet().stream() - .filter(property -> clientPropertyHasInvalidChangeRequested(currentClient, newClient, property.getKey(), property.getValue())) - .map(Map.Entry::getKey) - .collect(Collectors.toList()); - - if (validationErrors.size() > 0) { - throw new ClientTypeException( - "Cannot change property of client as it is not allowed by the specified client type.", - validationErrors.toArray()); - } - } - - protected ClientRepresentation augmentClient(ClientRepresentation client) { - clientType.getConfig().entrySet() - .forEach(property -> setClientProperty(client, property.getKey(), property.getValue())); - return client; - } - - private boolean clientPropertyHasInvalidChangeRequested( - ClientRepresentation oldClient, - ClientRepresentation newClient, - String propertyName, - ClientTypeRepresentation.PropertyConfig propertyConfig) { - Object newClientProperty = getClientProperty(newClient, propertyName); - Object oldClientProperty = getClientProperty(oldClient, propertyName); - - return ( - // Validate that non-applicable client properties were not changed. - !propertyConfig.getApplicable() && - !Objects.isNull(newClientProperty) && - !Objects.equals(oldClientProperty, newClientProperty) - ) || ( - // Validate that applicable read-only client properties were not changed. - propertyConfig.getApplicable() && - propertyConfig.getReadOnly() && - !Objects.isNull(newClientProperty) && - !Objects.equals(oldClientProperty, newClientProperty) - ); - } - - private void setClientProperty(ClientRepresentation client, - String propertyName, - ClientTypeRepresentation.PropertyConfig propertyConfig) { - - if (!propertyConfig.getApplicable() || propertyConfig.getDefaultValue() == null) { - return; - } - - if (clientRepresentationProperties.containsKey(propertyName)) { - // Java property on client representation - Method setter = clientRepresentationProperties.get(propertyName).getWriteMethod(); - try { - setter.invoke(client, propertyConfig.getDefaultValue()); - } catch (Exception e) { - logger.warnf("Cannot set property '%s' on client with value '%s'. Check configuration of the client type '%s'", propertyName, propertyConfig.getDefaultValue(), clientType.getName()); - throw new ClientTypeException("Cannot set property on client", e); - } - } else { - // Client attribute - if (client.getAttributes() == null) { - client.setAttributes(new HashMap<>()); - } - client.getAttributes().put(propertyName, propertyConfig.getDefaultValue().toString()); - } - } - - private Object getClientProperty(ClientRepresentation client, String propertyName) { - PropertyDescriptor propertyDescriptor = clientRepresentationProperties.get(propertyName); - - if (propertyDescriptor != null) { - // Java property - Method getter = propertyDescriptor.getReadMethod(); - try { - return getter.invoke(client); - } catch (Exception e) { - logger.warnf("Cannot read property '%s' on client '%s'. Client type is '%s'", propertyName, client.getClientId(), clientType.getName()); - throw new ClientTypeException("Cannot read property of client", e); - } - } else { - // Attribute - return client.getAttributes() == null ? null : client.getAttributes().get(propertyName); - } + public ClientModel augment(ClientModel client) { + return new TypeAwareClientModelDelegate(this, () -> client); } } \ No newline at end of file diff --git a/services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientTypeProvider.java b/services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientTypeProvider.java index 218b06e7ca4..10c12f83f85 100644 --- a/services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientTypeProvider.java +++ b/services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientTypeProvider.java @@ -18,11 +18,9 @@ package org.keycloak.services.clienttype.impl; -import java.beans.PropertyDescriptor; import java.util.Map; import org.jboss.logging.Logger; -import org.keycloak.models.KeycloakSession; import org.keycloak.representations.idm.ClientTypeRepresentation; import org.keycloak.client.clienttype.ClientType; import org.keycloak.client.clienttype.ClientTypeException; @@ -35,17 +33,9 @@ public class DefaultClientTypeProvider implements ClientTypeProvider { private static final Logger logger = Logger.getLogger(DefaultClientTypeProvider.class); - private final KeycloakSession session; - private final Map clientRepresentationProperties; - - public DefaultClientTypeProvider(KeycloakSession session, Map clientRepresentationProperties) { - this.session = session; - this.clientRepresentationProperties = clientRepresentationProperties; - } - @Override public ClientType getClientType(ClientTypeRepresentation clientTypeRep) { - return new DefaultClientType(session, clientTypeRep, clientRepresentationProperties); + return new DefaultClientType(clientTypeRep); } @Override diff --git a/services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientTypeProviderFactory.java b/services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientTypeProviderFactory.java index 474aad10730..04c7af37f31 100644 --- a/services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientTypeProviderFactory.java +++ b/services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientTypeProviderFactory.java @@ -18,23 +18,11 @@ package org.keycloak.services.clienttype.impl; -import java.beans.BeanInfo; -import java.beans.IntrospectionException; -import java.beans.Introspector; -import java.beans.PropertyDescriptor; -import java.lang.reflect.Method; -import java.util.Arrays; -import java.util.Map; -import java.util.Set; -import java.util.function.Function; -import java.util.stream.Collectors; - import org.keycloak.Config; import org.keycloak.common.Profile; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.provider.EnvironmentDependentProviderFactory; -import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.client.clienttype.ClientTypeProvider; import org.keycloak.client.clienttype.ClientTypeProviderFactory; @@ -45,41 +33,13 @@ public class DefaultClientTypeProviderFactory implements ClientTypeProviderFacto public static final String PROVIDER_ID = "default"; - private Map clientRepresentationProperties; - @Override public ClientTypeProvider create(KeycloakSession session) { - return new DefaultClientTypeProvider(session, clientRepresentationProperties); + return new DefaultClientTypeProvider(); } @Override - public void init(Config.Scope config) { - Set filtered = Arrays.stream(new String[] {"attributes", "type"}).collect(Collectors.toSet()); - - try { - BeanInfo bi = Introspector.getBeanInfo(ClientRepresentation.class); - PropertyDescriptor[] pd = bi.getPropertyDescriptors(); - clientRepresentationProperties = Arrays.stream(pd) - .filter(desc -> !filtered.contains(desc.getName())) - .filter(desc -> desc.getWriteMethod() != null) - .map(desc -> { - // Take "is" methods into consideration - if (desc.getReadMethod() == null && Boolean.class.equals(desc.getPropertyType())) { - String methodName = "is" + desc.getName().substring(0, 1).toUpperCase() + desc.getName().substring(1); - try { - Method getter = ClientRepresentation.class.getDeclaredMethod(methodName); - desc.setReadMethod(getter); - } catch (Exception e) { - throw new IllegalStateException("Getter method for property " + desc.getName() + " cannot be found"); - } - } - return desc; - }) - .collect(Collectors.toMap(PropertyDescriptor::getName, Function.identity())); - } catch (IntrospectionException ie) { - throw new IllegalStateException("Introspection of Client representation failed", ie); - } - } + public void init(Config.Scope config) {} @Override public void postInit(KeycloakSessionFactory factory) { diff --git a/services/src/main/java/org/keycloak/services/managers/ClientManager.java b/services/src/main/java/org/keycloak/services/managers/ClientManager.java index 2c499e4c622..cc623ce5332 100644 --- a/services/src/main/java/org/keycloak/services/managers/ClientManager.java +++ b/services/src/main/java/org/keycloak/services/managers/ClientManager.java @@ -42,8 +42,6 @@ import org.keycloak.representations.adapters.config.BaseRealmConfig; import org.keycloak.representations.adapters.config.PolicyEnforcerConfig; import org.keycloak.representations.idm.ClientRepresentation; -import org.keycloak.client.clienttype.ClientType; -import org.keycloak.client.clienttype.ClientTypeManager; import org.keycloak.sessions.AuthenticationSessionProvider; import java.net.URI; @@ -82,11 +80,6 @@ public ClientManager() { * @return */ public static ClientModel createClient(KeycloakSession session, RealmModel realm, ClientRepresentation rep) { - if (Profile.isFeatureEnabled(Profile.Feature.CLIENT_TYPES) && rep.getType() != null) { - ClientTypeManager mgr = session.getProvider(ClientTypeManager.class); - ClientType clientType = mgr.getClientType(realm, rep.getType()); - clientType.onCreate(rep); - } ClientModel client = RepresentationToModel.createClient(session, realm, rep); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java index ff70e8db472..099e3be25ac 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java @@ -152,17 +152,6 @@ public Response update(final ClientRepresentation rep) { session.setAttribute(ClientSecretConstants.CLIENT_SECRET_ROTATION_ENABLED,Boolean.FALSE); session.clientPolicy().triggerOnEvent(new AdminClientUpdateContext(rep, client, auth.adminAuth())); - if (Profile.isFeatureEnabled(Profile.Feature.CLIENT_TYPES)) { - if (!ObjectUtil.isEqualOrBothNull(rep.getType(), client.getType())) { - throw new ClientTypeException("Not supported to change client type"); - } - if (rep.getType() != null) { - ClientTypeManager mgr = session.getProvider(ClientTypeManager.class); - ClientType clientType = mgr.getClientType(realm, rep.getType()); - clientType.onUpdate(client, rep); - } - } - updateClientFromRep(rep, client, session); ValidationUtil.validateClient(session, client, false, r -> { diff --git a/services/src/main/resources/keycloak-default-client-types.json b/services/src/main/resources/keycloak-default-client-types.json index 0cbd2991825..8db24450499 100644 --- a/services/src/main/resources/keycloak-default-client-types.json +++ b/services/src/main/resources/keycloak-default-client-types.json @@ -6,8 +6,8 @@ "config": { "standardFlowEnabled": { "applicable": true, - "read-only": true, - "default-value": true + "default-value": true, + "read-only": true } } }, @@ -18,59 +18,69 @@ "alwaysDisplayInConsole": { "applicable": false }, + "authorizationServicesEnabled": { + "applicable": false + }, + "bearerOnly": { + "applicable": false + }, "consentRequired": { "applicable": true, - "read-only": true, - "default-value": false + "default-value": false, + "read-only": true + }, + "directAccessGrantsEnabled": { + "applicable": false + }, + "frontchannelLogout": { + "applicable": false + }, + "implicitFlowEnabled": { + "applicable": false }, "login_theme": { "applicable": false }, - "protocol": { - "applicable": true, - "read-only": true, - "default-value": "openid-connect" + "logoUri": { + "applicable": false }, - "publicClient": { - "applicable": true, - "read-only": true, - "default-value": false + "oauth2.device.authorization.grant.enabled": { + "applicable": false }, - "bearerOnly": { - "applicable": true, - "read-only": true, - "default-value": false + "oidc.ciba.grant.enabled": { + "applicable": false }, - "standardFlowEnabled": { - "applicable": true, - "read-only": true, - "default-value": false + "policyUri": { + "applicable": false }, - "implicitFlowEnabled": { + "protocol": { "applicable": true, - "read-only": true, - "default-value": false + "default-value": "openid-connect", + "read-only": true }, - "directAccessGrantsEnabled": { + "publicClient": { "applicable": true, - "read-only": true, - "default-value": false + "default-value": false, + "read-only": true + }, + "redirectUris": { + "applicable": false }, "serviceAccountsEnabled": { "applicable": true, - "read-only": true, - "default-value": true + "default-value": true, + "read-only": true }, - "logoUri": { + "standardFlowEnabled": { "applicable": false }, - "policyUri": { + "tosUri": { "applicable": false }, - "tosUri": { + "webOrigins": { "applicable": false } } } ] -} \ No newline at end of file +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientTypesTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientTypesTest.java index 259dd888bdf..592a4ca4a4f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientTypesTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientTypesTest.java @@ -126,14 +126,10 @@ public void testUpdateClientWithClientType() { clientRep.setServiceAccountsEnabled(true); - // Adding non-applicable attribute should not fail + // Adding non-applicable attribute should not fail but not update client attribute clientRep.getAttributes().put(ClientModel.LOGO_URI, "https://foo"); - try { - testRealm().clients().get(clientRep.getId()).update(clientRep); - Assert.fail("Not expected to update client"); - } catch (BadRequestException bre) { - assertErrorResponseContainsParams(bre.getResponse(), "logoUri"); - } + testRealm().clients().get(clientRep.getId()).update(clientRep); + assertEquals(testRealm().clients().get(clientRep.getId()).toRepresentation().getAttributes().get(ClientModel.LOGO_URI), null); // Update of supported attribute should be successful clientRep.getAttributes().remove(ClientModel.LOGO_URI); @@ -180,7 +176,7 @@ public void testClientTypesAdminRestAPI_globalTypes() { assertEquals("default", serviceAccountType.getProvider()); ClientTypeRepresentation.PropertyConfig cfg = serviceAccountType.getConfig().get("standardFlowEnabled"); - assertPropertyConfig("standardFlowEnabled", cfg, true, true, false); + assertPropertyConfig("standardFlowEnabled", cfg, false, null, null); cfg = serviceAccountType.getConfig().get("serviceAccountsEnabled"); assertPropertyConfig("serviceAccountsEnabled", cfg, true, true, true);