Repository navigation
Migrate KcOidcBrokerVaultTest - #52388
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Migrates KcOidcBrokerVaultTest from the legacy Arquillian suite to the new JUnit 5 testframework suite, configuring a file-based vault provider and verifying broker login works with vault-resolved IdP client secrets.
Changes:
- Replaced the legacy
KcOidcBrokerVaultTest+KcOidcBrokerVaultConfigurationwith a new JUnit 5KcOidcBrokerVaultTestundertests/base/.../broker/oidc/using@KeycloakIntegrationTest. - Updated OIDC broker base logic to locate the provider-side “consumer client” by placeholder redirect URI rather than by a fixed clientId.
- Added shared broker-test utilities to enable unmanaged user attributes and reliably bypass the first-broker-login review-profile interruption; added a reusable “user created” assertion hook.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerVaultTest.java | Removes legacy Arquillian vault test entrypoint. |
| testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerVaultConfiguration.java | Removes legacy broker configuration that injected the vault secret. |
| tests/base/src/test/java/org/keycloak/tests/broker/oidc/KcOidcBrokerVaultTest.java | Adds JUnit 5 integration test configuring file vault + setting IdP secret to a vault expression. |
| tests/base/src/test/java/org/keycloak/tests/broker/AbstractKcOidcBrokerTest.java | Adjusts endpoint configuration to match provider client via placeholder redirect URI. |
| tests/base/src/test/java/org/keycloak/tests/broker/AbstractBrokerTest.java | Adds shared setup for unmanaged attributes + helper to disable review-profile/VERIFY_PROFILE behavior. |
| tests/base/src/test/java/org/keycloak/tests/broker/AbstractBrokerLoginTest.java | Ensures post-login assertion that the consumer user is created; exposes helper for overrides. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Pure git rename of the legacy vault broker test 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>
sguilhen
force-pushed
the
52248-broker-oidc-vault
branch
from
September 4, 2026 15:58
5c2c928 to
4874957
Compare
Configures the file-based vault provider and sets the IDP client secret to a vault expression, verifying that broker login works with vault-resolved secrets. Also removes the legacy KcOidcBrokerVaultConfiguration and its now-orphaned vault fixture file (its only consumer, this test, is migrated). Adapted to the merged abstract-class baseline: the concrete class extends AbstractKcOidcBrokerTest with no realm field overrides (both provider and consumer use the default config), just the @beforeeach that points the consumer IDP's client secret at the vault expression, and the @KeycloakIntegrationTest server config that enables the file vault. Carries forward the fixes from prior batches (independently re-derived per batch since this branch is off upstream/main): visibility widening, configureBrokerEndpoints() placeholder-matching fix, enableUnmanagedAttributes(), disableUpdateProfileOnFirstLogin(), and the assertUserCreatedInConsumerRealm() extraction. Closes keycloak#52248 Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>
sguilhen
force-pushed
the
52248-broker-oidc-vault
branch
from
September 4, 2026 16:13
4874957 to
3516505
Compare
pedroigor
approved these changes
Sep 4, 2026
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.
Migrates
KcOidcBrokerVaultTestto the new JUnit 5 framework, landing intests/base/.../broker/oidc/. Configures the file-based vault provider and sets the IdP client secret to a vault expression, verifying broker login works with vault-resolved secrets.Two commits, following the project's move-then-adapt pattern.
Also removes the legacy
KcOidcBrokerVaultConfigurationand its now-orphaned vault fixture file (testsuite/.../src/test/resources/vault/consumer_oidc__idp) - both had this test as their only consumer (confirmed via grep).Adapted to the merged abstract-class baseline: the concrete class extends
AbstractKcOidcBrokerTestwith no realm field overrides (both provider and consumer use the default config) - just the@BeforeEachthat points the consumer IdP's client secret at the vault expression, and the@KeycloakIntegrationTestserver config that enables the file vault.Carries forward the fixes from #52372/#52376/#52379/#52382/#52383/#52387 (independently re-derived per batch, this branch is off
upstream/main): visibility widening,configureBrokerEndpoints()placeholder-matching fix,enableUnmanagedAttributes(),disableUpdateProfileOnFirstLogin(), andassertUserCreatedInConsumerRealm().Testing
mvn test -Dtest=KcOidcBrokerVaultTest- 2/2 greenKcOidcBrokerTest,KcOidcBrokerClientSecretBasicAuthTest,KcSamlBrokerTest(phase-1 baseline) - 6/6 greenmvn spotless:checkcleanCloses #52248