Skip to content

Commit

Permalink
Replace bean method calls with injection
Browse files Browse the repository at this point in the history
This is so that our configuration classes do not rely on CGLIB to proxy bean methods.

Fixes spring-projectsgh-6818
  • Loading branch information
eleftherias committed May 22, 2019
1 parent f699854 commit a50d9e3
Show file tree
Hide file tree
Showing 12 changed files with 337 additions and 36 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand Down Expand Up @@ -107,8 +107,7 @@ public AuthenticationManager getAuthenticationManager() throws Exception {
if (this.authenticationManagerInitialized) {
return this.authenticationManager;
}
AuthenticationManagerBuilder authBuilder = authenticationManagerBuilder(
this.objectPostProcessor, this.applicationContext);
AuthenticationManagerBuilder authBuilder = this.applicationContext.getBean(AuthenticationManagerBuilder.class);
if (this.buildingAuthenticationManager.getAndSet(true)) {
return new AuthenticationManagerDelegator(authBuilder);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand Down Expand Up @@ -29,10 +29,7 @@
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
import org.springframework.beans.factory.SmartInitializingSingleton;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.AdviceMode;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.ImportAware;
import org.springframework.context.annotation.*;
import org.springframework.core.annotation.AnnotationAttributes;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.core.type.AnnotationMetadata;
Expand Down Expand Up @@ -110,7 +107,6 @@ public <T> T postProcess(T object) {
* <li>{@link #accessDecisionManager()}</li>
* <li>{@link #afterInvocationManager()}</li>
* <li>{@link #authenticationManager()}</li>
* <li>{@link #methodSecurityMetadataSource()}</li>
* <li>{@link #runAsManager()}</li>
*
* </ul>
Expand All @@ -119,19 +115,19 @@ public <T> T postProcess(T object) {
* Subclasses can override this method to provide a different
* {@link MethodInterceptor}.
* </p>
* @param methodSecurityMetadataSource the default {@link MethodSecurityMetadataSource}.
*
* @return the {@link MethodInterceptor}.
* @throws Exception
*/
@Bean
public MethodInterceptor methodSecurityInterceptor() throws Exception {
public MethodInterceptor methodSecurityInterceptor(MethodSecurityMetadataSource methodSecurityMetadataSource) {
this.methodSecurityInterceptor = isAspectJ()
? new AspectJMethodSecurityInterceptor()
: new MethodSecurityInterceptor();
methodSecurityInterceptor.setAccessDecisionManager(accessDecisionManager());
methodSecurityInterceptor.setAfterInvocationManager(afterInvocationManager());
methodSecurityInterceptor
.setSecurityMetadataSource(methodSecurityMetadataSource());
.setSecurityMetadataSource(methodSecurityMetadataSource);
RunAsManager runAsManager = runAsManager();
if (runAsManager != null) {
methodSecurityInterceptor.setRunAsManager(runAsManager);
Expand Down Expand Up @@ -197,8 +193,8 @@ private void initializeMethodSecurityInterceptor() throws Exception {

/**
* Provide a custom {@link AfterInvocationManager} for the default implementation of
* {@link #methodSecurityInterceptor()}. The default is null if pre post is not
* enabled. Otherwise, it returns a {@link AfterInvocationProviderManager}.
* {@link #methodSecurityInterceptor(MethodSecurityMetadataSource)}. The default is null
* if pre post is not enabled. Otherwise, it returns a {@link AfterInvocationProviderManager}.
*
* <p>
* Subclasses should override this method to provide a custom
Expand All @@ -224,7 +220,7 @@ protected AfterInvocationManager afterInvocationManager() {

/**
* Provide a custom {@link RunAsManager} for the default implementation of
* {@link #methodSecurityInterceptor()}. The default is null.
* {@link #methodSecurityInterceptor(MethodSecurityMetadataSource)}. The default is null.
*
* @return the {@link RunAsManager} to use
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
* @since 5.0
*/
@Configuration
class ServerHttpSecurityConfiguration implements WebFluxConfigurer {
class ServerHttpSecurityConfiguration {
private static final String BEAN_NAME_PREFIX = "org.springframework.security.config.annotation.web.reactive.HttpSecurityConfiguration.";
private static final String HTTPSECURITY_BEAN_NAME = BEAN_NAME_PREFIX + "httpSecurity";

Expand Down Expand Up @@ -85,9 +85,15 @@ void setUserDetailsPasswordService(ReactiveUserDetailsPasswordService userDetail
this.userDetailsPasswordService = userDetailsPasswordService;
}

@Override
public void configureArgumentResolvers(ArgumentResolverConfigurer configurer) {
configurer.addCustomResolver(authenticationPrincipalArgumentResolver());
@Bean
public WebFluxConfigurer authenticationPrincipalArgumentResolverConfigurer(
AuthenticationPrincipalArgumentResolver authenticationPrincipalArgumentResolver) {
return new WebFluxConfigurer() {
@Override
public void configureArgumentResolvers(ArgumentResolverConfigurer configurer) {
configurer.addCustomResolver(authenticationPrincipalArgumentResolver);
}
};
}

@Bean
Expand All @@ -110,7 +116,6 @@ public CurrentSecurityContextArgumentResolver reactiveCurrentSecurityContextArgu
return resolver;
}


@Bean(HTTPSECURITY_BEAN_NAME)
@Scope("prototype")
public ServerHttpSecurity httpSecurity() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand Down Expand Up @@ -103,10 +103,10 @@ public void addArgumentResolvers(List<HandlerMethodArgumentResolver> argumentRes

@Override
public final void configureClientInboundChannel(ChannelRegistration registration) {
ChannelSecurityInterceptor inboundChannelSecurity = inboundChannelSecurity();
registration.setInterceptors(securityContextChannelInterceptor());
ChannelSecurityInterceptor inboundChannelSecurity = context.getBean(ChannelSecurityInterceptor.class);
registration.setInterceptors(context.getBean(SecurityContextChannelInterceptor.class));
if (!sameOriginDisabled()) {
registration.setInterceptors(csrfChannelInterceptor());
registration.setInterceptors(context.getBean(CsrfChannelInterceptor.class));
}
if (inboundRegistry.containsMapping()) {
registration.setInterceptors(inboundChannelSecurity);
Expand Down Expand Up @@ -153,9 +153,9 @@ public CsrfChannelInterceptor csrfChannelInterceptor() {
}

@Bean
public ChannelSecurityInterceptor inboundChannelSecurity() {
public ChannelSecurityInterceptor inboundChannelSecurity(MessageSecurityMetadataSource messageSecurityMetadataSource) {
ChannelSecurityInterceptor channelSecurityInterceptor = new ChannelSecurityInterceptor(
inboundMessageSecurityMetadataSource());
messageSecurityMetadataSource);
MessageExpressionVoter<Object> voter = new MessageExpressionVoter<>();
voter.setExpressionHandler(getMessageExpressionHandler());

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand Down Expand Up @@ -34,6 +34,7 @@
import org.springframework.security.authentication.TestingAuthenticationToken;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.authentication.dao.DaoAuthenticationProvider;
import org.springframework.security.config.annotation.AlreadyBuiltException;
import org.springframework.security.config.annotation.ObjectPostProcessor;
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
import org.springframework.security.config.annotation.configuration.ObjectPostProcessorConfiguration;
Expand Down Expand Up @@ -64,9 +65,7 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.startsWith;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.*;

public class AuthenticationConfigurationTests {

Expand Down Expand Up @@ -530,4 +529,20 @@ public AuthenticationManager manager2() {
}

}

@Test
public void getAuthenticationManagerWhenAuthenticationConfigurationSubclassedThenBuildsUsingBean()
throws Exception {
this.spring.register(AuthenticationConfigurationSubclass.class).autowire();
AuthenticationManagerBuilder ap = this.spring.getContext().getBean(AuthenticationManagerBuilder.class);

this.spring.getContext().getBean(AuthenticationConfiguration.class).getAuthenticationManager();

assertThatThrownBy(ap::build)
.isInstanceOf(AlreadyBuiltException.class);
}

@Configuration(proxyBeanMethods = false)
static class AuthenticationConfigurationSubclass extends AuthenticationConfiguration {
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand All @@ -20,6 +20,7 @@
import java.util.Map;
import javax.sql.DataSource;

import org.aopalliance.intercept.MethodInterceptor;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
Expand Down Expand Up @@ -553,4 +554,20 @@ static class CustomAuthorityService {
public void emptyPrefixRoleUser() {}
}
}

@Test
public void methodSecurityInterceptorUsesMetadataSourceBeanWhenProxyingDisabled() {
this.spring.register(CustomMetadataSourceProxylessConfig.class).autowire();
MethodSecurityInterceptor methodInterceptor =
(MethodSecurityInterceptor) this.spring.getContext().getBean(MethodInterceptor.class);
MethodSecurityMetadataSource methodSecurityMetadataSource =
this.spring.getContext().getBean(MethodSecurityMetadataSource.class);

assertThat(methodInterceptor.getSecurityMetadataSource()).isSameAs(methodSecurityMetadataSource);
}

@EnableGlobalMethodSecurity(prePostEnabled = true)
@Configuration(proxyBeanMethods = false)
public static class CustomMetadataSourceProxylessConfig extends GlobalMethodSecurityConfiguration {
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand Down Expand Up @@ -163,8 +163,8 @@ public void methodSecurityWhenCustomAuthenticationManagerThenAuthorizes() {
public static class CustomAuthenticationConfig extends GlobalMethodSecurityConfiguration {

@Override
public MethodInterceptor methodSecurityInterceptor() throws Exception {
MethodInterceptor interceptor = super.methodSecurityInterceptor();
public MethodInterceptor methodSecurityInterceptor(MethodSecurityMetadataSource methodSecurityMetadataSource) {
MethodInterceptor interceptor = super.methodSecurityInterceptor(methodSecurityMetadataSource);
((MethodSecurityInterceptor) interceptor).setAlwaysReauthenticate(true);
return interceptor;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,24 @@ GrantedAuthorityDefaults grantedAuthorityDefaults() {
}
}

@Test
public void rolePrefixWithGrantedAuthorityDefaultsAndSubclassWithProxyingDisabled() {
this.spring.register(SubclassConfig.class).autowire();

TestingAuthenticationToken authentication = new TestingAuthenticationToken(
"principal", "credential", "ROLE_ABC");
MockMethodInvocation methodInvocation = mock(MockMethodInvocation.class);

EvaluationContext context = this.methodSecurityExpressionHandler
.createEvaluationContext(authentication, methodInvocation);
SecurityExpressionRoot root = (SecurityExpressionRoot) context.getRootObject()
.getValue();

assertThat(root.hasRole("ROLE_ABC")).isTrue();
assertThat(root.hasRole("ABC")).isTrue();
}

@Configuration(proxyBeanMethods = false)
static class SubclassConfig extends ReactiveMethodSecurityConfiguration {
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand Down Expand Up @@ -34,6 +34,7 @@
import org.springframework.security.access.expression.SecurityExpressionHandler;
import org.springframework.security.authentication.TestingAuthenticationToken;
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
import org.springframework.security.config.annotation.authentication.configuration.EnableGlobalAuthentication;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.builders.WebSecurity;
import org.springframework.security.config.test.SpringTestRule;
Expand Down Expand Up @@ -403,4 +404,52 @@ public void getMethodDelegatingApplicationListenerWhenWebSecurityConfigurationTh
Method method = ClassUtils.getMethod(WebSecurityConfiguration.class, "delegatingApplicationListener", null);
assertThat(Modifier.isStatic(method.getModifiers())).isTrue();
}

@Test
public void loadConfigWhenProxyingDisabledAndSubclassThenFilterChainsCreated() {
this.spring.register(GlobalAuthenticationWebSecurityConfigurerAdaptersConfig.class, SubclassConfig.class).autowire();

FilterChainProxy filterChainProxy = this.spring.getContext().getBean(FilterChainProxy.class);
List<SecurityFilterChain> filterChains = filterChainProxy.getFilterChains();

assertThat(filterChains).hasSize(4);
}

@Configuration(proxyBeanMethods = false)
static class SubclassConfig extends WebSecurityConfiguration {
}

@Import(AuthenticationTestConfiguration.class)
@EnableGlobalAuthentication
static class GlobalAuthenticationWebSecurityConfigurerAdaptersConfig {
@Configuration
@Order(1)
static class WebConfigurer1 extends WebSecurityConfigurerAdapter {
@Override
public void configure(WebSecurity web) throws Exception {
web
.ignoring()
.antMatchers("/ignore1", "/ignore2");
}

@Override
protected void configure(HttpSecurity http) throws Exception {
http
.antMatcher("/anonymous/**")
.authorizeRequests()
.anyRequest().anonymous();
}
}

@Configuration
static class WebConfigurer2 extends WebSecurityConfigurerAdapter {

@Override
protected void configure(HttpSecurity http) throws Exception {
http
.authorizeRequests()
.anyRequest().authenticated();
}
}
}
}
Loading

0 comments on commit a50d9e3

Please sign in to comment.