From 3f014c72994fdf48cfc4446e09a1f1d019021dd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9da=20Housni=20Alaoui?= Date: Mon, 13 Nov 2023 19:13:01 +0100 Subject: [PATCH] Cannot display 'Authentication Flows' screen when a realm contains more than ~4000 clients (#21058) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #21010 Signed-off-by: Réda Housni Alaoui --- .../models/cache/infinispan/RealmAdapter.java | 5 ++ .../cache/infinispan/RealmCacheSession.java | 5 ++ .../keycloak/models/jpa/JpaRealmProvider.java | 47 +++++++++++++++++++ .../org/keycloak/models/jpa/RealmAdapter.java | 5 ++ .../storage/ClientStorageManager.java | 5 ++ .../ui/rest/model/AuthenticationMapper.java | 13 ++--- .../util/IdentityBrokerStateTestHelpers.java | 6 +++ .../java/org/keycloak/models/ClientModel.java | 2 + .../java/org/keycloak/models/RealmModel.java | 2 + .../storage/client/ClientLookupProvider.java | 16 +++++-- .../HardcodedClientStorageProvider.java | 5 ++ 11 files changed, 102 insertions(+), 9 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java index ee082565d7..8095236847 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java @@ -799,6 +799,11 @@ public class RealmAdapter implements CachedRealmModel { return cacheSession.searchClientsByAttributes(this, attributes, firstResult, maxResults); } + @Override + public Stream searchClientByAuthenticationFlowBindingOverrides(Map overrides, Integer firstResult, Integer maxResults) { + return cacheSession.searchClientsByAuthenticationFlowBindingOverrides(this, overrides, firstResult, maxResults); + } + @Override public Stream getClientsStream(Integer firstResult, Integer maxResults) { return cacheSession.getClientsStream(this, firstResult, maxResults); 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 49ae627a33..c278713923 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 @@ -1212,6 +1212,11 @@ public class RealmCacheSession implements CacheRealmProvider { return getClientDelegate().searchClientsByAttributes(realm, attributes, firstResult, maxResults); } + @Override + public Stream searchClientsByAuthenticationFlowBindingOverrides(RealmModel realm, Map overrides, Integer firstResult, Integer maxResults) { + return getClientDelegate().searchClientsByAuthenticationFlowBindingOverrides(realm, overrides, firstResult, maxResults); + } + @Override public ClientModel getClientByClientId(RealmModel realm, String clientId) { String cacheKey = getClientByClientIdCacheKey(clientId, realm.getId()); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java index 936d9e5852..09a57ffd3f 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java @@ -28,6 +28,8 @@ import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaDelete; import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.Join; +import jakarta.persistence.criteria.JoinType; +import jakarta.persistence.criteria.MapJoin; import jakarta.persistence.criteria.Predicate; import jakarta.persistence.criteria.Root; import java.util.ArrayList; @@ -838,6 +840,51 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc .map(id -> session.clients().getClientById(realm, id)); } + @Override + public Stream searchClientsByAuthenticationFlowBindingOverrides(RealmModel realm, Map overrides, Integer firstResult, Integer maxResults) { + CriteriaBuilder builder = em.getCriteriaBuilder(); + CriteriaQuery queryBuilder = builder.createQuery(String.class); + Root root = queryBuilder.from(ClientEntity.class); + queryBuilder.select(root.get("id")); + + List predicates = new ArrayList<>(); + + predicates.add(builder.equal(root.get("realmId"), realm.getId())); + + //noinspection resource + String dbProductName = em.unwrap(Session.class).doReturningWork(connection -> connection.getMetaData().getDatabaseProductName()); + + for (Map.Entry entry : overrides.entrySet()) { + String bindingName = entry.getKey(); + String authenticationFlowId = entry.getValue(); + + MapJoin authFlowBindings = root.joinMap("authFlowBindings", JoinType.LEFT); + + Predicate attrNamePredicate = builder.equal(authFlowBindings.key(), bindingName); + + Predicate attrValuePredicate; + if (dbProductName.equals("Oracle")) { + // SELECT * FROM client_attributes WHERE ... DBMS_LOB.COMPARE(value, '0') = 0 ...; + // Oracle is not able to compare a CLOB with a VARCHAR unless it being converted with TO_CHAR + // But for this all values in the table need to be smaller than 4K, otherwise the cast will fail with + // "ORA-22835: Buffer too small for CLOB to CHAR" (even if it is in another row). + // This leaves DBMS_LOB.COMPARE as the option to compare the CLOB with the value. + attrValuePredicate = builder.equal(builder.function("DBMS_LOB.COMPARE", Integer.class, authFlowBindings.value(), builder.literal(authenticationFlowId)), 0); + } else { + attrValuePredicate = builder.equal(authFlowBindings.value(), authenticationFlowId); + } + + predicates.add(builder.and(attrNamePredicate, attrValuePredicate)); + } + + Predicate finalPredicate = builder.and(predicates.toArray(new Predicate[0])); + queryBuilder.where(finalPredicate).orderBy(builder.asc(root.get("clientId"))); + + TypedQuery query = em.createQuery(queryBuilder); + return closing(paginateQuery(query, firstResult, maxResults).getResultStream()) + .map(id -> session.clients().getClientById(realm, id)); + } + @Override public void removeClients(RealmModel realm) { TypedQuery query = em.createNamedQuery("getClientIdsByRealm", String.class); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java index 174290a405..655f3d601d 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java @@ -775,6 +775,11 @@ public class RealmAdapter implements LegacyRealmModel, JpaModel { return session.clients().searchClientsByAttributes(this, attributes, firstResult, maxResults); } + @Override + public Stream searchClientByAuthenticationFlowBindingOverrides(Map overrides, Integer firstResult, Integer maxResults) { + return session.clients().searchClientsByAuthenticationFlowBindingOverrides(this, overrides, firstResult, maxResults); + } + private static final String BROWSER_HEADER_PREFIX = "_browser_header."; @Override diff --git a/model/legacy-private/src/main/java/org/keycloak/storage/ClientStorageManager.java b/model/legacy-private/src/main/java/org/keycloak/storage/ClientStorageManager.java index f3f6591292..9b7131cb1d 100644 --- a/model/legacy-private/src/main/java/org/keycloak/storage/ClientStorageManager.java +++ b/model/legacy-private/src/main/java/org/keycloak/storage/ClientStorageManager.java @@ -167,6 +167,11 @@ public class ClientStorageManager implements ClientProvider { return query((p, f, m) -> p.searchClientsByAttributes(realm, attributes, f, m), realm, firstResult, maxResults); } + @Override + public Stream searchClientsByAuthenticationFlowBindingOverrides(RealmModel realm, Map overrides, Integer firstResult, Integer maxResults) { + return query((p, f, m) -> p.searchClientsByAuthenticationFlowBindingOverrides(realm, overrides, f, m), realm, firstResult, maxResults); + } + @FunctionalInterface interface PaginatedQuery { Stream query(ClientLookupProvider provider, Integer firstResult, Integer maxResults); diff --git a/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/model/AuthenticationMapper.java b/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/model/AuthenticationMapper.java index 1e4d52533f..5040f862c6 100644 --- a/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/model/AuthenticationMapper.java +++ b/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/model/AuthenticationMapper.java @@ -1,5 +1,6 @@ package org.keycloak.admin.ui.rest.model; +import java.util.Collections; import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -15,7 +16,6 @@ public class AuthenticationMapper { public static Authentication convertToModel(AuthenticationFlowModel flow, RealmModel realm) { final Stream identityProviders = realm.getIdentityProvidersStream(); - final Stream clients = realm.getClientsStream(); final Authentication authentication = new Authentication(); authentication.setId(flow.getId()); @@ -30,11 +30,12 @@ public class AuthenticationMapper { authentication.setUsedBy(new UsedBy(UsedBy.UsedByType.SPECIFIC_PROVIDERS, usedByIdp)); } - final List usedClients = clients.filter( - c -> c.getAuthenticationFlowBindingOverrides().get("browser") != null && c.getAuthenticationFlowBindingOverrides() - .get("browser").equals(flow.getId()) || c.getAuthenticationFlowBindingOverrides() - .get("direct_grant") != null && c.getAuthenticationFlowBindingOverrides().get("direct_grant").equals(flow.getId())) - .map(ClientModel::getClientId).limit(MAX_USED_BY).collect(Collectors.toList()); + + Stream browserFlowOverridingClients = realm.searchClientByAuthenticationFlowBindingOverrides(Collections.singletonMap("browser", flow.getId()), 0, MAX_USED_BY); + Stream directGrantFlowOverridingClients = realm.searchClientByAuthenticationFlowBindingOverrides(Collections.singletonMap("direct_grant", flow.getId()), 0, MAX_USED_BY); + final List usedClients = Stream.concat(browserFlowOverridingClients, directGrantFlowOverridingClients) + .limit(MAX_USED_BY) + .map(ClientModel::getClientId).collect(Collectors.toList()); if (!usedClients.isEmpty()) { authentication.setUsedBy(new UsedBy(UsedBy.UsedByType.SPECIFIC_CLIENTS, usedClients)); diff --git a/server-spi-private/src/test/java/org/keycloak/broker/provider/util/IdentityBrokerStateTestHelpers.java b/server-spi-private/src/test/java/org/keycloak/broker/provider/util/IdentityBrokerStateTestHelpers.java index acea768b6e..982c87d8e5 100644 --- a/server-spi-private/src/test/java/org/keycloak/broker/provider/util/IdentityBrokerStateTestHelpers.java +++ b/server-spi-private/src/test/java/org/keycloak/broker/provider/util/IdentityBrokerStateTestHelpers.java @@ -1143,6 +1143,12 @@ public class IdentityBrokerStateTestHelpers { return null; } + @Override + public Stream searchClientByAuthenticationFlowBindingOverrides(Map overrides, Integer firstResult, Integer maxResults) { + return null; + } + + @Override public void updateRequiredCredentials(Set creds) { 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 da3cb8dbc4..b4c9a19cde 100755 --- a/server-spi/src/main/java/org/keycloak/models/ClientModel.java +++ b/server-spi/src/main/java/org/keycloak/models/ClientModel.java @@ -54,6 +54,8 @@ public interface ClientModel extends ClientScopeModel, RoleContainerModel, Prot * is always checked for equality, and the value is checked per the operator. */ public static final SearchableModelField ATTRIBUTE = new SearchableModelField<>("attribute", String[].class); + + public static final SearchableModelField AUTHENTICATION_FLOW_BINDING_OVERRIDE = new SearchableModelField<>("authenticationFlowBindingOverrides", String[].class); } interface ClientCreationEvent extends ProviderEvent { diff --git a/server-spi/src/main/java/org/keycloak/models/RealmModel.java b/server-spi/src/main/java/org/keycloak/models/RealmModel.java index b57c4313d1..9abd8964c8 100755 --- a/server-spi/src/main/java/org/keycloak/models/RealmModel.java +++ b/server-spi/src/main/java/org/keycloak/models/RealmModel.java @@ -358,6 +358,8 @@ public interface RealmModel extends RoleContainerModel { Stream searchClientByAttributes(Map attributes, Integer firstResult, Integer maxResults); + Stream searchClientByAuthenticationFlowBindingOverrides(Map overrides, Integer firstResult, Integer maxResults); + void updateRequiredCredentials(Set creds); Map getBrowserSecurityHeaders(); 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 ace8a202a9..217cafa547 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 @@ -20,9 +20,7 @@ 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; /** @@ -32,7 +30,7 @@ import java.util.stream.Stream; * @version $Revision: 1 $ */ public interface ClientLookupProvider { - + /** * Exact search for a client by its internal ID. * @param realm Realm to limit the search. @@ -63,6 +61,18 @@ public interface ClientLookupProvider { Stream searchClientsByAttributes(RealmModel realm, Map attributes, Integer firstResult, Integer maxResults); + default Stream searchClientsByAuthenticationFlowBindingOverrides(RealmModel realm, Map overrides, Integer firstResult, Integer maxResults) { + Stream clients = searchClientsByAttributes(realm, Map.of(), null, null) + .filter(client -> overrides.entrySet().stream().allMatch(override -> override.getValue().equals(client.getAuthenticationFlowBindingOverrides().get(override.getKey())))); + if (firstResult != null && firstResult >= 0) { + clients = clients.skip(firstResult); + } + if (maxResults != null && maxResults >= 0 ) { + clients = clients.limit(maxResults); + } + return clients; + } + /** * Return all default scopes (if {@code defaultScope} is {@code true}) or all optional scopes (if {@code defaultScope} is {@code false}) linked with the client * 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 723c38970b..25e4d73af0 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 @@ -97,6 +97,11 @@ public class HardcodedClientStorageProvider implements ClientStorageProvider, Cl return Stream.empty(); } + @Override + public Stream searchClientsByAuthenticationFlowBindingOverrides(RealmModel realm, Map overrides, Integer firstResult, Integer maxResults) { + return Stream.empty(); + } + @Override public Map getClientScopes(RealmModel realm, ClientModel client, boolean defaultScope) { if (defaultScope) {