This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/claims in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 090f5987ed7db2211e8403d2b6470879344f75a5 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Thu Nov 5 14:31:52 2026 +0000 Handle SAML claims without a NameFormat and combined claim annotations --- .../interceptor/ClaimsAuthorizingInterceptor.java | 34 ++++++++---- .../ClaimsAuthorizingInterceptorTest.java | 61 ++++++++++++++++++++++ 2 files changed, 85 insertions(+), 10 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 2c52512a6ed..4236388a6dc 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 @@ -53,6 +53,9 @@ public class ClaimsAuthorizingInterceptor extends AbstractPhaseInterceptor<Messa private static final Set<String> SKIP_METHODS = new HashSet<>( Arrays.asList("wait", "notify", "notifyAll", "equals", "toString", "hashCode")); + // The SAML 2.0 default for an Attribute without a NameFormat, and the @Claim default format + private static final String UNSPECIFIED_NAME_FORMAT = + "urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"; // Claims configured explicitly via setClaims, keyed by method name private Map<String, List<ClaimBean>> claims = new HashMap<>(); @@ -157,16 +160,23 @@ public class ClaimsAuthorizingInterceptor extends AbstractPhaseInterceptor<Messa org.apache.cxf.rt.security.claims.ClaimCollection actualClaims = sc.getClaims(); for (ClaimBean claimBean : list) { + String claimType = claimBean.getClaim().getClaimType(); org.apache.cxf.rt.security.claims.Claim matchingClaim = null; for (org.apache.cxf.rt.security.claims.Claim cl : actualClaims) { + if (claimType == null) { + break; + } if (cl instanceof SAMLClaim) { - // If it's a SAMLClaim the name + nameformat must match what's configured - if (((SAMLClaim)cl).getName().equals(claimBean.getClaim().getClaimType()) - && ((SAMLClaim)cl).getNameFormat().equals(claimBean.getClaimFormat())) { + // If it's a SAMLClaim the name + nameformat must match what's configured. The + // NameFormat is optional, in which case it is the unspecified name format + SAMLClaim samlClaim = (SAMLClaim)cl; + if (claimType.equals(samlClaim.getName()) + && getNameFormat(claimBean.getClaimFormat()) + .equals(getNameFormat(samlClaim.getNameFormat()))) { matchingClaim = cl; break; } - } else if (cl.getClaimType().equals(claimBean.getClaim().getClaimType())) { + } else if (claimType.equals(cl.getClaimType())) { matchingClaim = cl; break; } @@ -256,6 +266,10 @@ public class ClaimsAuthorizingInterceptor extends AbstractPhaseInterceptor<Messa } } + private static String getNameFormat(String nameFormat) { + return nameFormat == null ? UNSPECIFIED_NAME_FORMAT : nameFormat; + } + private static boolean isClaimOverridden(ClaimBean bean, List<ClaimBean> mClaims) { for (ClaimBean methodBean : mClaims) { if (bean.getClaim().getClaimType().equals(methodBean.getClaim().getClaimType()) @@ -270,13 +284,13 @@ public class ClaimsAuthorizingInterceptor extends AbstractPhaseInterceptor<Messa Claims claimsAnn, Claim claimAnn) { List<ClaimBean> claimsList = new ArrayList<>(); - final List<Claim> annClaims; + // Both @Claims and @Claim may be present, in which case all their claims apply + final List<Claim> annClaims = new ArrayList<>(); if (claimsAnn != null) { - annClaims = Arrays.asList(claimsAnn.value()); - } else if (claimAnn != null) { - annClaims = Collections.singletonList(claimAnn); - } else { - annClaims = Collections.emptyList(); + annClaims.addAll(Arrays.asList(claimsAnn.value())); + } + if (claimAnn != null) { + annClaims.add(claimAnn); } for (Claim ann : annClaims) { org.apache.cxf.rt.security.claims.Claim claim = new org.apache.cxf.rt.security.claims.Claim(); 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 e0da1ca8352..09c66236acc 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 @@ -536,6 +536,43 @@ public class ClaimsAuthorizingInterceptorTest { assertDenied(in, prepareMessage(method, createDefaultClaim("user"))); } + @Test + public void testClaimWithoutNameFormat() throws Exception { + // A SAML attribute without a NameFormat has the unspecified name format, the default + ClaimsAuthorizingInterceptor in = createInterceptor(new NameFormatService()); + Method method = NameFormatService.class.getMethod("defaultFormat"); + + in.handleMessage(prepareMessage(method, + createClaim(SAMLClaim.SAML_ROLE_ATTRIBUTENAME_DEFAULT, null, "admin"))); + assertDenied(in, prepareMessage(method, + createClaim(SAMLClaim.SAML_ROLE_ATTRIBUTENAME_DEFAULT, null, "user"))); + } + + @Test + public void testClaimWithoutNameFormatDoesNotMatchOtherFormat() throws Exception { + ClaimsAuthorizingInterceptor in = createInterceptor(new NameFormatService()); + Method method = NameFormatService.class.getMethod("specificFormat"); + + assertDenied(in, prepareMessage(method, createClaim("http://cxf/roles";, null, "admin"))); + in.handleMessage(prepareMessage(method, createClaim("http://cxf/roles";, "http://claims";, "admin"))); + } + + @Test + public void testClaimAndClaimsAnnotationsAreCombined() throws Exception { + ClaimsAuthorizingInterceptor in = createInterceptor(new CombinedClaimsService()); + Method method = CombinedClaimsService.class.getMethod("update"); + org.apache.cxf.rt.security.claims.Claim location = createClaim("http://location";, + "urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified", "UK"); + + in.handleMessage(prepareMessage(method, createDefaultClaim("admin"), location)); + assertDenied(in, prepareMessage(method, createDefaultClaim("admin"))); + assertDenied(in, prepareMessage(method, location)); + + Method classMethod = CombinedClaimsService.class.getMethod("list"); + in.handleMessage(prepareMessage(classMethod, createDefaultClaim("user"), location)); + assertDenied(in, prepareMessage(classMethod, createDefaultClaim("user"))); + } + private static ClaimsAuthorizingInterceptor createInterceptor(Object service) { ClaimsAuthorizingInterceptor in = new ClaimsAuthorizingInterceptor(); in.setSecuredObject(service); @@ -756,6 +793,30 @@ public class ClaimsAuthorizingInterceptorTest { @Claim(name = "http://location";, value = "UK") public static class LocationInheritingService extends BaseService { } + public static class NameFormatService { + @Claim("admin") + public void defaultFormat() { + + } + + @Claim(name = "http://cxf/roles";, format = "http://claims";, value = "admin") + public void specificFormat() { + + } + } + @Claims({@Claim(name = "http://location";, value = "UK") }) + @Claim("user") + public static class CombinedClaimsService { + @Claims({@Claim(name = "http://location";, value = "UK") }) + @Claim("admin") + public void update() { + + } + + public void list() { + + } + } public static class OverridingService extends BaseService { @Override @Claim("admin")
