Skip to content

Conversation

sberyozkin
Copy link
Member

Fixes #48085

We already have an integration test with the DPoP value, so I just moved the check to OidcUtils to test a few more variations

@gastaldi gastaldi requested a review from Copilot May 27, 2025 14:50
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Enables case-insensitive handling of the DPoP authorization scheme and centralizes the logic in OidcUtils.

  • Introduce isDPoPScheme in OidcUtils for case-insensitive comparisons
  • Update BearerAuthenticationMechanism to use the new utility method
  • Add unit tests covering uppercase and lowercase scheme values

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
OidcUtils.java Added isDPoPScheme to perform a case-insensitive check against DPOP_SCHEME
BearerAuthenticationMechanism.java Updated the DPoP check to use OidcUtils.isDPoPScheme
OidcUtilsTest.java Added testDpopScheme to verify behavior with different casing
Comments suppressed due to low confidence (2)

extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcUtilsTest.java:37

  • [nitpick] Consider renaming this test to testIsDPoPScheme to clearly reflect the method under test and align with naming conventions.
public void testDpopScheme() throws Exception {

extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcUtilsTest.java:40

  • Add an assertion for a mixed-case input (e.g., "dPoP") to ensure full coverage of case-insensitivity.
assertTrue(OidcUtils.isDPoPScheme("dpop"));

}
return token;
}

Copy link
Preview

Copilot AI May 27, 2025

Choose a reason for hiding this comment

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

[nitpick] Add a brief Javadoc comment explaining the purpose of this method, its parameters, and that it performs a case-insensitive comparison.

Suggested change
/**
* Checks if the provided authorization scheme matches the DPoP (Demonstration of Proof-of-Possession) scheme.
*
* @param authorizationScheme the authorization scheme to check; may be null
* @return {@code true} if the authorization scheme matches the DPoP scheme (case-insensitive), {@code false} otherwise
*/

Copilot uses AI. Check for mistakes.

Copy link
Member

Choose a reason for hiding this comment

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

Let's not clutter our code with useless javadoc for very simple methods, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

@gsmet Sounds good, as I was about to ask for feedback on this one, as I was hesitating a bit :-).

@gsmet
Copy link
Member

gsmet commented May 27, 2025

@sberyozkin I think it should go in 3.20 too, right? AFAICS, DPoP was introduced in 3.19. I added the label. Shout loudly if it was a mistake :)

@sberyozkin
Copy link
Member Author

Thanks @gsmet, indeed, should also be backported to 3.20

Copy link

quarkus-bot bot commented May 27, 2025

Status for workflow Quarkus CI

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

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

@gsmet gsmet merged commit fdcdefe into quarkusio:main May 27, 2025
28 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.24 - main milestone May 27, 2025
@gsmet gsmet modified the milestones: 3.24 - main, 3.23.1 Jun 3, 2025
@jmartisk jmartisk modified the milestones: 3.23.1, 3.20.2 Jun 25, 2025
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.

Quarkus does not enforce DPoP proof with DPoP scheme
4 participants