Skip to content
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

Support for OAuth2 Demonstrating Proof of Possession #45891

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Jan 27, 2025

Fixes #42115.

This PR adds a complete DPoP token verification support, with tests.

DPoP is currently restricted to the public clients, which explains why the test structure is created around FrontendResource emulating SPA.

Support for the custom DPoP nonce providers can be offered in the future.

All in all, the implementation is just a translation of https://datatracker.ietf.org/doc/html/rfc9449#name-checking-dpop-proofs, goes via every recommended verification step:

  • DPoP header exists
  • Its HTTP method and URIs are correct
  • It has non-private public JWK key and the proof signature is correct
  • It has an access token hash which matches the provided access token
  • The access token confirmation claim has a JWK thumbprint which matches the DPoP's public jwk thumbprint

Some tuning for the URI matches might be needed going forward but should work fine for typical cases.

To support the test cases I updated Keycloak Dev service to enable experimental features for users be able to use DPoP, etc.

The actual tests can be improved further, I'd like to enable in a separate PR to report exception causes not only in devmode but also in test mode.

@sberyozkin sberyozkin requested a review from pedroigor January 27, 2025 16:23
@sberyozkin sberyozkin force-pushed the oidc_dpop branch 9 times, most recently from 5a924c3 to 89a452a Compare January 30, 2025 18:56
@sberyozkin sberyozkin force-pushed the oidc_dpop branch 2 times, most recently from 72cf471 to ff297e7 Compare February 10, 2025 17:09
@quarkus-bot quarkus-bot bot added area/docstyle issues related for manual docstyle review area/documentation labels Feb 10, 2025
@sberyozkin sberyozkin marked this pull request as ready for review February 10, 2025 17:10

This comment has been minimized.

Copy link

github-actions bot commented Feb 10, 2025

🙈 The PR is closed and the preview is expired.

@sberyozkin
Copy link
Member Author

Oops, I realized I forgot starting containers for testing a few negative rests I added today

@sberyozkin sberyozkin marked this pull request as draft February 10, 2025 18:26
@sberyozkin sberyozkin marked this pull request as ready for review February 10, 2025 23:20
@sberyozkin
Copy link
Member Author

I mistyped the RSA key size (2024 as opposed to 2048), now the tests should be fine

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@sberyozkin
Copy link
Member Author

sberyozkin commented Feb 11, 2025

Sorry, needs more work, some side-effects likely related to enabling all features in the devservice

@sberyozkin sberyozkin marked this pull request as draft February 11, 2025 11:48
@sberyozkin sberyozkin marked this pull request as ready for review February 11, 2025 13:43

This comment has been minimized.

@sberyozkin
Copy link
Member Author

I ended up adding a dedicated features property to the Keycloak devservice config. One can also use the custom start command but if all the users want is to enable, for example, the dpop feature, then features=dpop will be a much simpler way to achieve it compared to copying the rest of the start command options...

Copy link

quarkus-bot bot commented Feb 11, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 802f60c.

✅ 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.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

quarkus-bot bot commented Feb 11, 2025

Status for workflow Quarkus CI

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

✅ 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
Copy link
Member Author

That could've been a great new feature for LTS

@sberyozkin
Copy link
Member Author

Also CC @mposolda

@sberyozkin
Copy link
Member Author

sberyozkin commented Feb 12, 2025

Just to clarify that at the test level, FrontendResource emulates a public SPA client which authenticates users to Keycloak and then creates one DPoP proof to complete the authorization code flow with Keycloak and then passes the token with another DPoP proof to Quarkus - this PR is only about verifying the DPoP proof and the bound access token at the Quarkus end

Copy link
Contributor

@pedroigor pedroigor left a comment

Choose a reason for hiding this comment

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

@sberyozkin
Copy link
Member Author

Thanks very much for the time, @pedroigor.

@mposolda, you are always welcome to comment as well, I'll CC you when I open a follow up PR to allow the registration of the custom DPoP Nonce providers

@sberyozkin sberyozkin merged commit 209fdfc into quarkusio:main Feb 14, 2025
71 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.21 - main milestone Feb 14, 2025
@sberyozkin sberyozkin deleted the oidc_dpop branch February 14, 2025 14:00
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Feb 14, 2025
@sberyozkin
Copy link
Member Author

@gsmet, Hi, it is quite an important feature, if it is not too late yet then please consider for a backport, but please don't hesitate to drop the backport label if you feel it may be too sensitive to backport

@gsmet gsmet modified the milestones: 3.21 - main, 3.19.0 Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Demonstrating Proof of Possession (DPoP) in quarkus-oidc
3 participants