Only filter default organization related scopes based on dynamic scope format

Closes #42877

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
This commit is contained in:
Pedro Igor 2025-09-25 15:42:09 -03:00
parent b68284f6c1
commit 6e851ce80e
14 changed files with 239 additions and 56 deletions

View File

@ -29,6 +29,7 @@ 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;
@ -39,6 +40,8 @@ import java.util.stream.Stream;
* @version $Revision: 1 $
*/
public class ClientAdapter implements ClientModel, CachedObject {
private final LazyModel<ClientModel> modelSupplier;
protected RealmCacheSession cacheSession;
protected RealmModel cachedRealm;
@ -49,12 +52,13 @@ public class ClientAdapter implements ClientModel, CachedObject {
this.cachedRealm = cachedRealm;
this.cacheSession = cacheSession;
this.cached = cached;
this.modelSupplier = new LazyModel<>(this::getClient);
}
private void getDelegateForUpdate() {
if (updated == null) {
cacheSession.registerClientInvalidation(cached.getId(), cached.getClientId(), cachedRealm.getId());
updated = cacheSession.getClientDelegate().getClientById(cachedRealm, cached.getId());
updated = modelSupplier.get();
if (updated == null) throw new IllegalStateException("Not found in database");
}
}
@ -340,7 +344,7 @@ public class ClientAdapter implements ClientModel, CachedObject {
@Override
public Stream<ProtocolMapperModel> getProtocolMappersStream() {
if (isUpdated()) return updated.getProtocolMappersStream();
return cached.getProtocolMappers().stream();
return cached.getProtocolMappers(cacheSession.session, modelSupplier);
}
@Override
@ -365,18 +369,17 @@ public class ClientAdapter implements ClientModel, CachedObject {
@Override
public ProtocolMapperModel getProtocolMapperById(String id) {
for (ProtocolMapperModel mapping : cached.getProtocolMappers()) {
if (mapping.getId().equals(id)) return mapping;
}
return null;
return cached.getProtocolMapperById(cacheSession.session, modelSupplier, id);
}
@Override
public List<ProtocolMapperModel> getProtocolMapperByType(String type) {
return cached.getProtocolMapperByType(cacheSession.session, modelSupplier, type);
}
@Override
public ProtocolMapperModel getProtocolMapperByName(String protocol, String name) {
for (ProtocolMapperModel mapping : cached.getProtocolMappers()) {
if (mapping.getProtocol().equals(protocol) && mapping.getName().equals(name)) return mapping;
}
return null;
return cached.getProtocolMapperByName(cacheSession.session, modelSupplier, protocol, name);
}
@Override
@ -620,6 +623,10 @@ public class ClientAdapter implements ClientModel, CachedObject {
return RoleUtils.hasRole(getRolesStream(), role);
}
private ClientModel getClient() {
return cacheSession.getClientDelegate().getClientById(cachedRealm, cached.getId());
}
@Override
public boolean equals(Object o) {
if (this == o) return true;

View File

@ -25,7 +25,9 @@ import org.keycloak.models.cache.infinispan.entities.CachedClientScope;
import org.keycloak.models.utils.RoleUtils;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;
import java.util.stream.Stream;
/**
@ -39,16 +41,19 @@ public class ClientScopeAdapter implements ClientScopeModel {
protected ClientScopeModel updated;
protected CachedClientScope cached;
private final Supplier<ClientScopeModel> modelSupplier;
public ClientScopeAdapter(RealmModel cachedRealm, CachedClientScope cached, RealmCacheSession cacheSession) {
this.cachedRealm = cachedRealm;
this.cacheSession = cacheSession;
this.cached = cached;
this.modelSupplier = new LazyModel<>(this::getClientScope);
}
private void getDelegateForUpdate() {
if (updated == null) {
cacheSession.registerClientScopeInvalidation(cached.getId(), cachedRealm.getId());
updated = cacheSession.getClientScopeDelegate().getClientScopeById(cachedRealm, cached.getId());
updated = modelSupplier.get();
if (updated == null) throw new IllegalStateException("Not found in database");
}
}
@ -81,7 +86,7 @@ public class ClientScopeAdapter implements ClientScopeModel {
@Override
public Stream<ProtocolMapperModel> getProtocolMappersStream() {
if (isUpdated()) return updated.getProtocolMappersStream();
return cached.getProtocolMappers().stream();
return cached.getProtocolMappers(cacheSession.session, modelSupplier);
}
@Override
@ -106,18 +111,17 @@ public class ClientScopeAdapter implements ClientScopeModel {
@Override
public ProtocolMapperModel getProtocolMapperById(String id) {
for (ProtocolMapperModel mapping : cached.getProtocolMappers()) {
if (mapping.getId().equals(id)) return mapping;
}
return null;
return cached.getProtocolMapperById(cacheSession.session, modelSupplier, id);
}
@Override
public List<ProtocolMapperModel> getProtocolMapperByType(String type) {
return cached.getProtocolMapperByType(cacheSession.session, modelSupplier, type);
}
@Override
public ProtocolMapperModel getProtocolMapperByName(String protocol, String name) {
for (ProtocolMapperModel mapping : cached.getProtocolMappers()) {
if (mapping.getProtocol().equals(protocol) && mapping.getName().equals(name)) return mapping;
}
return null;
return cached.getProtocolMapperByName(cacheSession.session, modelSupplier, protocol, name);
}
@Override
@ -224,6 +228,9 @@ public class ClientScopeAdapter implements ClientScopeModel {
return copy;
}
private ClientScopeModel getClientScope() {
return cacheSession.getClientScopeDelegate().getClientScopeById(cachedRealm, cached.getId());
}
@Override
public boolean equals(Object o) {

View File

@ -0,0 +1,59 @@
package org.keycloak.models.cache.infinispan.entities;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ProtocolMapperModel;
import org.keycloak.models.cache.infinispan.DefaultLazyLoader;
import org.keycloak.models.cache.infinispan.LazyLoader;
abstract class AbstractCachedClientScope<D extends ClientScopeModel> extends AbstractRevisioned implements InRealm {
private final LazyLoader<D, Map<String, ProtocolMapperModel>> mappersById;
private final LazyLoader<D, Map<String, String>> mappersByName;
private final LazyLoader<D, Map<String, List<String>>> mappersByType;
public AbstractCachedClientScope(Long revision, ClientScopeModel model) {
super(revision, model.getId());
mappersById = new DefaultLazyLoader<>(scope -> scope.getProtocolMappersStream()
.collect(Collectors.toMap(ProtocolMapperModel::getId, ProtocolMapperModel::new)),
Collections::emptyMap);
mappersByName = new DefaultLazyLoader<>(scope -> scope.getProtocolMappersStream()
.collect(Collectors.toMap(mapper -> mapper.getProtocol() + "." + mapper.getName(),
ProtocolMapperModel::getId)),
Collections::emptyMap);
mappersByType = new DefaultLazyLoader<>(scope ->
scope.getProtocolMappersStream()
.collect(Collectors.groupingBy(ProtocolMapperModel::getProtocolMapper,
Collectors.mapping(ProtocolMapperModel::getId, Collectors.toList()))),
Collections::emptyMap);
}
public Stream<ProtocolMapperModel> getProtocolMappers(KeycloakSession session, Supplier<D> model) {
return mappersById.get(session, model).values().stream();
}
public ProtocolMapperModel getProtocolMapperById(KeycloakSession session, Supplier<D> model, String id) {
if (id == null) {
return null;
}
return mappersById.get(session, model).get(id);
}
public List<ProtocolMapperModel> getProtocolMapperByType(KeycloakSession session, Supplier<D> model, String type) {
return mappersByType.get(session, model).getOrDefault(type, List.of()).stream()
.map(id -> getProtocolMapperById(session, model, id))
.collect(Collectors.toList());
}
public ProtocolMapperModel getProtocolMapperByName(KeycloakSession session, Supplier<D> model, String protocol, String name) {
String id = mappersByName.get(session, model).get(protocol + "." + name);
return getProtocolMapperById(session, model, id);
}
}

View File

@ -25,7 +25,6 @@ import java.util.TreeMap;
import java.util.stream.Collectors;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ProtocolMapperModel;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel;
@ -33,7 +32,7 @@ import org.keycloak.models.RoleModel;
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @version $Revision: 1 $
*/
public class CachedClient extends AbstractRevisioned implements InRealm {
public class CachedClient extends AbstractCachedClientScope<ClientModel> {
protected String clientId;
protected String name;
protected String description;
@ -53,7 +52,6 @@ public class CachedClient extends AbstractRevisioned implements InRealm {
protected int notBefore;
protected Set<String> scope = new HashSet<>();
protected Set<String> webOrigins = new HashSet<>();
protected Set<ProtocolMapperModel> protocolMappers = new HashSet<>();
protected boolean surrogateAuthRequired;
protected String managementUrl;
protected String rootUrl;
@ -68,7 +66,7 @@ public class CachedClient extends AbstractRevisioned implements InRealm {
protected Map<String, Integer> registeredNodes;
public CachedClient(Long revision, RealmModel realm, ClientModel model) {
super(revision, model.getId());
super(revision, model);
clientAuthenticatorType = model.getClientAuthenticatorType();
secret = model.getSecret();
registrationToken = model.getRegistrationToken();
@ -88,7 +86,6 @@ public class CachedClient extends AbstractRevisioned implements InRealm {
redirectUris.addAll(model.getRedirectUris());
webOrigins.addAll(model.getWebOrigins());
scope.addAll(model.getScopeMappingsStream().map(RoleModel::getId).collect(Collectors.toSet()));
protocolMappers.addAll(model.getProtocolMappersStream().collect(Collectors.toSet()));
surrogateAuthRequired = model.isSurrogateAuthRequired();
managementUrl = model.getManagementUrl();
rootUrl = model.getRootUrl();
@ -176,10 +173,6 @@ public class CachedClient extends AbstractRevisioned implements InRealm {
return frontchannelLogout;
}
public Set<ProtocolMapperModel> getProtocolMappers() {
return protocolMappers;
}
public boolean isSurrogateAuthRequired() {
return surrogateAuthRequired;
}

View File

@ -18,7 +18,6 @@
package org.keycloak.models.cache.infinispan.entities;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.ProtocolMapperModel;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel;
@ -32,23 +31,21 @@ import java.util.stream.Collectors;
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @version $Revision: 1 $
*/
public class CachedClientScope extends AbstractRevisioned implements InRealm {
public class CachedClientScope extends AbstractCachedClientScope<ClientScopeModel> {
private String name;
private String description;
private String realm;
private String protocol;
private Set<String> scope = new HashSet<>();
private Set<ProtocolMapperModel> protocolMappers = new HashSet<>();
private Map<String, String> attributes = new HashMap<>();
public CachedClientScope(Long revision, RealmModel realm, ClientScopeModel model) {
super(revision, model.getId());
super(revision, model);
name = model.getName();
description = model.getDescription();
this.realm = realm.getId();
protocol = model.getProtocol();
protocolMappers.addAll(model.getProtocolMappersStream().collect(Collectors.toSet()));
scope.addAll(model.getScopeMappingsStream().map(RoleModel::getId).collect(Collectors.toSet()));
attributes.putAll(model.getAttributes());
}
@ -64,9 +61,6 @@ public class CachedClientScope extends AbstractRevisioned implements InRealm {
public String getRealm() {
return realm;
}
public Set<ProtocolMapperModel> getProtocolMappers() {
return protocolMappers;
}
public String getProtocol() {
return protocol;

View File

@ -38,6 +38,7 @@ 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;
@ -469,6 +470,14 @@ public class ClientAdapter implements ClientModel, JpaModel<ClientEntity> {
return entityToModel(entity);
}
@Override
public List<ProtocolMapperModel> getProtocolMapperByType(String type) {
return this.entity.getProtocolMappers().stream()
.filter((mapper) -> mapper.getProtocolMapper().equals(type))
.map(this::entityToModel)
.toList();
}
@Override
public ProtocolMapperModel getProtocolMapperByName(String protocol, String name) {
ProtocolMapperEntity entity = getProtocolMapperEntityByName(protocol, name);

View File

@ -31,8 +31,10 @@ import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.models.utils.RoleUtils;
import jakarta.persistence.EntityManager;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Stream;
@ -192,6 +194,14 @@ public class ClientScopeAdapter implements ClientScopeModel, JpaModel<ClientScop
return entityToModel(entity);
}
@Override
public List<ProtocolMapperModel> getProtocolMapperByType(String type) {
return this.entity.getProtocolMappers().stream()
.filter((mapper) -> mapper.getProtocolMapper().equals(type))
.map(this::entityToModel)
.toList();
}
@Override
public ProtocolMapperModel getProtocolMapperByName(String protocol, String name) {
ProtocolMapperEntity entity = getProtocolMapperEntityByName(protocol, name);

View File

@ -24,6 +24,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;
@ -614,6 +615,11 @@ public class ClientModelLazyDelegate implements ClientModel {
return getDelegate().getProtocolMapperById(id);
}
@Override
public List<ProtocolMapperModel> getProtocolMapperByType(String type) {
return getDelegate().getProtocolMapperByType(type);
}
@Override
public ProtocolMapperModel getProtocolMapperByName(String protocol, String name) {
return getDelegate().getProtocolMapperByName(protocol, name);

View File

@ -452,6 +452,11 @@ public class CredentialScopeModel implements ClientScopeModel {
return clientScope.getProtocolMapperById(id);
}
@Override
public List<ProtocolMapperModel> getProtocolMapperByType(String type) {
return clientScope.getProtocolMapperByType(type);
}
@Override
public ProtocolMapperModel getProtocolMapperByName(String protocol, String name) {
return clientScope.getProtocolMapperByName(protocol, name);

View File

@ -17,6 +17,7 @@
package org.keycloak.models;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;
@ -170,6 +171,11 @@ public class ClientScopeDecorator implements ClientScopeModel {
return delegate.getProtocolMapperById(id);
}
@Override
public List<ProtocolMapperModel> getProtocolMapperByType(String type) {
return delegate.getProtocolMapperByType(type);
}
@Override
public ProtocolMapperModel getProtocolMapperByName(String protocol, String name) {
return delegate.getProtocolMapperByName(protocol, name);

View File

@ -17,6 +17,7 @@
package org.keycloak.models;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@ -41,4 +42,8 @@ public interface ProtocolMapperContainerModel {
ProtocolMapperModel getProtocolMapperById(String id);
ProtocolMapperModel getProtocolMapperByName(String protocol, String name);
default List<ProtocolMapperModel> getProtocolMapperByType(String type) {
return List.of();
}
}

View File

@ -18,6 +18,7 @@
package org.keycloak.models;
import java.io.Serializable;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
@ -37,6 +38,21 @@ public class ProtocolMapperModel implements Serializable {
protected String consentText;
protected Map<String, String> config;
public ProtocolMapperModel() {
}
public ProtocolMapperModel(ProtocolMapperModel model) {
this.id = model.id;
this.name = model.name;
this.protocol = model.protocol;
this.protocolMapper = model.protocolMapper;
this.consentRequired = model.consentRequired;
this.consentText = model.consentText;
if (model.config != null) {
this.config = new HashMap<>(model.config);
}
}
public String getId() {
return id;

View File

@ -62,6 +62,7 @@ import org.keycloak.models.light.LightweightUserAdapter;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.models.utils.SessionExpirationUtils;
import org.keycloak.models.utils.RoleUtils;
import org.keycloak.organization.protocol.mappers.oidc.OrganizationMembershipMapper;
import org.keycloak.organization.protocol.mappers.oidc.OrganizationScope;
import org.keycloak.protocol.ProtocolMapper;
import org.keycloak.protocol.ProtocolMapperUtils;
@ -638,10 +639,13 @@ public class TokenManager {
return clientScopes;
}
// skip scopes that were explicitly requested using the dynamic scope format
// skip organization-related scopes that were explicitly requested using the dynamic scope format
// we don't want dynamic and default client scopes duplicated
// always include the dedicated client scope that maps to the client itself
clientScopes = clientScopes.filter(scope -> scope.equals(client) || !scopeParam.contains(scope.getName() + ClientScopeModel.VALUE_SEPARATOR));
clientScopes = clientScopes.filter(scope -> {
return scope.equals(client)
|| !scopeParam.contains(scope.getName() + ClientScopeModel.VALUE_SEPARATOR)
|| scope.getProtocolMapperByType(OrganizationMembershipMapper.PROVIDER_ID).isEmpty();
});
Map<String, ClientScopeModel> allOptionalScopes = client.getClientScopes(false);

View File

@ -1831,26 +1831,17 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
client.setName("test");
clientResource.update(client);
// creates a client scope using the dynamic scope format and add it to the client as optional scope
ClientScopeRepresentation scopeRep = new ClientScopeRepresentation();
scopeRep.setName("test:create-mapper");
scopeRep.setProtocol("openid-connect");
scopeRep.setProtocolMappers(Collections.singletonList(createHardCodedMapper("from-mapper", "from-mapper", "value")));
try (Response resp1 = realm.clientScopes().create(scopeRep)) {
assertEquals(201, resp1.getStatus());
String clientScopeId = ApiUtil.getCreatedId(resp1);
getCleanup().addClientScopeId(clientScopeId);
clientResource.addOptionalClientScope(clientScopeId);
}
String expectedScopeName = "test:create-mapper";
createClientScope(realm, clientResource, expectedScopeName, "from-mapper", "value", false);
// creates a dedicated mapper to the client
ProtocolMappersResource dedicatedClientMappers = clientResource.getProtocolMappers();
dedicatedClientMappers.createMapper(createHardCodedMapper("from-dedicated-mapper", "from-dedicated-mapper", "value")).close();
// request the test:create-mapper scope so that mappers are included
oauth.scope("openid " + scopeRep.getName());
oauth.scope("openid " + expectedScopeName);
AccessTokenResponse response = oauth.doPasswordGrantRequest("test-user@localhost", "password");
assertTrue(response.getScope().contains(scopeRep.getName()));
assertTrue(response.getScope().contains(expectedScopeName));
IDToken idToken = oauth.verifyIDToken(response.getIdToken());
assertNotNull(idToken.getOtherClaims());
@ -1862,14 +1853,85 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
assertEquals("value", idToken.getOtherClaims().get("from-dedicated-mapper"));
AccessToken accessToken = oauth.verifyToken(response.getAccessToken());
assertTrue(response.getScope().contains(scopeRep.getName()));
assertTrue(accessToken.getScope().contains(expectedScopeName));
assertNotNull(accessToken.getOtherClaims());
assertNotNull(accessToken.getOtherClaims().get("from-mapper"));
// claim mapped by client scope mapper
assertEquals("value", accessToken.getOtherClaims().get("from-mapper"));
assertNotNull(idToken.getOtherClaims().get("from-dedicated-mapper"));
assertNotNull(accessToken.getOtherClaims().get("from-dedicated-mapper"));
// claim mapped by dedicated client mapper
assertEquals("value", idToken.getOtherClaims().get("from-dedicated-mapper"));
assertEquals("value", accessToken.getOtherClaims().get("from-dedicated-mapper"));
}
@Test
public void testStaticScopeUsingDynamicScopeFormatPrefixedWithScopeAsDefaultScope() {
RealmResource realm = adminClient.realm("test");
ClientResource clientResource = findClientResourceByClientId(realm, "test-app");
ClientRepresentation client = clientResource.toRepresentation();
// make sure the name of the client maps to the prefix of the dynamic scope name
client.setName("test");
clientResource.update(client);
// creates a client scope using the dynamic scope format and add it to the client as default scope
createClientScope(realm, clientResource, "test", "from-scope-mapper", "value", true);
createClientScope(realm, clientResource, "test:create", "from-dynamic-scope", "value", true);
oauth.scope("openid test:create");
AccessTokenResponse response = oauth.doPasswordGrantRequest("test-user@localhost", "password");
assertTrue(response.getScope().contains("test:create"));
AccessToken accessToken = oauth.verifyToken(response.getAccessToken());
assertTrue(accessToken.getScope().contains("test:create"));
assertNotNull(accessToken.getOtherClaims());
assertNotNull(accessToken.getOtherClaims().get("from-dynamic-scope"));
assertEquals("value", accessToken.getOtherClaims().get("from-dynamic-scope"));
assertNotNull(accessToken.getOtherClaims().get("from-scope-mapper"));
assertEquals("value", accessToken.getOtherClaims().get("from-scope-mapper"));
}
@Test
public void testStaticScopeUsingDynamicScopeFormatPrefixedWithScopeAsOptionalScope() {
RealmResource realm = adminClient.realm("test");
ClientResource clientResource = findClientResourceByClientId(realm, "test-app");
ClientRepresentation client = clientResource.toRepresentation();
// make sure the name of the client maps to the prefix of the dynamic scope name
client.setName("test");
clientResource.update(client);
// creates a client scope using the dynamic scope format and add it to the client as optional scope
createClientScope(realm, clientResource, "test", "from-scope-mapper", "value", false);
createClientScope(realm, clientResource, "test:create", "from-dynamic-scope", "value", false);
oauth.scope("openid test:create");
AccessTokenResponse response = oauth.doPasswordGrantRequest("test-user@localhost", "password");
assertTrue(response.getScope().contains("test:create"));
AccessToken accessToken = oauth.verifyToken(response.getAccessToken());
assertTrue(accessToken.getScope().contains("test:create"));
assertNotNull(accessToken.getOtherClaims());
assertNotNull(accessToken.getOtherClaims().get("from-dynamic-scope"));
// claim mapped by client scope mapper
assertEquals("value", accessToken.getOtherClaims().get("from-dynamic-scope"));
}
private void createClientScope(RealmResource realm, ClientResource clientResource, String name, String claim, String value, boolean isDefault) {
ClientScopeRepresentation scopeRep = new ClientScopeRepresentation();
scopeRep.setName(name);
scopeRep.setProtocol("openid-connect");
scopeRep.setProtocolMappers(Collections.singletonList(createHardCodedMapper(name + "from-scope-mapper", claim, value)));
try (Response resp1 = realm.clientScopes().create(scopeRep)) {
assertEquals(201, resp1.getStatus());
String clientScopeId = ApiUtil.getCreatedId(resp1);
getCleanup().addClientScopeId(clientScopeId);
if (isDefault) {
clientResource.addDefaultClientScope(clientScopeId);
} else {
clientResource.addOptionalClientScope(clientScopeId);
}
}
}
private void assertRoles(List<String> actualRoleList, String ...expectedRoles){