Migrate OIDC broker config-variant and parameter-forwarding tests by sguilhen · Pull Request #52372 · keycloak/keycloak

sguilhen · 2026-09-03T12:55:35Z

Migrates the OIDC config-variant and parameter-forwarding broker tests to the new JUnit 5 framework, landing in tests/base/.../broker/oidc/. These are config-only variants with no own @Test methods - they exercise the two inherited AbstractKcOidcBrokerTest login tests under different IdP configurations.

Two commits, following the project's usual move-then-adapt pattern (via tests/migration-util/commit-migration.sh) so the actual content rewrite is reviewable on its own:

Pure rename - the 8 legacy Arquillian tests move directly from testsuite/integration-arquillian/.../testsuite/broker/ to tests/base/.../tests/broker/oidc/ with zero content changes (git-recognized 100% renames). The two phase-1 classes already in the new suite (KcOidcBrokerTest, KcOidcBrokerClientSecretBasicAuthTest) also relocate from the flat broker/ package into the oidc/ package this batch establishes.
Content adaptation - rewrites all 10 files to the new-testsuite abstract-class baseline and updates the shared support classes.

Ported from the broker-test-migration-phase-2 branch and adapted to the merged phase-1 baseline (#51859), which replaced the original interfaces-with-default-methods design with the abstract-class hierarchy. Concrete classes now extend AbstractKcOidcBrokerTest instead of implements BrokerLoginTest, KcOidcBrokerConfigSupport, dropping the oauth/webDriver/loginPage/updateProfilePage getter boilerplate (inherited fields) and the provider/consumer realm getters where the default config applies - only KcOidcBrokerUiLocalesWithIdpHintTest needs both realms customized, so it shadows both fields with no getter overrides.

Two small additive extractions on the shared base (not specific to this batch, needed by several of these variants):

AbstractBrokerLoginTest.assertUserCreatedInConsumerRealm(): the post-login assertion block was inlined into loginUser() during the abstract-class rework; pulled back out into its own protected method so tests overriding loginUser() with a custom pre-login step can reuse it.
AbstractKcOidcBrokerTest.createDefaultProviderClient(): the provider client construction (with the legacy-fidelity-fixed protocol mappers) was inlined into OidcProviderRealmConfig; extracted so KcOidcBrokerUiLocalesWithIdpHintTest's custom provider realm config can reuse it instead of duplicating it.
createOidcIdentityProvider()/configureConsumerRealm()/createDefaultProviderClient() widened from package-private to protected: this is the first batch to place concrete classes in the oidc/ subpackage (previously everything lived at the broker/ root), so cross-package subclass access needs protected visibility.

The 8 legacy classes carry no @Test methods of their own (verified 0 @Test annotations, no subclasses, no suite-file references), so they're fully removed rather than left duplicated.

Testing

mvn test -Dtest=KcOidcBrokerAcrParameterTest,KcOidcBrokerNoLoginHintTest,KcOidcBrokerParameterForwardTest,KcOidcBrokerPkceTest,KcOidcBrokerPromptParameterTest,KcOidcBrokerUiLocalesDisabledTest,KcOidcBrokerUiLocalesEnabledTest,KcOidcBrokerUiLocalesWithIdpHintTest,KcOidcBrokerTest,KcOidcBrokerClientSecretBasicAuthTest,KcSamlBrokerTest - 22/22 green
mvn spotless:check clean
Confirmed no remaining source references to the 8 removed legacy classes

Closes #52242

Copilot

🟢 Approval recommended

The migrations preserve the legacy scenarios, reuse the shared framework correctly, and have focused passing coverage.

Pull request overview

Migrates eight OIDC broker configuration variants to the JUnit 5 test framework while reusing shared broker setup and assertions.

Changes:

Adds OIDC parameter-forwarding, PKCE, prompt, login-hint, and locale variants.
Extracts reusable provider-client configuration and post-login assertions.
Widens shared configuration helpers for subclass access.

File summaries

File	Description
`AbstractBrokerLoginTest.java`	Extracts reusable consumer-user assertions.
`AbstractKcOidcBrokerTest.java`	Exposes configuration helpers and extracts provider-client creation.
`KcOidcBrokerAcrParameterTest.java`	Tests ACR forwarding.
`KcOidcBrokerNoLoginHintTest.java`	Tests disabled login-hint forwarding.
`KcOidcBrokerParameterForwardTest.java`	Tests configured parameter forwarding and encoding.
`KcOidcBrokerPkceTest.java`	Adds the PKCE-enabled broker variant.
`KcOidcBrokerPromptParameterTest.java`	Tests client prompt forwarding.
`KcOidcBrokerUiLocalesDisabledTest.java`	Tests disabled locale forwarding.
`KcOidcBrokerUiLocalesEnabledTest.java`	Tests enabled locale forwarding.
`KcOidcBrokerUiLocalesWithIdpHintTest.java`	Tests Hungarian locale propagation with an IdP hint.

Review details

Files reviewed: 10/10 changed files
Comments generated: 0
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

🟢 Approval recommended

The migration preserves variant behavior, shared setup, inherited coverage, and removes the corresponding legacy tests cleanly.

Review details

Files reviewed: 20/20 changed files
Comments generated: 0 new
Review effort level: Balanced

@test

Migrate KcOidcBrokerIdpHintTest (4 own tests), KcOidcBrokerHiddenIdpHintTest (1 own test), and KcOidcBrokerNonceParameterTest (2 own tests, including a new testNonceSet that verifies the nonce is present in the token response - legacy only had testNonceNotSet). Adapted to the merged abstract-class baseline: concrete classes extend AbstractKcOidcBrokerTest instead of implementing KcOidcBrokerConfigSupport directly (the phase-2 branch deliberately skipped BrokerLoginTest here since these classes bring their own @test methods and didn't want the generic login tests as extra, possibly-conflicting coverage). The current abstract hierarchy bakes testLogInAsUserInIDP()/testLoginWithExistingUser() into AbstractBrokerLoginTest, which AbstractKcOidcBrokerTest always extends, so there's no way to pull in just the OIDC config layer without them: - KcOidcBrokerIdpHintTest and KcOidcBrokerNonceParameterTest use configs compatible with the inherited login tests (default or purely additive), so they run as harmless extra coverage. - KcOidcBrokerHiddenIdpHintTest's IDP is hidden from the login page, which would break the inherited tests' logInWithBroker() (clicks a social button that isn't rendered) - overridden and @disabled with a reason, mirroring the existing KcOidcBrokerSubMatchIntrospectionTest pattern, rather than restructuring the shared hierarchy for one test class. Added AbstractBrokerTest.disableUpdateProfileOnFirstLogin(): both nonce tests need to skip the first-broker-login review-profile interruption to test the nonce flow directly. Not present in the merged base (a phase-1 ripple fix that predates the abstract-class rework); re-added here in the form the phase-2 branch already converged on the phase-1 fix (disables both the review-profile flow step and the consumer's VERIFY_PROFILE required action, mirroring the legacy suite which never enabled VERIFY_PROFILE). Also carries forward two fixes independently re-derived per batch since this branch is off upstream/main: createOidcIdentityProvider()/ configureConsumerRealm() widened to protected (oidc/ subpackage cross-package access, same as keycloak#52372/keycloak#52376), and the configureBrokerEndpoints() provider- client lookup fixed to match by placeholder redirect URI instead of the fixed CLIENT_ID (same fix as keycloak#52376). Closes keycloak#52244 Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

@test

Migrate KcOidcBrokerIdpHintTest (4 own tests), KcOidcBrokerHiddenIdpHintTest (1 own test), and KcOidcBrokerNonceParameterTest (2 own tests, including a new testNonceSet that verifies the nonce is present in the token response - legacy only had testNonceNotSet). Adapted to the merged abstract-class baseline: concrete classes extend AbstractKcOidcBrokerTest instead of implementing KcOidcBrokerConfigSupport directly (the phase-2 branch deliberately skipped BrokerLoginTest here since these classes bring their own @test methods and didn't want the generic login tests as extra, possibly-conflicting coverage). The current abstract hierarchy bakes testLogInAsUserInIDP()/testLoginWithExistingUser() into AbstractBrokerLoginTest, which AbstractKcOidcBrokerTest always extends, so there's no way to pull in just the OIDC config layer without them: - KcOidcBrokerIdpHintTest and KcOidcBrokerNonceParameterTest use configs compatible with the inherited login tests (default or purely additive), so they run as harmless extra coverage. - KcOidcBrokerHiddenIdpHintTest's IDP is hidden from the login page, which would break the inherited tests' logInWithBroker() (clicks a social button that isn't rendered) - overridden and @disabled with a reason, mirroring the existing KcOidcBrokerSubMatchIntrospectionTest pattern, rather than restructuring the shared hierarchy for one test class. Added AbstractBrokerTest.disableUpdateProfileOnFirstLogin(): both nonce tests need to skip the first-broker-login review-profile interruption to test the nonce flow directly. Not present in the merged base (a phase-1 ripple fix that predates the abstract-class rework); re-added here in the form the phase-2 branch already converged on the phase-1 fix (disables both the review-profile flow step and the consumer's VERIFY_PROFILE required action, mirroring the legacy suite which never enabled VERIFY_PROFILE). Also carries forward two fixes independently re-derived per batch since this branch is off upstream/main: createOidcIdentityProvider()/ configureConsumerRealm() widened to protected (oidc/ subpackage cross-package access, same as #52372/#52376), and the configureBrokerEndpoints() provider- client lookup fixed to match by placeholder redirect URI instead of the fixed CLIENT_ID (same fix as #52376). Closes #52244 Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

Copilot

🟢 Approval recommended

The migrated configurations preserve the legacy scenarios, reuse the new test hierarchy correctly, and pass the focused test suite.

Review details

Files reviewed: 19/19 changed files
Comments generated: 0 new
Review effort level: Balanced

@test

Migrate KcOidcBrokerClientSecretJwtTest, KcOidcBrokerClientSecretJwtCustomSignAlgTest, KcOidcBrokerColonAliasClientSecretBasicAuthTest, KcOidcBrokerPrivateKeyJwtTest, KcOidcBrokerPrivateKeyJwtCustomSignAlgTest, KcOidcBrokerPrivateKeyJwtCustomAudienceTest, KcOidcBrokerPrivateKeyJwtWithX509HeadersTest, and KcOidcBrokerSubMatchIntrospectionTest. Config-only variants except KcOidcBrokerSubMatchIntrospectionTest, which overrides testLogInAsUserInIDP and disables testLoginWithExistingUser (renamed from loginWithExistingUser per phase-1's 2nd review round - the base method was renamed and any override must follow or it silently detaches into a non-@test method). Adapted to the merged abstract-class baseline: concrete classes extend AbstractKcOidcBrokerTest instead of implementing BrokerLoginTest + KcOidcBrokerConfigSupport, dropping the oauth/webDriver/loginPage/ updateProfilePage getter boilerplate and the realm getters. Two of the eight classes (PrivateKeyJwtCustomAudienceTest, PrivateKeyJwtWithX509HeadersTest) reuse KcOidcBrokerPrivateKeyJwtTest's package-private nested PrivateKeyJwtProviderRealmConfig directly, which needs no visibility change since all three classes now live in the same oidc/ package. Two additive extractions on AbstractKcOidcBrokerTest, mirroring the same provider/consumer decomposition the phase-2 branch's KcOidcBrokerConfigSupport already had (independently re-derived here since this branch is based on upstream/main, not on the not-yet-merged keycloak#52372 which added the first one): - createDefaultProviderClient(): same extraction as keycloak#52372, needed here too since half these tests build a customized provider client (different secret/auth method) while reusing the default mapper set. - configureProviderRealm(RealmBuilder, ClientBuilder): new - the realm-level scaffolding (name, event listener, default user) factored out from OidcProviderRealmConfig so a customized client can reuse it instead of duplicating the realm setup. Bugfix in AbstractKcOidcBrokerTest.configureBrokerEndpoints() (pre-existing, not introduced by this batch): the provider-side client redirect-URI rewrite looked up the client by the fixed CLIENT_ID ("brokerapp"), which silently no-ops for any test that creates the provider client under a different clientId - exactly what KcOidcBrokerColonAliasClientSecretBasicAuthTest does to mirror its legacy counterpart's colon-containing client ID. Legacy handled this via a getIDPClientIdInProviderRealm() override precisely because the same lookup-by-fixed-id problem exists there. Fixed by matching on the client's seeded placeholder redirect URI instead of its clientId, which every provider client created via createDefaultProviderClient() carries regardless of what clientId a test assigns it. Caught by actually running this test end-to-end against the new-testsuite framework, not by inspection. Closes keycloak#52243 Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

Migrate KcOidcBrokerJWETest (1 own test), KcOidcBrokerJWEUserInfoJustEncryptedTest, and all 8 ECDH-ES variant tests, all landing in oidc/. Architecture change from the phase-2 branch: the phase-2/interim design used interface traits (JweBrokerConfigSupport, EcdhEsJweBrokerConfigSupport, JweUserInfoBrokerTest) because that's how the whole shared support layer was built at the time. Converted to a 2-layer abstract-class extension instead - AbstractKcOidcJweBrokerTest extends AbstractKcOidcBrokerTest (JWE key/algorithm config + the shared testIdentityClaimsFromUserInfoEndpoint()), and AbstractKcOidcEcdhEsJweBrokerTest extends AbstractKcOidcJweBrokerTest (ECDH-specific key overrides + abstract getCurve()) - rather than keeping them as interfaces mixed in alongside BrokerLoginTest/EcdhEsJweBrokerConfigSupport. Re-examined whether this is genuinely a "multi-axis" case (the documented reason interfaces are reserved over abstract classes elsewhere in this migration): it isn't. Every one of the 10 variants wants the full stack - protocol config, basic login tests, and JWE support - in the same order, with no case needing to skip a layer (unlike e.g. KcOidcBrokerHiddenIdpHintTest in chain expresses this without forcing AbstractBr 50E0 okerTest's protected fields/methods (oauth, webDriver, logInAsUserInIDP(), etc.) to be widened to public just so interface default methods could call them - interface methods are implicitly public, and a protected inherited method does not satisfy a public abstract interface method, so keeping these as interfaces against the current abstract-class baseline would have forced that wider visibility change. Concrete classes drop straight to their per-variant algorithm/curve overrides (4 one-line methods, or none at all for the RSA base case). Also removes the legacy AbstractKcOidcBrokerJWEEcdhEsTest shared base (all 8 subclasses migrated) and drops all 10 now-migrated class names from the legacy fips-suite list (they are not yet added to the new-testsuite FIPS suites - already tracked separately as deferred pending a test-framework HTTPS-trust fix, per the migration plan's Infrastructure section). Found and fixed another real gap in the merged AbstractBrokerTest (same category as keycloak#52244's disableUpdateProfileOnFirstLogin): enableUnmanagedAttributes() was dropped during the abstract-class rework. Without it, the declarative user profile silently drops attributes outside its managed schema - exactly what this batch's IdP mapper does (maps a custom OIDC claim onto a "user-info" user attribute), so testIdentityClaimsFromUserInfoEndpoint failed identically across all 10 variants until this was restored. Caught by running the tests, not by inspection. Carries forward the visibility widening + configureBrokerEndpoints() and assertUserCreatedInConsumerRealm() fixes from keycloak#52372/keycloak#52376/keycloak#52379 (independently re-derived per batch, this branch is off upstream/main). Closes keycloak#52245 Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

Copilot

🟢 Approval recommended

The migrated variants preserve their legacy scenarios, reuse the established test hierarchy, and have focused passing coverage.

Review details

Files re BAF0 viewed: 20/20 changed files
Comments generated: 0 new
Review effort level: Balanced

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-09-04T17:09:22Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.federation.ldap.LDAPUserLoginTest#loginLDAPUserAuthenticationNoneEncryptionSSL

Keycloak CI - Java Distribution IT (windows-latest - temurin - 21)

java.lang.IllegalStateException: User search expected one result. Found 0 users.
	at org.keycloak.testsuite.AbstractTestRealmKeycloakTest.findUser(AbstractTestRealmKeycloakTest.java:57)
	at org.keycloak.testsuite.federation.ldap.LDAPUserLoginTest.verifyLoginSucceededAndLogout(LDAPUserLoginTest.java:148)
	at org.keycloak.testsuite.federation.ldap.LDAPUserLoginTest.runLDAPLoginTest(LDAPUserLoginTest.java:190)
	at org.keycloak.testsuite.federation.ldap.LDAPUserLoginTest.loginLDAPUserAuthenticationNoneEncryptionSSL(LDAPUserLoginTest.java:292)
...

Report flaky test

Migrate KcOidcBrokerJWETest (1 own test), KcOidcBrokerJWEUserInfoJustEncryptedTest, and all 8 ECDH-ES variant tests, all landing in oidc/. Architecture change from the phase-2 branch: the phase-2/interim design used interface traits (JweBrokerConfigSupport, EcdhEsJweBrokerConfigSupport, JweUserInfoBrokerTest) because that's how the whole shared support layer was built at the time. Converted to a 2-layer abstract-class extension instead - AbstractKcOidcJweBrokerTest extends AbstractKcOidcBrokerTest (JWE key/algorithm config + the shared testIdentityClaimsFromUserInfoEndpoint()), and AbstractKcOidcEcdhEsJweBrokerTest extends AbstractKcOidcJweBrokerTest (ECDH-specific key overrides + abstract getCurve()) - rather than keeping them as interfaces mixed in alongside BrokerLoginTest/EcdhEsJweBrokerConfigSupport. Re-examined whether this is genuinely a "multi-axis" case (the documented reason interfaces are reserved over abstract classes elsewhere in this migration): it isn't. Every one of the 10 variants wants the full stack - protocol config, basic login tests, and JWE support - in the same order, with no case needing to skip a layer (unlike e.g. KcOidcBrokerHiddenIdpHintTest in chain expresses this without forcing AbstractBrokerTest's protected fields/methods (oauth, webDriver, logInAsUserInIDP(), etc.) to be widened to public just so interface default methods could call them - interface methods are implicitly public, and a protected inherited method does not satisfy a public abstract interface method, so keeping these as interfaces against the current abstract-class baseline would have forced that wider visibility change. Concrete classes drop straight to their per-variant algorithm/curve overrides (4 one-line methods, or none at all for the RSA base case). Also removes the legacy AbstractKcOidcBrokerJWEEcdhEsTest shared base (all 8 subclasses migrated) and drops all 10 now-migrated class names from the legacy fips-suite list (they are not yet added to the new-testsuite FIPS suites - already tracked separately as deferred pending a test-framework HTTPS-trust fix, per the migration plan's Infrastructure section). Found and fixed another real gap in the merged AbstractBrokerTest (same category as #52244's disableUpdateProfileOnFirstLogin): enableUnmanagedAttributes() was dropped during the abstract-class rework. Without it, the declarative user profile silently drops attributes outside its managed schema - exactly what this batch's IdP mapper does (maps a custom OIDC claim onto a "user-info" user attribute), so testIdentityClaimsFromUserInfoEndpoint failed identically across all 10 variants until this was restored. Caught by running the tests, not by inspection. Carries forward the visibility widening + configureBrokerEndpoints() and assertUserCreatedInConsumerRealm() fixes from #52372/#52376/#52379 (independently re-derived per batch, this branch is off upstream/main). Closes #52245 Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

@test

Migrate KcOidcBrokerClientSecretJwtTest, KcOidcBrokerClientSecretJwtCustomSignAlgTest, KcOidcBrokerColonAliasClientSecretBasicAuthTest, KcOidcBrokerPrivateKeyJwtTest, KcOidcBrokerPrivateKeyJwtCustomSignAlgTest, KcOidcBrokerPrivateKeyJwtCustomAudienceTest, KcOidcBrokerPrivateKeyJwtWithX509HeadersTest, and KcOidcBrokerSubMatchIntrospectionTest. Config-only variants except KcOidcBrokerSubMatchIntrospectionTest, which overrides testLogInAsUserInIDP and disables testLoginWithExistingUser (renamed from loginWithExistingUser per phase-1's 2nd review round - the base method was renamed and any override must follow or it silently detaches into a non-@test method). Adapted to the merged abstract-class baseline: concrete classes extend AbstractKcOidcBrokerTest instead of implementing BrokerLoginTest + KcOidcBrokerConfigSupport, dropping the oauth/webDriver/loginPage/ updateProfilePage getter boilerplate and the realm getters. Two of the eight classes (PrivateKeyJwtCustomAudienceTest, PrivateKeyJwtWithX509HeadersTest) reuse KcOidcBrokerPrivateKeyJwtTest's package-private nested PrivateKeyJwtProviderRealmConfig directly, which needs no visibility change since all three classes now live in the same oidc/ package. Two additive extractions on AbstractKcOidcBrokerTest, mirroring the same provider/consumer decomposition the phase-2 branch's KcOidcBrokerConfigSupport already had (independently re-derived here since this branch is based on upstream/main, not on the not-yet-merged #52372 which added the first one): - createDefaultProviderClient(): same extraction as #52372, needed here too since half these tests build a customized provider client (different secret/auth method) while reusing the default mapper set. - configureProviderRealm(RealmBuilder, ClientBuilder): new - the realm-level scaffolding (name, event listener, default user) factored out from OidcProviderRealmConfig so a customized client can reuse it instead of duplicating the realm setup. Bugfix in AbstractKcOidcBrokerTest.configureBrokerEndpoints() (pre-existing, not introduced by this batch): the provider-side client redirect-URI rewrite looked up the client by the fixed CLIENT_ID ("brokerapp"), which silently no-ops for any test that creates the provider client under a different clientId - exactly what KcOidcBrokerColonAliasClientSecretBasicAuthTest does to mirror its legacy counterpart's colon-containing client ID. Legacy handled this via a getIDPClientIdInProviderRealm() override precisely because the same lookup-by-fixed-id problem exists there. Fixed by matching on the client's seeded placeholder redirect URI instead of its clientId, which every provider client created via createDefaultProviderClient() carries regardless of what clientId a test assigns it. Caught by actually running this test end-to-end against the new-testsuite framework, not by inspection. Closes #52243 Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

…ite oidc package Pure git rename of the eight legacy config-variant and parameter-forwarding broker tests into the new testsuite oidc package. The two phase-1 tests already in the new suite (KcOidcBrokerTest, KcOidcBrokerClientSecretBasicAuthTest) also move from the flat broker package into oidc/. No content changes; the new-testsuite adaptation follows in the next commit. Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

Migrate KcOidcBrokerAcrParameterTest, KcOidcBrokerNoLoginHintTest, KcOidcBrokerParameterForwardTest, KcOidcBrokerPkceTest, KcOidcBrokerPromptParameterTest, KcOidcBrokerUiLocalesDisabledTest, KcOidcBrokerUiLocalesEnabledTest, and KcOidcBrokerUiLocalesWithIdpHintTest. These are all config-only variants with no own test methods - they run the two inherited AbstractKcOidcBrokerTest login tests with different IDP configurations. Also relocates the two phase-1 classes already in the new suite (KcOidcBrokerTest, KcOidcBrokerClientSecretBasicAuthTest) into the oidc/ package established by this batch, fixing their package declaration and adding the now-needed cross-package import of AbstractKcOidcBrokerTest. Adapted to the merged abstract-class baseline (AbstractBrokerTest / AbstractBrokerLoginTest / AbstractKcOidcBrokerTest) rather than the interfaces-with-default-methods design the phase-2 branch this content was sourced from was originally built against: concrete classes extend AbstractKcOidcBrokerTest instead of implementing BrokerLoginTest + KcOidcBrokerConfigSupport, dropping the oauth/webDriver/loginPage/ updateProfilePage getter boilerplate entirely (inherited fields) and the provider/consumer realm getters where the default config is used - only KcOidcBrokerUiLocalesWithIdpHintTest needs both realms customized, so it shadows both fields with no getter overrides needed. Two small additive extractions on the shared base, needed by several of these variants and not specific to this batch: - AbstractBrokerLoginTest.assertUserCreatedInConsumerRealm(): the post-login assertion block was inlined into loginUser() during the abstract-class rework; pulled back out into its own protected method so tests overriding loginUser() with a custom pre-login step can still reuse it. - AbstractKcOidcBrokerTest.createDefaultProviderClient(): the provider client construction (with the legacy-fidelity-fixed protocol mappers) was inlined into OidcProviderRealmConfig; extracted so KcOidcBrokerUiLocalesWithIdpHintTest's custom provider realm config can reuse the same client instead of duplicating it. - createOidcIdentityProvider()/configureConsumerRealm()/ createDefaultProviderClient() widened from package-private to protected: they were only callable from same-package subclasses before (all concrete classes lived at the broker/ root); this batch is the first to place concrete classes in the oidc/ subpackage, so cross-package subclass access needs protected visibility. Closes keycloak#52242 Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

Copilot

🟢 Approval recommended

The migrated variants preserve their legacy behavior, reuse the JUnit 5 broker infrastructure correctly, and have focused passing coverage.

Review details

Files reviewed: 18/18 changed files
Comments generated: 0 new
Review effort level: Balanced

Copilot AI balanced review requested due to automatic review settings September 3, 2026 12:55

sguilhen requested a review from a team as a code owner September 3, 2026 12:55

keycloak-github-bot Bot added team/continuous-delivery team/core-iam labels Sep 3, 2026

Copilot started reviewing on behalf of sguilhen September 3, 2026 13:07

View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 3, 2026 13:19

sguilhen force-pushed the 52242-broker-oidc-config-variants branch from 6252856 to 4f49095 Compare September 3, 2026 13:19

sguilhen requested review from a team as code owners September 3, 2026 13:19

Copilot started reviewing on behalf of sguilhen September 3, 2026 13:34

View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

pedroigor previously approved these changes Sep 4, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 4, 2026 15:58

sguilhen force-pushed the 52242-broker-oidc-config-variants branch from 4f49095 to b671d77 Compare September 4, 2026 15:58

Copilot started reviewing on behalf of sguilhen September 4, 2026 15:58

View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 4, 2026 16:13

sguilhen dismissed pedroigor’s stale review via 690f17f September 4, 2026 16:13

sguilhen force-pushed the 52242-broker-oidc-config-variants branch from b671d77 to 690f17f Compare September 4, 2026 16:13

Copilot started reviewing on behalf of sguilhen September 4, 2026 16:13

View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

keycloak-github-bot Bot added the flaky-test label Sep 4, 2026

keycloak-github-bot Bot reviewed Sep 4, 2026

View reviewed changes

sguilhen added 2 commits September 4, 2026 15:52

Copilot AI review requested due to automatic review settings September 4, 2026 19:11

sguilhen force-pushed the 52242-broker-oidc-config-variants branch from 690f17f to 514a844 Compare September 4, 2026 19:11

Copilot started reviewing on behalf of sguilhen September 4, 2026 19:20

View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

pedroigor approved these changes Sep 8, 2026

View reviewed changes

pedroigor merged commit 965edcc into keycloak:main Sep 8, 2026
94 checks passed

This was referenced Sep 15, 2026

Migrate KcOidcFirstBrokerLoginDetectExistingUserTest to tests/base #52787

Merged

migration broker package #51881

Closed

sguilhen deleted the 52242-broker-oidc-config-variants branch September 21, 2026 18:57

Navigation Menu

Repository navigation

Migrate OIDC broker config-variant and parameter-forwarding tests - #52372

Migrate OIDC broker config-variant and parameter-forwarding tests#52372
pedroigor merged 2 commits into
keycloak:mainfrom
sguilhen:52242-broker-oidc-config-variants

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Uh oh!

Testing

Uh oh!

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Unreported flaky test detected

org.keycloak.testsuite.federation.ldap.LDAPUserLoginTest#loginLDAPUserAuthenticationNoneEncryptionSSL

Uh oh!

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants