From 34d964eb08369305abe4e3ae1d30340dfc4d0299 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Wed, 7 Aug 2024 14:34:40 -0600 Subject: [PATCH] Default Handler Resolution to Reflection-Based Closes gh-15496 --- ...tAuthorizeExpressionAttributeRegistry.java | 3 +- ...eAuthorizeExpressionAttributeRegistry.java | 3 +- ...ctiveMethodAuthorizationDeniedHandler.java | 67 +++++++++++++++++ ...ostAuthorizeAuthorizationManagerTests.java | 71 +++++++++++++++++++ ...PreAuthorizeAuthorizationManagerTests.java | 70 ++++++++++++++++++ 5 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 core/src/main/java/org/springframework/security/authorization/method/ReflectiveMethodAuthorizationDeniedHandler.java diff --git a/core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeExpressionAttributeRegistry.java b/core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeExpressionAttributeRegistry.java index bd118bd6c8..a12f0bcf2c 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeExpressionAttributeRegistry.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeExpressionAttributeRegistry.java @@ -42,7 +42,8 @@ final class PostAuthorizeExpressionAttributeRegistry extends AbstractExpressionA private Function, MethodAuthorizationDeniedHandler> handlerResolver; PostAuthorizeExpressionAttributeRegistry() { - this.handlerResolver = (clazz) -> this.defaultHandler; + this.handlerResolver = (clazz) -> new ReflectiveMethodAuthorizationDeniedHandler(clazz, + PostAuthorizeAuthorizationManager.class); } @NonNull diff --git a/core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeExpressionAttributeRegistry.java b/core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeExpressionAttributeRegistry.java index 0a06f64c32..95de9ba6c5 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeExpressionAttributeRegistry.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeExpressionAttributeRegistry.java @@ -42,7 +42,8 @@ final class PreAuthorizeExpressionAttributeRegistry extends AbstractExpressionAt private Function, MethodAuthorizationDeniedHandler> handlerResolver; PreAuthorizeExpressionAttributeRegistry() { - this.handlerResolver = (clazz) -> this.defaultHandler; + this.handlerResolver = (clazz) -> new ReflectiveMethodAuthorizationDeniedHandler(clazz, + PreAuthorizeAuthorizationManager.class); } @NonNull diff --git a/core/src/main/java/org/springframework/security/authorization/method/ReflectiveMethodAuthorizationDeniedHandler.java b/core/src/main/java/org/springframework/security/authorization/method/ReflectiveMethodAuthorizationDeniedHandler.java new file mode 100644 index 0000000000..25fd350e64 --- /dev/null +++ b/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); + } + } + +} diff --git a/core/src/test/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManagerTests.java b/core/src/test/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManagerTests.java index 37383b40d5..eb7d624579 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManagerTests.java +++ b/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; + } + + } + } diff --git a/core/src/test/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManagerTests.java b/core/src/test/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManagerTests.java index cb43868dbf..57c4d705e9 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManagerTests.java +++ b/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; + } + + } + }