Allow filtering non-collection types

marcusdacoregio · Mar 8, 2024 · a8ff86c · a8ff86c
1 parent bade66e
commit a8ff86c
Show file tree

Hide file tree

Showing 12 changed files with 416 additions and 19 deletions.
diff --git a/...k/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java b/...k/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java
@@ -102,6 +102,7 @@ static MethodInterceptor preAuthorizeAuthorizationMethodInterceptor(
 		AuthorizationManagerBeforeMethodInterceptor preAuthorize = AuthorizationManagerBeforeMethodInterceptor
 			.preAuthorize(manager(manager, registryProvider));
 		preAuthorize.setOrder(preAuthorize.getOrder() + configuration.interceptorOrderOffset);
+		preAuthorize.setApplicationContext(context);
 		return new DeferringMethodInterceptor<>(preAuthorize, (f) -> {
 			methodSecurityDefaultsProvider.ifAvailable(manager::setTemplateDefaults);
 			manager.setExpressionHandler(expressionHandlerProvider
@@ -125,6 +126,7 @@ static MethodInterceptor postAuthorizeAuthorizationMethodInterceptor(
 		AuthorizationManagerAfterMethodInterceptor postAuthorize = AuthorizationManagerAfterMethodInterceptor
 			.postAuthorize(manager(manager, registryProvider));
 		postAuthorize.setOrder(postAuthorize.getOrder() + configuration.interceptorOrderOffset);
+		postAuthorize.setApplicationContext(context);
 		return new DeferringMethodInterceptor<>(postAuthorize, (f) -> {
 			methodSecurityDefaultsProvider.ifAvailable(manager::setTemplateDefaults);
 			manager.setExpressionHandler(expressionHandlerProvider

diff --git a/...pringframework/security/config/annotation/method/configuration/MethodSecurityService.java b/...pringframework/security/config/annotation/method/configuration/MethodSecurityService.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,16 +17,21 @@
 package org.springframework.security.config.annotation.method.configuration;
 
 import java.util.List;
+import java.util.Optional;
 
 import jakarta.annotation.security.DenyAll;
 import jakarta.annotation.security.PermitAll;
 import jakarta.annotation.security.RolesAllowed;
+import org.aopalliance.intercept.MethodInvocation;
 
 import org.springframework.security.access.annotation.Secured;
 import org.springframework.security.access.prepost.PostAuthorize;
 import org.springframework.security.access.prepost.PostFilter;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.security.access.prepost.PreFilter;
+import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.authorization.method.DeniedHandler;
+import org.springframework.security.authorization.method.MethodAccessDeniedHandler;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.parameters.P;
 
@@ -108,4 +113,55 @@ public interface MethodSecurityService {
 	@RequireAdminRole
 	void repeatedAnnotations();
 
+	@PostFilter("hasRole('ADMIN')")
+	@DeniedHandler(CardNumberMaskingHandler.class)
+	String postFilterGetCardNumberIfAdmin(String cardNumber);
+
+	@PreFilter("hasRole('ADMIN')")
+	@DeniedHandler(CardNumberMaskingHandler.class)
+	String preFilterGetCardNumberIfAdmin(String cardNumber);
+
+	@PostAuthorize("hasRole('ADMIN')")
+	@DeniedHandler(CardNumberMaskingHandler.class)
+	String postAuthorizeGetCardNumberIfAdmin(String cardNumber);
+
+	@PreAuthorize("hasRole('ADMIN')")
+	@DeniedHandler(CardNumberMaskingHandler.class)
+	String preAuthorizeGetCardNumberIfAdmin(String cardNumber);
+
+	@PostFilter("hasRole('ADMIN')")
+	@DeniedHandler(MaskingHandler.class)
+	Optional<String> getOptionalIfAdmin(String value);
+
+	@DenyAll
+	@DeniedHandler(MaskingHandler.class)
+	String denyAllMasking();
+
+	class MaskingHandler implements MethodAccessDeniedHandler {
+
+		@Override
+		public Object handle(Object deniedObject, AuthorizationDecision decision) {
+			return "***";
+		}
+
+	}
+
+	class CardNumberMaskingHandler implements MethodAccessDeniedHandler {
+
+		static String MASK = "****-****-****-";
+
+		@Override
+		public Object handle(Object deniedObject, AuthorizationDecision decision) {
+			if (deniedObject instanceof String cardNumber) {
+				return MASK + cardNumber.substring(cardNumber.length() - 4);
+			}
+			if (deniedObject instanceof MethodInvocation mi) {
+				String cardNumber = (String) mi.getArguments()[0];
+				return MASK + cardNumber.substring(cardNumber.length() - 4);
+			}
+			return "***";
+		}
+
+	}
+
 }
diff --git a/...gframework/security/config/annotation/method/configuration/MethodSecurityServiceImpl.java b/...gframework/security/config/annotation/method/configuration/MethodSecurityServiceImpl.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
 package org.springframework.security.config.annotation.method.configuration;
 
 import java.util.List;
+import java.util.Optional;
 
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
@@ -126,4 +127,34 @@ public List<String> allAnnotations(List<String> list) {
 	public void repeatedAnnotations() {
 	}
 
+	@Override
+	public String postFilterGetCardNumberIfAdmin(String cardNumber) {
+		return cardNumber;
+	}
+
+	@Override
+	public String preFilterGetCardNumberIfAdmin(String cardNumber) {
+		return cardNumber;
+	}
+
+	@Override
+	public String postAuthorizeGetCardNumberIfAdmin(String cardNumber) {
+		return cardNumber;
+	}
+
+	@Override
+	public String preAuthorizeGetCardNumberIfAdmin(String cardNumber) {
+		return cardNumber;
+	}
+
+	@Override
+	public Optional<String> getOptionalIfAdmin(String value) {
+		return Optional.ofNullable(value);
+	}
+
+	@Override
+	public String denyAllMasking() {
+		return "ok";
+	}
+
 }
diff --git a/...urity/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java b/...urity/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java
@@ -22,6 +22,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Optional;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
@@ -662,6 +663,69 @@ public void methodWhenPostFilterMetaAnnotationThenFilters() {
 			.containsExactly("dave");
 	}
 
+	@Test
+	@WithMockUser
+	void getCardNumberWhenPostFilterAndNotAdminThenReturnMasked() {
+		this.spring.register(AuthzConfig.class, MethodSecurityServiceEnabledConfig.class).autowire();
+		MethodSecurityService service = this.spring.getContext().getBean(MethodSecurityService.class);
+		String cardNumber = service.postFilterGetCardNumberIfAdmin("4444-3333-2222-1111");
+		assertThat(cardNumber).isEqualTo("****-****-****-1111");
+	}
+
+	@Test
+	@WithMockUser
+	void getCardNumberWhenPreFilterAndNotAdminThenReturnMasked() {
+		this.spring.register(AuthzConfig.class, MethodSecurityServiceEnabledConfig.class).autowire();
+		MethodSecurityService service = this.spring.getContext().getBean(MethodSecurityService.class);
+		String cardNumber = service.preFilterGetCardNumberIfAdmin("4444-3333-2222-1111");
+		assertThat(cardNumber).isEqualTo("****-****-****-1111");
+	}
+
+	@Test
+	@WithMockUser
+	void getCardNumberWhenPostAuthorizeAndNotAdminThenReturnMasked() {
+		this.spring.register(AuthzConfig.class, MethodSecurityServiceEnabledConfig.class).autowire();
+		MethodSecurityService service = this.spring.getContext().getBean(MethodSecurityService.class);
+		String cardNumber = service.postAuthorizeGetCardNumberIfAdmin("4444-3333-2222-1111");
+		assertThat(cardNumber).isEqualTo("****-****-****-1111");
+	}
+
+	@Test
+	@WithMockUser
+	void getCardNumberWhenPreAuthorizeAndNotAdminThenReturnMasked() {
+		this.spring.register(AuthzConfig.class, MethodSecurityServiceEnabledConfig.class).autowire();
+		MethodSecurityService service = this.spring.getContext().getBean(MethodSecurityService.class);
+		String cardNumber = service.preAuthorizeGetCardNumberIfAdmin("4444-3333-2222-1111");
+		assertThat(cardNumber).isEqualTo("****-****-****-1111");
+	}
+
+	@Test
+	@WithMockUser(roles = "ADMIN")
+	void getCardNumberWhenPostFilterAndIsAdminThenReturnOriginal() {
+		this.spring.register(AuthzConfig.class, MethodSecurityServiceEnabledConfig.class).autowire();
+		MethodSecurityService service = this.spring.getContext().getBean(MethodSecurityService.class);
+		String cardNumber = service.postFilterGetCardNumberIfAdmin("4444-3333-2222-1111");
+		assertThat(cardNumber).isEqualTo("4444-3333-2222-1111");
+	}
+
+	@Test
+	@WithMockUser
+	void getOptionalIfAdminWhenNotAdminThenMasked() {
+		this.spring.register(AuthzConfig.class, MethodSecurityServiceEnabledConfig.class).autowire();
+		MethodSecurityService service = this.spring.getContext().getBean(MethodSecurityService.class);
+		Optional<String> opt = service.getOptionalIfAdmin("value");
+		assertThat(opt).hasValue("***");
+	}
+
+	@Test
+	@WithMockUser
+	void denyAllWhenMaskHandlerThenMasked() {
+		this.spring.register(AuthzConfig.class, MethodSecurityServiceEnabledConfig.class).autowire();
+		MethodSecurityService service = this.spring.getContext().getBean(MethodSecurityService.class);
+		String value = service.denyAllMasking();
+		assertThat(value).isEqualTo("***");
+	}
+
 	private static Consumer<ConfigurableWebApplicationContext> disallowBeanOverriding() {
 		return (context) -> ((AnnotationConfigWebApplicationContext) context).setAllowBeanDefinitionOverriding(false);
 	}
@@ -675,6 +739,16 @@ private static Advisor returnAdvisor(int order) {
 		return advisor;
 	}
 
+	@Configuration
+	static class AuthzConfig {
+
+		@Bean
+		Authz authz() {
+			return new Authz();
+		}
+
+	}
+
 	@Configuration
 	@EnableCustomMethodSecurity
 	static class CustomMethodSecurityServiceConfig {

diff --git a/...ngframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java b/...ngframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,13 +23,15 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.function.Supplier;
 import java.util.stream.Stream;
 
 import org.aopalliance.intercept.MethodInvocation;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.context.ApplicationContext;
 import org.springframework.core.ParameterNameDiscoverer;
 import org.springframework.core.log.LogMessage;
 import org.springframework.expression.EvaluationContext;
@@ -40,6 +42,9 @@
 import org.springframework.security.access.expression.ExpressionUtils;
 import org.springframework.security.authentication.AuthenticationTrustResolver;
 import org.springframework.security.authentication.AuthenticationTrustResolverImpl;
+import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.authorization.method.MethodAccessDeniedHandler;
+import org.springframework.security.authorization.method.MethodAccessDeniedHandlerResolver;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.parameters.DefaultSecurityParameterNameDiscoverer;
 import org.springframework.util.Assert;
@@ -67,6 +72,8 @@ public class DefaultMethodSecurityExpressionHandler extends AbstractSecurityExpr
 
 	private String defaultRolePrefix = "ROLE_";
 
+	private MethodAccessDeniedHandlerResolver accessDeniedHandlerResolver = new MethodAccessDeniedHandlerResolver();
+
 	public DefaultMethodSecurityExpressionHandler() {
 	}
 
@@ -133,8 +140,32 @@ public Object filter(Object filterTarget, Expression filterExpression, Evaluatio
 		if (filterTarget instanceof Stream) {
 			return filterStream((Stream<?>) filterTarget, filterExpression, ctx, rootObject);
 		}
-		throw new IllegalArgumentException(
-				"Filter target must be a collection, array, map or stream type, but was " + filterTarget);
+		if (filterTarget instanceof Optional<?> optional) {
+			return filterOptional(optional, filterExpression, ctx, rootObject);
+		}
+		return filterObject(filterTarget, filterExpression, ctx, rootObject);
+	}
+
+	private Object filterObject(Object filterObject, Expression filterExpression, EvaluationContext ctx,
+			MethodSecurityExpressionOperations rootObject) {
+		rootObject.setFilterObject(filterObject);
+		if (ExpressionUtils.evaluateAsBoolean(filterExpression, ctx)) {
+			return filterObject;
+		}
+		if (!(ctx instanceof MethodSecurityEvaluationContext methodCtx)) {
+			return null;
+		}
+		MethodAccessDeniedHandler deniedHandler = this.accessDeniedHandlerResolver.resolve(methodCtx.getMethod());
+		return (deniedHandler != null) ? deniedHandler.handle(filterObject, new AuthorizationDecision(false)) : null;
+	}
+
+	private <T> Object filterOptional(Optional<T> filterTarget, Expression filterExpression, EvaluationContext ctx,
+								MethodSecurityExpressionOperations rootObject) {
+		if (filterTarget.isEmpty()) {
+			return filterTarget;
+		}
+		Object filtered = filterObject(filterTarget.get(), filterExpression, ctx, rootObject);
+		return Optional.ofNullable(filtered);
 	}
 
 	private <T> Object filterCollection(Collection<T> filterTarget, Expression filterExpression, EvaluationContext ctx,
@@ -271,4 +302,10 @@ protected String getDefaultRolePrefix() {
 		return this.defaultRolePrefix;
 	}
 
+	@Override
+	public void setApplicationContext(ApplicationContext applicationContext) {
+		super.setApplicationContext(applicationContext);
+		this.accessDeniedHandlerResolver.setApplicationContext(applicationContext);
+	}
+
 }
diff --git a/...rg/springframework/security/access/expression/method/MethodSecurityEvaluationContext.java b/...rg/springframework/security/access/expression/method/MethodSecurityEvaluationContext.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,6 +39,8 @@
  */
 class MethodSecurityEvaluationContext extends MethodBasedEvaluationContext {
 
+	private Method method;
+
 	/**
 	 * Intended for testing. Don't use in practice as it creates a new parameter resolver
 	 * for each instance. Use the constructor which takes the resolver, as an argument
@@ -51,15 +53,21 @@ class MethodSecurityEvaluationContext extends MethodBasedEvaluationContext {
 	MethodSecurityEvaluationContext(Authentication user, MethodInvocation mi,
 			ParameterNameDiscoverer parameterNameDiscoverer) {
 		super(mi.getThis(), getSpecificMethod(mi), mi.getArguments(), parameterNameDiscoverer);
+		this.method = getSpecificMethod(mi);
 	}
 
 	MethodSecurityEvaluationContext(MethodSecurityExpressionOperations root, MethodInvocation mi,
 			ParameterNameDiscoverer parameterNameDiscoverer) {
 		super(root, getSpecificMethod(mi), mi.getArguments(), parameterNameDiscoverer);
+		this.method = getSpecificMethod(mi);
 	}
 
 	private static Method getSpecificMethod(MethodInvocation mi) {
 		return AopUtils.getMostSpecificMethod(mi.getMethod(), AopProxyUtils.ultimateTargetClass(mi.getThis()));
 	}
 
+	public Method getMethod() {
+		return this.method;
+	}
+
 }