Skip to content

[CVE-2026-17048] Keycloak Admin REST API Leaks Vault-Resolved Rotated Client Secrets - #51209

Open
mabartos wants to merge 1 commit into
keycloak:mainfrom
mabartos:KC-51145
Open

[CVE-2026-17048] Keycloak Admin REST API Leaks Vault-Resolved Rotated Client Secrets#51209
mabartos wants to merge 1 commit into
keycloak:mainfrom
mabartos:KC-51145

Conversation

@mabartos

Copy link
Copy Markdown
Member

@keycloak-github-bot

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.cluster.UserFederationInvalidationClusterTest#crudWithoutFailover

Keycloak CI - Clustering IT

org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
	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.AssertFalse.failNotFalse(AssertFalse.java:63)
	at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:36)
...

Report flaky test

org.keycloak.testsuite.cluster.UserFederationInvalidationClusterTest#crudWithFailover

Keycloak CI - Clustering IT

java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "entity" is null
	at org.keycloak.testsuite.cluster.AbstractInvalidationClusterTest.sortFields(AbstractInvalidationClusterTest.java:172)
	at org.keycloak.testsuite.cluster.AbstractInvalidationClusterTest.assertEntityOnSurvivorNodesEqualsTo(AbstractInvalidationClusterTest.java:154)
	at org.keycloak.testsuite.cluster.UserFederationInvalidationClusterTest.assertEntityOnSurvivorNodesEqualsTo(UserFederationInvalidationClusterTest.java:94)
	at org.keycloak.testsuite.cluster.UserFederationInvalidationClusterTest.assertEntityOnSurvivorNodesEqualsTo(UserFederationInvalidationClusterTest.java:19)
...

Report flaky test

@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

@mabartos
mabartos marked this pull request as ready for review July 28, 2026 07:32
@mabartos
mabartos requested a review from a team as a code owner July 28, 2026 07:32
Copilot AI review requested due to automatic review settings July 28, 2026 07:32

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.

Pull request overview

Note

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.

Addresses CVE-2026-17048 by preventing vault resolution of rotated client secrets when returned via the Keycloak Admin REST API, avoiding leakage of vault-backed secret values.

Changes:

  • Add an API path that returns rotated secrets without vault resolution, and an internal path that can resolve vault expressions for authentication.
  • Update rotated-secret validation/authentication flow to explicitly resolve vault expressions.
  • Add an integration test and test vault secret resource to verify Admin API does not return resolved values.

Reviewed changes

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

File Description
tests/base/src/test/resources/org/keycloak/tests/admin/vault/default_rotated__secret Adds a vault-backed rotated secret test resource.
tests/base/src/test/java/org/keycloak/tests/admin/ClientVaultTest.java Adds coverage ensuring the Admin API returns the raw rotated-secret placeholder, and enables secret-rotation feature for the test server.
services/src/main/java/org/keycloak/protocol/oidc/OIDCClientSecretConfigWrapper.java Splits rotated-secret retrieval into “no vault resolution” vs “resolve for auth” paths and updates validation/auth usage accordingly.

@mabartos
mabartos marked this pull request as draft July 28, 2026 07:55
@mabartos
mabartos marked this pull request as ready for review July 28, 2026 08:21

@msdaly200 msdaly200 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.

Just one nit to add a regression test, but everything looks good to fix the vulnerability.

… Client Secrets

Closes keycloak#51145

Signed-off-by: Martin Bartoš <mabartos@redhat.com>
Copilot AI review requested due to automatic review settings July 28, 2026 14:53

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.

Pull request overview

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

Comments suppressed due to low confidence (4)

services/src/main/java/org/keycloak/protocol/oidc/OIDCClientSecretConfigWrapper.java:142

  • This changes the behavior of the existing public method getClientRotatedSecret(KeycloakSession) from 'resolve when session is present' to 'never resolve'. If any existing callers relied on the old behavior, this is a breaking semantic change. Consider preserving backward-compatible behavior (e.g., keep getClientRotatedSecret(session) resolving by default) and introducing a clearly named raw/unresolved getter for Admin API usage, or renaming the methods to make the resolution behavior explicit.
    public String getClientRotatedSecret(KeycloakSession session) {
        return getClientRotatedSecret(session, false);
    }

    /**
     * Returns the rotated client secret value, optionally resolving vault expressions.
     * Vault resolution should only be enabled for authentication validation, never for
     * returning values through the Admin API to avoid leaking sensitive vault-backed values.
     * Use {@link #hasRotatedSecret()} to check whether a rotated secret is effectively present before calling this method.
     *
     * @param session the keycloak session
     * @param resolveVault if {@code true}, vault expressions like {@code ${vault.key}} are resolved to their actual values;
     *                     if {@code false}, the raw stored value (potentially a vault placeholder) is returned
     */
    public String getClientRotatedSecret(KeycloakSession session, boolean resolveVault) {
        String secret = getAttribute(CLIENT_ROTATED_SECRET);
        if (resolveVault && session != null) {
            return session.vault().getStringSecret(secret).get().orElse(secret);
        }
        return secret;
    }

services/src/main/java/org/keycloak/protocol/oidc/OIDCClientSecretConfigWrapper.java:254

  • Avoid relying on the platform default charset when converting secrets to bytes. Use an explicit charset (for both inputs) such as UTF-8 to ensure consistent behavior across environments.
        return MessageDigest.isEqual(secret.getBytes(), getClientRotatedSecret(session, true).getBytes());

tests/base/src/test/java/org/keycloak/tests/admin/ClientVaultTest.java:156

  • The creation/expiration timestamps are derived from separate System.currentTimeMillis() calls, which can produce inconsistent values and makes the test setup less deterministic. Compute nowSeconds once and reuse it for both attributes (creation = nowSeconds, expiration = nowSeconds + 86400).
            realm.clients(ClientBuilder.create("myclient-with-rotated-vault-secret")
                    .publicClient(false)
                    .directAccessGrantsEnabled(true)
                    .secret("some-primary-secret")
                    .attribute(ClientSecretConstants.CLIENT_ROTATED_SECRET, "${vault.rotated_secret}")
                    .attribute(ClientSecretConstants.CLIENT_ROTATED_SECRET_CREATION_TIME, String.valueOf(System.currentTimeMillis() / 1000))
                    .attribute(ClientSecretConstants.CLIENT_ROTATED_SECRET_EXPIRATION_TIME, String.valueOf(System.currentTimeMillis() / 1000 + 86400)));

tests/base/src/test/resources/org/keycloak/tests/admin/vault/default_rotated__secret:1

  • Add a trailing newline at end-of-file to avoid tooling/lint issues and keep consistent formatting across resource files.
rotatedsecret

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CVE-2026-17048] Keycloak Admin REST API Leaks Vault-Resolved Rotated Client Secrets

3 participants