Ensure IDPs returned from infinispan provider are ordered by alias

Closes #33243

Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>
This commit is contained in:
Stefan Guilhen 2025-01-28 10:51:55 -03:00 committed by Pedro Igor
parent 58596b916f
commit 0fc0dcd119
3 changed files with 35 additions and 11 deletions

View File

@ -135,7 +135,7 @@ export default function IdentityProvidersSection() {
params.search = search;
}
const providers = await adminClient.identityProviders.find(params);
return sortBy(providers, "alias");
return providers;
};
const navigateToCreate = (providerId: string) =>

View File

@ -17,6 +17,7 @@
package org.keycloak.models.cache.infinispan.idp;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
@ -216,13 +217,14 @@ public class InfinispanIdentityProviderStorageProvider implements IdentityProvid
// there is a cache entry, but the current search is not yet cached
cache.invalidateObject(cacheKey);
Long loaded = cache.getCurrentRevision(cacheKey);
cached = idpDelegate.getByOrganization(orgId, first, max).map(IdentityProviderModel::getInternalId).collect(Collectors.toSet());
cached = idpDelegate.getByOrganization(orgId, first, max).map(IdentityProviderModel::getInternalId)
.collect(Collectors.toCollection(LinkedHashSet::new));
query = new IdentityProviderListQuery(loaded, cacheKey, realm, searchKey, cached, query);
cache.addRevisioned(query, cache.getCurrentCounter());
}
}
Set<IdentityProviderModel> identityProviders = new HashSet<>();
Set<IdentityProviderModel> identityProviders = new LinkedHashSet<>();
for (String id : cached) {
IdentityProviderModel idp = session.identityProviders().getById(id);
if (idp == null) {
@ -251,7 +253,8 @@ public class InfinispanIdentityProviderStorageProvider implements IdentityProvid
if (query == null) {
// not cached yet
Long loaded = cache.getCurrentRevision(cacheKey);
cached = idpDelegate.getForLogin(mode, organizationId).map(IdentityProviderModel::getInternalId).collect(Collectors.toSet());
cached = idpDelegate.getForLogin(mode, organizationId).map(IdentityProviderModel::getInternalId)
.collect(Collectors.toCollection(LinkedHashSet::new));
query = new IdentityProviderListQuery(loaded, cacheKey, getRealm(), searchKey, cached);
cache.addRevisioned(query, startupRevision);
} else {
@ -266,7 +269,7 @@ public class InfinispanIdentityProviderStorageProvider implements IdentityProvid
}
}
Set<IdentityProviderModel> identityProviders = new HashSet<>();
Set<IdentityProviderModel> identityProviders = new LinkedHashSet<>();
for (String id : cached) {
IdentityProviderModel idp = session.identityProviders().getById(id);
if (idp == null) {

View File

@ -18,6 +18,7 @@
package org.keycloak.testsuite.admin;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
@ -77,6 +78,7 @@ import org.keycloak.models.Constants;
import org.keycloak.models.IdentityProviderMapperModel;
import org.keycloak.models.IdentityProviderMapperSyncMode;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.IdentityProviderStorageProvider;
import org.keycloak.models.utils.StripSecretsUtils;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.representations.idm.AdminEventRepresentation;
@ -171,6 +173,25 @@ public class IdentityProviderTest extends AbstractAdminTest {
Assert.assertFalse("Config should be present in full representation", results.iterator().next().getConfig().isEmpty());
}
@Test
public void testFindForLoginPreservesOrderByAlias() {
create(createRep("twitter", "twitter"));
create(createRep("linkedin-openid-connect", "linkedin-openid-connect"));
create(createRep("google", "google"));
create(createRep("github", "github"));
create(createRep("facebook", "facebook"));
create(createRep("stackoverflow", "stackoverflow"));
create(createRep("openshift-v4", "openshift-v4"));
getTestingClient().server(REALM_NAME).run(session -> {
// fetch the list of idps available for login (should match all from above list) and ensure they come ordered by alias.
List<String> aliases = session.identityProviders().getForLogin(IdentityProviderStorageProvider.FetchMode.ALL, null)
.map(IdentityProviderModel::getAlias).toList();
assertThat(aliases, contains("facebook", "github", "google", "linkedin-openid-connect", "openshift-v4", "stackoverflow", "twitter"));
});
}
@Test
public void testCreateWithReservedCharacterForAlias() {
IdentityProviderRepresentation newIdentityProvider = createRep("ne$&w-identity-provider", "oidc");
@ -601,18 +622,18 @@ public class IdentityProviderTest extends AbstractAdminTest {
}
}
private IdentityProviderRepresentation createRep(String id, String providerId) {
return createRep(id, providerId,true, null);
private IdentityProviderRepresentation createRep(String alias, String providerId) {
return createRep(alias, providerId,true, null);
}
private IdentityProviderRepresentation createRep(String id, String providerId,boolean enabled, Map<String, String> config) {
return createRep(id, id, providerId, enabled, config);
private IdentityProviderRepresentation createRep(String alias, String providerId,boolean enabled, Map<String, String> config) {
return createRep(alias, alias, providerId, enabled, config);
}
private IdentityProviderRepresentation createRep(String id, String displayName, String providerId, boolean enabled, Map<String, String> config) {
private IdentityProviderRepresentation createRep(String alias, String displayName, String providerId, boolean enabled, Map<String, String> config) {
IdentityProviderRepresentation idp = new IdentityProviderRepresentation();
idp.setAlias(id);
idp.setAlias(alias);
idp.setDisplayName(displayName);
idp.setProviderId(providerId);
idp.setEnabled(enabled);