Skip to content

Simplify public key registration with OIDC client registration #48525

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

Merged
merged 1 commit into from
Jun 23, 2025

Conversation

sberyozkin
Copy link
Member

I've been planning to do it for a while, this minor PR makes it easier to deal with the advanced cases where the client metadata must include the public key in JWK format, the test code we have for it is good for tests, but is unlikely to work for users in general, but it was the only way to do it since the ClientMetadata builder API that I originally added was pretty limited in what it could do.

So I added two jwks methods to the builder to supply PublicKey directly, defaulting to the use=sig - which I expect to be the only case anyway as we don't deal at the OIDC level with OIDC providers supplying encryption keys in the public JWK set.

I also added one jwks method accepting a Map when users need to have more JWK properties like key id (it does not have to be crypto-safe but only unique in a given key set), and other properties.

In the updated test example, I also dropped the client_uri property which implies, in the dynamic client reg case, that in this case it can be for example Quarkus that supplies this page... OIDC provider may allocate it itself in some cases too. It is not important for the test in any case.

This comment has been minimized.

Copy link
Member

@michalvavrik michalvavrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I agree this will help to users.

} catch (NoSuchAlgorithmException e) {
throw new RuntimeException("Failed to generate key id", e);
}
private ClientMetadata createClientMetadata(String keycloakUrl) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can drop keycloakUrl as the client URI is not set anymore, but it's a test so probably not wort fixing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let me do it

@sberyozkin sberyozkin force-pushed the oidc_client_reg_public_key branch from 7e9e10c to fbb7056 Compare June 23, 2025 09:37
@sberyozkin sberyozkin requested a review from gastaldi June 23, 2025 09:37
Copy link

quarkus-bot bot commented Jun 23, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit fbb7056.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@sberyozkin sberyozkin merged commit 029bb77 into quarkusio:main Jun 23, 2025
28 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.25 - main milestone Jun 23, 2025
@sberyozkin sberyozkin deleted the oidc_client_reg_public_key branch June 23, 2025 14:59
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.

3 participants