Browse Source

Default Handler Resolution to Reflection-Based

Closes gh-15496
Josh Cummings 1 year ago
parent
commit
34d964eb08

+ 2 - 1
core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeExpressionAttributeRegistry.java

@@ -42,7 +42,8 @@ final class PostAuthorizeExpressionAttributeRegistry extends AbstractExpressionA
 	private Function<Class<? extends MethodAuthorizationDeniedHandler>, MethodAuthorizationDeniedHandler> handlerResolver;
 
 	PostAuthorizeExpressionAttributeRegistry() {
-		this.handlerResolver = (clazz) -> this.defaultHandler;
+		this.handlerResolver = (clazz) -> new ReflectiveMethodAuthorizationDeniedHandler(clazz,
+				PostAuthorizeAuthorizationManager.class);
 	}
 
 	@NonNull

+ 2 - 1
core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeExpressionAttributeRegistry.java

@@ -42,7 +42,8 @@ final class PreAuthorizeExpressionAttributeRegistry extends AbstractExpressionAt
 	private Function<Class<? extends MethodAuthorizationDeniedHandler>, MethodAuthorizationDeniedHandler> handlerResolver;
 
 	PreAuthorizeExpressionAttributeRegistry() {
-		this.handlerResolver = (clazz) -> this.defaultHandler;
+		this.handlerResolver = (clazz) -> new ReflectiveMethodAuthorizationDeniedHandler(clazz,
+				PreAuthorizeAuthorizationManager.class);
 	}
 
 	@NonNull

+ 67 - 0
core/src/main/java/org/springframework/security/authorization/method/ReflectiveMethodAuthorizationDeniedHandler.java

@@ -0,0 +1,67 @@
+/*
+ * 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://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.springframework.security.authorization.method;
+
+import org.aopalliance.intercept.MethodInvocation;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+import org.springframework.security.authorization.AuthorizationResult;
+
+final class ReflectiveMethodAuthorizationDeniedHandler implements MethodAuthorizationDeniedHandler {
+
+	private final Log logger = LogFactory.getLog(getClass());
+
+	private final Class<?> targetClass;
+
+	private final Class<?> managerClass;
+
+	ReflectiveMethodAuthorizationDeniedHandler(Class<?> targetClass, Class<?> managerClass) {
+		this.logger.debug(
+				"Will attempt to instantiate handlerClass attributes using reflection since no application context was supplied to "
+						+ managerClass);
+		this.targetClass = targetClass;
+		this.managerClass = managerClass;
+	}
+
+	@Override
+	public Object handleDeniedInvocation(MethodInvocation methodInvocation, AuthorizationResult authorizationResult) {
+		return constructMethodAuthorizationDeniedHandler().handleDeniedInvocation(methodInvocation,
+				authorizationResult);
+	}
+
+	@Override
+	public Object handleDeniedInvocationResult(MethodInvocationResult methodInvocationResult,
+			AuthorizationResult authorizationResult) {
+		return constructMethodAuthorizationDeniedHandler().handleDeniedInvocationResult(methodInvocationResult,
+				authorizationResult);
+	}
+
+	private MethodAuthorizationDeniedHandler constructMethodAuthorizationDeniedHandler() {
+		try {
+			return ((MethodAuthorizationDeniedHandler) this.targetClass.getConstructor().newInstance());
+		}
+		catch (Exception ex) {
+			throw new IllegalArgumentException("Failed to construct instance of " + this.targetClass
+					+ ". Please either add a public default constructor to the class "
+					+ " or publish an instance of it as a Spring bean. If you publish it as a Spring bean, "
+					+ " either add `@EnableMethodSecurity` to your configuration or "
+					+ " provide the `ApplicationContext` directly to " + this.managerClass, ex);
+		}
+	}
+
+}

+ 71 - 0
core/src/test/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManagerTests.java

@@ -23,8 +23,10 @@ import java.util.Collections;
 import java.util.List;
 import java.util.function.Supplier;
 
+import org.aopalliance.intercept.MethodInvocation;
 import org.junit.jupiter.api.Test;
 
+import org.springframework.context.support.GenericApplicationContext;
 import org.springframework.core.annotation.AnnotationConfigurationException;
 import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
 import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler;
@@ -33,6 +35,7 @@ import org.springframework.security.access.prepost.PostAuthorize;
 import org.springframework.security.authentication.TestAuthentication;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.authorization.AuthorizationResult;
 import org.springframework.security.core.Authentication;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -167,6 +170,34 @@ public class PostAuthorizeAuthorizationManagerTests {
 			.isThrownBy(() -> manager.check(authentication, result));
 	}
 
+	@Test
+	public void checkWhenHandlerDeniedNoApplicationContextThenReflectivelyConstructs() throws Exception {
+		PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager();
+		assertThat(handleDeniedInvocationResult("methodOne", manager)).isNull();
+		assertThatExceptionOfType(IllegalArgumentException.class)
+			.isThrownBy(() -> handleDeniedInvocationResult("methodTwo", manager));
+	}
+
+	@Test
+	public void checkWhenHandlerDeniedApplicationContextThenLooksForBean() throws Exception {
+		GenericApplicationContext context = new GenericApplicationContext();
+		context.registerBean(NoDefaultConstructorHandler.class, () -> new NoDefaultConstructorHandler(new Object()));
+		context.refresh();
+		PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager();
+		manager.setApplicationContext(context);
+		assertThat(handleDeniedInvocationResult("methodTwo", manager)).isNull();
+		assertThatExceptionOfType(IllegalStateException.class)
+			.isThrownBy(() -> handleDeniedInvocationResult("methodOne", manager));
+	}
+
+	private Object handleDeniedInvocationResult(String methodName, PostAuthorizeAuthorizationManager manager)
+			throws Exception {
+		MethodInvocation invocation = new MockMethodInvocation(new UsingHandleDeniedAuthorization(),
+				UsingHandleDeniedAuthorization.class, methodName);
+		MethodInvocationResult result = new MethodInvocationResult(invocation, null);
+		return manager.handleDeniedInvocationResult(result, null);
+	}
+
 	public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
 
 		public void doSomething() {
@@ -237,4 +268,44 @@ public class PostAuthorizeAuthorizationManagerTests {
 
 	}
 
+	public static final class UsingHandleDeniedAuthorization {
+
+		@HandleAuthorizationDenied(handlerClass = NullHandler.class)
+		@PostAuthorize("denyAll()")
+		public String methodOne() {
+			return "ok";
+		}
+
+		@HandleAuthorizationDenied(handlerClass = NoDefaultConstructorHandler.class)
+		@PostAuthorize("denyAll()")
+		public String methodTwo() {
+			return "ok";
+		}
+
+	}
+
+	public static final class NullHandler implements MethodAuthorizationDeniedHandler {
+
+		@Override
+		public Object handleDeniedInvocation(MethodInvocation methodInvocation,
+				AuthorizationResult authorizationResult) {
+			return null;
+		}
+
+	}
+
+	public static final class NoDefaultConstructorHandler implements MethodAuthorizationDeniedHandler {
+
+		private NoDefaultConstructorHandler(Object parameter) {
+
+		}
+
+		@Override
+		public Object handleDeniedInvocation(MethodInvocation methodInvocation,
+				AuthorizationResult authorizationResult) {
+			return null;
+		}
+
+	}
+
 }

+ 70 - 0
core/src/test/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManagerTests.java

@@ -20,9 +20,11 @@ import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.function.Supplier;
 
+import org.aopalliance.intercept.MethodInvocation;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.aop.TargetClassAware;
+import org.springframework.context.support.GenericApplicationContext;
 import org.springframework.core.annotation.AnnotationConfigurationException;
 import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
 import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler;
@@ -31,6 +33,7 @@ import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.security.authentication.TestAuthentication;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.authorization.AuthorizationResult;
 import org.springframework.security.core.Authentication;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -147,6 +150,33 @@ public class PreAuthorizeAuthorizationManagerTests {
 		assertThat(decision.isGranted()).isTrue();
 	}
 
+	@Test
+	public void checkWhenHandlerDeniedNoApplicationContextThenReflectivelyConstructs() throws Exception {
+		PreAuthorizeAuthorizationManager manager = new PreAuthorizeAuthorizationManager();
+		assertThat(handleDeniedInvocationResult("methodOne", manager)).isNull();
+		assertThatExceptionOfType(IllegalArgumentException.class)
+			.isThrownBy(() -> handleDeniedInvocationResult("methodTwo", manager));
+	}
+
+	@Test
+	public void checkWhenHandlerDeniedApplicationContextThenLooksForBean() throws Exception {
+		GenericApplicationContext context = new GenericApplicationContext();
+		context.registerBean(NoDefaultConstructorHandler.class, () -> new NoDefaultConstructorHandler(new Object()));
+		context.refresh();
+		PreAuthorizeAuthorizationManager manager = new PreAuthorizeAuthorizationManager();
+		manager.setApplicationContext(context);
+		assertThat(handleDeniedInvocationResult("methodTwo", manager)).isNull();
+		assertThatExceptionOfType(IllegalStateException.class)
+			.isThrownBy(() -> handleDeniedInvocationResult("methodOne", manager));
+	}
+
+	private Object handleDeniedInvocationResult(String methodName, PreAuthorizeAuthorizationManager manager)
+			throws Exception {
+		MethodInvocation invocation = new MockMethodInvocation(new UsingHandleDeniedAuthorization(),
+				UsingHandleDeniedAuthorization.class, methodName);
+		return manager.handleDeniedInvocation(invocation, null);
+	}
+
 	public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
 
 		public void doSomething() {
@@ -241,4 +271,44 @@ public class PreAuthorizeAuthorizationManagerTests {
 
 	}
 
+	public static final class UsingHandleDeniedAuthorization {
+
+		@HandleAuthorizationDenied(handlerClass = NullHandler.class)
+		@PreAuthorize("denyAll()")
+		public String methodOne() {
+			return "ok";
+		}
+
+		@HandleAuthorizationDenied(handlerClass = NoDefaultConstructorHandler.class)
+		@PreAuthorize("denyAll()")
+		public String methodTwo() {
+			return "ok";
+		}
+
+	}
+
+	public static final class NullHandler implements MethodAuthorizationDeniedHandler {
+
+		@Override
+		public Object handleDeniedInvocation(MethodInvocation methodInvocation,
+				AuthorizationResult authorizationResult) {
+			return null;
+		}
+
+	}
+
+	public static final class NoDefaultConstructorHandler implements MethodAuthorizationDeniedHandler {
+
+		private NoDefaultConstructorHandler(Object parameter) {
+
+		}
+
+		@Override
+		public Object handleDeniedInvocation(MethodInvocation methodInvocation,
+				AuthorizationResult authorizationResult) {
+			return null;
+		}
+
+	}
+
 }