Remove redundant logic from OAuth2MethodSecurityConfiguration

Previously, OAuth2MethodSecurityConfiguration set the
PermissionEvaluator on the expression evaluator by looking in the
context for a PermissionEvaluator bean. This is unnecessary as
GlobalMethodSecurityConfiguration already does the same thing and does
so after the post-processor in OAuth2MethodSecurityConfiguration has
run. This commit removes the redundant logic and adds tests to check
that both the PermissionEvaluator and the RoleHierarchy are set use
beans in the context.

Closes gh-7979
This commit is contained in:
Andy Wilkinson 2017-01-13 07:25:56 -05:00
parent 8b0ecab181
commit 6f7d1de167
2 changed files with 59 additions and 7 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2015 the original author or authors.
* Copyright 2012-2017 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.
@ -26,7 +26,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.access.PermissionEvaluator;
import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
import org.springframework.security.authentication.AuthenticationTrustResolver;
import org.springframework.security.config.annotation.method.configuration.GlobalMethodSecurityConfiguration;
@ -99,10 +98,6 @@ public class OAuth2MethodSecurityConfiguration
if (trustResolver != null) {
handler.setTrustResolver(trustResolver);
}
PermissionEvaluator permissions = findInContext(PermissionEvaluator.class);
if (permissions != null) {
handler.setPermissionEvaluator(permissions);
}
handler.setExpressionParser(bean.getExpressionParser());
return handler;
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2016 the original author or authors.
* Copyright 2012-2017 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.
@ -49,12 +49,15 @@ import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.RequestEntity;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.PermissionEvaluator;
import org.springframework.security.access.annotation.Jsr250MethodSecurityMetadataSource;
import org.springframework.security.access.annotation.SecuredAnnotationSecurityMetadataSource;
import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler;
import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
import org.springframework.security.access.method.DelegatingMethodSecurityMetadataSource;
import org.springframework.security.access.method.MethodSecurityMetadataSource;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.access.prepost.PreInvocationAuthorizationAdvice;
import org.springframework.security.access.prepost.PrePostAnnotationSecurityMetadataSource;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
@ -96,6 +99,7 @@ import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RestController;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
/**
* Verify Spring Security OAuth2 auto-configuration secures end points properly, accepts
@ -141,6 +145,39 @@ public class OAuth2AutoConfigurationTests {
.isEmpty();
}
@Test
public void methodSecurityExpressionHandlerIsConfiguredWithRoleHierarchyFromTheContext() {
this.context = new AnnotationConfigEmbeddedWebApplicationContext();
this.context.register(RoleHierarchyConfiguration.class,
AuthorizationAndResourceServerConfiguration.class,
MinimalSecureWebApplication.class);
this.context.refresh();
PreInvocationAuthorizationAdvice advice = this.context
.getBean(PreInvocationAuthorizationAdvice.class);
MethodSecurityExpressionHandler expressionHandler = (MethodSecurityExpressionHandler) ReflectionTestUtils
.getField(advice, "expressionHandler");
RoleHierarchy roleHierarchy = (RoleHierarchy) ReflectionTestUtils
.getField(expressionHandler, "roleHierarchy");
assertThat(roleHierarchy).isSameAs(this.context.getBean(RoleHierarchy.class));
}
@Test
public void methodSecurityExpressionHandlerIsConfiguredWithPermissionEvaluatorFromTheContext() {
this.context = new AnnotationConfigEmbeddedWebApplicationContext();
this.context.register(PermissionEvaluatorConfiguration.class,
AuthorizationAndResourceServerConfiguration.class,
MinimalSecureWebApplication.class);
this.context.refresh();
PreInvocationAuthorizationAdvice advice = this.context
.getBean(PreInvocationAuthorizationAdvice.class);
MethodSecurityExpressionHandler expressionHandler = (MethodSecurityExpressionHandler) ReflectionTestUtils
.getField(advice, "expressionHandler");
PermissionEvaluator permissionEvaluator = (PermissionEvaluator) ReflectionTestUtils
.getField(expressionHandler, "permissionEvaluator");
assertThat(permissionEvaluator)
.isSameAs(this.context.getBean(PermissionEvaluator.class));
}
@Test
public void testEnvironmentalOverrides() {
this.context = new AnnotationConfigEmbeddedWebApplicationContext();
@ -571,4 +608,24 @@ public class OAuth2AutoConfigurationTests {
}
@Configuration
protected static class RoleHierarchyConfiguration {
@Bean
public RoleHierarchy roleHierarchy() {
return mock(RoleHierarchy.class);
}
}
@Configuration
protected static class PermissionEvaluatorConfiguration {
@Bean
public PermissionEvaluator permissionEvaluator() {
return mock(PermissionEvaluator.class);
}
}
}