Skip to content

Migrate JWE and ECDH-ES broker tests - #52382

Merged
pedroigor merged 2 commits into
keycloak:mainfrom
sguilhen:52245-broker-oidc-jwe-ecdh-es
Sep 4, 2026
Merged

Migrate JWE and ECDH-ES broker tests#52382
pedroigor merged 2 commits into
keycloak:mainfrom
sguilhen:52245-broker-oidc-jwe-ecdh-es

Conversation

@sguilhen

@sguilhen sguilhen commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Migrates the JWE and ECDH-ES token-encryption broker tests to the new JUnit 5 framework, landing in tests/base/.../broker/oidc/.

Two commits, following the project's move-then-adapt pattern:

  1. Pure rename - the 10 legacy Arquillian tests move directly to tests/base/.../tests/broker/oidc/ with zero content changes.
  2. Content adaptation - rewrites all 10 files, adds shared JWE support, removes the legacy shared abstract base and fips-suite entries.

Other changes

  • Removes the legacy AbstractKcOidcBrokerJWEEcdhEsTest shared base (all 8 subclasses migrated) and drops all 10 migrated class names from the legacy fips-suite list. They're not yet in the new-testsuite FIPS suites - already tracked as deferred pending a test-framework HTTPS-trust fix (see the migration plan's Infrastructure section), not something this PR changes.
  • Found and fixed another real gap in the merged AbstractBrokerTest: 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). 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() fix, and assertUserCreatedInConsumerRealm() extraction from Migrate OIDC broker config-variant and parameter-forwarding tests #52372/Migrate OIDC client-authentication and sub-mismatch broker tests #52376/Migrate IdP-hint and nonce-parameter broker tests #52379 (independently re-derived, this branch is off upstream/main).

Testing

  • mvn test -Dtest=KcOidcBrokerJWETest,KcOidcBrokerJWEUserInfoJustEncryptedTest,KcOidcBrokerJWEEcdhEsA128KwP256A128CbcHs256Test,KcOidcBrokerJWEEcdhEsA128KwP256A128GcmTest,KcOidcBrokerJWEEcdhEsA192KwP384A192CbcHs384Test,KcOidcBrokerJWEEcdhEsA192KwP384A192GcmTest,KcOidcBrokerJWEEcdhEsA256KwP521A256CbcHs512Test,KcOidcBrokerJWEEcdhEsA256KwP521A256GcmTest,KcOidcBrokerJWEEcdhEsP384A192CbcHs384Test,KcOidcBrokerJWEEcdhEsP384A192GcmTest - 30/30 green
  • Re-ran KcOidcBrokerTest, KcOidcBrokerClientSecretBasicAuthTest, KcSamlBrokerTest (phase-1 baseline) - 6/6 green
  • mvn spotless:check clean

Closes #52245

Copilot AI balanced review requested due to automatic review settings September 3, 2026 14:21
@sguilhen
sguilhen requested review from a team as code owners September 3, 2026 14:21
@sguilhen sguilhen mentioned this pull request Sep 3, 2026
Merged
Copilot AI reviewed Sep 3, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The migration preserves the legacy algorithm matrix and shared behavior with focused passing coverage.

Pull request overview

Migrates ten JWE and ECDH-ES OIDC broker tests from Arquillian to the JUnit 5 test framework.

Changes:

  • Adds shared abstract classes for JWE configuration, ECDH key setup, and encrypted UserInfo testing.
  • Ports all ten algorithm variants while preserving their legacy configurations.
  • Restores shared broker setup and removes obsolete legacy/FIPS entries.
File summaries
File Description
testsuite/integration-arquillian/tests/base/testsuites/fips-suite Removes migrated legacy tests.
testsuite/.../broker/KcOidcBrokerJWEUserInfoJustEncryptedTest.java Removes legacy test.
testsuite/.../broker/KcOidcBrokerJWETest.java Removes legacy JWE base test.
testsuite/.../broker/KcOidcBrokerJWEEcdhEsP384A192GcmTest.java Removes legacy variant.
testsuite/.../broker/KcOidcBrokerJWEEcdhEsP384A192CbcHs384Test.java Removes legacy variant.
testsuite/.../broker/KcOidcBrokerJWEEcdhEsA256KwP521A256GcmTest.java Removes legacy variant.
testsuite/.../broker/KcOidcBrokerJWEEcdhEsA256KwP521A256CbcHs512Test.java Removes legacy variant.
testsuite/.../broker/KcOidcBrokerJWEEcdhEsA192KwP384A192GcmTest.java Removes legacy variant.
testsuite/.../broker/KcOidcBrokerJWEEcdhEsA192KwP384A192CbcHs384Test.java Removes legacy variant.
testsuite/.../broker/KcOidcBrokerJWEEcdhEsA128KwP256A128GcmTest.java Removes legacy variant.
testsuite/.../broker/KcOidcBrokerJWEEcdhEsA128KwP256A128CbcHs256Test.java Removes legacy variant.
testsuite/.../broker/AbstractKcOidcBrokerJWEEcdhEsTest.java Removes obsolete shared base.
tests/base/.../oidc/KcOidcBrokerJWEUserInfoJustEncryptedTest.java Adds encrypted-only JUnit 5 variant.
tests/base/.../oidc/KcOidcBrokerJWETest.java Adds default RSA JWE variant.
tests/base/.../oidc/KcOidcBrokerJWEEcdhEsP384A192GcmTest.java Adds ECDH-ES/GCM variant.
tests/base/.../oidc/KcOidcBrokerJWEEcdhEsP384A192CbcHs384Test.java Adds ECDH-ES/CBC variant.
tests/base/.../oidc/KcOidcBrokerJWEEcdhEsA256KwP521A256GcmTest.java Adds A256KW/P-521/GCM variant.
tests/base/.../oidc/KcOidcBrokerJWEEcdhEsA256KwP521A256CbcHs512Test.java Adds A256KW/P-521/CBC variant.
tests/base/.../oidc/KcOidcBrokerJWEEcdhEsA192KwP384A192GcmTest.java Adds A192KW/P-384/GCM variant.
tests/base/.../oidc/KcOidcBrokerJWEEcdhEsA192KwP384A192CbcHs384Test.java Adds A192KW/P-384/CBC variant.
tests/base/.../oidc/KcOidcBrokerJWEEcdhEsA128KwP256A128GcmTest.java Adds A128KW/P-256/GCM variant.
tests/base/.../oidc/KcOidcBrokerJWEEcdhEsA128KwP256A128CbcHs256Test.java Adds A128KW/P-256/CBC variant.
tests/base/.../broker/AbstractKcOidcJweBrokerTest.java Adds shared JWE and UserInfo behavior.
tests/base/.../broker/AbstractKcOidcEcdhEsJweBrokerTest.java Adds shared ECDH key configuration.
tests/base/.../broker/AbstractKcOidcBrokerTest.java Improves endpoint lookup and visibility.
tests/base/.../broker/AbstractBrokerTest.java Enables unmanaged profile attributes.
tests/base/.../broker/AbstractBrokerLoginTest.java Extracts reusable user assertions.
Review details
  • Files reviewed: 27/27 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.

This was referenced Sep 3, 2026
Merged
Merged
Merged
keycloak-github-bot[bot]
keycloak-github-bot Bot reviewed Sep 3, 2026
View reviewed changes

@keycloak-github-bot keycloak-github-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unreported flaky test detected, please review

@keycloak-github-bot

keycloak-github-bot Bot commented Sep 3, 2026

Copy link
Copy Markdown

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.adapter.servlet.SAMLServletAdapterTest#employeeSigPostNoIdpKeyTest

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: 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.SAMLServletAdapterTest#salesPostAssertionAndResponseSigTest

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: 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.SAMLServletAdapterTest#salesPostSigEmailTest

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: 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.SAMLServletAdapterTest#salesPostSigTest

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: 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.SAMLServletAdapterTest#testAdapterRoleMappings

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: 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.SAMLServletAdapterTest#salesPostEncTest

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: 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.SAMLServletAdapterTest#idpInitiatedUnauthorizedLoginTest

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: 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.SAMLServletAdapterTest#salesPostEncSignedAssertionsOnlyTest

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: 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.broker.KcOidcBrokerTest#loginWithExistingUserWithBruteForceEnabled

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

org.opentest4j.AssertionFailedError: Expected page 'login-error' to be loaded, but currently on page 'null' after timeout
	at org.keycloak.testsuite.pages.AbstractPage.waitForPage(AbstractPage.java:62)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.jboss.arquillian.graphene.proxy.GrapheneProxyHandler.invokeReal(GrapheneProxyHandler.java:143)
...

Report flaky test

@sguilhen
Move JWE and ECDH-ES tests to new testsuite oidc package
61b6d68 
Pure git rename of the ten legacy JWE and ECDH-ES broker tests into the new
testsuite oidc package. No content changes; the new-testsuite adaptation
follows in the next commit.

Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>
Copilot AI review requested due to automatic review settings September 4, 2026 15:58
@sguilhen
sguilhen force-pushed the 52245-broker-oidc-jwe-ecdh-es branch from 86fd4a6 to 42dffaf Compare September 4, 2026 15:58
Copilot AI reviewed Sep 4, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The migration preserves the complete algorithm matrix and shared behavior with focused passing coverage.

Review details
  • Files reviewed: 24/24 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@sguilhen
Migrate JWE and ECDH-ES broker tests to new testsuite
5cdb112 
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 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 AI review requested due to automatic review settings September 4, 2026 16:13
@sguilhen
sguilhen force-pushed the 52245-broker-oidc-jwe-ecdh-es branch from 42dffaf to 5cdb112 Compare September 4, 2026 16:13
Copilot AI reviewed Sep 4, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The migration preserves the algorithm matrix and shared test behavior without identified correctness issues.

Review details
  • Files reviewed: 25/25 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@pedroigor
pedroigor merged commit cbb83ef into keycloak:main Sep 4, 2026
94 checks passed
This was referenced Sep 15, 2026
Merged
Closed
@sguilhen
sguilhen deleted the 52245-broker-oidc-jwe-ecdh-es branch September 21, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@pedroigor pedroigor pedroigor approved these changes
+1 more reviewer
@keycloak-github-bot keycloak-github-bot[bot] keycloak-github-bot[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

flaky-test team/continuous-delivery team/core-iam

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Migrate JWE and ECDH-ES broker tests

3 participants

@sguilhen @pedroigor