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 5948451179a Resolve generic overrides exactly in
SecureAnnotationsInterceptor (#3679)
5948451179a is described below
commit 5948451179a375fbda69b8c0ec6d898d3b166647
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Wed Nov 4 15:26:26 2026 +0000
Resolve generic overrides exactly in SecureAnnotationsInterceptor (#3679)
---
.../security/SecureAnnotationsInterceptor.java | 33 ++++--
.../security/SecureAnnotationsInterceptorTest.java | 112 ++++++++++++++++++++-
2 files changed, 133 insertions(+), 12 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 284a8d5364f..0ec35183c1f 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
@@ -52,6 +52,9 @@ public class SecureAnnotationsInterceptor extends
SimpleAuthorizingInterceptor {
// the secured object's hierarchy. Keying by signature means that
overloaded methods do not
// share a rule
private final Map<String, List<RolesDeclaration>> declarationsBySignature
= new HashMap<>();
+ // The erased signatures a method overrides with a different signature,
e.g. update(Object)
+ // for update(String) implementing update(T) of Repository<String>
+ private final Map<String, Set<String>> overriddenSignatures = new
HashMap<>();
private final Map<String, Set<String>> signaturesByName = new HashMap<>();
private final Set<Class<?>> securedClasses = new HashSet<>();
// The classes passed to setSecuredObject, so their roles can be read
again if the
@@ -93,6 +96,7 @@ public class SecureAnnotationsInterceptor extends
SimpleAuthorizingInterceptor {
return;
}
declarationsBySignature.clear();
+ overriddenSignatures.clear();
signaturesByName.clear();
securedClasses.clear();
for (Class<?> cls : securedObjectClasses) {
@@ -152,6 +156,12 @@ public class SecureAnnotationsInterceptor extends
SimpleAuthorizingInterceptor {
declarations.add(new RolesDeclaration(classRolesDeclarer,
classRoles, false));
}
signaturesByName.computeIfAbsent(m.getName(), k -> new
HashSet<>()).add(signature);
+ if (!m.isBridge()) {
+ for (Method overridden :
ReflectionUtil.getGenericallyOverriddenMethods(cls, m)) {
+ overriddenSignatures.computeIfAbsent(signature, k -> new
HashSet<>())
+ .add(ReflectionUtil.getMethodSignature(overridden));
+ }
+ }
}
findRoles(cls.getSuperclass(), rolesMap);
for (Class<?> interfaceCls : cls.getInterfaces()) {
@@ -205,18 +215,21 @@ public class SecureAnnotationsInterceptor extends
SimpleAuthorizingInterceptor {
}
private List<List<String>> getAnnotationRoles(Method method) {
- List<List<String>> rules =
getAnnotationRoles(ReflectionUtil.getMethodSignature(method));
- if (!rules.isEmpty()) {
- return rules;
+ String signature = ReflectionUtil.getMethodSignature(method);
+ Set<String> signatures = new HashSet<>();
+ if (declarationsBySignature.containsKey(signature)) {
+ signatures.add(signature);
+ signatures.addAll(overriddenSignatures.getOrDefault(signature,
Collections.emptySet()));
+ } else {
+ // The method is not one of the secured object's methods, so every
rule
+ // recorded for its name applies
+ signatures.addAll(signaturesByName.getOrDefault(method.getName(),
Collections.emptySet()));
}
- // No rule applies to the method's signature, e.g. because it
implements a generic
- // interface method whose erased signature differs, so every rule
recorded for its
- // name applies
- Set<List<String>> nameRules = new LinkedHashSet<>();
- for (String s : signaturesByName.getOrDefault(method.getName(),
Collections.emptySet())) {
- nameRules.addAll(getAnnotationRoles(s));
+ Set<List<String>> rules = new LinkedHashSet<>();
+ for (String s : signatures) {
+ rules.addAll(getAnnotationRoles(s));
}
- return new ArrayList<>(nameRules);
+ return new ArrayList<>(rules);
}
private List<List<String>> getAnnotationRoles(String signature) {
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 4956b01fb5a..290c9ca1a4c 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
@@ -351,11 +351,61 @@ public class SecureAnnotationsInterceptorTest {
}
@Test
- public void testGenericInterfaceMethodRolesStillApply() throws Exception {
+ public void testGenericInterfaceMethodRolesAreEnforced() throws Exception {
+ // update(String) overrides the erased update(Object), but not the
update() overload
Method target = StringRepository.class.getMethod("update",
String.class);
-
+ assertAllowed(new StringRepository(), target, "admin");
assertDenied(new StringRepository(), target, "user");
assertDenied(new StringRepository(), target);
+
+ Method userMethod = StringRepository.class.getMethod("update");
+ assertAllowed(new StringRepository(), userMethod, "user");
+ assertDenied(new StringRepository(), userMethod, "admin");
+ }
+
+ @Test
+ public void testGenericInterfaceMethodRolesApplyTogetherWithClassRoles()
throws Exception {
+ Method target = ClassRolesStringRepository.class.getMethod("update",
String.class);
+
+ assertAllowed(new ClassRolesStringRepository(), target, "user",
"admin");
+ assertDenied(new ClassRolesStringRepository(), target, "user");
+ assertDenied(new ClassRolesStringRepository(), target, "admin");
+ }
+
+ @Test
+ public void
testGenericInterfaceMethodRolesApplyToInheritedImplementation() throws
Exception {
+ Method target = InheritingStringRepository.class.getMethod("update",
String.class);
+
+ assertAllowed(new InheritingStringRepository(), target, "admin");
+ assertDenied(new InheritingStringRepository(), target, "user");
+ }
+
+ @Test
+ public void testGenericInterfaceMethodRolesApplyThroughTypeVariables()
throws Exception {
+ Method target = TypedStringRepository.class.getMethod("update",
String.class);
+
+ assertAllowed(new TypedStringRepository(), target, "admin");
+ assertDenied(new TypedStringRepository(), target, "user");
+ }
+
+ @Test
+ public void testUnrelatedOverloadIsNotLinkedToGenericInterfaceMethod()
throws Exception {
+ Method integerMethod =
OverloadingStringRepository.class.getMethod("update", Integer.class);
+ assertAllowed(new OverloadingStringRepository(), integerMethod,
"user");
+ assertDenied(new OverloadingStringRepository(), integerMethod,
"admin");
+
+ Method stringMethod =
OverloadingStringRepository.class.getMethod("update", String.class);
+ assertAllowed(new OverloadingStringRepository(), stringMethod,
"admin");
+ assertDenied(new OverloadingStringRepository(), stringMethod, "user");
+ }
+
+ @Test
+ public void testUnannotatedOverloadIsNotSecuredByOtherOverloads() throws
Exception {
+ Method target = UnannotatedOverloadService.class.getMethod("delete");
+
+ assertAllowed(new UnannotatedOverloadService(), target, "user");
+ assertDenied(new UnannotatedOverloadService(),
+ UnannotatedOverloadService.class.getMethod("delete",
String.class), "user");
}
@Test
@@ -627,6 +677,64 @@ public class SecureAnnotationsInterceptorTest {
}
}
+ @RolesAllowed("user")
+ public static class ClassRolesStringRepository implements
Repository<String> {
+ @Override
+ public void update(String value) {
+ }
+
+ @Override
+ public void update() {
+ }
+ }
+
+ public static class StringUpdater {
+ public void update(String value) {
+ }
+
+ public void update() {
+ }
+ }
+
+ public static class InheritingStringRepository extends StringUpdater
implements Repository<String> {
+ }
+
+ public abstract static class AbstractRepository<T> implements
Repository<T> { // NOPMD
+ }
+
+ public static class TypedStringRepository extends
AbstractRepository<String> {
+ @Override
+ public void update(String value) {
+ }
+
+ @Override
+ public void update() {
+ }
+ }
+
+ public static class OverloadingStringRepository implements
Repository<String> {
+ @Override
+ public void update(String value) {
+ }
+
+ @Override
+ public void update() {
+ }
+
+ @RolesAllowed("user")
+ public void update(Integer value) {
+ }
+ }
+
+ public static class UnannotatedOverloadService {
+ @RolesAllowed("admin")
+ public void delete(String id) {
+ }
+
+ public void delete() {
+ }
+ }
+
public static class UnsecuredService {
public void delete() {
}
