This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/OAuthScopesFilter in repository https://gitbox.apache.org/repos/asf/cxf.git
commit d445d2ec42e1bb39f5b3335c53b56c6ece6f3ca2 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Sun Nov 1 16:08:26 2026 +0000 Keep OAuthScopesFilter rules regardless of setter order --- .../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) { }
