Migrate basic broker infrastructure to new testsuite by sguilhen · Pull Request #51859 · keycloak/keycloak

sguilhen · 2026-08-19T13:06:02Z

This is the foundational PR of the broker test migration - moving the broker suite from the legacy Arquillian testsuite (testsuite/integration-arquillian/.../broker) to the new JUnit 5 framework under tests/base/.../broker.

Scope

Kept deliberately small so the shared infrastructure can be reviewed on its own:

The reusable support layer every later batch builds on: AbstractBrokerTest, AbstractBrokerLoginTest, and the protocol bases AbstractKcOidcBrokerTest / AbstractKcSamlBrokerTest (abstract-class hierarchy; concrete tests inherit the injected clients/pages and the @Test login logic). The protocol bases also inject the default provider/consumer realms, so a concrete test using the default config needs no fields or getters at all; a test that needs a different config for one or both realms shadows just the field(s) it varies (same name/type, different @InjectRealm#config) - the framework resolves the shadowed and inherited field to the same deployed realm, so no getter override is needed either way.
Three basic login tests exercising that layer: KcOidcBrokerTest, KcOidcBrokerClientSecretBasicAuthTest, KcSamlBrokerTest.

The design uses abstract classes by default (the new framework injects into class fields, not interfaces, so this removes the getter boilerplate and matches the rest of tests/base); interfaces are kept only for cases that must mix two independent @Test axes (JWE variants, the token-store matrix). This is the pattern the rest of the migration follows, so that is the main thing to scrutinize.

Closes #51954

martin-kanis

@sguilhen thanks, I added few comments/questions. Probably someone from the new framework creators will need to eventually chime in.

Copilot

Pull request overview

Migrates foundational broker login testing from Arquillian to the JUnit 5 test framework for #51954.

Changes:

Adds reusable OIDC/SAML broker configuration and login-test interfaces.
Adds three basic broker integration tests.
Adds broker-related UI page objects.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`BrokerConfigSupport.java`	Shared broker helpers and constants.
`BrokerLoginTest.java`	Reusable broker login/logout tests.
`OidcBrokerConfigSupport.java`	OIDC realm and broker configuration.
`SamlBrokerConfigSupport.java`	SAML realm and broker configuration.
`KcOidcBrokerTest.java`	Basic OIDC broker tests.
`KcOidcBrokerClientSecretBasicAuthTest.java`	OIDC basic-auth variant.
`KcSamlBrokerTest.java`	Basic SAML broker tests.
`IdpConfirmLinkPage.java`	IdP account-link confirmation page object.
`LoginTotpPage.java`	OTP login page object.

Suppressed comments (2)

test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/LoginTotpPage.java:26

This regresses the existing page-object contract where login(null) submits an empty OTP to test validation; Selenium rejects sendKeys(null) before the form is submitted. Preserve null as an empty input.

        otpInput.sendKeys(totp);

test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/LoginTotpPage.java:44

The login template prefixes its login-otp page ID with login-, so the rendered body ID is login-login-otp. With the current value, assertCurrent() always times out on the OTP page.

        return "login-otp";

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

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

+        Map<String, String> config = mapper.getConfig();
+        config.put(ProtocolMapperUtils.USER_ATTRIBUTE, userAttribute);
+        config.put(AttributeStatementHelper.SAML_ATTRIBUTE_NAME, samlAttributeName);
+        config.put(AttributeStatementHelper.SAML_ATTRIBUTE_NAMEFORMAT, nameFormat);
+        if (friendlyName != null) {
+            config.put(AttributeStatementHelper.FRIENDLY_NAME, friendlyName);
+        }


+        String consumerBrokerEndpoint = consumerBaseUrl + "/broker/" + IDP_SAML_ALIAS + "/endpoint";
+        // ManagedRealm#getBaseUrl() is "<serverRoot>/realms/<realm>"; strip the realm suffix to get the
+        // server root that the consumer-side SP client URLs (sales-post) must point at.
+        String consumerServerRoot = consumerBaseUrl.substring(0, consumerBaseUrl.indexOf("/realms/"));


+    }
+
+    private boolean profilePageAppeared() {
+        Set<String> profilePageIds = Set.of("login-login-update-profile", "login-idp-review-user-profile");


Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-08-26T20:36:30Z

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.LDAPGroupMapperTest#test06_addingUserToNewKeycloakGroup

Keycloak CI - Base IT (5)

java.lang.AssertionError: 

Expected: a collection with size <4>
     but: collection size was <3>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
...

Report flaky test

org.keycloak.testsuite.forms.BruteForceTest#testGrantInvalidPassword

Keycloak CI - Forms IT (chrome)

org.opentest4j.AssertionFailedError: Expected error event ==> expected: <true> but was: <false>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
...

Report flaky test

org.keycloak.testsuite.adapter.servlet.SAMLClockSkewAdapterTest#testTokenTimeIsValid

Keycloak CI - Adapter IT Strict Cookies

org.jboss.arquillian.graphene.enricher.exception.PageObjectInitializationException: 
Can not instantiate Page Object 
class org.keycloak.testsuite.adapter.page.SalesPostClockSkewServlet
 declared in: 
org.keycloak.testsuite.adapter.servlet.SAMLClockSkewAdapterTest
...

Report flaky test

martin-kanis

@sguilhen Thanks for all of the iterations. Looks good to me.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Add shared test infrastructure (BrokerConfigSupport, KcOidcBrokerConfigSupport, SamlBrokerConfigSupport, BrokerLoginTest) and migrate KcOidcBrokerTest, KcOidcBrokerClientSecretBasicAuthTest, and KcSamlBrokerTest. The shared OIDC config brokers through the Keycloak-specific "keycloak-oidc" identity provider, matching the legacy KcOidcBrokerConfiguration baseline (with prompt=login and loginHint forwarding); a generic-provider variant can be added where a test actually needs it. The provider user is created without a first/last name, as the legacy suite did, so the imported consumer user is incomplete and the consumer's first-broker-login review-profile page is shown. The default helper asserts that page appears before updating it (mirroring the legacy assertCurrent()), and VERIFY_PROFILE is disabled on the provider realm since the new testsuite enables it by default and would otherwise stop the incomplete user at the provider. A tolerant updateAccountInformationIfPresent() variant remains for flows that intentionally skip profile review. The login flows run against the migrated broker-app client (via BrokerAppClientConfig) rather than the framework default test-app, and the OIDC provider-side client carries the legacy broker callback configuration (redirect/admin/backchannel-logout URLs, post-logout redirect URIs) rewritten from the consumer base URL, mirroring the SAML base. The legacy test classes are retained since they carry additional test methods not yet covered by this migration. Per Copilot review: the protocol bases (AbstractKcOidcBrokerTest / AbstractKcSamlBrokerTest) now inject the default provider/consumer realms and implement getProviderRealm()/getConsumerRealm() once, instead of leaving them abstract for every concrete test to implement. A concrete test using the default config needs no realm fields or getters at all; a test that needs a different config for one side shadows just that field (same name/type, different @InjectRealm#config) - the framework resolves the shadowed and inherited field to the same deployed realm, so no getter override is needed either way (same pattern already used by AccountRestServiceLightweightTokenTest#managedRealm). Closes keycloak#51954 Signed-off-by: Stefan Guilhen <sguilhen@redhat.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

sguilhen requested a review from martin-kanis August 19, 2026 13:06

martin-kanis reviewed Aug 20, 2026

View reviewed changes

sguilhen force-pushed the broker-test-migration branch from 729d613 to c2bc943 Compare August 20, 2026 12:57

sguilhen requested a review from martin-kanis August 20, 2026 13:22

sguilhen force-pushed the broker-test-migration branch 2 times, most recently from 938cdcc to 1917b99 Compare August 21, 2026 18:58

sguilhen changed the title ~~WIP: Migrate basic broker login tests to new testsuite~~ Migrate basic broker infrastructure to new testsuite Aug 21, 2026

This was referenced Aug 21, 2026

Migrate OIDC/SAML broker config-variant and infrastructure-dependent tests to the new testsuite #51949

Closed

Migrate basic broker infrastructure and login tests #51954

Closed

sguilhen force-pushed the broker-test-migration branch 2 times, most recently from ffd075c to 4f10e2b Compare August 23, 2026 16:50

keycloak-github-bot Bot added the flaky-test label Aug 23, 2026

keycloak-github-bot Bot mentioned this pull request Aug 23, 2026

Flaky test: org.keycloak.testsuite.broker.KcOidcBrokerTest#loginWithExistingUserWithBruteForceEnabled #51600

Closed

sguilhen marked this pull request as ready for review August 24, 2026 12:17

sguilhen requested a review from a team as a code owner August 24, 2026 12:17

Copilot AI balanced review requested due to automatic review settings August 24, 2026 12:17

Copilot started reviewing on behalf of sguilhen August 24, 2026 12:17

View session

Copilot AI reviewed Aug 24, 2026

View reviewed changes

Comment thread test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/IdpConfirmLinkPage.java Resolved

Comment thread test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/LoginTotpPage.java Resolved

Copilot AI review requested due to automatic review settings August 25, 2026 12:44

sguilhen force-pushed the broker-test-migration branch from 4f10e2b to 5fb6603 Compare August 25, 2026 12:44

Copilot started reviewing on behalf of sguilhen August 25, 2026 12:57

View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Comment thread tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerConfigSupport.java Resolved

Comment thread tests/base/src/test/java/org/keycloak/tests/broker/AbstractBrokerTest.java Resolved

Copilot AI review requested due to automatic review settings August 25, 2026 14:13

sguilhen force-pushed the broker-test-migration branch from 5fb6603 to 7b3578f Compare August 25, 2026 14:13

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Copilot started reviewing on behalf of sguilhen August 25, 2026 14:33

View session

Copilot AI review requested due to automatic review settings August 25, 2026 16:23

sguilhen force-pushed the broker-test-migration branch from 7b3578f to b163149 Compare August 25, 2026 16:23

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 25, 2026 17:06

Copilot AI reviewed Aug 26, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 26, 2026 19:09

Copilot started reviewing on behalf of sguilhen August 26, 2026 19:10

View session

Copilot started reviewing on behalf of sguilhen August 26, 2026 19:17

View session

Copilot AI reviewed Aug 26, 2026

View reviewed changes

keycloak-github-bot Bot reviewed Aug 26, 2026

View reviewed changes

martin-kanis approved these changes Aug 26, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 27, 2026 12:45

sguilhen force-pushed the broker-test-migration branch from a0d4f31 to f858e3f Compare August 27, 2026 12:45

Copilot started reviewing on behalf of sguilhen August 27, 2026 12:46

View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

Comment thread tests/base/src/test/java/org/keycloak/tests/broker/AbstractBrokerTest.java Resolved

Copilot AI review requested due to automatic review settings August 31, 2026 12:10

pedroigor force-pushed the broker-test-migration branch from f858e3f to 408146b Compare August 31, 2026 12:10

Copilot started reviewing on behalf of pedroigor August 31, 2026 12:11

View session

Copilot AI reviewed Aug 31, 2026

View reviewed changes

Comment thread tests/base/src/test/java/org/keycloak/tests/broker/AbstractBrokerTest.java Resolved

Copilot AI review requested due to automatic review settings August 31, 2026 12:59

sguilhen force-pushed the broker-test-migration branch from 408146b to 8639b57 Compare August 31, 2026 12:59

Copilot started reviewing on behalf of sguilhen August 31, 2026 13:03

View session

Copilot AI reviewed Aug 31, 2026

View reviewed changes

martin-kanis approved these changes Sep 1, 2026

View reviewed changes

pedroigor approved these changes Sep 1, 2026

View reviewed changes

pedroigor merged commit f277758 into keycloak:main Sep 1, 2026
151 of 153 checks passed

This was referenced Sep 3, 2026

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

Merged

Migrate OIDC client-authentication and sub-mismatch broker tests #52376

Merged

Migrate KcOidcBrokerIdpPublicKeyMissingUseTest and port the MissingUseJwks provider #52383

Merged

This was referenced Sep 15, 2026

Migrate KcOidcFirstBrokerLoginDetectExistingUserTest to tests/base #52787

Merged

migration broker package #51881

Closed

sguilhen deleted the broker-test-migration branch September 21, 2026 18:58

Conversation

sguilhen commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Scope

Uh oh!

martin-kanis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot Bot commented Aug 26, 2026

Unreported flaky test detected

org.keycloak.testsuite.federation.ldap.LDAPGroupMapperTest#test06_addingUserToNewKeycloakGroup

org.keycloak.testsuite.forms.BruteForceTest#testGrantInvalidPassword

org.keycloak.testsuite.adapter.servlet.SAMLClockSkewAdapterTest#testTokenTimeIsValid

Uh oh!

martin-kanis left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Uh oh!

sguilhen commented Aug 19, 2026 •

edited

Loading