This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/SecureAnnotationsInterceptor in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 07cabc977d81ff0155e637902deaa7cbb655b2fa Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Mon Nov 2 06:30:19 2026 +0000 Apply SecureAnnotationsInterceptor annotation class set after the secured object --- .../security/SecureAnnotationsInterceptor.java | 53 +++++++++++++ .../security/SecureAnnotationsInterceptorTest.java | 88 ++++++++++++++++++++++ 2 files changed, 141 insertions(+) diff --git a/core/src/main/java/org/apache/cxf/interceptor/security/SecureAnnotationsInterceptor.java b/core/src/main/java/org/apache/cxf/interceptor/security/SecureAnnotationsInterceptor.java index cfd69cedcb1..fa4f590df8e 100644 --- a/core/src/main/java/org/apache/cxf/interceptor/security/SecureAnnotationsInterceptor.java +++ b/core/src/main/java/org/apache/cxf/interceptor/security/SecureAnnotationsInterceptor.java @@ -20,9 +20,12 @@ package org.apache.cxf.interceptor.security; import java.lang.annotation.Annotation; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.logging.Level; @@ -42,6 +45,12 @@ public class SecureAnnotationsInterceptor extends SimpleAuthorizingInterceptor { Arrays.asList("wait", "notify", "notifyAll", "equals", "toString", "hashCode")); private String annotationClassName = DEFAULT_ANNOTATION_CLASS_NAME; + // The classes passed to setSecuredObject, so their roles can be read again if the + // annotation class changes afterwards + private final List<Class<?>> securedObjectClasses = new ArrayList<>(); + // The methodRolesMap entries added from the annotations, so they can be replaced + // without touching the entries configured via setMethodRolesMap + private final Map<String, List<String>> annotationRoles = new HashMap<>(); public SecureAnnotationsInterceptor() { this(true); @@ -58,10 +67,41 @@ public class SecureAnnotationsInterceptor extends SimpleAuthorizingInterceptor { ex.printStackTrace(); throw new IllegalArgumentException("Annotation class " + name + " is not available"); } + resolveSecuredObjectRoles(); } public void setSecuredObject(Object object) { Class<?> cls = ClassHelper.getRealClass(object); + securedObjectClasses.add(cls); + addRoles(cls, Collections.emptyMap()); + } + + /** + * The roles are read from the annotations when the secured object is set, so read them + * again with the new annotation class for any secured object which was set before it. + * Otherwise the roles would come from the default annotation class, and a service using + * another annotation would be left unprotected. + */ + private void resolveSecuredObjectRoles() { + if (securedObjectClasses.isEmpty()) { + return; + } + for (Map.Entry<String, List<String>> entry : annotationRoles.entrySet()) { + // Keep an entry which setMethodRolesMap has replaced since + if (methodRolesMap.get(entry.getKey()) == entry.getValue()) { + methodRolesMap.remove(entry.getKey()); + } + } + annotationRoles.clear(); + // What is left was configured via setMethodRolesMap after the secured object, so it + // takes precedence, as it would have if the annotation class had been set first + Map<String, List<String>> configuredRoles = new HashMap<>(methodRolesMap); + for (Class<?> cls : securedObjectClasses) { + addRoles(cls, configuredRoles); + } + } + + private void addRoles(Class<?> cls, Map<String, List<String>> configuredRoles) { Map<String, String> rolesMap = new HashMap<>(); findRoles(cls, rolesMap); if (rolesMap.isEmpty()) { @@ -71,7 +111,20 @@ public class SecureAnnotationsInterceptor extends SimpleAuthorizingInterceptor { LOG.fine("Method: " + entry.getKey() + ", roles: " + entry.getValue()); } } + Map<String, List<String>> previousRoles = new HashMap<>(methodRolesMap); super.setMethodRolesMap(rolesMap); + // Record the entries the annotations added or replaced, using the keys as stored + // by setMethodRolesMap, and restore any configured entry they replaced + for (Map.Entry<String, List<String>> entry : methodRolesMap.entrySet()) { + if (entry.getValue() != previousRoles.get(entry.getKey())) { + List<String> configured = configuredRoles.get(entry.getKey()); + if (configured != null) { + entry.setValue(configured); + } else { + annotationRoles.put(entry.getKey(), entry.getValue()); + } + } + } } protected void findRoles(Class<?> cls, Map<String, String> rolesMap) { diff --git a/core/src/test/java/org/apache/cxf/interceptor/security/SecureAnnotationsInterceptorTest.java b/core/src/test/java/org/apache/cxf/interceptor/security/SecureAnnotationsInterceptorTest.java index 7f2b948ccf1..57e23e6b343 100644 --- a/core/src/test/java/org/apache/cxf/interceptor/security/SecureAnnotationsInterceptorTest.java +++ b/core/src/test/java/org/apache/cxf/interceptor/security/SecureAnnotationsInterceptorTest.java @@ -24,7 +24,10 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.Method; import java.security.Principal; +import java.util.Collections; +import java.util.List; +import jakarta.annotation.security.RolesAllowed; import org.apache.cxf.common.security.SimplePrincipal; import org.apache.cxf.message.Exchange; import org.apache.cxf.message.ExchangeImpl; @@ -38,6 +41,7 @@ import org.apache.cxf.service.model.BindingOperationInfo; import org.junit.Before; import org.junit.Test; +import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -84,6 +88,80 @@ public class SecureAnnotationsInterceptorTest { in.handleMessage(message); } + @Test(expected = AccessDeniedException.class) + public void testAnnotationClassNameSetAfterSecuredObject() { + SecureAnnotationsInterceptor in = new SecureAnnotationsInterceptor(); + in.setSecuredObject(new TestService2()); + in.setAnnotationClassName(SecureRolesAllowed.class.getName()); + in.handleMessage(message); + } + + @Test + public void testAnnotationClassNameChangeReplacesAnnotationRoles() throws Exception { + Method echo = MixedService.class.getMethod("echo"); + SecureAnnotationsInterceptor in = new SecureAnnotationsInterceptor(); + in.setSecuredObject(new MixedService()); + assertEquals(Collections.singletonList("defaultRole"), in.getExpectedRoles(echo)); + + in.setAnnotationClassName(SecureRolesAllowed.class.getName()); + assertEquals(Collections.singletonList("secureRole"), in.getExpectedRoles(echo)); + } + + @Test + public void testAnnotationClassNameChangeKeepsConfiguredMethodRoles() throws Exception { + Method echo = MixedService.class.getMethod("echo"); + Method other = MixedService.class.getMethod("other"); + SecureAnnotationsInterceptor in = new SecureAnnotationsInterceptor(); + in.setSecuredObject(new MixedService()); + in.setMethodRolesMap(Collections.singletonMap("other", "admin")); + + in.setAnnotationClassName(SecureRolesAllowed.class.getName()); + assertEquals(Collections.singletonList("secureRole"), in.getExpectedRoles(echo)); + assertEquals(Collections.singletonList("admin"), in.getExpectedRoles(other)); + } + + @Test + public void testConfiguredMethodNameRolesKeptAfterAnnotationClassChange() throws Exception { + Method echo = MixedService.class.getMethod("echo"); + SecureAnnotationsInterceptor in = new SecureAnnotationsInterceptor(); + in.setSecuredObject(new MixedService()); + in.setMethodRolesMap(Collections.singletonMap("echo", "admin")); + + in.setAnnotationClassName(SecureRolesAllowed.class.getName()); + // As when the annotation class is set first: the annotation's signature key takes + // precedence, and the configured method name entry is kept + assertEquals(Collections.singletonList("secureRole"), in.getExpectedRoles(echo)); + assertEquals(Collections.singletonList("admin"), in.methodRolesMap.get("echo")); + } + + @Test + public void testConfiguredMethodRolesMatchAnnotationClassSetFirst() throws Exception { + Method echo = MixedService.class.getMethod("echo"); + SecureAnnotationsInterceptor in = new SecureAnnotationsInterceptor(); + in.setAnnotationClassName(SecureRolesAllowed.class.getName()); + in.setSecuredObject(new MixedService()); + in.setMethodRolesMap(Collections.singletonMap("echo", "admin")); + + assertEquals(Collections.singletonList("secureRole"), in.getExpectedRoles(echo)); + assertEquals(Collections.singletonList("admin"), in.methodRolesMap.get("echo")); + } + + @Test + public void testAnnotationClassNameChangeWithCustomMethodSig() throws Exception { + Method echo = MixedService.class.getMethod("echo"); + SecureAnnotationsInterceptor in = new SecureAnnotationsInterceptor() { + @Override + protected String createMethodSig(Method m) { + return " " + super.createMethodSig(m) + " "; + } + }; + in.setSecuredObject(new MixedService()); + in.setAnnotationClassName(SecureRolesAllowed.class.getName()); + assertEquals(Collections.singletonList("secureRole"), in.getExpectedRoles(echo)); + for (List<String> roles : in.methodRolesMap.values()) { + assertEquals(Collections.singletonList("secureRole"), roles); + } + } @Retention (RetentionPolicy.RUNTIME) @Target({ElementType.TYPE, ElementType.METHOD }) @@ -103,6 +181,16 @@ public class SecureAnnotationsInterceptorTest { } } + private static final class MixedService { + @RolesAllowed("defaultRole") + @SecureRolesAllowed("secureRole") + public void echo() { + } + + public void other() { + } + } + private static final class TestSecurityContext implements SecurityContext { public Principal getUserPrincipal() {
