From d1ab921c50ad1ffe3a8d1970fae3abab2668a619 Mon Sep 17 00:00:00 2001 From: vramik Date: Tue, 18 Apr 2023 19:15:34 +0200 Subject: [PATCH] JpaUserProvider count methods are inconsistent with searchForUser's param filter handling Closes #17581 --- .../keycloak/models/jpa/JpaUserProvider.java | 312 ++++++++---------- .../keycloak/testsuite/admin/UsersTest.java | 137 +++----- 2 files changed, 188 insertions(+), 261 deletions(-) diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java index 3ccd00ac12..7b4c29e844 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java @@ -255,6 +255,7 @@ public class JpaUserProvider implements UserProvider, UserCredentialStore { updateGrantedConsentEntity(consentEntity, consent); } + @Override public boolean revokeConsentForClient(RealmModel realm, String userId, String clientId) { UserConsentEntity consentEntity = getGrantedConsentEntity(userId, clientId, LockModeType.PESSIMISTIC_WRITE); if (consentEntity == null) return false; @@ -523,7 +524,7 @@ public class JpaUserProvider implements UserProvider, UserCredentialStore { query.setParameter("username", username.toLowerCase()); query.setParameter("realmId", realm.getId()); List results = query.getResultList(); - if (results.size() == 0) return null; + if (results.isEmpty()) return null; return new UserAdapter(session, realm, em, results.get(0)); } @@ -666,38 +667,9 @@ public class JpaUserProvider implements UserProvider, UserCredentialStore { Expression count = qb.count(from); userQuery = userQuery.select(count); - List restrictions = new ArrayList<>(); + List restrictions = predicates(params, from); restrictions.add(qb.equal(from.get("realmId"), realm.getId())); - for (Map.Entry entry : params.entrySet()) { - String key = entry.getKey(); - String value = entry.getValue(); - if (key == null || value == null) { - continue; - } - - switch (key) { - case UserModel.USERNAME: - restrictions.add(qb.like(from.get("username"), "%" + value + "%")); - break; - case UserModel.FIRST_NAME: - restrictions.add(qb.like(from.get("firstName"), "%" + value + "%")); - break; - case UserModel.LAST_NAME: - restrictions.add(qb.like(from.get("lastName"), "%" + value + "%")); - break; - case UserModel.EMAIL: - restrictions.add(qb.like(from.get("email"), "%" + value + "%")); - break; - case UserModel.ENABLED: - restrictions.add(qb.equal(from.get("enabled"), Boolean.parseBoolean(value.toLowerCase()))); - break; - case UserModel.EMAIL_VERIFIED: - restrictions.add(qb.equal(from.get("emailVerified"), Boolean.parseBoolean(value.toLowerCase()))); - break; - } - } - userQuery = userQuery.where(restrictions.toArray(new Predicate[0])); TypedQuery query = em.createQuery(userQuery); Long result = query.getSingleResult(); @@ -706,52 +678,25 @@ public class JpaUserProvider implements UserProvider, UserCredentialStore { } @Override + @SuppressWarnings("unchecked") public int getUsersCount(RealmModel realm, Map params, Set groupIds) { if (groupIds == null || groupIds.isEmpty()) { return 0; } - CriteriaBuilder qb = em.getCriteriaBuilder(); - CriteriaQuery userQuery = qb.createQuery(Long.class); - Root from = userQuery.from(UserGroupMembershipEntity.class); - Expression count = qb.count(from.get("user")); - userQuery = userQuery.select(count); + CriteriaBuilder cb = em.getCriteriaBuilder(); - List restrictions = new ArrayList<>(); - restrictions.add(qb.equal(from.get("user").get("realmId"), realm.getId())); - restrictions.add(from.get("groupId").in(groupIds)); + CriteriaQuery countQuery = cb.createQuery(Long.class); + Root root = countQuery.from(UserEntity.class); + countQuery.select(cb.count(root)); - for (Map.Entry entry : params.entrySet()) { - String key = entry.getKey(); - String value = entry.getValue(); - if (key == null || value == null) { - continue; - } + List restrictions = predicates(params, root); + restrictions.add(cb.equal(root.get("realmId"), realm.getId())); + + groupsWithPermissionsSubquery(countQuery, groupIds, root, restrictions); - switch (key) { - case UserModel.USERNAME: - restrictions.add(qb.like(from.get("user").get("username"), "%" + value + "%")); - break; - case UserModel.FIRST_NAME: - restrictions.add(qb.like(from.get("user").get("firstName"), "%" + value + "%")); - break; - case UserModel.LAST_NAME: - restrictions.add(qb.like(from.get("user").get("lastName"), "%" + value + "%")); - break; - case UserModel.EMAIL: - restrictions.add(qb.like(from.get("user").get("email"), "%" + value + "%")); - break; - case UserModel.ENABLED: - restrictions.add(qb.equal(from.get("enabled"), Boolean.parseBoolean(value.toLowerCase()))); - break; - case UserModel.EMAIL_VERIFIED: - restrictions.add(qb.equal(from.get("emailVerified"), Boolean.parseBoolean(value.toLowerCase()))); - break; - } - } - - userQuery = userQuery.where(restrictions.toArray(new Predicate[0])); - TypedQuery query = em.createQuery(userQuery); + countQuery.where(restrictions.toArray(new Predicate[0])); + TypedQuery query = em.createQuery(countQuery); Long result = query.getSingleResult(); return result.intValue(); @@ -783,124 +728,23 @@ public class JpaUserProvider implements UserProvider, UserCredentialStore { } @Override + @SuppressWarnings("unchecked") public Stream searchForUserStream(RealmModel realm, Map attributes, Integer firstResult, Integer maxResults) { CriteriaBuilder builder = em.getCriteriaBuilder(); CriteriaQuery queryBuilder = builder.createQuery(UserEntity.class); Root root = queryBuilder.from(UserEntity.class); - List predicates = new ArrayList<>(); - List attributePredicates = new ArrayList<>(); + List predicates = predicates(attributes, root); predicates.add(builder.equal(root.get("realmId"), realm.getId())); - Join federatedIdentitiesJoin = null; - - for (Map.Entry entry : attributes.entrySet()) { - String key = entry.getKey(); - String value = entry.getValue(); - - if (value == null) { - continue; - } - - switch (key) { - case UserModel.SEARCH: - for (String stringToSearch : value.trim().split("\\s+")) { - predicates.add(builder.or(getSearchOptionPredicateArray(stringToSearch, builder, root))); - } - break; - case FIRST_NAME: - case LAST_NAME: - if (Boolean.valueOf(attributes.getOrDefault(UserModel.EXACT, Boolean.FALSE.toString()))) { - predicates.add(builder.equal(builder.lower(root.get(key)), value.toLowerCase())); - } else { - predicates.add(builder.like(builder.lower(root.get(key)), "%" + value.toLowerCase() + "%")); - } - break; - case USERNAME: - case EMAIL: - if (Boolean.valueOf(attributes.getOrDefault(UserModel.EXACT, Boolean.FALSE.toString()))) { - predicates.add(builder.equal(root.get(key), value.toLowerCase())); - } else { - predicates.add(builder.like(root.get(key), "%" + value.toLowerCase() + "%")); - } - break; - case EMAIL_VERIFIED: - predicates.add(builder.equal(root.get(key), Boolean.parseBoolean(value.toLowerCase()))); - break; - case UserModel.ENABLED: - predicates.add(builder.equal(root.get(key), Boolean.parseBoolean(value))); - break; - case UserModel.IDP_ALIAS: - if (federatedIdentitiesJoin == null) { - federatedIdentitiesJoin = root.join("federatedIdentities"); - } - predicates.add(builder.equal(federatedIdentitiesJoin.get("identityProvider"), value)); - break; - case UserModel.IDP_USER_ID: - if (federatedIdentitiesJoin == null) { - federatedIdentitiesJoin = root.join("federatedIdentities"); - } - predicates.add(builder.equal(federatedIdentitiesJoin.get("userId"), value)); - break; - case UserModel.EXACT: - break; - // All unknown attributes will be assumed as custom attributes - default: - Join attributesJoin = root.join("attributes", JoinType.LEFT); - - attributePredicates.add(builder.and( - builder.equal(builder.lower(attributesJoin.get("name")), key.toLowerCase()), - builder.equal(builder.lower(attributesJoin.get("value")), value.toLowerCase()))); - - break; - case UserModel.INCLUDE_SERVICE_ACCOUNT: { - if (!attributes.containsKey(UserModel.INCLUDE_SERVICE_ACCOUNT) - || !Boolean.parseBoolean(attributes.get(UserModel.INCLUDE_SERVICE_ACCOUNT))) { - predicates.add(root.get("serviceAccountClientLink").isNull()); - } - break; - } - } - } - - if (!attributePredicates.isEmpty()) { - predicates.add(builder.and(attributePredicates.toArray(new Predicate[0]))); - } - Set userGroups = (Set) session.getAttribute(UserModel.GROUPS); if (userGroups != null) { - Subquery subquery = queryBuilder.subquery(String.class); - Root from = subquery.from(UserGroupMembershipEntity.class); - - subquery.select(builder.literal(1)); - - List subPredicates = new ArrayList<>(); - - subPredicates.add(from.get("groupId").in(userGroups)); - subPredicates.add(builder.equal(from.get("user").get("id"), root.get("id"))); - - Subquery subquery1 = queryBuilder.subquery(String.class); - - subquery1.select(builder.literal(1)); - Root from1 = subquery1.from(ResourceEntity.class); - - List subs = new ArrayList<>(); - - Expression groupId = from.get("groupId"); - subs.add(builder.like(from1.get("name"), builder.concat("group.resource.", groupId))); - - subquery1.where(subs.toArray(new Predicate[subs.size()])); - - subPredicates.add(builder.exists(subquery1)); - - subquery.where(subPredicates.toArray(new Predicate[subPredicates.size()])); - - predicates.add(builder.exists(subquery)); + groupsWithPermissionsSubquery(queryBuilder, userGroups, root, predicates); } - queryBuilder.where(predicates.toArray(new Predicate[predicates.size()])).orderBy(builder.asc(root.get(UserModel.USERNAME))); + queryBuilder.where(predicates.toArray(new Predicate[0])).orderBy(builder.asc(root.get(UserModel.USERNAME))); TypedQuery query = em.createQuery(queryBuilder); @@ -927,7 +771,7 @@ public class JpaUserProvider implements UserProvider, UserCredentialStore { query.setParameter("identityProvider", identityProvider); query.setLockMode(lockMode); List results = query.getResultList(); - return results.size() > 0 ? results.get(0) : null; + return !results.isEmpty() ? results.get(0) : null; } @@ -1090,4 +934,122 @@ public class JpaUserProvider implements UserProvider, UserCredentialStore { UserEntity user = em.getReference(UserEntity.class, id); return em.contains(user) ? user : null; } + + private List predicates(Map attributes, Root root) { + CriteriaBuilder builder = em.getCriteriaBuilder(); + + List predicates = new ArrayList<>(); + List attributePredicates = new ArrayList<>(); + + Join federatedIdentitiesJoin = null; + + for (Map.Entry entry : attributes.entrySet()) { + String key = entry.getKey(); + String value = entry.getValue(); + + if (value == null) { + continue; + } + + switch (key) { + case UserModel.SEARCH: + for (String stringToSearch : value.trim().split("\\s+")) { + predicates.add(builder.or(getSearchOptionPredicateArray(stringToSearch, builder, root))); + } + break; + case FIRST_NAME: + case LAST_NAME: + if (Boolean.parseBoolean(attributes.get(UserModel.EXACT))) { + predicates.add(builder.equal(builder.lower(root.get(key)), value.toLowerCase())); + } else { + predicates.add(builder.like(builder.lower(root.get(key)), "%" + value.toLowerCase() + "%")); + } + break; + case USERNAME: + case EMAIL: + if (Boolean.parseBoolean(attributes.get(UserModel.EXACT))) { + predicates.add(builder.equal(root.get(key), value.toLowerCase())); + } else { + predicates.add(builder.like(root.get(key), "%" + value.toLowerCase() + "%")); + } + break; + case EMAIL_VERIFIED: + predicates.add(builder.equal(root.get(key), Boolean.valueOf(value.toLowerCase()))); + break; + case UserModel.ENABLED: + predicates.add(builder.equal(root.get(key), Boolean.valueOf(value))); + break; + case UserModel.IDP_ALIAS: + if (federatedIdentitiesJoin == null) { + federatedIdentitiesJoin = root.join("federatedIdentities"); + } + predicates.add(builder.equal(federatedIdentitiesJoin.get("identityProvider"), value)); + break; + case UserModel.IDP_USER_ID: + if (federatedIdentitiesJoin == null) { + federatedIdentitiesJoin = root.join("federatedIdentities"); + } + predicates.add(builder.equal(federatedIdentitiesJoin.get("userId"), value)); + break; + case UserModel.EXACT: + break; + // All unknown attributes will be assumed as custom attributes + default: + Join attributesJoin = root.join("attributes", JoinType.LEFT); + + attributePredicates.add(builder.and( + builder.equal(builder.lower(attributesJoin.get("name")), key.toLowerCase()), + builder.equal(builder.lower(attributesJoin.get("value")), value.toLowerCase()))); + + break; + case UserModel.INCLUDE_SERVICE_ACCOUNT: { + if (!attributes.containsKey(UserModel.INCLUDE_SERVICE_ACCOUNT) + || !Boolean.parseBoolean(attributes.get(UserModel.INCLUDE_SERVICE_ACCOUNT))) { + predicates.add(root.get("serviceAccountClientLink").isNull()); + } + break; + } + } + } + + if (!attributePredicates.isEmpty()) { + predicates.add(builder.and(attributePredicates.toArray(new Predicate[0]))); + } + + return predicates; + } + + @SuppressWarnings("unchecked") + private void groupsWithPermissionsSubquery(CriteriaQuery query, Set groupIds, Root root, List restrictions) { + CriteriaBuilder cb = em.getCriteriaBuilder(); + + Subquery subquery = query.subquery(String.class); + + Root from = subquery.from(UserGroupMembershipEntity.class); + + subquery.select(cb.literal(1)); + + List subPredicates = new ArrayList<>(); + + subPredicates.add(from.get("groupId").in(groupIds)); + subPredicates.add(cb.equal(from.get("user").get("id"), root.get("id"))); + + Subquery subquery1 = query.subquery(String.class); + + subquery1.select(cb.literal(1)); + Root from1 = subquery1.from(ResourceEntity.class); + + List subs = new ArrayList<>(); + + Expression groupId = from.get("groupId"); + subs.add(cb.like(from1.get("name"), cb.concat("group.resource.", groupId))); + + subquery1.where(subs.toArray(new Predicate[0])); + + subPredicates.add(cb.exists(subquery1)); + + subquery.where(subPredicates.toArray(new Predicate[0])); + + restrictions.add(cb.exists(subquery)); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UsersTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UsersTest.java index 226ebdb33c..84e34b719f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UsersTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UsersTest.java @@ -56,6 +56,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.not; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; public class UsersTest extends AbstractAdminTest { @@ -185,76 +186,39 @@ public class UsersTest extends AbstractAdminTest { createUser(REALM_NAME, "user3", "password", "user3FirstName", "user3LastName", "user3@example.com", rep -> rep.setEmailVerified(true)); // Prefix search count - Integer count = realm.users().count("user"); - assertThat(count, is(3)); - - count = realm.users().count("user*"); - assertThat(count, is(3)); - - count = realm.users().count("er"); - assertThat(count, is(0)); - - count = realm.users().count(""); - assertThat(count, is(3)); - - count = realm.users().count("*"); - assertThat(count, is(3)); - - count = realm.users().count("user2FirstName"); - assertThat(count, is(1)); - - count = realm.users().count("user2First"); - assertThat(count, is(1)); - - count = realm.users().count("user2First*"); - assertThat(count, is(1)); - - count = realm.users().count("user1@example"); - assertThat(count, is(1)); - - count = realm.users().count("user1@example*"); - assertThat(count, is(1)); - - count = realm.users().count(null); - assertThat(count, is(3)); + assertSearchMatchesCount(realm, "user", 3); + assertSearchMatchesCount(realm, "user*", 3); + assertSearchMatchesCount(realm, "er", 0); + assertSearchMatchesCount(realm, "", 3); + assertSearchMatchesCount(realm, "*", 3); + assertSearchMatchesCount(realm, "user2FirstName", 1); + assertSearchMatchesCount(realm, "user2First", 1); + assertSearchMatchesCount(realm, "user2First*", 1); + assertSearchMatchesCount(realm, "user1@example", 1); + assertSearchMatchesCount(realm, "user1@example*", 1); + assertSearchMatchesCount(realm, null, 3); // Infix search count - count = realm.users().count("*user*"); - assertThat(count, is(3)); - - count = realm.users().count("**"); - assertThat(count, is(3)); - - count = realm.users().count("*foobar*"); - assertThat(count, is(0)); - - count = realm.users().count("*LastName*"); - assertThat(count, is(3)); - - count = realm.users().count("*FirstName*"); - assertThat(count, is(3)); - - count = realm.users().count("*@example.com*"); - assertThat(count, is(3)); - + assertSearchMatchesCount(realm, "*user*", 3); + assertSearchMatchesCount(realm, "**", 3); + assertSearchMatchesCount(realm, "*foobar*", 0); + assertSearchMatchesCount(realm, "*LastName*", 3); + assertSearchMatchesCount(realm, "*FirstName*", 3); + assertSearchMatchesCount(realm, "*@example.com*", 3); + // Exact search count - count = realm.users().count("\"user1\""); - assertThat(count, is(1)); + assertSearchMatchesCount(realm, "\"user1\"", 1); + assertSearchMatchesCount(realm, "\"1\"", 0); + assertSearchMatchesCount(realm, "\"\"", 0); + assertSearchMatchesCount(realm, "\"user1FirstName\"", 1); + assertSearchMatchesCount(realm, "\"user1LastName\"", 1); + assertSearchMatchesCount(realm, "\"user1@example.com\"", 1); + } - count = realm.users().count("\"1\""); - assertThat(count, is(0)); - - count = realm.users().count("\"\""); - assertThat(count, is(0)); - - count = realm.users().count("\"user1FirstName\""); - assertThat(count, is(1)); - - count = realm.users().count("\"user1LastName\""); - assertThat(count, is(1)); - - count = realm.users().count("\"user1@example.com\""); - assertThat(count, is(1)); + private void assertSearchMatchesCount(RealmResource realm, String search, Integer expectedCount) { + Integer count = realm.users().count(search); + assertThat(count, is(expectedCount)); + assertThat(realm.users().search(search, null, null), hasSize(count)); } @Test @@ -305,22 +269,22 @@ public class UsersTest extends AbstractAdminTest { public void countUsersBySearchWithGroupViewPermission() throws CertificateException, NoSuchAlgorithmException, KeyStoreException, KeyManagementException, IOException { RealmResource testRealmResource = setupTestEnvironmentWithPermissions(true); //search all - assertThat(testRealmResource.users().count("user"), is(3)); + assertSearchMatchesCount(testRealmResource, "user", 3); //search first name - assertThat(testRealmResource.users().count("*FirstName*"), is(3)); - assertThat(testRealmResource.users().count("user2FirstName"), is(1)); + assertSearchMatchesCount(testRealmResource, "*FirstName*", 3); + assertSearchMatchesCount(testRealmResource, "user2FirstName", 1); //search last name - assertThat(testRealmResource.users().count("*LastName*"), is(3)); - assertThat(testRealmResource.users().count("user2LastName"), is(1)); + assertSearchMatchesCount(testRealmResource, "*LastName*", 3); + assertSearchMatchesCount(testRealmResource, "user2LastName", 1); //search in email - assertThat(testRealmResource.users().count("*@example.com*"), is(3)); - assertThat(testRealmResource.users().count("user1@example.com"), is(1)); + assertSearchMatchesCount(testRealmResource, "*@example.com*", 3); + assertSearchMatchesCount(testRealmResource, "user1@example.com", 1); //search for something not existing - assertThat(testRealmResource.users().count("notExisting"), is(0)); + assertSearchMatchesCount(testRealmResource, "notExisting", 0); //search for empty string - assertThat(testRealmResource.users().count(""), is(3)); + assertSearchMatchesCount(testRealmResource, "", 3); //search not specified (defaults to simply /count) - assertThat(testRealmResource.users().count(null), is(3)); + assertSearchMatchesCount(testRealmResource, null, 3); } @Test @@ -328,6 +292,7 @@ public class UsersTest extends AbstractAdminTest { public void countUsersByFiltersWithGroupViewPermission() throws CertificateException, NoSuchAlgorithmException, KeyStoreException, KeyManagementException, IOException { RealmResource testRealmResource = setupTestEnvironmentWithPermissions(true); //search username + assertThat(testRealmResource.users().count(null, null, null, "user"), equalTo(testRealmResource.users().search("user", null, null, null, null, null).size())); assertThat(testRealmResource.users().count(null, null, null, "user"), is(3)); assertThat(testRealmResource.users().count(null, null, null, "user1"), is(1)); assertThat(testRealmResource.users().count(null, null, null, "notExisting"), is(0)); @@ -371,22 +336,22 @@ public class UsersTest extends AbstractAdminTest { public void countUsersBySearchWithNoViewPermission() throws CertificateException, NoSuchAlgorithmException, KeyStoreException, KeyManagementException, IOException { RealmResource testRealmResource = setupTestEnvironmentWithPermissions(false); //search all - assertThat(testRealmResource.users().count("user"), is(0)); + assertSearchMatchesCount(testRealmResource, "user", 0); //search first name - assertThat(testRealmResource.users().count("FirstName"), is(0)); - assertThat(testRealmResource.users().count("user2FirstName"), is(0)); + assertSearchMatchesCount(testRealmResource, "FirstName", 0); + assertSearchMatchesCount(testRealmResource, "user2FirstName", 0); //search last name - assertThat(testRealmResource.users().count("LastName"), is(0)); - assertThat(testRealmResource.users().count("user2LastName"), is(0)); + assertSearchMatchesCount(testRealmResource, "LastName", 0); + assertSearchMatchesCount(testRealmResource, "user2LastName", 0); //search in email - assertThat(testRealmResource.users().count("@example.com"), is(0)); - assertThat(testRealmResource.users().count("user1@example.com"), is(0)); + assertSearchMatchesCount(testRealmResource, "@example.com", 0); + assertSearchMatchesCount(testRealmResource, "user1@example.com", 0); //search for something not existing - assertThat(testRealmResource.users().count("notExisting"), is(0)); + assertSearchMatchesCount(testRealmResource, "notExisting", 0); //search for empty string - assertThat(testRealmResource.users().count(""), is(0)); + assertSearchMatchesCount(testRealmResource, "", 0); //search not specified (defaults to simply /count) - assertThat(testRealmResource.users().count(null), is(0)); + assertSearchMatchesCount(testRealmResource, null, 0); } @Test