davsclaus commented on code in PR #24032:
URL: https://github.com/apache/camel/pull/24032#discussion_r3413989081
##########
components/camel-oauth/src/main/java/org/apache/camel/oauth/UserProfile.java:
##########
@@ -180,15 +180,16 @@ private static JsonObject verifyToken(OAuthConfig config,
String token, boolean
// Fetch Keycloak public key
Review Comment:
Nit: this comment is a leftover from when the code was Keycloak-specific.
The verification is now provider-agnostic (works with any OIDC IdP). Consider
updating:
```suggestion
// Verify JWS signature against configured JWK set
```
##########
components/camel-oauth/src/main/java/org/apache/camel/oauth/UserProfile.java:
##########
@@ -180,15 +180,16 @@ private static JsonObject verifyToken(OAuthConfig config,
String token, boolean
// Fetch Keycloak public key
var jwkSet = config.getJWKSet();
- if (!jwkSet.isEmpty()) {
- var rsaKey = (RSAKey) jwkSet.getKeyByKeyId(keyID);
- if (rsaKey == null) {
- throw new OAuthException("No matching key found for: " +
keyID);
- }
- RSAPublicKey publicKey = rsaKey.toRSAPublicKey();
- if (!signedJWT.verify(new RSASSAVerifier(publicKey))) {
- throw new OAuthException("Invalid token signature");
- }
+ if (jwkSet == null || jwkSet.isEmpty()) {
Review Comment:
Good catch adding the `null` check here — the old code would have thrown an
NPE if `getJWKSet()` returned `null` (which it can, since `OAuthConfig.jwkSet`
has no default). This is a correctness fix beyond the stated scope of the PR
and worth calling out in the description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
