libre.sh/keycloak-scim
2
0
Fork 0
Code Issues 14 Pull requests Releases Packages Activity Actions 6

Client type OIDC base read only defaults (#29706)

Browse source 
closes #29742
closes #29422

Signed-off-by: Patrick Jennings <pajennin@redhat.com>
This commit is contained in:
Patrick Jennings 2024-05-22 03:07:19 -04:00 committed by GitHub
parent 68b2e40b38
commit 84acc953dd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 131 additions and 119 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
common/src/main/java/org/keycloak/common/util/CollectionUtil.java
View file

@ -20,6 +20,7 @@ package org.keycloak.common.util;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
@ -89,4 +90,8 @@ public class CollectionUtil {
.filter(searchCollection::contains)
.collect(Collectors.toSet());
}
public static <T> Set<T> collectionToSet(Collection<T> collection) {
return collection == null ? null : new HashSet<>(collection);
}
}

24
core/src/main/java/org/keycloak/representations/idm/ClientTypeRepresentation.java
View file

@ -65,11 +65,8 @@ public class ClientTypeRepresentation {
@JsonProperty("applicable")
private Boolean applicable;
@JsonProperty("read-only")
private Boolean readOnly;
@JsonProperty("default-value")
private Object defaultValue;
@JsonProperty("value")
private Object value;
public Boolean getApplicable() {
return applicable;
@ -79,20 +76,13 @@ public class ClientTypeRepresentation {
this.applicable = applicable;
}
public Boolean getReadOnly() {
return readOnly;
public Object getValue() {
return value;
}
public void setReadOnly(Boolean readOnly) {
this.readOnly = readOnly;
}
public Object getDefaultValue() {
return defaultValue;
}
public void setDefaultValue(Object defaultValue) {
this.defaultValue = defaultValue;
public void setValue(Object value) {
this.value = value;
}
}
}

11
server-spi-private/src/main/java/org/keycloak/client/clienttype/ClientType.java
View file

@ -19,9 +19,8 @@
package org.keycloak.client.clienttype;
import org.keycloak.models.ClientModel;
import org.keycloak.representations.idm.ClientTypeRepresentation;
import java.util.Map;
import java.util.Set;
/**
* TODO:client-types javadocs
@ -36,14 +35,10 @@ public interface ClientType {
// Can be property name (like "standardFlow" or "rootUrl") or attributeName (like "pkceEnabled")
boolean isApplicable(String optionName);
// Return if option is configurable by clientType or not...
boolean isReadOnly(String optionName);
// Return the value of particular option (if it can be provided by clientType) or return null if this option is not provided by client type
<T> T getDefaultValue(String optionName, Class<T> optionType);
<T> T getTypeValue(String optionName, Class<T> optionType);
Map<String, ClientTypeRepresentation.PropertyConfig> getConfiguration();
Set<String> getOptionNames();
// Augment at the client type
ClientModel augment(ClientModel client);

15
server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java
View file

@ -18,7 +18,6 @@
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;
@ -61,6 +60,7 @@ import org.keycloak.client.clienttype.ClientTypeException;
import org.keycloak.client.clienttype.ClientTypeManager;
import org.keycloak.common.Profile;
import org.keycloak.common.Profile.Feature;
import org.keycloak.common.util.CollectionUtil;
import org.keycloak.common.util.MultivaluedHashMap;
import org.keycloak.common.util.ObjectUtil;
import org.keycloak.common.util.UriUtils;
@ -413,7 +413,7 @@ public class RepresentationToModel {
public static void updateClient(ClientRepresentation rep, ClientModel resource, KeycloakSession session) {
if (Profile.isFeatureEnabled(Profile.Feature.CLIENT_TYPES)) {
if (!ObjectUtil.isEqualOrBothNull(rep.getType(), rep.getType())) {
if (!ObjectUtil.isEqualOrBothNull(resource.getType(), rep.getType())) {
throw new ClientTypeException("Not supported to change client type");
}
if (rep.getType() != null) {
@ -526,8 +526,8 @@ public class RepresentationToModel {
// 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)));
add(updatePropertyAction(client::setRedirectUris, () -> CollectionUtil.collectionToSet(rep.getRedirectUris()), client::getRedirectUris));
add(updatePropertyAction(client::setWebOrigins, () -> CollectionUtil.collectionToSet(rep.getWebOrigins()), () -> defaultWebOrigins(client)));
}};
// Extended client attributes
@ -689,6 +689,7 @@ public class RepresentationToModel {
* @return {@link Supplier} with results of operation.
* @param <T> Type of property.
*/
@SafeVarargs
private static <T> Supplier<ClientTypeException> updatePropertyAction(Consumer<T> modelSetter, Supplier<T>... getters) {
Stream<T> firstNonNullSupplied = Stream.of(getters)
.map(Supplier::get)
@ -1645,12 +1646,6 @@ public class RepresentationToModel {
return toModel(representation, authorization, client);
}
private static <T> Set<T> collectionToSet(Collection<T> collection) {
return Optional.ofNullable(collection)
.map(HashSet::new)
.orElse(null);
}
private static void updateOrganizationBroker(RealmModel realm, IdentityProviderRepresentation representation, KeycloakSession session) {
if (!Profile.isFeatureEnabled(Feature.ORGANIZATION)) {
return;

31
services/src/main/java/org/keycloak/services/clienttype/client/TypeAwareClientModelDelegate.java
View file

@ -49,7 +49,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
@Override
public boolean isStandardFlowEnabled() {
return TypedClientSimpleAttribute.STANDARD_FLOW_ENABLED
.getClientAttribute(clientType, super::isStandardFlowEnabled, Boolean.class);
.getClientAttribute(clientType, Boolean.class);
}
@Override
@ -61,7 +61,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
@Override
public boolean isBearerOnly() {
return TypedClientSimpleAttribute.BEARER_ONLY
.getClientAttribute(clientType, super::isBearerOnly, Boolean.class);
.getClientAttribute(clientType, Boolean.class);
}
@Override
@ -73,7 +73,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
@Override
public boolean isConsentRequired() {
return TypedClientSimpleAttribute.CONSENT_REQUIRED
.getClientAttribute(clientType, super::isConsentRequired, Boolean.class);
.getClientAttribute(clientType, Boolean.class);
}
@Override
@ -85,7 +85,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
@Override
public boolean isDirectAccessGrantsEnabled() {
return TypedClientSimpleAttribute.DIRECT_ACCESS_GRANTS_ENABLED
.getClientAttribute(clientType, super::isDirectAccessGrantsEnabled, Boolean.class);
.getClientAttribute(clientType, Boolean.class);
}
@Override
@ -97,7 +97,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
@Override
public boolean isAlwaysDisplayInConsole() {
return TypedClientSimpleAttribute.ALWAYS_DISPLAY_IN_CONSOLE
.getClientAttribute(clientType, super::isAlwaysDisplayInConsole, Boolean.class);
.getClientAttribute(clientType, Boolean.class);
}
@Override
@ -109,7 +109,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
@Override
public boolean isFrontchannelLogout() {
return TypedClientSimpleAttribute.FRONTCHANNEL_LOGOUT
.getClientAttribute(clientType, super::isFrontchannelLogout, Boolean.class);
.getClientAttribute(clientType, Boolean.class);
}
@Override
@ -121,7 +121,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
@Override
public boolean isImplicitFlowEnabled() {
return TypedClientSimpleAttribute.IMPLICIT_FLOW_ENABLED
.getClientAttribute(clientType, super::isImplicitFlowEnabled, Boolean.class);
.getClientAttribute(clientType, Boolean.class);
}
@Override
@ -133,7 +133,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
@Override
public boolean isServiceAccountsEnabled() {
return TypedClientSimpleAttribute.SERVICE_ACCOUNTS_ENABLED
.getClientAttribute(clientType, super::isServiceAccountsEnabled, Boolean.class);
.getClientAttribute(clientType, Boolean.class);
}
@Override
@ -145,7 +145,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
@Override
public String getProtocol() {
return TypedClientSimpleAttribute.PROTOCOL
.getClientAttribute(clientType, super::getProtocol, String.class);
.getClientAttribute(clientType, String.class);
}
@Override
@ -157,7 +157,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
@Override
public boolean isPublicClient() {
return TypedClientSimpleAttribute.PUBLIC_CLIENT
.getClientAttribute(clientType, super::isPublicClient, Boolean.class);
.getClientAttribute(clientType, Boolean.class);
}
@Override
@ -169,7 +169,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
@Override
public Set<String> getWebOrigins() {
return TypedClientSimpleAttribute.WEB_ORIGINS
.getClientAttribute(clientType, super::getWebOrigins, Set.class);
.getClientAttribute(clientType, Set.class);
}
@Override
@ -193,7 +193,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
@Override
public Set<String> getRedirectUris() {
return TypedClientSimpleAttribute.REDIRECT_URIS
.getClientAttribute(clientType, super::getRedirectUris, Set.class);
.getClientAttribute(clientType, Set.class);
}
@Override
@ -238,7 +238,7 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
public String getAttribute(String name) {
TypedClientExtendedAttribute attribute = TypedClientExtendedAttribute.getAttributesByName().get(name);
if (attribute != null) {
return attribute.getClientAttribute(clientType, () -> super.getAttribute(name), String.class);
return attribute.getClientAttribute(clientType, String.class);
} else {
return super.getAttribute(name);
}
@ -251,9 +251,8 @@ public class TypeAwareClientModelDelegate extends ClientModelLazyDelegate {
// Get extended client type attributes and values from the client type configuration.
Set<String> extendedClientTypeAttributes =
clientType.getConfiguration().entrySet().stream()
.map(Map.Entry::getKey)
.filter(entry -> TypedClientExtendedAttribute.getAttributesByName().containsKey(entry))
clientType.getOptionNames().stream()
.filter(optionName -> TypedClientExtendedAttribute.getAttributesByName().containsKey(optionName))
.collect(Collectors.toSet());
// Augment client type attributes on top of attributes on the delegate.

34
services/src/main/java/org/keycloak/services/clienttype/client/TypedClientSimpleAttribute.java → services/src/main/java/org/keycloak/services/clienttype/client/TypedClientAttribute.java
View file

@ -11,7 +11,6 @@ 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
@ -89,7 +88,7 @@ enum TypedClientExtendedAttribute implements TypedClientAttribute {
interface TypedClientAttribute {
Logger logger = Logger.getLogger(TypedClientAttribute.class);
default <T> T getClientAttribute(ClientType clientType, Supplier<T> clientGetter, Class<T> tClass) {
default <T> T getClientAttribute(ClientType clientType, Class<T> tClass) {
String propertyName = getPropertyName();
Object nonApplicableValue = getNonApplicableValue();
@ -103,32 +102,25 @@ interface TypedClientAttribute {
}
}
// 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();
return clientType.getTypeValue(propertyName, tClass);
}
default <T> void setClientAttribute(ClientType clientType, T newValue, Consumer<T> clientSetter, Class<T> 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());
// If feature is set as non-applicable, return directly
if (!clientType.isApplicable(propertyName)) {
if(!Objects.equals(getNonApplicableValue(), 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);
}
// If there is an attempt to change a value for an applicable field with a read-only value set, then throw an exception.
T oldVal = clientType.getTypeValue(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

32
services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientType.java
View file

@ -23,7 +23,8 @@ import org.keycloak.models.ClientModel;
import org.keycloak.representations.idm.ClientTypeRepresentation;
import org.keycloak.services.clienttype.client.TypeAwareClientModelDelegate;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@ -43,34 +44,31 @@ public class DefaultClientType implements ClientType {
@Override
public boolean isApplicable(String optionName) {
ClientTypeRepresentation.PropertyConfig cfg = clientType.getConfig().get(optionName);
// Each property is applicable by default if not configured for the particular client type
return (cfg != null && cfg.getApplicable() != null) ? cfg.getApplicable() : true;
return getConfiguration(optionName)
.map(ClientTypeRepresentation.PropertyConfig::getApplicable)
.orElse(true);
}
@Override
public boolean isReadOnly(String optionName) {
ClientTypeRepresentation.PropertyConfig cfg = clientType.getConfig().get(optionName);
public <T> T getTypeValue(String optionName, Class<T> optionType) {
// Each property is writable by default if not configured for the particular type
return (cfg != null && cfg.getReadOnly() != null) ? cfg.getReadOnly() : false;
return getConfiguration(optionName)
.map(ClientTypeRepresentation.PropertyConfig::getValue)
.map(optionType::cast).orElse(null);
}
@Override
public <T> T getDefaultValue(String optionName, Class<T> optionType) {
ClientTypeRepresentation.PropertyConfig cfg = clientType.getConfig().get(optionName);
return (cfg != null && cfg.getDefaultValue() != null) ? optionType.cast(cfg.getDefaultValue()) : null;
}
@Override
public Map<String, ClientTypeRepresentation.PropertyConfig> getConfiguration() {
return clientType.getConfig();
public Set<String> getOptionNames() {
return clientType.getConfig().keySet();
}
@Override
public ClientModel augment(ClientModel client) {
return new TypeAwareClientModelDelegate(this, () -> client);
}
private Optional<ClientTypeRepresentation.PropertyConfig> getConfiguration(String optionName) {
return Optional.ofNullable(clientType.getConfig().get(optionName));
}
}

4
services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientTypeProvider.java
View file

@ -50,8 +50,8 @@ public class DefaultClientTypeProvider implements ClientTypeProvider {
throw new ClientTypeException("Invalid configuration of 'applicable' property on client type");
}
// Not supported to set read-only or default-value for properties, which are not applicable for the particular client
if (!propConfig.getApplicable() && (propConfig.getReadOnly() != null || propConfig.getDefaultValue() != null)) {
// Not supported to set value for properties, which are not applicable for the particular client
if (!propConfig.getApplicable() && propConfig.getValue() != null) {
logger.errorf("Property '%s' is not applicable and so should not have read-only or default-value set for client type '%s'", propertyName, clientType.getName());
throw new ClientTypeException("Invalid configuration of property on client type");
}

61
services/src/main/resources/keycloak-default-client-types.json
View file

@ -6,8 +6,53 @@
"config": {
"standardFlowEnabled": {
"applicable": true,
"default-value": true,
"read-only": true
"value": true
}
}
},
{
"name": "oidc",
"provider": "default",
"config": {
"authorizationServicesEnabled": {
"applicable": true
},
"consentRequired": {
"applicable": true
},
"directAccessGrantsEnabled": {
"applicable": true
},
"frontchannelLogout": {
"applicable": true
},
"fullScopeAllowed": {
"applicable": true
},
"implicitFlowEnabled": {
"applicable": true
},
"nodeReRegistrationTimeout": {
"applicable": true
},
"oauth2.device.authorization.grant.enabled": {
"applicable": true
},
"oidc.ciba.grant.enabled": {
"applicable": true
},
"protocol": {
"applicable": true,
"value": "openid-connect"
},
"publicClient": {
"applicable": true
},
"serviceAccountsEnabled": {
"applicable": true
},
"standardFlowEnabled": {
"applicable": true
}
}
},
@ -26,8 +71,7 @@
},
"consentRequired": {
"applicable": true,
"default-value": false,
"read-only": true
"value": false
},
"directAccessGrantsEnabled": {
"applicable": false
@ -55,21 +99,18 @@
},
"protocol": {
"applicable": true,
"default-value": "openid-connect",
"read-only": true
"value": "openid-connect"
},
"publicClient": {
"applicable": true,
"default-value": false,
"read-only": true
"value": false
},
"redirectUris": {
"applicable": false
},
"serviceAccountsEnabled": {
"applicable": true,
"default-value": true,
"read-only": true
"value": true
},
"standardFlowEnabled": {
"applicable": false

33
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientTypesTest.java
View file

@ -22,7 +22,6 @@ import jakarta.ws.rs.BadRequestException;
import jakarta.ws.rs.core.Response;
import org.junit.Test;
import org.keycloak.client.clienttype.ClientTypeManager;
import org.keycloak.common.util.ObjectUtil;
import org.keycloak.models.ClientModel;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.representations.idm.ClientRepresentation;
@ -45,7 +44,9 @@ import java.util.List;
import java.util.stream.Collectors;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.everyItem;
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.in;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
import static org.keycloak.common.Profile.Feature.CLIENT_TYPES;
@ -154,8 +155,8 @@ public class ClientTypesTest extends AbstractTestRealmKeycloakTest {
assertEquals(Response.Status.BAD_REQUEST, response.getStatusInfo());
ErrorRepresentation errorRepresentation = response.readEntity(ErrorRepresentation.class);
assertThat(
List.of(errorRepresentation.getParams()),
containsInAnyOrder(items));
List.of(items),
everyItem(in(errorRepresentation.getParams())));
}
@Test
@ -164,10 +165,9 @@ public class ClientTypesTest extends AbstractTestRealmKeycloakTest {
assertEquals(0, clientTypes.getRealmClientTypes().size());
List<String> globalClientTypeNames = clientTypes.getGlobalClientTypes().stream()
.map(ClientTypeRepresentation::getName)
List<ClientTypeRepresentation> globalClientTypeNames = clientTypes.getGlobalClientTypes().stream()
.collect(Collectors.toList());
Assert.assertNames(globalClientTypeNames, "sla", "service-account");
assertNames(globalClientTypeNames, "sla", "service-account");
ClientTypeRepresentation serviceAccountType = clientTypes.getGlobalClientTypes().stream()
.filter(clientType -> "service-account".equals(clientType.getName()))
@ -176,16 +176,15 @@ public class ClientTypesTest extends AbstractTestRealmKeycloakTest {
assertEquals("default", serviceAccountType.getProvider());
ClientTypeRepresentation.PropertyConfig cfg = serviceAccountType.getConfig().get("standardFlowEnabled");
assertPropertyConfig("standardFlowEnabled", cfg, false, null, null);
assertPropertyConfig("standardFlowEnabled", cfg, false, null);
cfg = serviceAccountType.getConfig().get("serviceAccountsEnabled");
assertPropertyConfig("serviceAccountsEnabled", cfg, true, true, true);
assertPropertyConfig("serviceAccountsEnabled", cfg, true, true);
cfg = serviceAccountType.getConfig().get("tosUri");
assertPropertyConfig("tosUri", cfg, false, null, null);
assertPropertyConfig("tosUri", cfg, false, null);
}
@Test
public void testClientTypesAdminRestAPI_realmTypes() {
ClientTypesRepresentation clientTypes = testRealm().clientTypes().getClientTypes();
@ -218,8 +217,7 @@ public class ClientTypesTest extends AbstractTestRealmKeycloakTest {
try {
ClientTypeRepresentation.PropertyConfig cfg = clientType.getConfig().get("standardFlowEnabled");
cfg.setApplicable(false);
cfg.setReadOnly(true);
cfg.setDefaultValue(true);
cfg.setValue(true);
testRealm().clientTypes().updateClientTypes(clientTypes);
Assert.fail("Not expected to update client types");
} catch (BadRequestException bre) {
@ -278,14 +276,13 @@ public class ClientTypesTest extends AbstractTestRealmKeycloakTest {
List<String> names = clientTypes.stream()
.map(ClientTypeRepresentation::getName)
.collect(Collectors.toList());
Assert.assertNames(names, expectedNames);
assertThat(names, hasItems(expectedNames));
}
private void assertPropertyConfig(String propertyName, ClientTypeRepresentation.PropertyConfig cfg, Boolean expectedApplicable, Boolean expectedReadOnly, Object expectedDefaultValue) {
assertThat("'applicable' for property " + propertyName + " not equal", ObjectUtil.isEqualOrBothNull(expectedApplicable, cfg.getApplicable()));
assertThat("'read-only' for property " + propertyName + " not equal", ObjectUtil.isEqualOrBothNull(expectedReadOnly, cfg.getReadOnly()));
assertThat("'default-value' ;for property " + propertyName + " not equal", ObjectUtil.isEqualOrBothNull(expectedDefaultValue, cfg.getDefaultValue()));
private void assertPropertyConfig(String propertyName, ClientTypeRepresentation.PropertyConfig cfg, Boolean expectedApplicable, Object expectedValue) {
assertEquals("'applicable' for property " + propertyName + " not equal", expectedApplicable, cfg.getApplicable());
assertEquals("'value' for property " + propertyName + " not equal", expectedValue, cfg.getValue());
}
private ClientRepresentation createClientWithType(String clientId, String clientType) {