From c3b9c669418f7f4a55b5645ea0f3aa8321369345 Mon Sep 17 00:00:00 2001 From: vramik Date: Tue, 16 Mar 2021 15:24:25 +0100 Subject: [PATCH] KEYCLOAK-17460 invalidate client when assigning scope --- .../cache/infinispan/ClientAdapter.java | 31 +++-------- .../cache/infinispan/ClientScopeAdapter.java | 4 ++ .../cache/infinispan/RealmCacheManager.java | 2 + .../cache/infinispan/RealmCacheSession.java | 51 ++++++++++++++++++ .../infinispan/entities/CachedClient.java | 4 +- .../entities/ClientScopeListQuery.java | 12 +++++ .../infinispan/entities/ClientScopeQuery.java | 2 +- .../keycloak/models/jpa/ClientAdapter.java | 52 +++---------------- .../models/jpa/ClientScopeAdapter.java | 6 ++- .../keycloak/models/jpa/JpaRealmProvider.java | 52 ++++++++++++++++++- .../ClientScopeClientMappingEntity.java | 40 +++++++------- .../META-INF/jpa-changelog-13.0.0.xml | 6 +++ .../map/client/AbstractClientModel.java | 21 ++++++-- .../models/map/client/MapClientAdapter.java | 37 +------------ .../models/map/client/MapClientProvider.java | 52 +++++++++++++++++++ .../migration/migrators/MigrateTo4_0_0.java | 4 +- .../models/utils/KeycloakModelUtils.java | 4 +- .../models/utils/ModelToRepresentation.java | 4 +- .../models/utils/RepresentationToModel.java | 6 +-- .../AbstractLoginProtocolFactory.java | 4 +- .../java/org/keycloak/models/ClientModel.java | 7 ++- .../org/keycloak/models/ClientProvider.java | 21 ++++++++ .../storage/client/ClientLookupProvider.java | 12 +++++ .../freemarker/model/ApplicationsBean.java | 4 +- .../keycloak/protocol/oidc/TokenManager.java | 4 +- .../condition/ClientScopesCondition.java | 4 +- .../ClientScopesClientRegistrationPolicy.java | 4 +- .../resources/admin/ClientResource.java | 2 +- .../resources/admin/RealmAdminResource.java | 1 + .../storage/ClientStorageManager.java | 31 +++++++++++ .../OpenshiftClientStorageProvider.java | 9 ++++ .../openshift/OpenshiftSAClientAdapter.java | 2 +- .../HardcodedClientStorageProvider.java | 29 ++++++----- .../admin/client/ClientScopeTest.java | 1 + .../keycloak/testsuite/cli/KcinitTest.java | 2 + .../ClientRegistrationPoliciesTest.java | 4 ++ .../client/ClientRegistrationTest.java | 11 ++-- .../client/OIDCClientRegistrationTest.java | 8 ++- .../testsuite/model/ClientModelTest.java | 17 +++--- 39 files changed, 381 insertions(+), 186 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java index 8e32eef256..abfca1ec94 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java @@ -27,8 +27,8 @@ import org.keycloak.models.cache.infinispan.entities.CachedClient; import org.keycloak.models.utils.RoleUtils; import java.security.MessageDigest; +import java.util.Collections; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -99,41 +99,22 @@ public class ClientAdapter implements ClientModel, CachedObject { @Override public void addClientScope(ClientScopeModel clientScope, boolean defaultScope) { - getDelegateForUpdate(); - updated.addClientScope(clientScope, defaultScope); + cacheSession.addClientScopes(getRealm(), this, Collections.singleton(clientScope), defaultScope); } @Override public void addClientScopes(Set clientScopes, boolean defaultScope) { - for (ClientScopeModel clientScope : clientScopes) { - addClientScope(clientScope, defaultScope); - } + cacheSession.addClientScopes(getRealm(), this, clientScopes, defaultScope); } @Override public void removeClientScope(ClientScopeModel clientScope) { - getDelegateForUpdate(); - updated.removeClientScope(clientScope); + cacheSession.removeClientScope(getRealm(), this, clientScope); } @Override - public Map getClientScopes(boolean defaultScope, boolean filterByProtocol) { - if (isUpdated()) return updated.getClientScopes(defaultScope, filterByProtocol); - List clientScopeIds = defaultScope ? cached.getDefaultClientScopesIds() : cached.getOptionalClientScopesIds(); - - // Defaults to openid-connect - String clientProtocol = getProtocol() == null ? "openid-connect" : getProtocol(); - - Map clientScopes = new HashMap<>(); - for (String scopeId : clientScopeIds) { - ClientScopeModel clientScope = cacheSession.getClientScopeById(cachedRealm, scopeId); - if (clientScope != null) { - if (!filterByProtocol || clientScope.getProtocol().equals(clientProtocol)) { - clientScopes.put(clientScope.getName(), clientScope); - } - } - } - return clientScopes; + public Map getClientScopes(boolean defaultScope) { + return cacheSession.getClientScopes(getRealm(), this, defaultScope); } public void addWebOrigin(String webOrigin) { diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientScopeAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientScopeAdapter.java index f98c99af6d..5ae50e85f7 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientScopeAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientScopeAdapter.java @@ -232,4 +232,8 @@ public class ClientScopeAdapter implements ClientScopeModel { return getId().hashCode(); } + @Override + public String toString() { + return String.format("%s@%08x", getId(), hashCode()); + } } diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java index 0684c59c18..b72c3f8053 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java @@ -97,6 +97,8 @@ public class RealmCacheManager extends CacheManager { public void clientUpdated(String realmId, String clientUuid, String clientId, Set invalidations) { invalidations.add(RealmCacheSession.getClientByClientIdCacheKey(clientId, realmId)); + invalidations.add(RealmCacheSession.getClientScopesCacheKey(clientUuid, true)); + invalidations.add(RealmCacheSession.getClientScopesCacheKey(clientUuid, false)); } // Client roles invalidated separately diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java index c3c63c20a7..acecf05e54 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java @@ -97,6 +97,8 @@ public class RealmCacheSession implements CacheRealmProvider { protected static final Logger logger = Logger.getLogger(RealmCacheSession.class); public static final String REALM_CLIENTS_QUERY_SUFFIX = ".realm.clients"; public static final String ROLES_QUERY_SUFFIX = ".roles"; + private static final String SCOPE_KEY_DEFAULT = "default"; + private static final String SCOPE_KEY_OPTIONAL = "optional"; protected RealmCacheManager cache; protected KeycloakSession session; protected RealmProvider realmDelegate; @@ -542,6 +544,10 @@ public class RealmCacheSession implements CacheRealmProvider { return realm + ".clientscopes"; } + static String getClientScopesCacheKey(String client, boolean defaultScope) { + return client + "." + (defaultScope ? SCOPE_KEY_DEFAULT : SCOPE_KEY_OPTIONAL) + ".clientscopes"; + } + static String getTopGroupsQueryCacheKey(String realm) { return realm + ".top.groups"; } @@ -1305,6 +1311,51 @@ public class RealmCacheSession implements CacheRealmProvider { realm.getClientScopesStream().map(ClientScopeModel::getId).forEach(id -> removeClientScope(realm, id)); } + @Override + public void addClientScopes(RealmModel realm, ClientModel client, Set clientScopes, boolean defaultScope) { + getClientDelegate().addClientScopes(realm, client, clientScopes, defaultScope); + registerClientInvalidation(client.getId(), client.getId(), realm.getId()); + } + + @Override + public void removeClientScope(RealmModel realm, ClientModel client, ClientScopeModel clientScope) { + getClientDelegate().removeClientScope(realm, client, clientScope); + registerClientInvalidation(client.getId(), client.getId(), realm.getId()); + } + + @Override + public Map getClientScopes(RealmModel realm, ClientModel client, boolean defaultScopes) { + String cacheKey = getClientScopesCacheKey(client.getId(), defaultScopes); + boolean queryDB = invalidations.contains(cacheKey) || invalidations.contains(client.getId()) || listInvalidations.contains(realm.getId()); + if (queryDB) { + return getClientDelegate().getClientScopes(realm, client, defaultScopes); + } + ClientScopeListQuery query = cache.get(cacheKey, ClientScopeListQuery.class); + + if (query == null) { + Long loaded = cache.getCurrentRevision(cacheKey); + Map model = getClientDelegate().getClientScopes(realm, client, defaultScopes); + if (model == null) return null; + Set ids = model.values().stream().map(ClientScopeModel::getId).collect(Collectors.toSet()); + query = new ClientScopeListQuery(loaded, cacheKey, realm, client.getId(), ids); + logger.tracev("adding assigned client scopes cache miss: client {0} key {1}", client.getClientId(), cacheKey); + cache.addRevisioned(query, startupRevision); + return model; + } + Map assignedScopes = new HashMap<>(); + for (String id : query.getClientScopes()) { + ClientScopeModel clientScope = session.clientScopes().getClientScopeById(realm, id); + if (clientScope == null) { + invalidations.add(cacheKey); + return getClientDelegate().getClientScopes(realm, client, defaultScopes); + } + if (clientScope.getProtocol().equals((client.getProtocol() == null) ? "openid-connect" : client.getProtocol())) { + assignedScopes.put(clientScope.getName(), clientScope); + } + } + return assignedScopes; + } + // Don't cache ClientInitialAccessModel for now @Override public ClientInitialAccessModel createClientInitialAccessModel(RealmModel realm, int expiration, int count) { diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedClient.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedClient.java index 54dd62aa2d..10dc3c40fa 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedClient.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedClient.java @@ -109,11 +109,11 @@ public class CachedClient extends AbstractRevisioned implements InRealm { registeredNodes = new TreeMap<>(model.getRegisteredNodes()); defaultClientScopesIds = new LinkedList<>(); - for (ClientScopeModel clientScope : model.getClientScopes(true, false).values()) { + for (ClientScopeModel clientScope : model.getClientScopes(true).values()) { defaultClientScopesIds.add(clientScope.getId()); } optionalClientScopesIds = new LinkedList<>(); - for (ClientScopeModel clientScope : model.getClientScopes(false, false).values()) { + for (ClientScopeModel clientScope : model.getClientScopes(false).values()) { optionalClientScopesIds.add(clientScope.getId()); } } diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/ClientScopeListQuery.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/ClientScopeListQuery.java index 0a13b34bf2..d19f9bd5e5 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/ClientScopeListQuery.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/ClientScopeListQuery.java @@ -25,6 +25,7 @@ public class ClientScopeListQuery extends AbstractRevisioned implements ClientSc private final Set clientScopes; private final String realm; private final String realmName; + private String clientUuid; public ClientScopeListQuery(Long revisioned, String id, RealmModel realm, Set clientScopes) { super(revisioned, id); @@ -33,6 +34,11 @@ public class ClientScopeListQuery extends AbstractRevisioned implements ClientSc this.clientScopes = clientScopes; } + public ClientScopeListQuery(Long revisioned, String id, RealmModel realm, String clientUuid, Set clientScopes) { + this(revisioned, id, realm, clientScopes); + this.clientUuid = clientUuid; + } + @Override public Set getClientScopes() { return clientScopes; @@ -43,11 +49,17 @@ public class ClientScopeListQuery extends AbstractRevisioned implements ClientSc return realm; } + @Override + public String getClientId() { + return clientUuid; + } + @Override public String toString() { return "ClientScopeListQuery{" + "id='" + getId() + "'" + ", realmName='" + realmName + '\'' + + ", clientUuid='" + clientUuid + '\'' + '}'; } } diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/ClientScopeQuery.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/ClientScopeQuery.java index ef2258f8ac..72098a2bba 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/ClientScopeQuery.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/ClientScopeQuery.java @@ -19,6 +19,6 @@ package org.keycloak.models.cache.infinispan.entities; import java.util.Set; -public interface ClientScopeQuery extends InRealm { +public interface ClientScopeQuery extends InClient { Set getClientScopes(); } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java index 9a4ee881ad..cb635c1516 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java @@ -26,20 +26,17 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.jpa.entities.ClientAttributeEntity; import org.keycloak.models.jpa.entities.ClientEntity; -import org.keycloak.models.jpa.entities.ClientScopeClientMappingEntity; import org.keycloak.models.jpa.entities.ProtocolMapperEntity; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.RoleUtils; -import org.keycloak.protocol.oidc.OIDCLoginProtocol; import javax.persistence.EntityManager; -import javax.persistence.TypedQuery; import java.security.MessageDigest; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -265,7 +262,7 @@ public class ClientAdapter implements ClientModel, JpaModel { @Override public void setProtocol(String protocol) { entity.setProtocol(protocol); - + session.getKeycloakSessionFactory().publish((ClientModel.ClientProtocolUpdatedEvent) () -> ClientAdapter.this); } @Override @@ -349,57 +346,22 @@ public class ClientAdapter implements ClientModel, JpaModel { @Override public void addClientScope(ClientScopeModel clientScope, boolean defaultScope) { - if (getClientScopes(defaultScope, false).containsKey(clientScope.getName())) return; - - persist(clientScope, defaultScope); + addClientScopes(Collections.singleton(clientScope), defaultScope); } @Override public void addClientScopes(Set clientScopes, boolean defaultScope) { - Map existingClientScopes = getClientScopes(defaultScope, false); - clientScopes.stream() - .filter(clientScope -> !existingClientScopes.containsKey(clientScope.getName())) - .forEach(clientScope -> persist(clientScope, defaultScope)); - } - - private void persist(ClientScopeModel clientScope, boolean defaultScope) { - ClientScopeClientMappingEntity entity = new ClientScopeClientMappingEntity(); - entity.setClientScopeId(clientScope.getId()); - entity.setClient(getEntity()); - entity.setDefaultScope(defaultScope); - em.persist(entity); - em.flush(); - em.detach(entity); + session.clients().addClientScopes(getRealm(), this, clientScopes, defaultScope); } @Override public void removeClientScope(ClientScopeModel clientScope) { - int numRemoved = em.createNamedQuery("deleteClientScopeClientMapping") - .setParameter("clientScopeId", clientScope.getId()) - .setParameter("client", getEntity()) - .executeUpdate(); - em.flush(); + session.clients().removeClientScope(getRealm(), this, clientScope); } @Override - public Map getClientScopes(boolean defaultScope, boolean filterByProtocol) { - TypedQuery query = em.createNamedQuery("clientScopeClientMappingIdsByClient", String.class); - query.setParameter("client", getEntity()); - query.setParameter("defaultScope", defaultScope); - List ids = query.getResultList(); - - // Defaults to openid-connect - String clientProtocol = getProtocol() == null ? OIDCLoginProtocol.LOGIN_PROTOCOL : getProtocol(); - - Map clientScopes = new HashMap<>(); - for (String clientScopeId : ids) { - ClientScopeModel clientScope = realm.getClientScopeById(clientScopeId); - if (clientScope == null) continue; - if (!filterByProtocol || clientScope.getProtocol().equals(clientProtocol)) { - clientScopes.put(clientScope.getName(), clientScope); - } - } - return clientScopes; + public Map getClientScopes(boolean defaultScope) { + return session.clients().getClientScopes(getRealm(), this, defaultScope); } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientScopeAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientScopeAdapter.java index 40e02769d5..ab5d4b3308 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientScopeAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientScopeAdapter.java @@ -300,7 +300,9 @@ public class ClientScopeAdapter implements ClientScopeModel, JpaModel this.removeClientScope(realm, id)); } + @Override + public void addClientScopes(RealmModel realm, ClientModel client, Set clientScopes, boolean defaultScope) { + // Defaults to openid-connect + String clientProtocol = client.getProtocol() == null ? OIDCLoginProtocol.LOGIN_PROTOCOL : client.getProtocol(); + + Map existingClientScopes = getClientScopes(realm, client, defaultScope); + + clientScopes.stream() + .filter(clientScope -> ! existingClientScopes.containsKey(clientScope.getName())) + .filter(clientScope -> Objects.equals(clientScope.getProtocol(), clientProtocol)) + .forEach(clientScope -> { + ClientScopeClientMappingEntity entity = new ClientScopeClientMappingEntity(); + entity.setClientScopeId(clientScope.getId()); + entity.setClientId(client.getId()); + entity.setDefaultScope(defaultScope); + em.persist(entity); + em.flush(); + em.detach(entity); + }); + } + + @Override + public void removeClientScope(RealmModel realm, ClientModel client, ClientScopeModel clientScope) { + em.createNamedQuery("deleteClientScopeClientMapping") + .setParameter("clientScopeId", clientScope.getId()) + .setParameter("clientId", client.getId()) + .executeUpdate(); + em.flush(); + } + + @Override + public Map getClientScopes(RealmModel realm, ClientModel client, boolean defaultScope) { + // Defaults to openid-connect + String clientProtocol = client.getProtocol() == null ? OIDCLoginProtocol.LOGIN_PROTOCOL : client.getProtocol(); + + TypedQuery query = em.createNamedQuery("clientScopeClientMappingIdsByClient", String.class); + query.setParameter("clientId", client.getId()); + query.setParameter("defaultScope", defaultScope); + + return query.getResultStream() + .map(clientScopeId -> session.clientScopes().getClientScopeById(realm, clientScopeId)) + .filter(Objects::nonNull) + .filter(clientScope -> Objects.equals(clientScope.getProtocol(), clientProtocol)) + .collect(Collectors.toMap(ClientScopeModel::getName, Function.identity())); + } + @Override public Stream searchForGroupByNameStream(RealmModel realm, String search, Integer first, Integer max) { TypedQuery query = em.createNamedQuery("getGroupIdsByNameContaining", String.class) diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientScopeClientMappingEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientScopeClientMappingEntity.java index 6067fd8c89..1b00f6395d 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientScopeClientMappingEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientScopeClientMappingEntity.java @@ -21,11 +21,8 @@ import java.io.Serializable; import javax.persistence.Column; import javax.persistence.Entity; -import javax.persistence.FetchType; import javax.persistence.Id; import javax.persistence.IdClass; -import javax.persistence.JoinColumn; -import javax.persistence.ManyToOne; import javax.persistence.NamedQueries; import javax.persistence.NamedQuery; import javax.persistence.Table; @@ -36,9 +33,9 @@ import javax.persistence.Table; * @author Marek Posolda */ @NamedQueries({ - @NamedQuery(name="clientScopeClientMappingIdsByClient", query="select m.clientScopeId from ClientScopeClientMappingEntity m where m.client = :client and m.defaultScope = :defaultScope"), - @NamedQuery(name="deleteClientScopeClientMapping", query="delete from ClientScopeClientMappingEntity where client = :client and clientScopeId = :clientScopeId"), - @NamedQuery(name="deleteClientScopeClientMappingByClient", query="delete from ClientScopeClientMappingEntity where client = :client") + @NamedQuery(name="clientScopeClientMappingIdsByClient", query="select m.clientScopeId from ClientScopeClientMappingEntity m where m.clientId = :clientId and m.defaultScope = :defaultScope"), + @NamedQuery(name="deleteClientScopeClientMapping", query="delete from ClientScopeClientMappingEntity where clientId = :clientId and clientScopeId = :clientScopeId"), + @NamedQuery(name="deleteClientScopeClientMappingByClient", query="delete from ClientScopeClientMappingEntity where clientId = :clientId") }) @Entity @Table(name="CLIENT_SCOPE_CLIENT") @@ -50,9 +47,8 @@ public class ClientScopeClientMappingEntity { protected String clientScopeId; @Id - @ManyToOne(fetch= FetchType.LAZY) - @JoinColumn(name="CLIENT_ID") - protected ClientEntity client; + @Column(name="CLIENT_ID") + protected String clientId; @Column(name="DEFAULT_SCOPE") protected boolean defaultScope; @@ -65,12 +61,12 @@ public class ClientScopeClientMappingEntity { this.clientScopeId = clientScopeId; } - public ClientEntity getClient() { - return client; + public String getClientId() { + return clientId; } - public void setClient(ClientEntity client) { - this.client = client; + public void setClientId(String clientId) { + this.clientId = clientId; } public boolean isDefaultScope() { @@ -85,22 +81,22 @@ public class ClientScopeClientMappingEntity { protected String clientScopeId; - protected ClientEntity client; + protected String clientId; public Key() { } - public Key(String clientScopeId, ClientEntity client) { + public Key(String clientScopeId, String clientId) { this.clientScopeId = clientScopeId; - this.client = client; + this.clientId = clientId; } public String getClientScopeId() { return clientScopeId; } - public ClientEntity getClient() { - return client; + public String getClientId() { + return clientId; } @Override @@ -111,7 +107,7 @@ public class ClientScopeClientMappingEntity { ClientScopeClientMappingEntity.Key key = (ClientScopeClientMappingEntity.Key) o; if (clientScopeId != null ? !clientScopeId.equals(key.getClientScopeId() != null ? key.getClientScopeId() : null) : key.getClientScopeId() != null) return false; - if (client != null ? !client.getId().equals(key.client != null ? key.client.getId() : null) : key.client != null) return false; + if (clientId != null ? !clientId.equals(key.getClientId() != null ? key.getClientId() : null) : key.getClientId() != null) return false; return true; } @@ -119,7 +115,7 @@ public class ClientScopeClientMappingEntity { @Override public int hashCode() { int result = clientScopeId != null ? clientScopeId.hashCode() : 0; - result = 31 * result + (client != null ? client.getId().hashCode() : 0); + result = 31 * result + (clientId != null ? clientId.hashCode() : 0); return result; } } @@ -133,7 +129,7 @@ public class ClientScopeClientMappingEntity { ClientScopeClientMappingEntity key = (ClientScopeClientMappingEntity) o; if (clientScopeId != null ? !clientScopeId.equals(key.getClientScopeId() != null ? key.getClientScopeId() : null) : key.getClientScopeId() != null) return false; - if (client != null ? !client.getId().equals(key.client != null ? key.client.getId() : null) : key.client != null) return false; + if (clientId != null ? !clientId.equals(key.getClientId() != null ? key.getClientId() : null) : key.getClientId() != null) return false; return true; } @@ -141,7 +137,7 @@ public class ClientScopeClientMappingEntity { @Override public int hashCode() { int result = clientScopeId != null ? clientScopeId.hashCode() : 0; - result = 31 * result + (client != null ? client.getId().hashCode() : 0); + result = 31 * result + (clientId != null ? clientId.hashCode() : 0); return result; } } diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml index 3243afa857..e014e74b39 100644 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml @@ -41,5 +41,11 @@ + + + + + + diff --git a/model/map/src/main/java/org/keycloak/models/map/client/AbstractClientModel.java b/model/map/src/main/java/org/keycloak/models/map/client/AbstractClientModel.java index a5ba3123d7..364f0d52b4 100644 --- a/model/map/src/main/java/org/keycloak/models/map/client/AbstractClientModel.java +++ b/model/map/src/main/java/org/keycloak/models/map/client/AbstractClientModel.java @@ -16,6 +16,8 @@ */ package org.keycloak.models.map.client; +import java.util.Collections; +import java.util.Map; import org.keycloak.models.ClientModel; import org.keycloak.models.ClientScopeModel; import org.keycloak.models.KeycloakSession; @@ -49,9 +51,22 @@ public abstract class AbstractClientModel implements C @Override public void addClientScopes(Set clientScopes, boolean defaultScope) { - for (ClientScopeModel cs : clientScopes) { - addClientScope(cs, defaultScope); - } + session.clients().addClientScopes(getRealm(), this, clientScopes, defaultScope); + } + + @Override + public void addClientScope(ClientScopeModel clientScope, boolean defaultScope) { + addClientScopes(Collections.singleton(clientScope), defaultScope); + } + + @Override + public void removeClientScope(ClientScopeModel clientScope) { + session.clients().removeClientScope(getRealm(), this, clientScope); + } + + @Override + public Map getClientScopes(boolean defaultScope) { + return session.clients().getClientScopes(getRealm(), this, defaultScope); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java b/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java index 161ceb688f..9bb5ba5223 100644 --- a/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java +++ b/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java @@ -17,20 +17,16 @@ package org.keycloak.models.map.client; import org.keycloak.models.ClientModel; -import org.keycloak.models.ClientScopeModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.ProtocolMapperModel; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.utils.KeycloakModelUtils; -import org.keycloak.protocol.oidc.OIDCLoginProtocol; -import com.google.common.base.Functions; import java.security.MessageDigest; import java.util.HashMap; import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.stream.Collectors; import java.util.stream.Stream; /** @@ -241,6 +237,7 @@ public abstract class MapClientAdapter extends AbstractClientModel MapClientAdapter.this); } @Override @@ -385,38 +382,6 @@ public abstract class MapClientAdapter extends AbstractClientModel getClientScopes(boolean defaultScope, boolean filterByProtocol) { - Stream res = this.entity.getClientScopes(defaultScope) - .map(realm::getClientScopeById) - .filter(Objects::nonNull); - - if (filterByProtocol) { - String clientProtocol = getProtocol() == null ? OIDCLoginProtocol.LOGIN_PROTOCOL : getProtocol(); - res = res.filter(cs -> Objects.equals(cs.getProtocol(), clientProtocol)); - } - - return res.collect(Collectors.toMap(ClientScopeModel::getName, Functions.identity())); - } - /*************** Scopes mappings ****************/ @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/client/MapClientProvider.java b/model/map/src/main/java/org/keycloak/models/map/client/MapClientProvider.java index 498e2dfee3..b4bb27d627 100644 --- a/model/map/src/main/java/org/keycloak/models/map/client/MapClientProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/client/MapClientProvider.java @@ -29,8 +29,10 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.map.storage.MapKeycloakTransaction; import org.keycloak.models.map.common.Serialization; import java.util.Comparator; +import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -43,6 +45,8 @@ import org.keycloak.models.map.storage.MapStorage; import org.keycloak.models.map.storage.ModelCriteriaBuilder; import org.keycloak.models.map.storage.ModelCriteriaBuilder.Operator; import static org.keycloak.common.util.StackUtil.getShortStackTrace; +import org.keycloak.models.ClientScopeModel; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; import static org.keycloak.utils.StreamsUtil.paginatedStream; public class MapClientProvider implements ClientProvider { @@ -269,6 +273,54 @@ public class MapClientProvider implements ClientProvider { return paginatedStream(s, firstResult, maxResults).map(entityToAdapterFunc(realm)); } + @Override + public void addClientScopes(RealmModel realm, ClientModel client, Set clientScopes, boolean defaultScope) { + MapClientEntity entity = tx.read(UUID.fromString(client.getId())); + + if (entity == null) return; + + // Defaults to openid-connect + String clientProtocol = client.getProtocol() == null ? OIDCLoginProtocol.LOGIN_PROTOCOL : client.getProtocol(); + + LOG.tracef("addClientScopes(%s, %s, %s, %b)%s", realm, client, clientScopes, defaultScope, getShortStackTrace()); + + Map existingClientScopes = getClientScopes(realm, client, defaultScope); + + clientScopes.stream() + .filter(clientScope -> ! existingClientScopes.containsKey(clientScope.getName())) + .filter(clientScope -> Objects.equals(clientScope.getProtocol(), clientProtocol)) + .forEach(clientScope -> entity.addClientScope(clientScope.getId(), defaultScope)); + } + + @Override + public void removeClientScope(RealmModel realm, ClientModel client, ClientScopeModel clientScope) { + MapClientEntity entity = tx.read(UUID.fromString(client.getId())); + + if (entity == null) return; + + LOG.tracef("removeClientScope(%s, %s, %s)%s", realm, client, clientScope, getShortStackTrace()); + + entity.removeClientScope(clientScope.getId()); + } + + @Override + public Map getClientScopes(RealmModel realm, ClientModel client, boolean defaultScopes) { + MapClientEntity entity = tx.read(UUID.fromString(client.getId())); + + if (entity == null) return null; + + // Defaults to openid-connect + String clientProtocol = client.getProtocol() == null ? OIDCLoginProtocol.LOGIN_PROTOCOL : client.getProtocol(); + + LOG.tracef("getClientScopes(%s, %s, %b)%s", realm, client, defaultScopes, getShortStackTrace()); + + return entity.getClientScopes(defaultScopes) + .map(clientScopeId -> session.clientScopes().getClientScopeById(realm, clientScopeId)) + .filter(Objects::nonNull) + .filter(clientScope -> Objects.equals(clientScope.getProtocol(), clientProtocol)) + .collect(Collectors.toMap(ClientScopeModel::getName, Function.identity())); + } + @Override public void close() { diff --git a/server-spi-private/src/main/java/org/keycloak/migration/migrators/MigrateTo4_0_0.java b/server-spi-private/src/main/java/org/keycloak/migration/migrators/MigrateTo4_0_0.java index ac9cdae41d..c94119ee4d 100644 --- a/server-spi-private/src/main/java/org/keycloak/migration/migrators/MigrateTo4_0_0.java +++ b/server-spi-private/src/main/java/org/keycloak/migration/migrators/MigrateTo4_0_0.java @@ -102,7 +102,7 @@ public class MigrateTo4_0_0 implements Migration { realm.getClientsStream() .filter(MigrationUtils::isOIDCNonBearerOnlyClient) .filter(c -> c.hasScope(offlineAccessRole)) - .filter(c -> !c.getClientScopes(false, true).containsKey(OAuth2Constants.OFFLINE_ACCESS)) + .filter(c -> !c.getClientScopes(false).containsKey(OAuth2Constants.OFFLINE_ACCESS)) .peek(c -> { LOG.debugf("Adding client scope 'offline_access' as optional scope to client '%s' in realm '%s'.", c.getClientId(), realm.getName()); c.addClientScope(offlineAccessScope, false); @@ -119,7 +119,7 @@ public class MigrateTo4_0_0 implements Migration { // Clients with consentRequired, which don't have any client scopes will be added itself to require consent, so that consent screen is shown when users authenticate realm.getClientsStream() .filter(ClientModel::isConsentRequired) - .filter(c -> c.getClientScopes(true, true).isEmpty()) + .filter(c -> c.getClientScopes(true).isEmpty()) .forEach(c -> { LOG.debugf("Adding client '%s' of realm '%s' to display itself on consent screen", c.getClientId(), realm.getName()); c.setDisplayOnConsentScreen(true); diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java b/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java index c50612b71b..2747ba00a0 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java @@ -704,8 +704,8 @@ public final class KeycloakModelUtils { public static boolean isClientScopeUsed(RealmModel realm, ClientScopeModel clientScope) { return realm.getClientsStream() - .filter(c -> (c.getClientScopes(true, false).containsKey(clientScope.getName())) || - (c.getClientScopes(false, false).containsKey(clientScope.getName()))) + .filter(c -> (c.getClientScopes(true).containsKey(clientScope.getName())) || + (c.getClientScopes(false).containsKey(clientScope.getName()))) .findFirst().isPresent(); } diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index b77ff2e592..a1860d0f67 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -592,8 +592,8 @@ public class ModelToRepresentation { rep.setNodeReRegistrationTimeout(clientModel.getNodeReRegistrationTimeout()); rep.setClientAuthenticatorType(clientModel.getClientAuthenticatorType()); - rep.setDefaultClientScopes(new LinkedList<>(clientModel.getClientScopes(true, false).keySet())); - rep.setOptionalClientScopes(new LinkedList<>(clientModel.getClientScopes(false, false).keySet())); + rep.setDefaultClientScopes(new LinkedList<>(clientModel.getClientScopes(true).keySet())); + rep.setOptionalClientScopes(new LinkedList<>(clientModel.getClientScopes(false).keySet())); Set redirectUris = clientModel.getRedirectUris(); if (redirectUris != null) { 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 bb0f3e8f5e..4aa316ec3b 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 @@ -1462,12 +1462,12 @@ public class RepresentationToModel { if (resourceRep.getDefaultClientScopes() != null || resourceRep.getOptionalClientScopes() != null) { // First remove all default/built in client scopes - for (ClientScopeModel clientScope : client.getClientScopes(true, false).values()) { + for (ClientScopeModel clientScope : client.getClientScopes(true).values()) { client.removeClientScope(clientScope); } // First remove all default/built in client scopes - for (ClientScopeModel clientScope : client.getClientScopes(false, false).values()) { + for (ClientScopeModel clientScope : client.getClientScopes(false).values()) { client.removeClientScope(clientScope); } } @@ -2039,7 +2039,7 @@ public class RepresentationToModel { // Backwards compatibility. If user had consent for "offline_access" role, we treat it as he has consent for "offline_access" client scope if (consentRep.getGrantedRealmRoles() != null) { if (consentRep.getGrantedRealmRoles().contains(OAuth2Constants.OFFLINE_ACCESS)) { - ClientScopeModel offlineScope = client.getClientScopes(false, true).get(OAuth2Constants.OFFLINE_ACCESS); + ClientScopeModel offlineScope = client.getClientScopes(false).get(OAuth2Constants.OFFLINE_ACCESS); if (offlineScope == null) { logger.warn("Unable to find offline_access scope referenced in grantedRoles of user"); } diff --git a/server-spi-private/src/main/java/org/keycloak/protocol/AbstractLoginProtocolFactory.java b/server-spi-private/src/main/java/org/keycloak/protocol/AbstractLoginProtocolFactory.java index 117d4cb71b..af56344f6e 100755 --- a/server-spi-private/src/main/java/org/keycloak/protocol/AbstractLoginProtocolFactory.java +++ b/server-spi-private/src/main/java/org/keycloak/protocol/AbstractLoginProtocolFactory.java @@ -46,8 +46,8 @@ public abstract class AbstractLoginProtocolFactory implements LoginProtocolFacto factory.register(new ProviderEventListener() { @Override public void onEvent(ProviderEvent event) { - if (event instanceof ClientModel.ClientCreationEvent) { - ClientModel client = ((ClientModel.ClientCreationEvent)event).getCreatedClient(); + if (event instanceof ClientModel.ClientProtocolUpdatedEvent) { + ClientModel client = ((ClientModel.ClientProtocolUpdatedEvent)event).getClient(); addDefaultClientScopes(client.getRealm(), client); addDefaults(client); } diff --git a/server-spi/src/main/java/org/keycloak/models/ClientModel.java b/server-spi/src/main/java/org/keycloak/models/ClientModel.java index cdd3e1f051..3571bae196 100755 --- a/server-spi/src/main/java/org/keycloak/models/ClientModel.java +++ b/server-spi/src/main/java/org/keycloak/models/ClientModel.java @@ -58,6 +58,10 @@ public interface ClientModel extends ClientScopeModel, RoleContainerModel, Prot KeycloakSession getKeycloakSession(); } + interface ClientProtocolUpdatedEvent extends ProviderEvent { + ClientModel getClient(); + } + /** * Notifies other providers that this client has been updated. *

@@ -221,10 +225,9 @@ public interface ClientModel extends ClientScopeModel, RoleContainerModel, Prot * Return all default scopes (if 'defaultScope' is true) or all optional scopes (if 'defaultScope' is false) linked with this client * * @param defaultScope - * @param filterByProtocol if true, then just client scopes of same protocol like current client will be returned * @return map where key is the name of the clientScope, value is particular clientScope. Returns empty map if no scopes linked (never returns null). */ - Map getClientScopes(boolean defaultScope, boolean filterByProtocol); + Map getClientScopes(boolean defaultScope); /** *

Returns a {@link ClientScopeModel} associated with this client. diff --git a/server-spi/src/main/java/org/keycloak/models/ClientProvider.java b/server-spi/src/main/java/org/keycloak/models/ClientProvider.java index 2eddea5cc3..ee72cfdafe 100644 --- a/server-spi/src/main/java/org/keycloak/models/ClientProvider.java +++ b/server-spi/src/main/java/org/keycloak/models/ClientProvider.java @@ -20,6 +20,7 @@ import org.keycloak.provider.Provider; import org.keycloak.storage.client.ClientLookupProvider; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -146,4 +147,24 @@ public interface ClientProvider extends ClientLookupProvider, Provider { * @param realm Realm. */ void removeClients(RealmModel realm); + + /** + * Assign clientScopes to the client. Add as default scopes (if parameter 'defaultScope' is true) + * or optional scopes (if parameter 'defaultScope' is false) + * + * @param realm Realm. + * @param client Client. + * @param clientScopes to be assigned + * @param defaultScope if true the scopes are assigned as default, or optional in case of false + */ + void addClientScopes(RealmModel realm, ClientModel client, Set clientScopes, boolean defaultScope); + + /** + * Unassign clientScope from the client. + * + * @param realm Realm. + * @param client Client. + * @param clientScope to be unassigned + */ + void removeClientScope(RealmModel realm, ClientModel client, ClientScopeModel clientScope); } diff --git a/server-spi/src/main/java/org/keycloak/storage/client/ClientLookupProvider.java b/server-spi/src/main/java/org/keycloak/storage/client/ClientLookupProvider.java index 046d67c68d..ca66bbd1a9 100644 --- a/server-spi/src/main/java/org/keycloak/storage/client/ClientLookupProvider.java +++ b/server-spi/src/main/java/org/keycloak/storage/client/ClientLookupProvider.java @@ -17,9 +17,11 @@ package org.keycloak.storage.client; import org.keycloak.models.ClientModel; +import org.keycloak.models.ClientScopeModel; import org.keycloak.models.RealmModel; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -92,4 +94,14 @@ public interface ClientLookupProvider { * @return Stream of ClientModel or an empty stream if no client is found. Never returns {@code null}. */ Stream searchClientsByClientIdStream(RealmModel realm, String clientId, Integer firstResult, Integer maxResults); + + /** + * Return all default scopes (if {@code defaultScope} is {@code true}) or all optional scopes (if {@code defaultScope} is {@code false}) linked with the client + * + * @param realm Realm + * @param client Client + * @param defaultScopes if true default scopes, if false optional scopes, are returned + * @return map where key is the name of the clientScope, value is particular clientScope. Returns empty map if no scopes linked (never returns null). + */ + Map getClientScopes(RealmModel realm, ClientModel client, boolean defaultScopes); } diff --git a/services/src/main/java/org/keycloak/forms/account/freemarker/model/ApplicationsBean.java b/services/src/main/java/org/keycloak/forms/account/freemarker/model/ApplicationsBean.java index 67fd692b77..cadd1a830e 100755 --- a/services/src/main/java/org/keycloak/forms/account/freemarker/model/ApplicationsBean.java +++ b/services/src/main/java/org/keycloak/forms/account/freemarker/model/ApplicationsBean.java @@ -183,8 +183,8 @@ public class ApplicationsBean { // Construct scope parameter with all optional scopes to see all potentially available roles Stream allClientScopes = Stream.concat( - client.getClientScopes(true, true).values().stream(), - client.getClientScopes(false, true).values().stream()); + client.getClientScopes(true).values().stream(), + client.getClientScopes(false).values().stream()); allClientScopes = Stream.concat(allClientScopes, Stream.of(client)).distinct(); Set availableRoles = TokenManager.getAccess(user, client, allClientScopes); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java index 0035b35c74..2de154fbbf 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java @@ -548,14 +548,14 @@ public class TokenManager { public static Stream getRequestedClientScopes(String scopeParam, ClientModel client) { // Add all default client scopes automatically and client itself Stream clientScopes = Stream.concat( - client.getClientScopes(true, true).values().stream(), + client.getClientScopes(true).values().stream(), Stream.of(client)).distinct(); if (scopeParam == null) { return clientScopes; } - Map allOptionalScopes = client.getClientScopes(false, true); + Map allOptionalScopes = client.getClientScopes(false); // Add optional client scopes requested by scope parameter return Stream.concat(parseScopeParameter(scopeParam).map(allOptionalScopes::get).filter(Objects::nonNull), clientScopes).distinct(); diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientScopesCondition.java b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientScopesCondition.java index 9f9c263059..db88a6a082 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientScopesCondition.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientScopesCondition.java @@ -72,8 +72,8 @@ public class ClientScopesCondition extends AbstractClientPolicyConditionProvider private boolean isScopeMatched(String explicitScopes, ClientModel client) { if (explicitScopes == null) explicitScopes = ""; Collection explicitSpecifiedScopes = new HashSet<>(Arrays.asList(explicitScopes.split(" "))); - Set defaultScopes = client.getClientScopes(true, true).keySet(); - Set optionalScopes = client.getClientScopes(false, true).keySet(); + Set defaultScopes = client.getClientScopes(true).keySet(); + Set optionalScopes = client.getClientScopes(false).keySet(); Set expectedScopes = getScopesForMatching(); if (expectedScopes == null) expectedScopes = new HashSet<>(); diff --git a/services/src/main/java/org/keycloak/services/clientregistration/policy/impl/ClientScopesClientRegistrationPolicy.java b/services/src/main/java/org/keycloak/services/clientregistration/policy/impl/ClientScopesClientRegistrationPolicy.java index a0fe5a6641..08c751fe94 100644 --- a/services/src/main/java/org/keycloak/services/clientregistration/policy/impl/ClientScopesClientRegistrationPolicy.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/policy/impl/ClientScopesClientRegistrationPolicy.java @@ -73,10 +73,10 @@ public class ClientScopesClientRegistrationPolicy implements ClientRegistrationP // Allow scopes, which were already presented before if (requestedDefaultScopeNames != null) { - requestedDefaultScopeNames.removeAll(clientModel.getClientScopes(true, false).keySet()); + requestedDefaultScopeNames.removeAll(clientModel.getClientScopes(true).keySet()); } if (requestedOptionalScopeNames != null) { - requestedOptionalScopeNames.removeAll(clientModel.getClientScopes(false, false).keySet()); + requestedOptionalScopeNames.removeAll(clientModel.getClientScopes(false).keySet()); } List allowedDefaultScopeNames = getAllowedScopeNames(realm, true); 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 65cb65da3f..0825a433b8 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 @@ -318,7 +318,7 @@ public class ClientResource { private Stream getDefaultClientScopes(boolean defaultScope) { auth.clients().requireView(client); - return client.getClientScopes(defaultScope, true).values().stream().map(ClientResource::toRepresentation); + return client.getClientScopes(defaultScope).values().stream().map(ClientResource::toRepresentation); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java index bc10f4f159..ea408229b2 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java @@ -249,6 +249,7 @@ public class RealmAdminResource { ClientScopeRepresentation rep = new ClientScopeRepresentation(); rep.setId(clientScope.getId()); rep.setName(clientScope.getName()); + rep.setProtocol(clientScope.getProtocol()); return rep; }); } diff --git a/services/src/main/java/org/keycloak/storage/ClientStorageManager.java b/services/src/main/java/org/keycloak/storage/ClientStorageManager.java index aa30dcc606..d6c258cea3 100644 --- a/services/src/main/java/org/keycloak/storage/ClientStorageManager.java +++ b/services/src/main/java/org/keycloak/storage/ClientStorageManager.java @@ -16,6 +16,7 @@ */ package org.keycloak.storage; +import java.util.Map; import org.jboss.logging.Logger; import org.keycloak.common.util.reflections.Types; import org.keycloak.component.ComponentModel; @@ -31,7 +32,9 @@ import org.keycloak.storage.client.ClientStorageProviderModel; import org.keycloak.utils.ServicesUtils; import java.util.Objects; +import java.util.Set; import java.util.stream.Stream; +import org.keycloak.models.ClientScopeModel; /** * @author Bill Burke @@ -160,6 +163,18 @@ public class ClientStorageManager implements ClientProvider { return Stream.concat(local, ext); } + @Override + public Map getClientScopes(RealmModel realm, ClientModel client, boolean defaultScopes) { + StorageId storageId = new StorageId(client.getId()); + if (storageId.getProviderId() == null) { + return session.clientLocalStorage().getClientScopes(realm, client, defaultScopes); + } + ClientLookupProvider provider = (ClientLookupProvider)getStorageProvider(session, client.getRealm(), storageId.getProviderId()); + if (provider == null) return null; + if (!isStorageProviderEnabled(client.getRealm(), storageId.getProviderId())) return null; + return provider.getClientScopes(realm, client, defaultScopes); + } + @Override public ClientModel addClient(RealmModel realm, String clientId) { return session.clientLocalStorage().addClient(realm, clientId); @@ -195,6 +210,22 @@ public class ClientStorageManager implements ClientProvider { session.clientLocalStorage().removeClients(realm); } + @Override + public void addClientScopes(RealmModel realm, ClientModel client, Set clientScopes, boolean defaultScope) { + if (!StorageId.isLocalStorage(client.getId())) { + throw new RuntimeException("Federated clients do not support this operation"); + } + session.clientLocalStorage().addClientScopes(realm, client, clientScopes, defaultScope); + } + + @Override + public void removeClientScope(RealmModel realm, ClientModel client, ClientScopeModel clientScope) { + if (!StorageId.isLocalStorage(client.getId())) { + throw new RuntimeException("Federated clients do not support this operation"); + } + session.clientLocalStorage().removeClientScope(realm, client, clientScope); + } + @Override public void close() { diff --git a/services/src/main/java/org/keycloak/storage/openshift/OpenshiftClientStorageProvider.java b/services/src/main/java/org/keycloak/storage/openshift/OpenshiftClientStorageProvider.java index 43233b94a7..ac93a033d1 100644 --- a/services/src/main/java/org/keycloak/storage/openshift/OpenshiftClientStorageProvider.java +++ b/services/src/main/java/org/keycloak/storage/openshift/OpenshiftClientStorageProvider.java @@ -19,7 +19,10 @@ package org.keycloak.storage.openshift; import com.openshift.restclient.IClient; import com.openshift.restclient.NotFoundException; import com.openshift.restclient.model.IResource; +import java.util.Collections; +import java.util.Map; import org.keycloak.models.ClientModel; +import org.keycloak.models.ClientScopeModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.storage.StorageId; @@ -92,4 +95,10 @@ public class OpenshiftClientStorageProvider implements ClientStorageProvider { return null; } } + + @Override + public Map getClientScopes(RealmModel realm, ClientModel client, boolean defaultScopes) { + // TODO not sure about this, this implementation doesn't use it now + return Collections.EMPTY_MAP; + } } diff --git a/services/src/main/java/org/keycloak/storage/openshift/OpenshiftSAClientAdapter.java b/services/src/main/java/org/keycloak/storage/openshift/OpenshiftSAClientAdapter.java index 7dbc5dc56c..9aaafb45b5 100755 --- a/services/src/main/java/org/keycloak/storage/openshift/OpenshiftSAClientAdapter.java +++ b/services/src/main/java/org/keycloak/storage/openshift/OpenshiftSAClientAdapter.java @@ -275,7 +275,7 @@ public final class OpenshiftSAClientAdapter extends AbstractReadOnlyClientStorag } @Override - public Map getClientScopes(boolean defaultScope, boolean filterByProtocol) { + public Map getClientScopes(boolean defaultScope) { if (defaultScope) { return Collections.emptyMap(); } diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/HardcodedClientStorageProvider.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/HardcodedClientStorageProvider.java index 49ec0a1a8f..a90d4f32fc 100755 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/HardcodedClientStorageProvider.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/HardcodedClientStorageProvider.java @@ -92,6 +92,21 @@ public class HardcodedClientStorageProvider implements ClientStorageProvider, Cl return Stream.empty(); } + @Override + public Map getClientScopes(RealmModel realm, ClientModel client, boolean defaultScope) { + if (defaultScope) { + ClientScopeModel rolesScope = KeycloakModelUtils.getClientScopeByName(realm, OIDCLoginProtocolFactory.ROLES_SCOPE); + ClientScopeModel webOriginsScope = KeycloakModelUtils.getClientScopeByName(realm, OIDCLoginProtocolFactory.WEB_ORIGINS_SCOPE); + return Arrays.asList(rolesScope, webOriginsScope) + .stream() + .collect(Collectors.toMap(ClientScopeModel::getName, clientScope -> clientScope)); + + } else { + ClientScopeModel offlineScope = KeycloakModelUtils.getClientScopeByName(realm, "offline_access"); + return Collections.singletonMap("offline_access", offlineScope); + } + } + public class ClientAdapter extends AbstractReadOnlyClientStorageAdapter { public ClientAdapter(RealmModel realm) { @@ -241,18 +256,8 @@ public class HardcodedClientStorageProvider implements ClientStorageProvider, Cl } @Override - public Map getClientScopes(boolean defaultScope, boolean filterByProtocol) { - if (defaultScope) { - ClientScopeModel rolesScope = KeycloakModelUtils.getClientScopeByName(realm, OIDCLoginProtocolFactory.ROLES_SCOPE); - ClientScopeModel webOriginsScope = KeycloakModelUtils.getClientScopeByName(realm, OIDCLoginProtocolFactory.WEB_ORIGINS_SCOPE); - return Arrays.asList(rolesScope, webOriginsScope) - .stream() - .collect(Collectors.toMap(ClientScopeModel::getName, clientScope -> clientScope)); - - } else { - ClientScopeModel offlineScope = KeycloakModelUtils.getClientScopeByName(realm, "offline_access"); - return Collections.singletonMap("offline_access", offlineScope); - } + public Map getClientScopes(boolean defaultScope) { + return session.clients().getClientScopes(getRealm(), this, defaultScope); } @Override diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java index c0d5eeffb3..cc98637d8d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java @@ -341,6 +341,7 @@ public class ClientScopeTest extends AbstractClientTest { // Add client scope ClientScopeRepresentation scopeRep = new ClientScopeRepresentation(); scopeRep.setName("foo-scope"); + scopeRep.setProtocol("openid-connect"); String scopeId = createClientScope(scopeRep); // Add client with the clientScope diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/KcinitTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/KcinitTest.java index 3e26dff8ae..1b5edaac0c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/KcinitTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/KcinitTest.java @@ -40,6 +40,7 @@ import org.keycloak.models.UserModel; import org.keycloak.models.credential.OTPCredentialModel; import org.keycloak.models.utils.DefaultAuthenticationFlows; import org.keycloak.models.utils.TimeBasedOTP; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.RequiredActionProviderRepresentation; import org.keycloak.representations.idm.authorization.ClientPolicyRepresentation; @@ -112,6 +113,7 @@ public class KcinitTest extends AbstractTestRealmKeycloakTest { kcinit.setEnabled(true); kcinit.addRedirectUri("*"); kcinit.setPublicClient(true); + kcinit.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL); kcinit.removeRole(realm.getRole(OAuth2Constants.OFFLINE_ACCESS)); ClientModel app = realm.addClient(APP); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationPoliciesTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationPoliciesTest.java index c4f2b7d8e2..dd71ce9a38 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationPoliciesTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationPoliciesTest.java @@ -382,12 +382,14 @@ public class ClientRegistrationPoliciesTest extends AbstractClientRegistrationTe // Add some clientScopes ClientScopeRepresentation clientScope = new ClientScopeRepresentation(); clientScope.setName("foo"); + clientScope.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL); Response response = realmResource().clientScopes().create(clientScope); String fooScopeId = ApiUtil.getCreatedId(response); response.close(); clientScope = new ClientScopeRepresentation(); clientScope.setName("bar"); + clientScope.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL); response = realmResource().clientScopes().create(clientScope); String barScopeId = ApiUtil.getCreatedId(response); response.close(); @@ -436,6 +438,7 @@ public class ClientRegistrationPoliciesTest extends AbstractClientRegistrationTe // Add some clientScope through Admin REST ClientScopeRepresentation clientScope = new ClientScopeRepresentation(); clientScope.setName("foo"); + clientScope.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL); Response response = realmResource().clientScopes().create(clientScope); String clientScopeId = ApiUtil.getCreatedId(response); response.close(); @@ -475,6 +478,7 @@ public class ClientRegistrationPoliciesTest extends AbstractClientRegistrationTe // Add some clientScope through Admin REST ClientScopeRepresentation clientScope = new ClientScopeRepresentation(); clientScope.setName("foo"); + clientScope.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL); Response response = realmResource().clientScopes().create(clientScope); String clientScopeId = ApiUtil.getCreatedId(response); response.close(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java index 3d2b56f126..7a0e54750e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java @@ -55,6 +55,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; import static org.keycloak.services.clientregistration.ErrorCodes.INVALID_CLIENT_METADATA; import static org.keycloak.services.clientregistration.ErrorCodes.INVALID_REDIRECT_URI; @@ -558,10 +559,12 @@ public class ClientRegistrationTest extends AbstractClientRegistrationTest { @Test public void registerClientAsAdminWithoutScope() throws ClientRegistrationException { - Set realmDefaultClientScopes = new HashSet<>(adminClient.realm(REALM_NAME).getDefaultDefaultClientScopes() - .stream().map(i->i.getName()).collect(Collectors.toList())); - Set realmOptionalClientScopes = new HashSet<>(adminClient.realm(REALM_NAME).getDefaultOptionalClientScopes() - .stream().map(i->i.getName()).collect(Collectors.toList())); + Set realmDefaultClientScopes = new HashSet<>(adminClient.realm(REALM_NAME).getDefaultDefaultClientScopes().stream() + .filter(scope -> Objects.equals(scope.getProtocol(), OIDCLoginProtocol.LOGIN_PROTOCOL)) + .map(i->i.getName()).collect(Collectors.toList())); + Set realmOptionalClientScopes = new HashSet<>(adminClient.realm(REALM_NAME).getDefaultOptionalClientScopes().stream() + .filter(scope -> Objects.equals(scope.getProtocol(), OIDCLoginProtocol.LOGIN_PROTOCOL)) + .map(i->i.getName()).collect(Collectors.toList())); authManageClients(); ClientRepresentation client = new ClientRepresentation(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java index f2378386e4..3b3513f56b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java @@ -520,7 +520,9 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest { @Test public void testClientWithoutScope() throws ClientRegistrationException { Set realmOptionalClientScopes = new HashSet<>(adminClient.realm(REALM_NAME).getDefaultOptionalClientScopes() - .stream().map(i->i.getName()).collect(Collectors.toList())); + .stream() + .filter(scope -> Objects.equals(scope.getProtocol(), OIDCLoginProtocol.LOGIN_PROTOCOL)) + .map(i->i.getName()).collect(Collectors.toList())); OIDCClientRepresentation clientRep = null; OIDCClientRepresentation response = null; @@ -535,7 +537,9 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest { ClientRepresentation rep = clientResource.toRepresentation(); Set realmDefaultClientScopes = new HashSet<>(adminClient.realm(REALM_NAME).getDefaultDefaultClientScopes() - .stream().map(i->i.getName()).collect(Collectors.toList())); + .stream() + .filter(scope -> Objects.equals(scope.getProtocol(), OIDCLoginProtocol.LOGIN_PROTOCOL)) + .map(i->i.getName()).collect(Collectors.toList())); Set registeredDefaultClientScopes = new HashSet<>(rep.getDefaultClientScopes()); assertTrue(realmDefaultClientScopes.equals(new HashSet<>(registeredDefaultClientScopes))); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/ClientModelTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/ClientModelTest.java index 84e9b5fa77..4a913dab91 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/ClientModelTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/ClientModelTest.java @@ -371,12 +371,12 @@ public class ClientModelTest extends AbstractKeycloakTest { ClientScopeModel scope1 = scope1Atomic.get(); ClientScopeModel scope2 = scope2Atomic.get(); - Map clientScopes1 = client.getClientScopes(true, true); + Map clientScopes1 = client.getClientScopes(true); assertThat("Client Scope contains 'scope1':", clientScopes1.containsKey("scope1"), is(true)); assertThat("Client Scope contains 'scope2':", clientScopes1.containsKey("scope2"), is(false)); assertThat("Client Scope contains 'scope3':", clientScopes1.containsKey("scope3"), is(false)); - Map clientScopes2 = client.getClientScopes(false, true); + Map clientScopes2 = client.getClientScopes(false); assertThat("Client Scope contains 'scope1':", clientScopes2.containsKey("scope1"), is(false)); assertThat("Client Scope contains 'scope2':", clientScopes2.containsKey("scope2"), is(true)); assertThat("Client Scope contains 'scope3':", clientScopes2.containsKey("scope3"), is(true)); @@ -392,12 +392,12 @@ public class ClientModelTest extends AbstractKeycloakTest { client = realm.getClientByClientId("templatized"); ClientScopeModel scope3 = scope3Atomic.get(); - Map clientScopes1 = client.getClientScopes(true, true); + Map clientScopes1 = client.getClientScopes(true); assertThat("Client Scope contains 'scope1':", clientScopes1.containsKey("scope1"), is(false)); assertThat("Client Scope contains 'scope2':", clientScopes1.containsKey("scope2"), is(false)); assertThat("Client Scope contains 'scope3':", clientScopes1.containsKey("scope3"), is(false)); - Map clientScopes2 = client.getClientScopes(false, true); + Map clientScopes2 = client.getClientScopes(false); assertThat("Client Scope contains 'scope1':", clientScopes2.containsKey("scope1"), is(false)); assertThat("Client Scope contains 'scope2':", clientScopes2.containsKey("scope2"), is(false)); assertThat("Client Scope contains 'scope3':", clientScopes2.containsKey("scope3"), is(true)); @@ -420,6 +420,7 @@ public class ClientModelTest extends AbstractKeycloakTest { RealmModel realm = currentSession.realms().getRealmByName(realmName); client = realm.addClient("templatized"); ClientScopeModel scope1 = realm.addClientScope("template"); + scope1.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL); scope1Atomic.set(scope1); client.addClientScope(scope1, true); }); @@ -505,13 +506,13 @@ public class ClientModelTest extends AbstractKeycloakTest { ClientScopeModel scope2 = scope2Atomic.get(); - Map clientScopes1 = client.getClientScopes(true, true); + Map clientScopes1 = client.getClientScopes(true); assertThat("Client Scope contains 'scope1':", clientScopes1.containsKey("scope1"), is(true)); assertThat("Client Scope contains 'scope2':", clientScopes1.containsKey("scope2"), is(false)); assertThat("Client Scope contains 'scope3':", clientScopes1.containsKey("scope3"), is(false)); - Map clientScopes2 = client.getClientScopes(false, true); + Map clientScopes2 = client.getClientScopes(false); assertThat("Client Scope contains 'scope1':", clientScopes2.containsKey("scope1"), is(false)); assertThat("Client Scope contains 'scope2':", clientScopes2.containsKey("scope2"), is(true)); assertThat("Client Scope contains 'scope3':", clientScopes2.containsKey("scope3"), is(true)); @@ -534,12 +535,12 @@ public class ClientModelTest extends AbstractKeycloakTest { RealmModel realm = currentSession.realms().getRealmByName(realmName); client = realm.getClientByClientId("foo2"); - Map clientScopes1 = client.getClientScopes(true, true); + Map clientScopes1 = client.getClientScopes(true); assertThat("Client Scope contains 'scope1':", clientScopes1.containsKey("scope1"), is(false)); assertThat("Client Scope contains 'scope2':", clientScopes1.containsKey("scope2"), is(false)); assertThat("Client Scope contains 'scope3':", clientScopes1.containsKey("scope3"), is(false)); - Map clientScopes2 = client.getClientScopes(false, true); + Map clientScopes2 = client.getClientScopes(false); assertThat("Client Scope contains 'scope1':", clientScopes2.containsKey("scope1"), is(false)); assertThat("Client Scope contains 'scope2':", clientScopes2.containsKey("scope2"), is(false)); assertThat("Client Scope contains 'scope3':", clientScopes2.containsKey("scope3"), is(true));