From da57bac0616e4fde4f5e527de0c8ecc5b429ec1a Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Tue, 21 Jun 2022 17:11:43 -0600 Subject: [PATCH] Add SecurityContextHolderStrategy Java Configuration for Method Security Issue gh-11061 --- .../GlobalMethodSecurityConfiguration.java | 14 +++++- .../Jsr250MethodSecurityConfiguration.java | 17 ++++++- .../PrePostMethodSecurityConfiguration.java | 9 ++++ .../SecuredMethodSecurityConfiguration.java | 17 ++++++- ...ePostMethodSecurityConfigurationTests.java | 37 +++++++++++--- .../config/test/SpringTestContext.java | 13 ++++- ...ntApplicationContextExecutionListener.java | 48 +++++++++++++++++++ 7 files changed, 142 insertions(+), 13 deletions(-) create mode 100644 config/src/test/java/org/springframework/security/config/test/SpringTestParentApplicationContextExecutionListener.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java index 27ce85758c..36afc9e496 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -73,6 +73,8 @@ import org.springframework.security.config.annotation.ObjectPostProcessor; import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration; import org.springframework.security.config.core.GrantedAuthorityDefaults; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.util.Assert; /** @@ -101,6 +103,9 @@ public class GlobalMethodSecurityConfiguration implements ImportAware, SmartInit }; + private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder + .getContextHolderStrategy(); + private DefaultMethodSecurityExpressionHandler defaultMethodExpressionHandler = new DefaultMethodSecurityExpressionHandler(); private AuthenticationManager authenticationManager; @@ -143,6 +148,7 @@ public class GlobalMethodSecurityConfiguration implements ImportAware, SmartInit this.methodSecurityInterceptor.setAccessDecisionManager(accessDecisionManager()); this.methodSecurityInterceptor.setAfterInvocationManager(afterInvocationManager()); this.methodSecurityInterceptor.setSecurityMetadataSource(methodSecurityMetadataSource); + this.methodSecurityInterceptor.setSecurityContextHolderStrategy(this.securityContextHolderStrategy); RunAsManager runAsManager = runAsManager(); if (runAsManager != null) { this.methodSecurityInterceptor.setRunAsManager(runAsManager); @@ -411,6 +417,12 @@ public class GlobalMethodSecurityConfiguration implements ImportAware, SmartInit this.expressionHandler = handlers.get(0); } + @Autowired(required = false) + void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) { + Assert.notNull(securityContextHolderStrategy, "securityContextHolderStrategy cannot be null"); + this.securityContextHolderStrategy = securityContextHolderStrategy; + } + @Override public void setBeanFactory(BeanFactory beanFactory) throws BeansException { this.context = beanFactory; diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/Jsr250MethodSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/Jsr250MethodSecurityConfiguration.java index b144477a13..95a5841765 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/Jsr250MethodSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/Jsr250MethodSecurityConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -25,6 +25,8 @@ import org.springframework.context.annotation.Role; import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor; import org.springframework.security.authorization.method.Jsr250AuthorizationManager; import org.springframework.security.config.core.GrantedAuthorityDefaults; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.context.SecurityContextHolderStrategy; /** * {@link Configuration} for enabling JSR-250 Spring Security Method Security. @@ -40,10 +42,16 @@ final class Jsr250MethodSecurityConfiguration { private final Jsr250AuthorizationManager jsr250AuthorizationManager = new Jsr250AuthorizationManager(); + private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder + .getContextHolderStrategy(); + @Bean @Role(BeanDefinition.ROLE_INFRASTRUCTURE) Advisor jsr250AuthorizationMethodInterceptor() { - return AuthorizationManagerBeforeMethodInterceptor.jsr250(this.jsr250AuthorizationManager); + AuthorizationManagerBeforeMethodInterceptor interceptor = AuthorizationManagerBeforeMethodInterceptor + .jsr250(this.jsr250AuthorizationManager); + interceptor.setSecurityContextHolderStrategy(this.securityContextHolderStrategy); + return interceptor; } @Autowired(required = false) @@ -51,4 +59,9 @@ final class Jsr250MethodSecurityConfiguration { this.jsr250AuthorizationManager.setRolePrefix(grantedAuthorityDefaults.getRolePrefix()); } + @Autowired(required = false) + void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) { + this.securityContextHolderStrategy = securityContextHolderStrategy; + } + } diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java index 8786279b0a..1c2a48be72 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java @@ -34,6 +34,7 @@ import org.springframework.security.authorization.method.PostFilterAuthorization import org.springframework.security.authorization.method.PreAuthorizeAuthorizationManager; import org.springframework.security.authorization.method.PreFilterAuthorizationMethodInterceptor; import org.springframework.security.config.core.GrantedAuthorityDefaults; +import org.springframework.security.core.context.SecurityContextHolderStrategy; /** * Base {@link Configuration} for enabling Spring Security Method Security. @@ -109,6 +110,14 @@ final class PrePostMethodSecurityConfiguration { this.postFilterAuthorizationMethodInterceptor.setExpressionHandler(methodSecurityExpressionHandler); } + @Autowired(required = false) + void setSecurityContextHolderStrategy(SecurityContextHolderStrategy strategy) { + this.preFilterAuthorizationMethodInterceptor.setSecurityContextHolderStrategy(strategy); + this.preAuthorizeAuthorizationMethodInterceptor.setSecurityContextHolderStrategy(strategy); + this.postAuthorizeAuthorizaitonMethodInterceptor.setSecurityContextHolderStrategy(strategy); + this.postFilterAuthorizationMethodInterceptor.setSecurityContextHolderStrategy(strategy); + } + @Autowired(required = false) void setGrantedAuthorityDefaults(GrantedAuthorityDefaults grantedAuthorityDefaults) { this.expressionHandler.setDefaultRolePrefix(grantedAuthorityDefaults.getRolePrefix()); diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/SecuredMethodSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/SecuredMethodSecurityConfiguration.java index 79a5b25f57..2e30c747a4 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/SecuredMethodSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/SecuredMethodSecurityConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -17,12 +17,15 @@ package org.springframework.security.config.annotation.method.configuration; import org.springframework.aop.Advisor; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Role; import org.springframework.security.access.annotation.Secured; import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.context.SecurityContextHolderStrategy; /** * {@link Configuration} for enabling {@link Secured} Spring Security Method Security. @@ -36,10 +39,20 @@ import org.springframework.security.authorization.method.AuthorizationManagerBef @Role(BeanDefinition.ROLE_INFRASTRUCTURE) final class SecuredMethodSecurityConfiguration { + private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder + .getContextHolderStrategy(); + @Bean @Role(BeanDefinition.ROLE_INFRASTRUCTURE) Advisor securedAuthorizationMethodInterceptor() { - return AuthorizationManagerBeforeMethodInterceptor.secured(); + AuthorizationManagerBeforeMethodInterceptor interceptor = AuthorizationManagerBeforeMethodInterceptor.secured(); + interceptor.setSecurityContextHolderStrategy(this.securityContextHolderStrategy); + return interceptor; + } + + @Autowired(required = false) + void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) { + this.securityContextHolderStrategy = securityContextHolderStrategy; } } diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java index 4af1f89cf6..c45d5daa21 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java @@ -50,19 +50,24 @@ import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.authorization.method.AuthorizationInterceptorsOrder; import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor; import org.springframework.security.authorization.method.MethodInvocationResult; +import org.springframework.security.config.annotation.SecurityContextChangedListenerConfig; import org.springframework.security.config.core.GrantedAuthorityDefaults; import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContextExtension; +import org.springframework.security.config.test.SpringTestParentApplicationContextExecutionListener; import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.test.context.annotation.SecurityTestExecutionListeners; +import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.security.test.context.support.WithAnonymousUser; import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.security.test.context.support.WithSecurityContextTestExecutionListener; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestExecutionListeners; import org.springframework.test.context.junit.jupiter.SpringExtension; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -73,7 +78,9 @@ import static org.mockito.Mockito.verify; * @author Josh Cummings */ @ExtendWith({ SpringExtension.class, SpringTestContextExtension.class }) -@SecurityTestExecutionListeners +@ContextConfiguration(classes = SecurityContextChangedListenerConfig.class) +@TestExecutionListeners(listeners = { WithSecurityContextTestExecutionListener.class, + SpringTestParentApplicationContextExecutionListener.class }) public class PrePostMethodSecurityConfigurationTests { public final SpringTestContext spring = new SpringTestContext(this); @@ -137,6 +144,8 @@ public class PrePostMethodSecurityConfigurationTests { this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire(); assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(this.methodSecurityService::securedUser) .withMessage("Access Denied"); + SecurityContextHolderStrategy strategy = this.spring.getContext().getBean(SecurityContextHolderStrategy.class); + verify(strategy, atLeastOnce()).getContext(); } @WithMockUser @@ -162,6 +171,15 @@ public class PrePostMethodSecurityConfigurationTests { this.methodSecurityService.preAuthorizeAdmin(); } + @WithMockUser(roles = "ADMIN") + @Test + public void preAuthorizeAdminWhenSecurityContextHolderStrategyThenUses() { + this.spring.register(MethodSecurityServiceConfig.class).autowire(); + this.methodSecurityService.preAuthorizeAdmin(); + SecurityContextHolderStrategy strategy = this.spring.getContext().getBean(SecurityContextHolderStrategy.class); + verify(strategy, atLeastOnce()).getContext(); + } + @WithMockUser(authorities = "PREFIX_ADMIN") @Test public void preAuthorizeAdminWhenRoleAdminAndCustomPrefixThenPasses() { @@ -285,6 +303,8 @@ public class PrePostMethodSecurityConfigurationTests { this.spring.register(BusinessServiceConfig.class).autowire(); assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(this.businessService::rolesAllowedUser) .withMessage("Access Denied"); + SecurityContextHolderStrategy strategy = this.spring.getContext().getBean(SecurityContextHolderStrategy.class); + verify(strategy, atLeastOnce()).getContext(); } @WithMockUser @@ -480,12 +500,15 @@ public class PrePostMethodSecurityConfigurationTests { @Bean @Role(BeanDefinition.ROLE_INFRASTRUCTURE) - Advisor customBeforeAdvice() { + Advisor customBeforeAdvice(SecurityContextHolderStrategy strategy) { JdkRegexpMethodPointcut pointcut = new JdkRegexpMethodPointcut(); pointcut.setPattern(".*MethodSecurityServiceImpl.*securedUser"); AuthorizationManager authorizationManager = (a, o) -> new AuthorizationDecision("bob".equals(a.get().getName())); - return new AuthorizationManagerBeforeMethodInterceptor(pointcut, authorizationManager); + AuthorizationManagerBeforeMethodInterceptor before = new AuthorizationManagerBeforeMethodInterceptor( + pointcut, authorizationManager); + before.setSecurityContextHolderStrategy(strategy); + return before; } } @@ -495,11 +518,11 @@ public class PrePostMethodSecurityConfigurationTests { @Bean @Role(BeanDefinition.ROLE_INFRASTRUCTURE) - Advisor customAfterAdvice() { + Advisor customAfterAdvice(SecurityContextHolderStrategy strategy) { JdkRegexpMethodPointcut pointcut = new JdkRegexpMethodPointcut(); pointcut.setPattern(".*MethodSecurityServiceImpl.*securedUser"); MethodInterceptor interceptor = (mi) -> { - Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + Authentication auth = strategy.getContext().getAuthentication(); if ("bob".equals(auth.getName())) { return "granted"; } diff --git a/config/src/test/java/org/springframework/security/config/test/SpringTestContext.java b/config/src/test/java/org/springframework/security/config/test/SpringTestContext.java index 36a741b99a..1f4a00aefc 100644 --- a/config/src/test/java/org/springframework/security/config/test/SpringTestContext.java +++ b/config/src/test/java/org/springframework/security/config/test/SpringTestContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2022 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. @@ -19,6 +19,7 @@ package org.springframework.security.config.test; import java.io.Closeable; import java.util.ArrayList; import java.util.List; +import java.util.function.Consumer; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -56,6 +57,8 @@ public class SpringTestContext implements Closeable { private List filters = new ArrayList<>(); + private List> postProcessors = new ArrayList<>(); + public SpringTestContext(Object test) { setTest(test); } @@ -104,6 +107,11 @@ public class SpringTestContext implements Closeable { return this; } + public SpringTestContext postProcessor(Consumer contextConsumer) { + this.postProcessors.add(contextConsumer); + return this; + } + public SpringTestContext mockMvcAfterSpringSecurityOk() { return addFilter(new OncePerRequestFilter() { @Override @@ -131,6 +139,9 @@ public class SpringTestContext implements Closeable { public void autowire() { this.context.setServletContext(new MockServletContext()); this.context.setServletConfig(new MockServletConfig()); + for (Consumer postProcessor : this.postProcessors) { + postProcessor.accept(this.context); + } this.context.refresh(); if (this.context.containsBean(BeanIds.SPRING_SECURITY_FILTER_CHAIN)) { // @formatter:off diff --git a/config/src/test/java/org/springframework/security/config/test/SpringTestParentApplicationContextExecutionListener.java b/config/src/test/java/org/springframework/security/config/test/SpringTestParentApplicationContextExecutionListener.java new file mode 100644 index 0000000000..d0ea038d55 --- /dev/null +++ b/config/src/test/java/org/springframework/security/config/test/SpringTestParentApplicationContextExecutionListener.java @@ -0,0 +1,48 @@ +/* + * Copyright 2002-2022 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.config.test; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.List; + +import org.springframework.context.ApplicationContext; +import org.springframework.test.context.TestContext; +import org.springframework.test.context.TestExecutionListener; + +public class SpringTestParentApplicationContextExecutionListener implements TestExecutionListener { + + @Override + public void beforeTestMethod(TestContext testContext) throws Exception { + ApplicationContext parent = testContext.getApplicationContext(); + Object testInstance = testContext.getTestInstance(); + getContexts(testInstance).forEach((springTestContext) -> springTestContext + .postProcessor((applicationContext) -> applicationContext.setParent(parent))); + } + + private static List getContexts(Object test) throws IllegalAccessException { + Field[] declaredFields = test.getClass().getDeclaredFields(); + List result = new ArrayList<>(); + for (Field field : declaredFields) { + if (SpringTestContext.class.isAssignableFrom(field.getType())) { + result.add((SpringTestContext) field.get(test)); + } + } + return result; + } + +}