From f8b1b3ee03bb46d130efc0f7bce23f905022c135 Mon Sep 17 00:00:00 2001 From: cgeorgilakis-grnet Date: Fri, 6 Sep 2024 17:26:29 +0300 Subject: [PATCH] Search Identity Providers by alias or display name Closes #32588 Signed-off-by: cgeorgilakis-grnet --- .../JpaIdentityProviderStorageProvider.java | 5 ++-- .../models/IdentityProviderModel.java | 1 + .../account/LinkedAccountsResource.java | 8 +++--- .../LinkedAccountsRestServiceTest.java | 28 +++++++++++++------ .../testsuite/admin/IdentityProviderTest.java | 11 ++++++-- 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaIdentityProviderStorageProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaIdentityProviderStorageProvider.java index 9f6b2838c6..7548a60768 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaIdentityProviderStorageProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaIdentityProviderStorageProvider.java @@ -52,6 +52,7 @@ import org.keycloak.utils.StringUtil; import static org.keycloak.models.IdentityProviderModel.ALIAS; import static org.keycloak.models.IdentityProviderModel.ALIAS_NOT_IN; import static org.keycloak.models.IdentityProviderModel.AUTHENTICATE_BY_DEFAULT; +import static org.keycloak.models.IdentityProviderModel.DISPLAY_NAME; import static org.keycloak.models.IdentityProviderModel.ENABLED; import static org.keycloak.models.IdentityProviderModel.FIRST_BROKER_LOGIN_FLOW_ID; import static org.keycloak.models.IdentityProviderModel.HIDE_ON_LOGIN; @@ -506,13 +507,13 @@ public class JpaIdentityProviderStorageProvider implements IdentityProviderStora if (search.startsWith("\"") && search.endsWith("\"")) { // exact search - alias must be an exact match search = search.substring(1, search.length() - 1); - return builder.equal(idp.get(ALIAS), search); + return builder.or(builder.equal(idp.get(ALIAS), search),builder.equal(idp.get(DISPLAY_NAME), search)); } else { search = search.replace("%", "\\%").replace("_", "\\_").replace("*", "%"); if (!search.endsWith("%")) { search += "%"; // default to prefix search } - return builder.like(builder.lower(idp.get(ALIAS)), search.toLowerCase(), '\\'); + return builder.or(builder.like(builder.lower(idp.get(ALIAS)), search.toLowerCase(), '\\'),builder.like(builder.lower(idp.get(DISPLAY_NAME)), search.toLowerCase(), '\\')); } } diff --git a/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java b/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java index 8ff01eace5..1c16d00eca 100755 --- a/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java +++ b/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java @@ -38,6 +38,7 @@ public class IdentityProviderModel implements Serializable { public static final String CASE_SENSITIVE_ORIGINAL_USERNAME = "caseSensitiveOriginalUsername"; public static final String CLAIM_FILTER_NAME = "claimFilterName"; public static final String CLAIM_FILTER_VALUE = "claimFilterValue"; + public static final String DISPLAY_NAME = "displayName"; public static final String DO_NOT_STORE_USERS = "doNotStoreUsers"; public static final String ENABLED = "enabled"; public static final String FILTERED_BY_CLAIMS = "filteredByClaim"; diff --git a/services/src/main/java/org/keycloak/services/resources/account/LinkedAccountsResource.java b/services/src/main/java/org/keycloak/services/resources/account/LinkedAccountsResource.java index 3bc909ea2a..d21610282d 100644 --- a/services/src/main/java/org/keycloak/services/resources/account/LinkedAccountsResource.java +++ b/services/src/main/java/org/keycloak/services/resources/account/LinkedAccountsResource.java @@ -179,15 +179,15 @@ public class LinkedAccountsResource { return true; }else if (search.startsWith("\"") && search.endsWith("\"")) { final String name = search.substring(1, search.length() - 1); - return linkedAccount.getProviderAlias().equals(name); + return linkedAccount.getProviderAlias().equals(name) || linkedAccount.getDisplayName().equals(name); } else if (search.startsWith("*") && search.endsWith("*")) { final String name = search.substring(1, search.length() - 1); - return linkedAccount.getProviderAlias().contains(name); + return linkedAccount.getProviderAlias().contains(name) || linkedAccount.getDisplayName().contains(name); } else if (search.endsWith("*")) { final String name = search.substring(0, search.length() - 1); - return linkedAccount.getProviderAlias().startsWith(name); + return linkedAccount.getProviderAlias().startsWith(name) || linkedAccount.getDisplayName().startsWith(name); } else { - return linkedAccount.getProviderAlias().startsWith(search); + return linkedAccount.getProviderAlias().startsWith(search) || linkedAccount.getDisplayName().startsWith(search); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/LinkedAccountsRestServiceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/LinkedAccountsRestServiceTest.java index d9f1f92ad1..3bf4fc38be 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/LinkedAccountsRestServiceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/LinkedAccountsRestServiceTest.java @@ -89,17 +89,18 @@ public class LinkedAccountsRestServiceTest extends AbstractTestRealmKeycloakTest testRealm.getUsers().add(UserBuilder.create().username("no-account-access").password("password").build()); testRealm.getUsers().add(UserBuilder.create().username("view-account-access").role("account", "view-profile").password("password").build()); - String[] providers = new String[]{"saml:mysaml", "oidc:myoidc", "github", "gitlab", "twitter", "facebook", "bitbucket", "microsoft"}; + String[] providers = new String[]{"saml:mysaml:saml-idp", "oidc:myoidc:oidc-idp", "github", "gitlab", "twitter", "facebook", "bitbucket", "microsoft"}; for (int i = 0; i < providers.length; i++) { String[] idpInfo = providers[i].split(":"); testRealm.addIdentityProvider(IdentityProviderBuilder.create() .providerId(idpInfo[0]) .alias(idpInfo.length == 1 ? idpInfo[0] : idpInfo[1]) + .displayName(idpInfo.length == 1 ? null : idpInfo[2]) .setAttribute("guiOrder", String.valueOf(i)) .build()); } - addFederatedIdentities(testRealm, "github", "gitlab", "twitter"); + addFederatedIdentities(testRealm, "github", "gitlab", "mysaml"); } private void addFederatedIdentities(RealmRepresentation testRealm, String... idpAliases) { @@ -183,7 +184,7 @@ public class LinkedAccountsRestServiceTest extends AbstractTestRealmKeycloakTest List linkedAccountAliases = details.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); assertThat(linkedAccountAliases, contains("mysaml", "myoidc", "github", "gitlab", "twitter", "facebook", "bitbucket", "microsoft")); - List expectedConnectedAccounts = List.of("github", "gitlab", "twitter"); + List expectedConnectedAccounts = List.of("github", "gitlab", "mysaml"); for (LinkedAccountRepresentation account : details) { if (expectedConnectedAccounts.contains(account.getProviderAlias())) { assertTrue(account.isConnected()); @@ -201,7 +202,7 @@ public class LinkedAccountsRestServiceTest extends AbstractTestRealmKeycloakTest assertEquals(3, accounts.size()); List linkedAccountAliases = accounts.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); - assertThat(linkedAccountAliases, contains("github", "gitlab", "twitter")); + assertThat(linkedAccountAliases, contains("mysaml", "github", "gitlab")); for (LinkedAccountRepresentation account : accounts) { assertTrue(account.isConnected()); } @@ -210,12 +211,12 @@ public class LinkedAccountsRestServiceTest extends AbstractTestRealmKeycloakTest accounts = linkedAccountsRep("linked=true&first=0&max=2"); assertEquals(2, accounts.size()); linkedAccountAliases = accounts.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); - assertThat(linkedAccountAliases, contains("github", "gitlab")); + assertThat(linkedAccountAliases, contains("mysaml","github")); accounts = linkedAccountsRep("linked=true&first=2&max=4"); assertEquals(1, accounts.size()); linkedAccountAliases = accounts.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); - assertThat(linkedAccountAliases, contains("twitter")); + assertThat(linkedAccountAliases, contains("gitlab")); // now use a search string to further filter the results. accounts = linkedAccountsRep("linked=true&search=git*"); @@ -223,13 +224,18 @@ public class LinkedAccountsRestServiceTest extends AbstractTestRealmKeycloakTest linkedAccountAliases = accounts.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); assertThat(linkedAccountAliases, contains("github", "gitlab")); + accounts = linkedAccountsRep("linked=true&search=*l-id*"); + assertEquals(1, accounts.size()); + linkedAccountAliases = accounts.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); + assertThat(linkedAccountAliases, contains("mysaml")); + // search unlinked identity providers. accounts = linkedAccountsRep("linked=false&first=0&max=10"); assertEquals(5, accounts.size()); linkedAccountAliases = accounts.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); // the unlinked accounts are ordered by alias, not gui order (this test needs to be adjusted if the model is fixed to order by gui order) - assertThat(linkedAccountAliases, contains("bitbucket", "facebook", "microsoft", "myoidc", "mysaml")); + assertThat(linkedAccountAliases, contains("bitbucket", "facebook", "microsoft", "myoidc", "twitter")); for (LinkedAccountRepresentation account : accounts) { assertFalse(account.isConnected()); } @@ -243,7 +249,7 @@ public class LinkedAccountsRestServiceTest extends AbstractTestRealmKeycloakTest accounts = linkedAccountsRep("linked=false&first=3&max=3"); assertEquals(2, accounts.size()); linkedAccountAliases = accounts.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); - assertThat(linkedAccountAliases, contains("myoidc", "mysaml")); + assertThat(linkedAccountAliases, contains("myoidc", "twitter")); // now use a search string to filter the results. accounts = linkedAccountsRep("linked=false&search=*o*"); @@ -256,6 +262,12 @@ public class LinkedAccountsRestServiceTest extends AbstractTestRealmKeycloakTest assertEquals(1, accounts.size()); linkedAccountAliases = accounts.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); assertThat(linkedAccountAliases, contains("microsoft")); + + //search based on display name + accounts = linkedAccountsRep("linked=false&search=*c-id*"); + assertEquals(1, accounts.size()); + linkedAccountAliases = accounts.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); + assertThat(linkedAccountAliases, contains("myoidc")); } @Test diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java index 7f45bc1dc1..5c32de3693 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java @@ -140,7 +140,7 @@ public class IdentityProviderTest extends AbstractAdminTest { @Test public void testFind() { - create(createRep("twitter", "twitter", true, Collections.singletonMap("key1", "value1"))); + create(createRep("twitter", "twitter idp","twitter", true, Collections.singletonMap("key1", "value1"))); create(createRep("linkedin-openid-connect", "linkedin-openid-connect")); create(createRep("google", "google")); create(createRep("github", "github")); @@ -160,6 +160,9 @@ public class IdentityProviderTest extends AbstractAdminTest { Assert.assertNames(realm.identityProviders().find("*oo*", true, 0, 5), "google", "facebook"); + //based on display name search + Assert.assertNames(realm.identityProviders().find("*ter i*", true, 0, 5), "twitter"); + List results = realm.identityProviders().find("\"twitter\"", true, 0, 5); Assert.assertNames(results, "twitter"); Assert.assertTrue("Result is not in brief representation", results.iterator().next().getConfig().isEmpty()); @@ -603,10 +606,14 @@ public class IdentityProviderTest extends AbstractAdminTest { } private IdentityProviderRepresentation createRep(String id, String providerId,boolean enabled, Map config) { + return createRep(id, id, providerId, enabled, config); + } + + private IdentityProviderRepresentation createRep(String id, String displayName, String providerId, boolean enabled, Map config) { IdentityProviderRepresentation idp = new IdentityProviderRepresentation(); idp.setAlias(id); - idp.setDisplayName(id); + idp.setDisplayName(displayName); idp.setProviderId(providerId); idp.setEnabled(enabled); if (config != null) {