From 26dcfe21ef246522a8766eee564073c0f51f73d4 Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Wed, 14 Jan 2026 19:13:22 +0100 Subject: [PATCH] Check if two IDPs with the same issuer URL exist before caching them Closes #45453 Signed-off-by: Alexander Schwartz --- .../DefaultAlternativeLookupProvider.java | 16 +++- .../FederatedClientAuthConflictsTest.java | 31 +++++++- .../model/AlternativeLookupProviderTest.java | 78 +++++++++++++++++++ 3 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 tests/base/src/test/java/org/keycloak/tests/model/AlternativeLookupProviderTest.java diff --git a/services/src/main/java/org/keycloak/cache/DefaultAlternativeLookupProvider.java b/services/src/main/java/org/keycloak/cache/DefaultAlternativeLookupProvider.java index 73ca5622bbd..55aac81c67f 100644 --- a/services/src/main/java/org/keycloak/cache/DefaultAlternativeLookupProvider.java +++ b/services/src/main/java/org/keycloak/cache/DefaultAlternativeLookupProvider.java @@ -29,12 +29,20 @@ public class DefaultAlternativeLookupProvider implements AlternativeLookupProvid } } - IdentityProviderModel idp = session.identityProviders().getAllStream(IdentityProviderQuery.any()) + List idps = session.identityProviders().getAllStream(IdentityProviderQuery.any()) .filter(i -> issuerUrl.equals(i.getConfig().get(IdentityProviderModel.ISSUER))) - .findFirst().orElse(null); - if (idp != null && idp.getAlias() != null) { - lookupCache.put(alternativeKey, idp.getAlias()); + .limit(2) + .toList(); + IdentityProviderModel idp = null; + if (idps.size() == 1) { + idp = idps.get(0); + if (idp.getAlias() != null) { + lookupCache.put(alternativeKey, idp.getAlias()); + } + } else if (idps.size() > 1) { + throw new RuntimeException("Multiple IDPs match the same issuer: " + idps.stream().map(IdentityProviderModel::getAlias).toList()); } + return idp; } diff --git a/tests/base/src/test/java/org/keycloak/tests/client/authentication/external/FederatedClientAuthConflictsTest.java b/tests/base/src/test/java/org/keycloak/tests/client/authentication/external/FederatedClientAuthConflictsTest.java index 67b7113dc32..b98fb5f3c65 100644 --- a/tests/base/src/test/java/org/keycloak/tests/client/authentication/external/FederatedClientAuthConflictsTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/client/authentication/external/FederatedClientAuthConflictsTest.java @@ -48,21 +48,44 @@ public class FederatedClientAuthConflictsTest { createIdp("idp1", "http://127.0.0.1:8500"); createIdp("idp2", "http://127.0.0.1:8500"); + createClient("myclient", "external1", "idp1"); + + // Should fail as there are two IdPs with the same issuer URL + AccessTokenResponse response = oAuthClient.clientCredentialsGrantRequest().clientJwt(createDefaultToken("external1", "http://127.0.0.1:8500")).send(); + Assertions.assertFalse(response.isSuccess()); + Assertions.assertEquals("Invalid client or Invalid client credentials", response.getErrorDescription()); + } + + @Test + public void testChangedIssuer() { + IdentityProviderRepresentation idp1 = createIdp("idp1", "http://127.0.0.1:8500"); + ClientRepresentation clientRep = createClient("myclient", "external1", "idp1"); - // Should pass as the first matching IdP by alias is always used + // Should fail as there are two IdPs with the same issuer URL AccessTokenResponse response = oAuthClient.clientCredentialsGrantRequest().clientJwt(createDefaultToken("external1", "http://127.0.0.1:8500")).send(); Assertions.assertTrue(response.isSuccess()); Assertions.assertEquals("myclient", events.poll().getClientId()); - clientRep.getAttributes().put(FederatedJWTClientAuthenticator.JWT_CREDENTIAL_ISSUER_KEY, "idp2"); + createIdp("idp2", "http://127.0.0.1:8500"); + clientRep.getAttributes().put(FederatedJWTClientAuthenticator.JWT_CREDENTIAL_ISSUER_KEY, "idp2"); realm.admin().clients().get(clientRep.getId()).update(clientRep); - // Should fail since it's using the second IdP + // Should fail since the old entry is still cached response = oAuthClient.clientCredentialsGrantRequest().clientJwt(createDefaultToken("external1", "http://127.0.0.1:8500")).send(); Assertions.assertFalse(response.isSuccess()); - Assertions.assertNull(events.poll().getClientId()); + Assertions.assertEquals("Invalid client or Invalid client credentials", response.getErrorDescription()); + + // Update old entry, so next read will invalidate it + idp1.getConfig().put(IdentityProviderModel.ISSUER, "http://127.0.0.1:8501"); + realm.admin().identityProviders().get(idp1.getAlias()).update(idp1); + + // Should succeed as entry is updated in the cache + events.clear(); + response = oAuthClient.clientCredentialsGrantRequest().clientJwt(createDefaultToken("external1", "http://127.0.0.1:8500")).send(); + Assertions.assertTrue(response.isSuccess()); + Assertions.assertEquals("myclient", events.poll().getClientId()); } @Test diff --git a/tests/base/src/test/java/org/keycloak/tests/model/AlternativeLookupProviderTest.java b/tests/base/src/test/java/org/keycloak/tests/model/AlternativeLookupProviderTest.java new file mode 100644 index 00000000000..e6de8b88c90 --- /dev/null +++ b/tests/base/src/test/java/org/keycloak/tests/model/AlternativeLookupProviderTest.java @@ -0,0 +1,78 @@ +/* + * Copyright 2026 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.tests.model; + +import org.keycloak.cache.AlternativeLookupProvider; +import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.testframework.annotations.InjectRealm; +import org.keycloak.testframework.annotations.KeycloakIntegrationTest; +import org.keycloak.testframework.realm.ManagedRealm; +import org.keycloak.testframework.remote.annotations.TestOnServer; + +import org.junit.jupiter.api.Assertions; + +@KeycloakIntegrationTest +public class AlternativeLookupProviderTest { + + @InjectRealm(attachTo = "master") + ManagedRealm realm; + + @TestOnServer + public void testDuplicateIssuerFoundInDatabase(KeycloakSession session) { + try { + IdentityProviderModel idp = createModel("kubernetes1", "kubernetes"); + idp.getConfig().put("issuer", "https://localhost"); + session.identityProviders().create(idp); + + idp = createModel("kubernetes2", "kubernetes"); + idp.getConfig().put("issuer", "https://localhost"); + session.identityProviders().create(idp); + + RuntimeException ex = Assertions.assertThrows(RuntimeException.class, () -> + session.getProvider(AlternativeLookupProvider.class).lookupIdentityProviderFromIssuer(session, "https://localhost") + ); + Assertions.assertEquals("Multiple IDPs match the same issuer: [kubernetes1, kubernetes2]", ex.getMessage()); + + } finally { + session.identityProviders().remove("kubernetes1"); + session.identityProviders().remove("kubernetes2"); + } + } + + + protected IdentityProviderModel createModel(String alias, String providerId) { + return createModel(alias, providerId,true); + } + + protected IdentityProviderModel createModel(String alias, String providerId, boolean enabled) { + return createModel(alias, alias, providerId, enabled); + } + + protected IdentityProviderModel createModel(String alias, String displayName, String providerId, boolean enabled) { + IdentityProviderModel idp = new IdentityProviderModel(); + + idp.setAlias(alias); + idp.setDisplayName(displayName); + idp.setProviderId(providerId); + idp.setEnabled(enabled); + return idp; + } + + +}