-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
base: main
Are you sure you want to change the base?
Conversation
e70f337
to
2cdc36d
Compare
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? |
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. |
f45b82d
to
142b5c6
Compare
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>
142b5c6
to
696e7f5
Compare
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: 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: 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. |
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
Fixes #13102