Skip to content

Commit

Permalink
Default Handler Resolution to Reflection-Based
Browse files Browse the repository at this point in the history
Closes gh-15496
  • Loading branch information
jzheaux committed Aug 7, 2024
1 parent 35695de commit 34d964e
Show file tree
Hide file tree
Showing 5 changed files with 212 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@
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;
Expand All @@ -33,6 +35,7 @@
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;
Expand Down Expand Up @@ -167,6 +170,34 @@ public void checkInheritedAnnotationsWhenConflictingThenAnnotationConfigurationE
.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() {
Expand Down Expand Up @@ -237,4 +268,44 @@ public interface InterfaceAnnotationsThree {

}

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;
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
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;
Expand All @@ -31,6 +33,7 @@
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;
Expand Down Expand Up @@ -147,6 +150,33 @@ public void checkTargetClassAwareWhenInterfaceLevelAnnotationsThenApplies() thro
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() {
Expand Down Expand Up @@ -241,4 +271,44 @@ public void inheritedAnnotations() {

}

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;
}

}

}

0 comments on commit 34d964e

Please sign in to comment.