This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/ClaimsAuthorizingInterceptor in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 1e4dd0715c8e526f64fca384022911af0086674a Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Wed Oct 28 16:25:06 2026 +0000 Resolve ClaimsAuthorizingInterceptor annotation claims by method signature --- .../interceptor/ClaimsAuthorizingInterceptor.java | 79 +++++++-- .../ClaimsAuthorizingInterceptorTest.java | 180 ++++++++++++++++++++- 2 files changed, 240 insertions(+), 19 deletions(-) diff --git a/rt/security/src/main/java/org/apache/cxf/rt/security/claims/interceptor/ClaimsAuthorizingInterceptor.java b/rt/security/src/main/java/org/apache/cxf/rt/security/claims/interceptor/ClaimsAuthorizingInterceptor.java index 94a9c9d7b11..2f7afad4c2d 100644 --- a/rt/security/src/main/java/org/apache/cxf/rt/security/claims/interceptor/ClaimsAuthorizingInterceptor.java +++ b/rt/security/src/main/java/org/apache/cxf/rt/security/claims/interceptor/ClaimsAuthorizingInterceptor.java @@ -31,6 +31,7 @@ import java.util.logging.Logger; import org.apache.cxf.common.logging.LogUtils; import org.apache.cxf.common.util.ClassHelper; +import org.apache.cxf.common.util.ReflectionUtil; import org.apache.cxf.interceptor.Fault; import org.apache.cxf.interceptor.security.AccessDeniedException; import org.apache.cxf.message.Message; @@ -53,7 +54,18 @@ public class ClaimsAuthorizingInterceptor extends AbstractPhaseInterceptor<Messa private static final Set<String> SKIP_METHODS = new HashSet<>( Arrays.asList("wait", "notify", "notifyAll", "equals", "toString", "hashCode")); + // Claims configured explicitly via setClaims, keyed by method name private Map<String, List<ClaimBean>> claims = new HashMap<>(); + // Claims discovered from @Claim/@Claims annotations, keyed by method signature so that + // overloaded methods do not share a rule and an annotation on an overriding method is + // not replaced by one from a superclass or interface + private final Map<String, List<ClaimBean>> methodClaimsBySignature = new HashMap<>(); + private final Map<String, List<ClaimBean>> classClaimsBySignature = new HashMap<>(); + // All the annotation-derived rules for a given method name, enforced together when the + // invoked method's signature is not recorded (e.g. it implements a generic interface method) + private final Map<String, List<List<ClaimBean>>> annotationClaimsByName = new HashMap<>(); + // Signatures of all the methods of the secured object + private final Set<String> securedSignatures = new HashSet<>(); private Map<String, String> nameAliases = Collections.emptyMap(); private Map<String, String> formatAliases = Collections.emptyMap(); @@ -82,7 +94,39 @@ public class ClaimsAuthorizingInterceptor extends AbstractPhaseInterceptor<Messa } protected boolean authorize(ClaimsSecurityContext sc, Method method) { - List<ClaimBean> list = claims.get(method.getName()); + String signature = ReflectionUtil.getMethodSignature(method); + List<List<ClaimBean>> rules = new ArrayList<>(); + // A method-level annotation anywhere in the hierarchy takes precedence over a + // class-level one + List<ClaimBean> annotationClaims = methodClaimsBySignature.get(signature); + if (annotationClaims == null) { + annotationClaims = classClaimsBySignature.get(signature); + } + if (annotationClaims != null) { + rules.add(annotationClaims); + } else if (annotationClaimsByName.containsKey(method.getName())) { + rules.addAll(annotationClaimsByName.get(method.getName())); + } + List<ClaimBean> configuredClaims = claims.get(method.getName()); + if (configuredClaims != null) { + rules.add(configuredClaims); + } + + if (rules.isEmpty()) { + // A method of the secured object without claims requirements is permitted, + // any other method is not + return securedSignatures.contains(signature); + } + // Every rule which applies to the method has to be satisfied + for (List<ClaimBean> rule : rules) { + if (!isAuthorized(sc, rule)) { + return false; + } + } + return true; + } + + private static boolean isAuthorized(ClaimsSecurityContext sc, List<ClaimBean> list) { org.apache.cxf.rt.security.claims.ClaimCollection actualClaims = sc.getClaims(); for (ClaimBean claimBean : list) { @@ -129,7 +173,7 @@ public class ClaimsAuthorizingInterceptor extends AbstractPhaseInterceptor<Messa public void setSecuredObject(Object object) { Class<?> cls = ClassHelper.getRealClass(object); findClaims(cls); - if (claims.isEmpty()) { + if (methodClaimsBySignature.isEmpty() && classClaimsBySignature.isEmpty()) { LOG.warning("The claims list is empty, the service object is not protected"); } } @@ -147,26 +191,27 @@ public class ClaimsAuthorizingInterceptor extends AbstractPhaseInterceptor<Messa List<ClaimBean> methodClaims = getClaims(m.getAnnotation(Claims.class), m.getAnnotation(Claim.class)); - List<ClaimBean> allClaims = new ArrayList<>(methodClaims); - for (ClaimBean bean : clsClaims) { - if (isClaimOverridden(bean, methodClaims)) { - continue; + // Subclasses are scanned before their superclasses and interfaces, so the most + // specific declaration of a given signature is recorded first and kept + String signature = ReflectionUtil.getMethodSignature(m); + securedSignatures.add(signature); + if (!methodClaims.isEmpty()) { + List<ClaimBean> allClaims = new ArrayList<>(methodClaims); + for (ClaimBean bean : clsClaims) { + if (isClaimOverridden(bean, methodClaims)) { + continue; + } + allClaims.add(bean); } - allClaims.add(bean); + methodClaimsBySignature.putIfAbsent(signature, allClaims); + annotationClaimsByName.computeIfAbsent(m.getName(), k -> new ArrayList<>()).add(allClaims); + } else if (!clsClaims.isEmpty()) { + classClaimsBySignature.putIfAbsent(signature, clsClaims); + annotationClaimsByName.computeIfAbsent(m.getName(), k -> new ArrayList<>()).add(clsClaims); } - - claims.put(m.getName(), allClaims); - } - if (!claims.isEmpty()) { - return; } findClaims(cls.getSuperclass()); - - if (!claims.isEmpty()) { - return; - } - for (Class<?> interfaceCls : cls.getInterfaces()) { findClaims(interfaceCls); } diff --git a/rt/security/src/test/java/org/apache/cxf/rt/security/claims/interceptor/ClaimsAuthorizingInterceptorTest.java b/rt/security/src/test/java/org/apache/cxf/rt/security/claims/interceptor/ClaimsAuthorizingInterceptorTest.java index 2a2e19db945..688dc7ca6f8 100644 --- a/rt/security/src/test/java/org/apache/cxf/rt/security/claims/interceptor/ClaimsAuthorizingInterceptorTest.java +++ b/rt/security/src/test/java/org/apache/cxf/rt/security/claims/interceptor/ClaimsAuthorizingInterceptorTest.java @@ -22,6 +22,7 @@ import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.lang.reflect.Method; import java.security.Principal; import java.util.Arrays; import java.util.Collections; @@ -226,6 +227,110 @@ public class ClaimsAuthorizingInterceptorTest { } } + @Test + public void testOverloadedMethodWithoutClaimsDoesNotRemoveClaims() throws Exception { + ClaimsAuthorizingInterceptor in = createInterceptor(new OverloadingService()); + Method adminMethod = OverloadingService.class.getMethod("delete", String.class); + Method unsecuredMethod = OverloadingService.class.getMethod("delete"); + + in.handleMessage(prepareMessage(adminMethod, createDefaultClaim("admin"))); + assertDenied(in, prepareMessage(adminMethod, createDefaultClaim("user"))); + + // A method without claims of its own has to meet the claims of its overloads + in.handleMessage(prepareMessage(unsecuredMethod, createDefaultClaim("admin"))); + assertDenied(in, prepareMessage(unsecuredMethod, createDefaultClaim("user"))); + } + + @Test + public void testOverloadedMethodsKeepTheirOwnClaims() throws Exception { + ClaimsAuthorizingInterceptor in = createInterceptor(new OverloadingService()); + Method adminMethod = OverloadingService.class.getMethod("update", String.class); + Method userMethod = OverloadingService.class.getMethod("update"); + + in.handleMessage(prepareMessage(adminMethod, createDefaultClaim("admin"))); + assertDenied(in, prepareMessage(adminMethod, createDefaultClaim("user"))); + in.handleMessage(prepareMessage(userMethod, createDefaultClaim("user"))); + assertDenied(in, prepareMessage(userMethod, createDefaultClaim("admin"))); + } + + @Test + public void testOverridingMethodClaimsTakePrecedenceOverSuperclass() throws Exception { + ClaimsAuthorizingInterceptor in = createInterceptor(new OverridingService()); + Method method = OverridingService.class.getMethod("update"); + + in.handleMessage(prepareMessage(method, createDefaultClaim("admin"))); + assertDenied(in, prepareMessage(method, createDefaultClaim("user"))); + } + + @Test + public void testInterfaceClaimsAreEnforced() throws Exception { + ClaimsAuthorizingInterceptor in = createInterceptor(new InterfaceService()); + Method implMethod = InterfaceService.class.getMethod("delete"); + Method interfaceMethod = SecuredApi.class.getMethod("delete"); + + in.handleMessage(prepareMessage(implMethod, createDefaultClaim("admin"))); + assertDenied(in, prepareMessage(implMethod, createDefaultClaim("user"))); + assertDenied(in, prepareMessage(interfaceMethod, createDefaultClaim("user"))); + } + + @Test + public void testGenericInterfaceMethodClaimsAreEnforced() throws Exception { + ClaimsAuthorizingInterceptor in = createInterceptor(new StringRepository()); + Method updateMethod = StringRepository.class.getMethod("update", String.class); + + // The erased interface signature does not match, so every rule for the name applies + in.handleMessage(prepareMessage(updateMethod, createDefaultClaim("admin", "user"))); + assertDenied(in, prepareMessage(updateMethod, createDefaultClaim("user"))); + assertDenied(in, prepareMessage(updateMethod, createDefaultClaim("admin"))); + } + + @Test + public void testMethodWithoutConfiguredClaimsIsDenied() throws Exception { + ClaimsAuthorizingInterceptor in = new ClaimsAuthorizingInterceptor(); + in.setClaims(Collections.singletonMap("update", + Collections.singletonList(createDefaultClaimBean("admin")))); + + in.handleMessage(prepareMessage(OverloadingService.class.getMethod("update"), + createDefaultClaim("admin"))); + assertDenied(in, prepareMessage(OverloadingService.class.getMethod("delete"), + createDefaultClaim("admin"))); + } + + @Test + public void testConfiguredClaimsAreEnforcedTogetherWithAnnotations() throws Exception { + ClaimsAuthorizingInterceptor in = createInterceptor(new OverloadingService()); + in.setClaims(Collections.singletonMap("update", + Collections.singletonList(createDefaultClaimBean("manager")))); + Method method = OverloadingService.class.getMethod("update"); + + in.handleMessage(prepareMessage(method, createDefaultClaim("user", "manager"))); + assertDenied(in, prepareMessage(method, createDefaultClaim("user"))); + assertDenied(in, prepareMessage(method, createDefaultClaim("manager"))); + } + + private static ClaimsAuthorizingInterceptor createInterceptor(Object service) { + ClaimsAuthorizingInterceptor in = new ClaimsAuthorizingInterceptor(); + in.setSecuredObject(service); + return in; + } + + private static void assertDenied(ClaimsAuthorizingInterceptor in, Message m) { + try { + in.handleMessage(m); + fail("AccessDeniedException expected"); + } catch (AccessDeniedException ex) { + // expected + } + } + + private static ClaimBean createDefaultClaimBean(String value) { + org.apache.cxf.rt.security.claims.Claim claim = new org.apache.cxf.rt.security.claims.Claim(); + claim.setClaimType(SAMLClaim.SAML_ROLE_ATTRIBUTENAME_DEFAULT); + claim.addValue(value); + return new ClaimBean(claim, "urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified", + ClaimMode.STRICT, false); + } + private void doTestClaims(String methodName, org.apache.cxf.rt.security.claims.Claim... claim) throws Exception { @@ -245,6 +350,17 @@ public class ClaimsAuthorizingInterceptorTest { String roleName, org.apache.cxf.rt.security.claims.Claim... claim) throws Exception { + return prepareMessage(cls.getMethod(methodName, new Class[]{}), roleName, claim); + } + + private Message prepareMessage(Method method, + org.apache.cxf.rt.security.claims.Claim... claim) { + return prepareMessage(method, SAMLClaim.SAML_ROLE_ATTRIBUTENAME_DEFAULT, claim); + } + + private Message prepareMessage(Method method, + String roleName, + org.apache.cxf.rt.security.claims.Claim... claim) { ClaimCollection claims = new ClaimCollection(); Collections.addAll(claims, claim); @@ -293,8 +409,7 @@ public class ClaimsAuthorizingInterceptorTest { Message m = new MessageImpl(); m.setExchange(new ExchangeImpl()); m.put(SecurityContext.class, sc); - m.put("org.apache.cxf.resource.method", - cls.getMethod(methodName, new Class[]{})); + m.put("org.apache.cxf.resource.method", method); return m; } @@ -366,6 +481,67 @@ public class ClaimsAuthorizingInterceptorTest { } } + public static class OverloadingService { + @Claim("admin") + public void delete(String id) { + + } + + public void delete() { + + } + + @Claim("admin") + public void update(String value) { + + } + + @Claim("user") + public void update() { + + } + } + public static class BaseService { + @Claim("user") + public void update() { + + } + } + public static class OverridingService extends BaseService { + @Override + @Claim("admin") + public void update() { + + } + } + public interface SecuredApi { + @Claim("admin") + void delete(); + } + public static class InterfaceService implements SecuredApi { + @Override + public void delete() { + + } + } + public interface Repository<T> { + @Claim("admin") + void update(T value); + + @Claim("user") + void update(); + } + public static class StringRepository implements Repository<String> { + @Override + public void update(String value) { + + } + + @Override + public void update() { + + } + } @Target(ElementType.METHOD) @Retention(RetentionPolicy.RUNTIME) public @interface SecureRole {
