This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch 4.1.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/4.1.x-fixes by this push:
new 1c15944aced Don't leave methods unprotected when a role annotation
can't be read (#3687)
1c15944aced is described below
commit 1c15944aced0c1b7409aad8eab001225b7bb06d0
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Fri Nov 6 17:00:27 2026 +0000
Don't leave methods unprotected when a role annotation can't be read (#3687)
* Don't leave methods unprotected when a role annotation can't be read
* Addressing review comment
(cherry picked from commit ba370b89eb710e6bd88742b97ff907d5fdd3de80)
---
.../security/SecureAnnotationsInterceptor.java | 43 ++++++++++--
.../security/SecureAnnotationsInterceptorTest.java | 50 +++++++++++++
.../security/app/PackagePrivateAnnotationTest.java | 82 ++++++++++++++++++++++
3 files changed, 168 insertions(+), 7 deletions(-)
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 0ec35183c1f..29b32544552 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
@@ -69,16 +69,39 @@ public class SecureAnnotationsInterceptor extends
SimpleAuthorizingInterceptor {
}
public void setAnnotationClassName(String name) {
+ final Class<?> annotationClass;
try {
- ClassLoaderUtils.loadClass(name,
SecureAnnotationsInterceptor.class);
- annotationClassName = name;
+ annotationClass = ClassLoaderUtils.loadClass(name,
SecureAnnotationsInterceptor.class);
} catch (Throwable ex) {
ex.printStackTrace();
throw new IllegalArgumentException("Annotation class " + name + "
is not available");
}
+ // The roles are read from the annotation's value(), so reject an
annotation without one
+ // rather than leave the methods it is on unprotected
+ if (!annotationClass.isAnnotation() ||
!hasRolesValue(annotationClass)) {
+ throw new IllegalArgumentException("Annotation class " + name
+ + " is not an annotation with a String or String[] value()");
+ }
+ // The annotation may not be public, e.g. an application's
package-private annotation
+ try {
+ ReflectionUtil.setAccessible(annotationClass.getMethod("value"));
+ } catch (Exception ex) {
+ throw new IllegalArgumentException("The value() of annotation
class " + name
+ + " can not be accessed", ex);
+ }
+ annotationClassName = name;
resolveSecuredObjectRoles();
}
+ private static boolean hasRolesValue(Class<?> annotationClass) {
+ try {
+ Class<?> valueType =
annotationClass.getMethod("value").getReturnType();
+ return valueType == String.class || valueType == String[].class;
+ } catch (NoSuchMethodException ex) {
+ return false;
+ }
+ }
+
public void setSecuredObject(Object object) {
Class<?> cls = ClassHelper.getRealClass(object);
securedObjectClasses.add(cls);
@@ -280,13 +303,19 @@ public class SecureAnnotationsInterceptor extends
SimpleAuthorizingInterceptor {
for (Annotation ann : anns) {
if (ann.annotationType().getName().equals(annotationClassName)) {
try {
- Method valueMethod =
ann.annotationType().getMethod("value");
- String[] roles = (String[])valueMethod.invoke(ann);
- return Arrays.asList(roles);
+ Object roles =
+
ReflectionUtil.setAccessible(ann.annotationType().getMethod("value")).invoke(ann);
+ if (roles instanceof String[]) {
+ return Arrays.asList((String[])roles);
+ } else if (roles instanceof String) {
+ return Collections.singletonList((String)roles);
+ }
+ LOG.warning("The value of " + ann + " is not a String or
String[]");
} catch (Exception ex) {
- // ignore
+ LOG.log(Level.WARNING, "The roles of " + ann + " can not
be read", ex);
}
- break;
+ // The method is protected by the annotation, so deny every
user
+ return Collections.emptyList();
}
}
return null;
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 290c9ca1a4c..290a5dcf074 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
@@ -169,6 +169,33 @@ public class SecureAnnotationsInterceptorTest {
}
}
+ @Test
+ public void testSingleRoleAnnotation() throws Exception {
+ Method target = SingleRoleService.class.getMethod("operation");
+
+ SecureAnnotationsInterceptor in = createInterceptor(new
SingleRoleService(), SingleRole.class);
+ assertAllowed(in, target, "admin");
+ assertDenied(in, target, "user");
+ assertDenied(in, target);
+ }
+
+ @Test
+ public void testAnnotationClassWithoutRolesValueIsRejected() throws
Exception {
+ SecureAnnotationsInterceptor in = new SecureAnnotationsInterceptor();
+ assertThrows(IllegalArgumentException.class,
+ () -> in.setAnnotationClassName(IntValueRoles.class.getName()));
+ assertThrows(IllegalArgumentException.class,
+ () -> in.setAnnotationClassName(NoValueRoles.class.getName()));
+ assertThrows(IllegalArgumentException.class,
+ () -> in.setAnnotationClassName(String.class.getName()));
+
+ // The annotation class is unchanged
+ in.setSecuredObject(new SameClassOverloadingService());
+ Method target =
SameClassOverloadingService.class.getMethod("operation");
+ assertAllowed(in, target, "user");
+ assertDenied(in, target, "admin");
+ }
+
@Test
public void testOverloadedSubclassMethodKeepsItsOwnRoles() throws
Exception {
Method adminMethod = OverloadingService.class.getMethod("operation",
String.class);
@@ -746,6 +773,29 @@ public class SecureAnnotationsInterceptorTest {
String[] value();
}
+ @Retention (RetentionPolicy.RUNTIME)
+ @Target({ElementType.TYPE, ElementType.METHOD })
+ public @interface SingleRole {
+ String value();
+ }
+
+ @Retention (RetentionPolicy.RUNTIME)
+ @Target({ElementType.TYPE, ElementType.METHOD })
+ public @interface IntValueRoles {
+ int value();
+ }
+
+ @Retention (RetentionPolicy.RUNTIME)
+ @Target({ElementType.TYPE, ElementType.METHOD })
+ public @interface NoValueRoles {
+ }
+
+ public static class SingleRoleService {
+ @SingleRole("admin")
+ public void operation() {
+ }
+ }
+
private static final class TestService {
@SecureRolesAllowed("testRole")
public void echo() {
diff --git
a/core/src/test/java/org/apache/cxf/interceptor/security/app/PackagePrivateAnnotationTest.java
b/core/src/test/java/org/apache/cxf/interceptor/security/app/PackagePrivateAnnotationTest.java
new file mode 100644
index 00000000000..03f6ed164f4
--- /dev/null
+++
b/core/src/test/java/org/apache/cxf/interceptor/security/app/PackagePrivateAnnotationTest.java
@@ -0,0 +1,82 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cxf.interceptor.security.app;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.security.Principal;
+
+import org.apache.cxf.interceptor.security.AccessDeniedException;
+import org.apache.cxf.interceptor.security.SecureAnnotationsInterceptor;
+import org.apache.cxf.message.ExchangeImpl;
+import org.apache.cxf.message.Message;
+import org.apache.cxf.message.MessageImpl;
+import org.apache.cxf.security.SecurityContext;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertThrows;
+
+/**
+ * A role annotation which is not public, in a different package from the
interceptor.
+ */
+public class PackagePrivateAnnotationTest {
+
+ @Test
+ public void testPackagePrivateAnnotation() throws Exception {
+ SecureAnnotationsInterceptor in = new SecureAnnotationsInterceptor();
+ in.setAnnotationClassName(AppRoles.class.getName());
+ in.setSecuredObject(new AppService());
+
+ in.handleMessage(createMessage("admin"));
+ assertThrows(AccessDeniedException.class, () ->
in.handleMessage(createMessage("user")));
+ }
+
+ private static Message createMessage(String role) throws Exception {
+ Message message = new MessageImpl();
+ message.setExchange(new ExchangeImpl());
+ message.put("org.apache.cxf.resource.method",
AppService.class.getMethod("update"));
+ message.put(SecurityContext.class, new SecurityContext() {
+ @Override
+ public Principal getUserPrincipal() {
+ return () -> "bob";
+ }
+
+ @Override
+ public boolean isUserInRole(String r) {
+ return role.equals(r);
+ }
+ });
+ return message;
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target({ElementType.TYPE, ElementType.METHOD })
+ @interface AppRoles {
+ String[] value();
+ }
+
+ public static class AppService {
+ @AppRoles("admin")
+ public void update() {
+ }
+ }
+}
