From 6f7d1de167e17bc75aa3606a973fd638bd633737 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 13 Jan 2017 07:25:56 -0500 Subject: [PATCH] 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 --- .../OAuth2MethodSecurityConfiguration.java | 7 +-- .../oauth2/OAuth2AutoConfigurationTests.java | 59 ++++++++++++++++++- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/method/OAuth2MethodSecurityConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/method/OAuth2MethodSecurityConfiguration.java index dddd54683ff..223b9e1e2ea 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/method/OAuth2MethodSecurityConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/method/OAuth2MethodSecurityConfiguration.java @@ -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; } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/OAuth2AutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/OAuth2AutoConfigurationTests.java index f5a75ebdccd..4e81591463a 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/OAuth2AutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/OAuth2AutoConfigurationTests.java @@ -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); + } + + } + }