8000 Add support for looking up client secrets via Vault SPI by tsaarni · Pull Request #39650 · keycloak/keycloak · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add support for looking up client secrets via Vault SPI #39650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tsaarni
Copy link
Contributor
@tsaarni tsaarni commented May 12, 2025

This PR adds support for resolving client secrets via the Vault SPI, enabling retrieval from secure storage instead of storing them in plaintext in the database. While the Vault SPI has some usability limitations, this approach is less invasive than alternatives like hashing or introducing new encrypt/decrypt SPI. It also preserves the plaintext access to the secret, which is required by existing use cases.

Notes:

(1) At the moment, the admin console only supports generating new random client secrets. There is no way for administrators to specify a vault reference value. However, the Admin REST API does support setting the client secret explicitly, including reference values. To fully support this feature, the admin console will need to be extended to allow entering reference values as well. Note: If a new random client secret is generated via the admin console, it will overwrite the existing reference.

(2) The client secret rotation feature introduces additional complexity. Triggering rotation will overwrite any reference-based secret with a newly generated random secret.

(3) Since the client secret may be accessed frequently, using a remote Vault SPI implementation with network communication can introduce latency. To avoid repeated lookups, Vault SPI implementation may choose to add caching.

TODO

  • Add test cases for the new Vault-based client secret resolution.
  • Extend the admin console to support setting client secret references.
  • Add documentation.
  • Update documentation with new screenshot.
  • Decide how to handle client (config) export when using a vault reference.

Fixes #13102

@tsaarni tsaarni force-pushed the vault-client-secret branch from e70f337 to 2cdc36d Compare May 12, 2025 18:24
@tsaarni
Copy link
Contributor Author
tsaarni commented May 12, 2025

Hi @mhajas, I noticed that you had opened an issue requesting this feature a few years ago. I’m interested in contributing it and would be glad to continue working on it. Before proceeding, I wanted to check if this is still a feature you'd consider including in Keycloak?

@mhajas
Copy link
Contributor
mhajas commented May 13, 2025

Thank you for the PR @tsaarni. I would say it still makes sense to include this since it is highly upvoted issue. I would be happy to help or find someone else who would be willing to help.

I will have a look at the changes later this week. Let me know if you need something else at this stage.

@mhajas mhajas self-assigned this May 13, 2025
@tsaarni tsaarni force-pushed the vault-client-secret branch 3 times, most recently from f45b82d to 142b5c6 Compare May 26, 2025 07:37
tsaarni added 4 commits June 11, 2025 14:43
If vault SPI is defined, lookup client secret (and rotated client secret) from
vault implementation.

Signed-off-by: Tero Saarni <tero.saarni@est.tech>
Signed-off-by: Tero Saarni <tero.saarni@est.tech>
Signed-off-by: Tero Saarni <tero.saarni@est.tech>
Signed-off-by: Tero Saarni <tero.saarni@est.tech>
@tsaarni
Copy link
Contributor Author
tsaarni commented Jun 11, 2025

I've now marked the PR as ready for reviews.

I still have a couple of basic question regarding the UI changes.

(1) I didn't add an option to set the client secret during client creation. As a result, the client is always created with randomly generated client secret (as before), even if administrator already knows they want to use a vault reference. In this case, they'll need to first create the client with a random secret, then go to the "Credentials" tab to configure the vault reference.

Do you think this is acceptable from a user experience perspective?

(2) In he "Credentials" tab, there was previously a separator line between authenticator selector and the client secret field:

client-credentials-before

I've removed that separator and moved the Client Secret field above the "Save" button, to make i more consistent with the layout used for the other authenticators:

client-credentials

I haven't updated the documentation with a new UI screenshot yet, as I'd like to agree on the approach first.

(3) Side effect of this change is that administrators can now set any specific client secret directly through the UI, which wasn’t previously possible. This capability has existed via the REST API, but not in the UI until now. Hope this is acceptable.

(4) In other Vault SPI use cases, the secret stored in the vault is not accessible through the UI. However, for OIDC clients, exporting the client configuration from the UI previously included the actual secret. The configuration comes from the client representation, which contains the vault reference instead of the secret itself. I haven’t found a workaround yet, and I’m uncertain whether the secret should be resolved during export.

Lastly, I haven’t used React Hook Form before, but I tried to follow the existing patterns used elsewhere in the code.

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.

Add support for specifying client.secret using vault
2 participants
0