From fc0e47caa4763e0b563a237cda9c5962be13b0c3 Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Sun, 19 Mar 2023 11:25:45 +0100 Subject: [PATCH] Fix KcCustomOidcBrokerTest Fixes: #20541 --- .../models/map/realm/MapRealmAdapter.java | 25 +++++++++++++++++-- .../entity/MapIdentityProviderEntity.java | 5 ++-- .../broker/KcCustomOidcBrokerTest.java | 5 ++-- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmAdapter.java b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmAdapter.java index c792ce47aa8..4e1f96daed6 100644 --- a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmAdapter.java +++ b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmAdapter.java @@ -31,6 +31,9 @@ import java.util.stream.Stream; import org.jboss.logging.Logger; import org.keycloak.Config; +import org.keycloak.broker.provider.IdentityProvider; +import org.keycloak.broker.provider.IdentityProviderFactory; +import org.keycloak.broker.social.SocialIdentityProvider; import org.keycloak.common.enums.SslRequired; import org.keycloak.component.ComponentFactory; import org.keycloak.component.ComponentModel; @@ -943,7 +946,8 @@ public class MapRealmAdapter extends AbstractRealmModel implemen @Override public Stream getIdentityProvidersStream() { Set ips = entity.getIdentityProviders(); - return ips == null ? Stream.empty() : ips.stream().map(MapIdentityProviderEntity::toModel); + return ips == null ? Stream.empty() : ips.stream() + .map(e -> MapIdentityProviderEntity.toModel(e, () -> this.getModelFromProviderFactory(e.getProviderId()))); } @Override @@ -952,10 +956,27 @@ public class MapRealmAdapter extends AbstractRealmModel implemen return ips == null ? null : ips.stream() .filter(identityProvider -> Objects.equals(identityProvider.getAlias(), alias)) .findFirst() - .map(MapIdentityProviderEntity::toModel) + .map(e -> MapIdentityProviderEntity.toModel(e, () -> this.getModelFromProviderFactory(e.getProviderId()))) .orElse(null); } + // This is a violation of layering requirements, this should NOT be in store code. + // However, there is no easy way around this given the current number of IdentityProviderModel implementations + private IdentityProviderModel getModelFromProviderFactory(String providerId) { + Optional factory = Stream.concat(session.getKeycloakSessionFactory().getProviderFactoriesStream(IdentityProvider.class), + session.getKeycloakSessionFactory().getProviderFactoriesStream(SocialIdentityProvider.class)) + .filter(providerFactory -> Objects.equals(providerFactory.getId(), providerId)) + .map(IdentityProviderFactory.class::cast) + .findFirst(); + + if (factory.isPresent()) { + return factory.get().createConfig(); + } else { + LOG.warn("Couldn't find a suitable identity provider factory for " + providerId); + return new IdentityProviderModel(); + } + } + @Override public void addIdentityProvider(IdentityProviderModel model) { if (getIdentityProviderByAlias(model.getAlias()) != null) { diff --git a/model/map/src/main/java/org/keycloak/models/map/realm/entity/MapIdentityProviderEntity.java b/model/map/src/main/java/org/keycloak/models/map/realm/entity/MapIdentityProviderEntity.java index e4fd62b657c..254fd1e4851 100644 --- a/model/map/src/main/java/org/keycloak/models/map/realm/entity/MapIdentityProviderEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/realm/entity/MapIdentityProviderEntity.java @@ -26,6 +26,7 @@ import org.keycloak.models.utils.KeycloakModelUtils; import java.util.HashMap; import java.util.Map; +import java.util.function.Supplier; @GenerateEntityImplementations @DeepCloner.Root @@ -50,9 +51,9 @@ public interface MapIdentityProviderEntity extends UpdatableEntity, AbstractEnti return entity; } - static IdentityProviderModel toModel(MapIdentityProviderEntity entity) { + static IdentityProviderModel toModel(MapIdentityProviderEntity entity, Supplier instanceCreator) { if (entity == null) return null; - IdentityProviderModel model = new IdentityProviderModel(); + IdentityProviderModel model = instanceCreator.get(); model.setInternalId(entity.getId()); model.setAlias(entity.getAlias()); model.setDisplayName(entity.getDisplayName()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcCustomOidcBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcCustomOidcBrokerTest.java index 01d06652ee4..4503cb03e8e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcCustomOidcBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcCustomOidcBrokerTest.java @@ -20,11 +20,12 @@ import static org.keycloak.testsuite.broker.BrokerTestConstants.IDP_OIDC_ALIAS; import static org.keycloak.testsuite.broker.BrokerTestTools.createIdentityProvider; import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot; -import org.junit.Assert; import org.junit.Test; import org.keycloak.models.IdentityProviderSyncMode; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.testsuite.broker.oidc.TestKeycloakOidcIdentityProviderFactory; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; /** * Test methods for testing a custom OIDC broker @@ -51,6 +52,6 @@ public class KcCustomOidcBrokerTest extends AbstractInitializedBaseBrokerTest { @Test public void testCustomDisplayIcon() { driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName())); - Assert.assertTrue(driver.getPageSource().contains("my-custom-idp-icon")); + assertThat(driver.getPageSource(), containsString("my-custom-idp-icon")); } }