You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by "dxbjavid (via GitHub)" <gi...@apache.org> on 2026/10/19 07:16:46 UTC
[PR] require every configured scope in OAuthRequestFilter [cxf]
dxbjavid opened a new pull request, #3603:
URL: https://github.com/apache/cxf/pull/3603
OAuthRequestFilter enforces its requiredScopes by comparing the number of token permissions whose name is in requiredScopes with requiredScopes.size(), but scope strings are never de-duplicated on the issuance path (parseScope splits on spaces, validateScopes is a containsAll, consent hands the requested list back as is, convertScopeToPermissions adds one permission per entry, and JwtAccessTokenValidator splits the scope claim back into a list). A client that only holds read can ask for scope=read read and the resulting token, with two read permissions, satisfies a filter configured with requiredScopes of read and admin, so the admin requirement is bypassed; the same count check also rejects a token that carries every required scope plus a duplicate. The check now collects the matching scope names into a set and requires it to contain every configured scope, with two regression tests driving validateRequest with signed JWT access tokens.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] require every configured scope in OAuthRequestFilter [cxf]
Posted by "dxbjavid (via GitHub)" <gi...@apache.org>.
dxbjavid commented on code in PR #3603:
URL: https://github.com/apache/cxf/pull/3603#discussion_r4321547037
##########
rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/filters/OAuthRequestFilter.java:
##########
@@ -236,6 +238,17 @@ protected boolean checkRequestURI(HttpServletRequest request, List<String> uris,
}
return foundValidScope;
}
+
+ private boolean hasAllRequiredScopes(List<OAuthPermission> matchingPermissions) {
Review Comment:
good point, made it protected so a subclass can swap in its own containment logic.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] require every configured scope in OAuthRequestFilter [cxf]
Posted by "coheigea (via GitHub)" <gi...@apache.org>.
coheigea commented on code in PR #3603:
URL: https://github.com/apache/cxf/pull/3603#discussion_r4322088431
##########
rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/filters/OAuthRequestFilter.java:
##########
@@ -236,6 +238,17 @@ protected boolean checkRequestURI(HttpServletRequest request, List<String> uris,
}
return foundValidScope;
}
+
+ private boolean hasAllRequiredScopes(List<OAuthPermission> matchingPermissions) {
Review Comment:
Two further points please:
1. A subclass can’t see the required scopes (OAuthRequestFilter.java:83). requiredScopes is private and there’s only a setter (setRequiredScopes, line 383), no getter. A subclass that overrides hasAllRequiredScopes to do pattern matching has nothing to match against. Its only option is to also override setRequiredScopes and keep its own copy of the list. Either of these would fix it:
add protected List<String> getRequiredScopes(), or
give the method the list as a parameter: hasAllRequiredScopes(List<String> requiredScopes, List<OAuthPermission> matchingPermissions).
Overriding checkScopeProperty has the same problem today, so a getter would help both.
2. Javadoc (minor). The method is now an extension point, so the comment inside it could become a short Javadoc. It should say what it receives (only the permissions that passed the URI, HTTP-method and checkScopeProperty checks) and that subclasses overriding checkScopeProperty to match loosely should override this method as well. The current comment about duplicates can stay.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] require every configured scope in OAuthRequestFilter [cxf]
Posted by "dxbjavid (via GitHub)" <gi...@apache.org>.
dxbjavid commented on code in PR #3603:
URL: https://github.com/apache/cxf/pull/3603#discussion_r4322714543
##########
rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/filters/OAuthRequestFilter.java:
##########
@@ -236,6 +238,17 @@ protected boolean checkRequestURI(HttpServletRequest request, List<String> uris,
}
return foundValidScope;
}
+
+ private boolean hasAllRequiredScopes(List<OAuthPermission> matchingPermissions) {
Review Comment:
Fair point, without a getter an override of either method had nothing to match against. I've added a protected getRequiredScopes(), which I think suits checkScopeProperty overrides better than threading the list through as a parameter. The comment is now a short Javadoc covering what the method receives and noting that loose checkScopeProperty overrides should override this one too, and the note about duplicates is kept.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] require every configured scope in OAuthRequestFilter [cxf]
Posted by "coheigea (via GitHub)" <gi...@apache.org>.
coheigea commented on code in PR #3603:
URL: https://github.com/apache/cxf/pull/3603#discussion_r4314329805
##########
rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/filters/OAuthRequestFilter.java:
##########
@@ -236,6 +238,17 @@ protected boolean checkRequestURI(HttpServletRequest request, List<String> uris,
}
return foundValidScope;
}
+
+ private boolean hasAllRequiredScopes(List<OAuthPermission> matchingPermissions) {
Review Comment:
Make this protected so that subclasses can change this logic if required.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] require every configured scope in OAuthRequestFilter [cxf]
Posted by "coheigea (via GitHub)" <gi...@apache.org>.
coheigea merged PR #3603:
URL: https://github.com/apache/cxf/pull/3603
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] require every configured scope in OAuthRequestFilter [cxf]
Posted by "dxbjavid (via GitHub)" <gi...@apache.org>.
dxbjavid commented on PR #3603:
URL: https://github.com/apache/cxf/pull/3603#issuecomment-6308227728
TY!
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org