Repository navigation
migration broker package - #51881
Closed
edewit wants to merge 9 commits into
Closed
migration broker package#51881edewit wants to merge 9 commits into
edewit wants to merge 9 commits into
Conversation
edewit
commented
Aug 20, 2026
Contributor
- Moving files to the new test suite
- Migrate broker package tests to the new testsuite
- Migrate broker package tests to the new testsuite
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Port broker tests from integration-arquillian into tests/base with compatibility shims in test-framework and tests/base utilities, and register migrated custom broker providers in tests/custom-providers. This removes legacy dependency assumptions while keeping migrated broker flows compiling under the new framework. Closes keycloak#50754 Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
edewit
force-pushed
the
migration-broker-package
branch
from
August 24, 2026 07:50
63e3cb1 to
dfb7fab
Compare
Add the providers package to the base suite selection so check-base-suites.sh passes in CI after migrating broker tests. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 266 out of 290 changed files in this pull request and generated 10 comments.
Suppressed comments (1)
tests/base/src/test/java/org/keycloak/tests/broker/KcSamlParseIdPDescriptorTest.java:60
- The new test framework starts Keycloak at the root context (for example
http://localhost:8080), not the legacy/authcontext. Appending/authmakes both descriptor requests hit a 404 and also makes all expected entity/service URLs incorrect.
Comment on lines
+31
to
+33
| public static WebDriver getCurrentDriver() { | ||
| return driverQueue.peek(); | ||
| } |
Comment on lines
+24
to
+26
| public ReverseProxy(String host, String nodes) { | ||
| // No-op compatibility constructor kept for migrated broker tests. | ||
| } |
Comment on lines
+34
to
+37
| @Override | ||
| protected void before() throws Throwable { | ||
| // no-op compatibility rule for migrated tests | ||
| } |
Comment on lines
+15
to
+18
| @Retention(RUNTIME) | ||
| @Target({ElementType.TYPE, ElementType.METHOD}) | ||
| @Repeatable(EnableFeatures.class) | ||
| public @interface EnableFeature { |
Comment on lines
+11
to
+12
| @EnableVault | ||
| @KeycloakIntegrationTest(config = org.keycloak.tests.broker.BrokerServerConfig.class) |
Comment on lines
58
to
59
| @ClassRule | ||
| public static LDAPRule ldapRule = new LDAPRule(); |
| * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a> | ||
| * @version $Revision: 1 $ | ||
| */ | ||
| public class PassThroughFederatedUserStorageProviderFactory implements UserStorageProviderFactory<PassThroughFederatedUserStorageProvider> { |
Comment on lines
29
to
+30
| import org.junit.jupiter.api.Assertions; | ||
| import org.junit.jupiter.api.Test; |
Comment on lines
23
to
+24
| import org.junit.jupiter.api.Assertions; | ||
| import org.junit.jupiter.api.Test; |
Comment on lines
+54
to
+55
| @InjectRealm | ||
| ManagedRealm managedRealm; |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 266 out of 290 changed files in this pull request and generated 1 comment.
Suppressed comments (7)
tests/base/src/test/java/org/keycloak/testsuite/util/DroneUtils.java:33
- The migrated static UI helpers never register a driver, so this returns
null; calls such asURLUtils.navigateToUrithen fail with an NPE. Fall back to the driver already tracked byManagedWebDriver.
tests/base/src/test/java/org/keycloak/testsuite/util/ReverseProxy.java:26 - This replacement no longer starts a proxy, but the migrated frontend-URL tests send browser traffic to ports 8666/8667. Those tests will therefore fail to connect instead of exercising proxy-based URL handling; migrate the proxy as a Jupiter/test-framework resource rather than leaving this constructor and rule as no-ops.
tests/base/src/test/java/org/keycloak/testsuite/util/InfinispanTestTimeServiceRule.java:37 - The old rule installed Infinispan's testing time service before cache-expiry tests; this no-op means the migrated tests'
TimeOffSetinjections (which defaultenableForCachesto false) do not advance cache time. Use@InjectTimeOffSet(enableForCaches = true)in those tests or provide an equivalent lifecycle resource.
tests/base/src/test/java/org/keycloak/testsuite/arquillian/annotation/EnableFeature.java:18 - This annotation has no consumer in the new test framework, so migrated tests annotated with it still start
BrokerServerConfigwithout the requested features. Tests requiring TOKEN_EXCHANGE or TRANSIENT_USERS will run against a server where those features are disabled; migrate them to server configs that callfeatures(...)or add a Jupiter integration for this annotation.
tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerVaultTest.java:12 @EnableVaultwas handled only by the Arquillian enricher and is inert under@KeycloakIntegrationTest;BrokerServerConfigalso supplies novault/vault-diroptions. Consequently${vault.oidc_idp}cannot be resolved and this migrated vault broker test cannot exercise the intended login.
tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerLdapTest.java:44- After switching this class to Jupiter, the JUnit 4
@ClassRuleis not executed.ldapRule.getConfig()then dereferences an uninitializedldapTestConfigurationinonBefore, so the test fails before reaching its assertions; migrateLDAPRuleto a Jupiter/test-framework lifecycle resource.
tests/base/src/test/java/org/keycloak/tests/broker/KcSamlParseIdPDescriptorTest.java:64 - This is an Arquillian callback, but the class no longer extends the Arquillian base and nothing invokes it.
@InjectRealmcreates the default managed realm, while both tests address realmtest, so descriptor retrieval and admin calls receive 404; configure the injected realm with nametest(or use its generated name throughout).
| @InjectAdminClient | ||
| Keycloak adminClient; | ||
|
|
||
| private final URI authServerPage = URI.create(getAuthServerContextRoot() + "/auth"); |
Re-add legacy broker base and social login test classes in the integration-arquillian testsuite so remaining tests that still import org.keycloak.testsuite.broker types compile after the broker migration. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 261 out of 290 changed files in this pull request and generated 1 comment.
Suppressed comments (6)
tests/base/src/test/java/org/keycloak/tests/broker/KcSamlParseIdPDescriptorTest.java:55
addTestRealmsis no longer a framework callback after removingAbstractKeycloakTest, while this injection creates a generated-name default realm. Every test below still callsadminClient.realm("test"), so the descriptor request/import will target a realm that was never created; configure the injected realm with the nametest(or usemanagedRealmthroughout).
tests/base/src/test/java/org/keycloak/testsuite/util/ReverseProxy.java:26- This no-op constructor (together with the no-op
apply) never starts a proxy on ports 8666/8667. The migrated OIDC and SAML frontend-URL tests navigate toproxy.getUrl(), so they will fail to connect; replace this compatibility stub with a Jupiter-managed reverse proxy lifecycle.
tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerLdapTest.java:41 - Switching the setup to Jupiter leaves the
@ClassRule LDAPRuleinactive, because Jupiter does not execute JUnit 4 rules. ConsequentlyLDAPRule.before()never initializesldapTestConfigurationor starts embedded LDAP, andonBefore()fails whengetConfig()dereferences it; migrate this rule to a Jupiter lifecycle resource.
tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerVaultTest.java:12 @EnableVaultis only handled by the old Arquillian enricher and has no effect under@KeycloakIntegrationTest;BrokerServerConfigalso supplies no vault options oroidc_idpsecret. The literal${vault.oidc_idp}will therefore not resolve, so configure a file vault and test secret through the new server config.
tests/base/src/test/java/org/keycloak/tests/broker/OAuth2BrokerTest.java:30- The migration changes these methods to Jupiter tests but retains JUnit 4
@Ignore, which Jupiter does not recognize. These four intentionally empty overrides will be executed and reported as passing; replace@Ignorewith Jupiter@Disabledso the report does not claim coverage that did not run.
tests/base/src/test/java/org/keycloak/testsuite/arquillian/annotation/EnableFeature.java:18 - This copied Arquillian annotation is only a marker in the new module; the Jupiter test framework has no consumer for it. Migrated tests such as
KcOidcBrokerTransientSessionsTestandKcOidcBrokerTokenExchangeTesttherefore start without their required disabled-by-default features; move those features into theirKeycloakServerConfigimplementations or add an equivalent Jupiter extension.
Comment on lines
+48
to
+50
| public IdentityProviderAttributeUpdater setStoreToken(boolean storeToken) { | ||
| rep.setStoreToken(storeToken); | ||
| return this; |
Drop broker test entries that were moved to the new tests/base module from integration-arquillian suite lists so suite.sh no longer aborts CI jobs that still execute legacy base suites. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Replace the hard dependency on ResteasyJackson2Provider with a generic ObjectMapper ContextResolver so tests/base compiles under the jackson3 profile used by CI. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 261 out of 295 changed files in this pull request and generated 10 comments.
Suppressed comments (8)
Previously missed (1) — in code that hasn't changed since the last review.
tests/base/src/test/java/org/keycloak/tests/broker/SocialLoginTest.java:82
- After converting these tests to Jupiter, the remaining JUnit 4
@BeforeClassmethod is never executed. Consequently the missing-social-config assumption and configuration loading are skipped, so the social tests run with an empty configuration instead of being skipped.
tests/base/src/test/java/org/keycloak/testsuite/arquillian/annotation/EnableFeature.java:18
- This annotation is not consumed by the Jupiter test framework, so usages such as
KcOidcBrokerTransientSessionsTestnever enable their required experimental feature. Migrate this behavior to a Jupiter extension or configure the required features through each test'sKeycloakServerConfig; otherwise those migrated tests start the server without the feature they exercise.
tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerVaultTest.java:11 @EnableVaultwas handled by the Arquillian enricher, but the Jupiter framework has no consumer for this annotation andVaultUtilsis never invoked. This test therefore starts without the vault/provider needed to resolve${vault.oidc_idp}.
tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerLdapTest.java:42- This Jupiter lifecycle does not execute the JUnit 4
@ClassRuledeclared below.LDAPRule.before()therefore never initializesldapTestConfigurationor starts embedded LDAP, and this setup immediately dereferences null inldapRule.getConfig().
tests/base/src/test/java/org/keycloak/testsuite/util/ReverseProxy.java:25 - The compatibility constructor no longer creates or starts a proxy, yet the migrated frontend-URL tests direct browser traffic to port 8666. With no listener there, those tests fail before reaching Keycloak; the proxy must be implemented in the new framework or the tests rewritten to use a managed proxy resource.
tests/base/src/test/java/org/keycloak/tests/broker/KcSamlParseIdPDescriptorTest.java:55 - Default
@InjectRealmcreates the framework's default realm, while every request below targets realmtest. The legacyaddTestRealmsmethod is no longer a lifecycle hook, sotestis never created and these calls return 404; configure the injected realm astestand use it directly.
tests/base/src/test/java/org/keycloak/tests/broker/KcSamlParseIdPDescriptorTest.java:60 - The new test framework serves Keycloak directly at
getAuthServerContextRoot(); it does not configure the legacy/authrelative path. Appending/authmakes the descriptor request and all expected metadata URLs point at a nonexistent endpoint.
tests/base/src/test/java/org/keycloak/tests/broker/BrokerTestTools.java:162 - The broker callback URLs also retain
/auth, so they do not match the endpoints exposed by the new test server. Build these paths withgetAuthPath()just like the migrated OIDC configuration does.
| */ | ||
| // Remove @Ignore when closing Github issue 20642 | ||
| @Ignore | ||
| @KeycloakIntegrationTest(config = org.keycloak.tests.broker.BrokerServerConfig.class) |
Comment on lines
+43
to
44
| @KeycloakIntegrationTest(config = org.keycloak.tests.broker.BrokerServerConfig.class) | ||
| public final class KcOidcBrokerLdapReadOnlyTest extends AbstractInitializedBaseBrokerTest { |
Comment on lines
23
to
+24
| import org.junit.jupiter.api.Assertions; | ||
| import org.junit.jupiter.api.Test; |
Comment on lines
46
to
+47
| import org.junit.jupiter.api.Assertions; | ||
| import org.junit.jupiter.api.Test; |
Comment on lines
+57
to
+58
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; |
Comment on lines
+77
to
+78
| @InjectRealm | ||
| ManagedRealm managedRealm; |
Comment on lines
+122
to
+123
| @InjectRealm | ||
| ManagedRealm managedRealm; |
Comment on lines
+97
to
+98
| @InjectRealm | ||
| ManagedRealm managedRealm; |
Comment on lines
167
to
169
| urlRealmProvider = getAuthRoot() + "/auth/realms/" + REALM_PROV_NAME; | ||
| urlRealmConsumer = getAuthRoot() + "/auth/realms/" + REALM_CONS_NAME; | ||
| urlRealmConsumer2 = getAuthRoot() + "/auth/realms/" + REALM_CONS_NAME + "-2"; |
Comment on lines
+145
to
+148
| config.put("authorizationUrl", getProviderRoot() + "/auth/realms/" + idpRealm + "/protocol/openid-connect/auth"); | ||
| config.put("tokenUrl", getProviderRoot() + "/auth/realms/" + idpRealm + "/protocol/openid-connect/token"); | ||
| config.put("logoutUrl", getProviderRoot() + "/auth/realms/" + idpRealm + "/protocol/openid-connect/logout"); | ||
| config.put("userInfoUrl", getProviderRoot() + "/auth/realms/" + idpRealm + "/protocol/openid-connect/userinfo"); |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 261 out of 295 changed files in this pull request and generated 1 comment.
Suppressed comments (7)
tests/base/src/test/java/org/keycloak/testsuite/arquillian/annotation/EnableFeature.java:18
- This Arquillian annotation has no consumer in the JUnit 5 test framework. Consequently migrated tests such as
KcOidcBrokerTransientSessionsTestdo not enable their requested feature; replace these annotations with aKeycloakServerConfigthat callsfeatures(...), or add a JUnit 5 extension that applies them.
tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerVaultTest.java:12 @EnableVaultis only handled by the old Arquillian enricher; the new test framework does not process it, andBrokerServerConfigdoes not configure a vault. The${vault.oidc_idp}secret therefore cannot be resolved, so configure the file vault through the new server config/resource instead of retaining this annotation.
tests/base/src/test/java/org/keycloak/tests/broker/KcSamlParseIdPDescriptorTest.java:64addTestRealmsis an old Arquillian hook and is never invoked for this standalone JUnit 5 class. Only the unrelated defaultManagedRealmis created, so every access toadminClient.realm("test")and its SAML descriptor targets a nonexistent realm; configure the injected realm with aRealmConfignamedtestand derive the endpoint from that managed realm.
tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerLdapTest.java:42- Migrating the setup method to JUnit Jupiter leaves
ldapRuleas a JUnit 4@ClassRule, which Jupiter never executes.ldapTestConfigurationremains null andldapRule.getConfig()inonBeforefails; migrate LDAP setup to a Jupiter extension/test resource before running these tests.
tests/base/src/test/java/org/keycloak/testsuite/util/ReverseProxy.java:26 - The compatibility constructor no longer creates or starts a proxy, yet migrated frontend-URL tests connect to
proxy.kc.localtest.me:8666/8667. Those requests have no listener and the tests cannot exercise proxy-derived URLs; provide an actual proxy test resource in the new framework instead of a no-op rule.
tests/base/src/test/java/org/keycloak/tests/broker/OAuth2BrokerTest.java:30 - These methods are now JUnit Jupiter tests, but the class still uses JUnit 4
@Ignore; Jupiter does not honor that annotation. The four intentionally empty overrides therefore run and pass, silently replacing the inherited coverage; migrate them to@Disabled(or remove the overrides).
tests/base/src/test/java/org/keycloak/testsuite/util/InfinispanTestTimeServiceRule.java:37 - Making this rule a no-op removes the cache-aware time service that the multiple-tab and consent tests relied on. Their injected
TimeOffSetdefaults toenableForCaches = false, so cache expiry no longer advances consistently; migrate those callers to@InjectTimeOffSet(enableForCaches = true)and remove this rule.
Comment on lines
+19
to
+23
| public static CloseableHttpClient newCloseableHttpClient(String keyStorePath, String keyStorePassword, | ||
| String trustStorePath, String trustStorePassword, RedirectStrategy redirectStrategy) { | ||
| // Compatibility helper for migrated tests. Tests that need real mTLS should use | ||
| // dedicated providers in the current test framework. | ||
| return HttpClientBuilder.create().setRedirectStrategy(redirectStrategy).build(); |
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
| @@ -1,6 +1,3 @@ | |||
| KcSamlBrokerSessionNotOnOrAfterTest | |||
| OidcClaimToUserSessionNoteMapperTest | |||
| KcOidcBrokerTransientSessionsTest | |||
| KcAdmSessionTest | |||
| @@ -1,6 +1,3 @@ | |||
| KcSamlBrokerSessionNotOnOrAfterTest | |||
| OidcClaimToUserSessionNoteMapperTest | |||
| KcOidcBrokerTransientSessionsTest | |||
| KcAdmSessionTest | |||
| @@ -1,5 +1,4 @@ | |||
| DeployedScriptAuthenticatorTest | |||
| @@ -1,5 +1,2 @@ | |||
| org.keycloak.testsuite.actions | |||
| org.keycloak.testsuite.forms | |||
| @@ -13,19 +13,4 @@ KcAdmTest | |||
| KcAdmCreateTest | |||
| SAMLServletAdapterTest | |||
| SamlSignatureTest | |||
| @@ -38,7 +38,7 @@ protected int findLastLine(String regex) { | |||
| } | |||
|
|
|||
| protected int findClassDeclaration() { | |||
| return findLine("public class .*"); | |||
| return findLine("^\\s*(?:public\\s+)?(?:abstract\\s+|final\\s+)?class\\b.*"); | |||
1 task done
Contributor
Author
|
Closing as superseded by #52787. The monolithic broker migration in this PR conflicted with the incremental broker test migrations already merged to |
edewit
added a commit
to edewit/keycloak
that referenced
this pull request
Sep 24, 2026
Add BrokerConfiguration-based broker test infrastructure for first-broker-login scenarios that still need programmatic realm setup, and migrate the detect-existing- user OIDC broker tests as the first batch of the broker package migration for keycloak#50755. Supersedes the monolithic keycloak#51881 approach with a focused follow-up aligned to the incremental broker migrations already on main. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
shawkins
pushed a commit
that referenced
this pull request
Oct 2, 2026
…52787) * Migrate KcOidcFirstBrokerLoginDetectExistingUserTest to tests/base Add BrokerConfiguration-based broker test infrastructure for first-broker-login scenarios that still need programmatic realm setup, and migrate the detect-existing- user OIDC broker tests as the first batch of the broker package migration for #50755. Supersedes the monolithic #51881 approach with a focused follow-up aligned to the incremental broker migrations already on main. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> * Address Copilot review feedback for broker test infrastructure Initialize identity-provider and protocol-mapper config maps before use, fail fast when the provider broker client is missing, use ThreadLocal for broker server-root state, and tidy BrokerTestConstants modifier order. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> * Reuse AbstractKcOidcBrokerTest for detect-existing broker login test Drop the parallel BrokerConfiguration-based test hierarchy and unused helpers, addressing review feedback about duplication and dead code. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> --------- Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.