This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/main by this push:
new c1fabe2cb30 Keep OAuthScopesFilter rules regardless of setter order
(#3658)
c1fabe2cb30 is described below
commit c1fabe2cb30bbdb5647e9b24c236416210b8ea9f
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Mon Nov 2 06:02:40 2026 +0000
Keep OAuthScopesFilter rules regardless of setter order (#3658)
---
.../security/oauth2/filters/OAuthScopesFilter.java | 28 +++++---
.../oauth2/filters/OAuthScopesFilterTest.java | 76 ++++++++++++++++++++++
2 files changed, 96 insertions(+), 8 deletions(-)
diff --git
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/filters/OAuthScopesFilter.java
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/filters/OAuthScopesFilter.java
index c3c50bf4f0e..8caa89bdf65 100644
---
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/filters/OAuthScopesFilter.java
+++
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/filters/OAuthScopesFilter.java
@@ -57,8 +57,8 @@ public class OAuthScopesFilter implements
ContainerRequestFilter {
@Context
private MessageContext mc;
// Scopes configured explicitly via setScopesMap/setScopesStringMap, keyed
by method name
- private Map<String, List<String>> scopesMap = new HashMap<>();
- private Map<String, Boolean> scopesMatchAllMap = new HashMap<>();
+ private final Map<String, List<String>> scopesMap = new HashMap<>();
+ private final Map<String, Boolean> scopesMatchAllMap = new HashMap<>();
// The method-level and class-level @Scopes declarations which apply to
each method signature
// in the secured object's hierarchy. Keying by signature means that
overloaded methods do
// not share a rule
@@ -69,7 +69,10 @@ public class OAuthScopesFilter implements
ContainerRequestFilter {
private final Map<String, Set<String>> signaturesByName = new HashMap<>();
private final Set<Class<?>> securedClasses = new HashSet<>();
private boolean annotationScopesFound;
- private Set<String> confidentialClientMethods = new HashSet<>();
+ // Methods configured explicitly via setConfidentialClientMethods, and
methods found from
+ // @ConfidentialClient annotations, kept apart so the setter cannot
discard the latter
+ private final Set<String> confidentialClientMethods = new HashSet<>();
+ private final Set<String> annotationConfidentialClientMethods = new
HashSet<>();
public void setSecuredObject(Object object) {
Class<?> cls = ClassHelper.getRealClass(object);
@@ -118,7 +121,7 @@ public class OAuthScopesFilter implements
ContainerRequestFilter {
ConfidentialClient mConfClient =
m.getAnnotation(ConfidentialClient.class);
if (classConfClient != null || mConfClient != null) {
- confidentialClientMethods.add(m.getName());
+ annotationConfidentialClientMethods.add(m.getName());
}
}
checkSecureClass(cls.getSuperclass());
@@ -134,7 +137,8 @@ public class OAuthScopesFilter implements
ContainerRequestFilter {
checkScopes(m);
}
protected void checkClient(Method m) {
- if (confidentialClientMethods.contains(m.getName())) {
+ if (confidentialClientMethods.contains(m.getName())
+ || annotationConfidentialClientMethods.contains(m.getName())) {
OAuthContext context = OAuthContextUtils.getContext(mc);
if (!context.isClientConfidential()) {
LOG.warning("Non confidential client " + context.getClientId()
@@ -244,7 +248,10 @@ public class OAuthScopesFilter implements
ContainerRequestFilter {
}
public void setScopesMap(Map<String, List<String>> scopesMap) {
- this.scopesMap = scopesMap;
+ // Add to, rather than replace, the scopes already configured, e.g.
via setScopesStringMap
+ if (scopesMap != null) {
+ this.scopesMap.putAll(scopesMap);
+ }
}
public void setScopesStringMap(Map<String, String> scopesStringMap) {
@@ -254,11 +261,16 @@ public class OAuthScopesFilter implements
ContainerRequestFilter {
}
public void setScopesMatchAllMap(Map<String, Boolean> scopesMatchAllMap) {
- this.scopesMatchAllMap = scopesMatchAllMap;
+ if (scopesMatchAllMap != null) {
+ this.scopesMatchAllMap.putAll(scopesMatchAllMap);
+ }
}
public void setConfidentialClientMethods(Set<String>
confidentialClientMethods) {
- this.confidentialClientMethods = confidentialClientMethods;
+ this.confidentialClientMethods.clear();
+ if (confidentialClientMethods != null) {
+ this.confidentialClientMethods.addAll(confidentialClientMethods);
+ }
}
private static final class ScopesDeclaration {
diff --git
a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/filters/OAuthScopesFilterTest.java
b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/filters/OAuthScopesFilterTest.java
index c8d165e7f10..6ccbd96ced1 100644
---
a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/filters/OAuthScopesFilterTest.java
+++
b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/filters/OAuthScopesFilterTest.java
@@ -246,6 +246,59 @@ public class OAuthScopesFilterTest {
assertThrows(ForbiddenException.class, () -> filter(filter, method,
"admin"));
}
+ @Test
+ public void
testConfidentialClientAnnotationKeptWhenMethodsConfiguredAfterSecuredObject()
+ throws Exception {
+ Method transfer =
ConfidentialClientService.class.getMethod("transfer");
+ Method list = ConfidentialClientService.class.getMethod("list");
+
+ OAuthScopesFilter filter = createFilter(new
ConfidentialClientService());
+ filter.setConfidentialClientMethods(Collections.singleton("list"));
+ filter(filter, transfer, true, "read");
+ assertThrows(ForbiddenException.class, () -> filter(filter, transfer,
false, "read"));
+ filter(filter, list, true, "read");
+ assertThrows(ForbiddenException.class, () -> filter(filter, list,
false, "read"));
+ }
+
+ @Test
+ public void
testConfidentialClientAnnotationKeptWhenMethodsConfiguredBeforeSecuredObject()
+ throws Exception {
+ Method transfer =
ConfidentialClientService.class.getMethod("transfer");
+ Method list = ConfidentialClientService.class.getMethod("list");
+
+ OAuthScopesFilter filter = new OAuthScopesFilter();
+ filter.setConfidentialClientMethods(Collections.singleton("list"));
+ filter.setSecuredObject(new ConfidentialClientService());
+ assertThrows(ForbiddenException.class, () -> filter(filter, transfer,
false, "read"));
+ assertThrows(ForbiddenException.class, () -> filter(filter, list,
false, "read"));
+ }
+
+ @Test
+ public void testScopesMapDoesNotDiscardScopesStringMap() throws Exception {
+ Method transfer = ConfiguredService.class.getMethod("transfer");
+ Method list = ConfiguredService.class.getMethod("list");
+
+ OAuthScopesFilter filter = new OAuthScopesFilter();
+ filter.setScopesStringMap(Collections.singletonMap("transfer",
"write"));
+ filter.setScopesMap(Collections.singletonMap("list",
Collections.singletonList("read")));
+ filter(filter, transfer, "write");
+ assertThrows(ForbiddenException.class, () -> filter(filter, transfer,
"read"));
+ filter(filter, list, "read");
+ assertThrows(ForbiddenException.class, () -> filter(filter, list,
"write"));
+ }
+
+ @Test
+ public void testScopesMatchAllMapDoesNotDiscardEarlierEntries() throws
Exception {
+ Method transfer = ConfiguredService.class.getMethod("transfer");
+
+ OAuthScopesFilter filter = new OAuthScopesFilter();
+ filter.setScopesStringMap(Collections.singletonMap("transfer", "read
write"));
+ filter.setScopesMatchAllMap(Collections.singletonMap("transfer",
Boolean.TRUE));
+ filter.setScopesMatchAllMap(Collections.singletonMap("list",
Boolean.TRUE));
+ filter(filter, transfer, "read", "write");
+ assertThrows(ForbiddenException.class, () -> filter(filter, transfer,
"read"));
+ }
+
private static void assertAllowed(Object service, Method method, String...
scopes) throws Exception {
filter(createFilter(service), method, scopes);
}
@@ -262,11 +315,17 @@ public class OAuthScopesFilterTest {
}
private static void filter(OAuthScopesFilter filter, Method method,
String... scopes) throws Exception {
+ filter(filter, method, false, scopes);
+ }
+
+ private static void filter(OAuthScopesFilter filter, Method method,
boolean confidentialClient,
+ String... scopes) throws Exception {
List<OAuthPermission> permissions = new ArrayList<>();
for (String scope : scopes) {
permissions.add(new OAuthPermission(scope));
}
OAuthContext context = new OAuthContext(new UserSubject("alice"),
null, permissions, "code");
+ context.setClientConfidential(confidentialClient);
MessageContext mc = mock(MessageContext.class);
when(mc.get("org.apache.cxf.resource.method")).thenReturn(method);
@@ -485,6 +544,23 @@ public class OAuthScopesFilterTest {
}
}
+ public static class ConfidentialClientService {
+ @ConfidentialClient
+ public void transfer() {
+ }
+
+ public void list() {
+ }
+ }
+
+ public static class ConfiguredService {
+ public void transfer() {
+ }
+
+ public void list() {
+ }
+ }
+
public static class UnsecuredService {
public void operation(String value) {
}