gnodet-bot commented on code in PR #29592:
URL: https://github.com/apache/camel/pull/29592#discussion_r4536276722
##########
components/camel-oauth/src/main/java/org/apache/camel/oauth/JwtTokenValidator.java:
##########
@@ -186,23 +185,43 @@ static OAuthTokenValidationResult validate(
String issuer = claims.getIssuer();
long expiresAt = claims.getExpirationTime() != null ?
claims.getExpirationTime().getTime() / 1000L : 0;
- List<String> scopes = Collections.emptyList();
- Object scopeClaim = claims.getClaim("scope");
- if (scopeClaim instanceof String s) {
- scopes = Arrays.asList(s.split(" "));
- } else if (scopeClaim instanceof List<?> list) {
- scopes = new ArrayList<>();
- for (Object item : list) {
- scopes.add(String.valueOf(item));
- }
- }
+ List<String> scopes = readScopes(claims.getClaim("scope"),
claims.getClaim("scp"));
Map<String, Object> allClaims = claims.getClaims();
LOG.debug("JWT validated successfully: sub={}, iss={}, exp={}",
subject, issuer, expiresAt);
return OAuthTokenValidationResult.valid(subject, issuer, audience,
scopes, allClaims, expiresAt);
}
+ /**
+ * Reads OAuth scopes from the standard scope claim and the scp claim used
by some identity providers. Both claims
+ * may be space-delimited strings or collections of scope strings.
+ */
+ private static List<String> readScopes(Object scopeClaim, Object scpClaim)
{
+ List<String> scopes = new ArrayList<>();
+ addScopes(scopes, scopeClaim);
+ addScopes(scopes, scpClaim);
+ return scopes.isEmpty() ? Collections.emptyList() :
List.copyOf(scopes);
+ }
+
Review Comment:
**Nit: `List.contains()` for dedup is O(n).** `addScopes()` accumulates into
a `List<String>` and uses `scopes.contains(scope)` for deduplication, giving
O(n²) over all scope tokens. `parseRequiredScopes()` added in the same PR uses
`LinkedHashSet` for O(1) dedup. For typical OAuth token scope counts this has
zero measurable impact, but the inconsistency is worth aligning.
```suggestion
private static List<String> readScopes(Object scopeClaim, Object
scpClaim) {
Set<String> scopes = new LinkedHashSet<>();
addScopes(scopes, scopeClaim);
addScopes(scopes, scpClaim);
return scopes.isEmpty() ? Collections.emptyList() :
List.copyOf(scopes);
}
```
(Change `addScopes` parameter type to `Set<String>` accordingly.)
--
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]
