[CVE-2026-17048] Keycloak Admin REST API Leaks Vault-Resolved Rotated Client Secrets - #51209
[CVE-2026-17048] Keycloak Admin REST API Leaks Vault-Resolved Rotated Client Secrets#51209mabartos wants to merge 1 commit into
Conversation
mabartos
commented
Jul 27, 2026
- Closes [CVE-2026-17048] Keycloak Admin REST API Leaks Vault-Resolved Rotated Client Secrets #51145
Unreported flaky test detectedIf 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#crudWithoutFailoverorg.keycloak.testsuite.cluster.UserFederationInvalidationClusterTest#crudWithFailover |
There was a problem hiding this comment.
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. |
msdaly200
left a comment
There was a problem hiding this comment.
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>
There was a problem hiding this comment.
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., keepgetClientRotatedSecret(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. ComputenowSecondsonce 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