From a69fb43bbb85b82a002e8c49d5b374b2e238bf75 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Tue, 10 Sep 2019 05:33:16 -0600 Subject: [PATCH] Bearer WebClient Filter Authentication Propagation Fixes: gh-7418 --- config/spring-security-config.gradle | 1 + .../configuration/OAuth2ImportSelector.java | 27 ++- .../OAuth2ResourceServerConfiguration.java | 144 +++++++++++++++ ...Auth2ResourceServerConfigurationTests.java | 165 ++++++++++++++++++ .../ServletBearerExchangeFilterFunction.java | 17 +- .../TestBearerTokenAuthentications.java | 51 ++++++ ...vletBearerExchangeFilterFunctionTests.java | 32 ++-- 7 files changed, 407 insertions(+), 30 deletions(-) create mode 100644 config/src/main/java/org/springframework/security/config/annotation/web/configuration/OAuth2ResourceServerConfiguration.java create mode 100644 config/src/test/java/org/springframework/security/config/annotation/web/configuration/OAuth2ResourceServerConfigurationTests.java create mode 100644 oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/TestBearerTokenAuthentications.java diff --git a/config/spring-security-config.gradle b/config/spring-security-config.gradle index afce9fe0039..f584bf80272 100644 --- a/config/spring-security-config.gradle +++ b/config/spring-security-config.gradle @@ -35,6 +35,7 @@ dependencies { testCompile project(':spring-security-test') testCompile project(path : ':spring-security-core', configuration : 'tests') testCompile project(path : ':spring-security-oauth2-client', configuration : 'tests') + testCompile project(path : ':spring-security-oauth2-resource-server', configuration : 'tests') testCompile project(path : ':spring-security-web', configuration : 'tests') testCompile apachedsDependencies testCompile powerMock2Dependencies diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/OAuth2ImportSelector.java b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/OAuth2ImportSelector.java index 079b307a993..1012b119c6b 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/OAuth2ImportSelector.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/OAuth2ImportSelector.java @@ -15,15 +15,21 @@ */ package org.springframework.security.config.annotation.web.configuration; +import java.util.ArrayList; +import java.util.List; + import org.springframework.context.annotation.ImportSelector; import org.springframework.core.type.AnnotationMetadata; import org.springframework.util.ClassUtils; /** * Used by {@link EnableWebSecurity} to conditionally import {@link OAuth2ClientConfiguration} - * when the {@code spring-security-oauth2-client} module is present on the classpath. - + * when the {@code spring-security-oauth2-client} module is present on the classpath and + * {@link OAuth2ResourceServerConfiguration} when the {@code spring-security-oauth2-resource-server} + * module is on the classpath. + * * @author Joe Grandja + * @author Josh Cummings * @since 5.1 * @see OAuth2ClientConfiguration */ @@ -31,11 +37,18 @@ final class OAuth2ImportSelector implements ImportSelector { @Override public String[] selectImports(AnnotationMetadata importingClassMetadata) { - boolean oauth2ClientPresent = ClassUtils.isPresent( - "org.springframework.security.oauth2.client.registration.ClientRegistration", getClass().getClassLoader()); + List imports = new ArrayList<>(); + + if (ClassUtils.isPresent( + "org.springframework.security.oauth2.client.registration.ClientRegistration", getClass().getClassLoader())) { + imports.add("org.springframework.security.config.annotation.web.configuration.OAuth2ClientConfiguration"); + } + + if (ClassUtils.isPresent( + "org.springframework.security.oauth2.server.resource.BearerTokenError", getClass().getClassLoader())) { + imports.add("org.springframework.security.config.annotation.web.configuration.OAuth2ResourceServerConfiguration"); + } - return oauth2ClientPresent ? - new String[] { "org.springframework.security.config.annotation.web.configuration.OAuth2ClientConfiguration" } : - new String[] {}; + return imports.toArray(new String[0]); } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/OAuth2ResourceServerConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/OAuth2ResourceServerConfiguration.java new file mode 100644 index 00000000000..34ec7efedad --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/OAuth2ResourceServerConfiguration.java @@ -0,0 +1,144 @@ +/* + * 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. + * 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.annotation.web.configuration; + +import org.reactivestreams.Subscription; +import reactor.core.CoreSubscriber; +import reactor.core.publisher.Hooks; +import reactor.core.publisher.Operators; +import reactor.util.context.Context; + +import org.springframework.beans.factory.DisposableBean; +import org.springframework.beans.factory.InitializingBean; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.ImportSelector; +import org.springframework.core.type.AnnotationMetadata; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.util.ClassUtils; + +/** + * {@link Configuration} for OAuth 2.0 Resource Server support. + * + *

+ * This {@code Configuration} is conditionally imported by {@link OAuth2ImportSelector} + * when the {@code spring-security-oauth2-resource-server} module is present on the classpath. + * + * @author Josh Cummings + * @since 5.2 + * @see OAuth2ImportSelector + */ +@Import(OAuth2ResourceServerConfiguration.OAuth2ClientWebFluxImportSelector.class) +final class OAuth2ResourceServerConfiguration { + + static class OAuth2ClientWebFluxImportSelector implements ImportSelector { + + @Override + public String[] selectImports(AnnotationMetadata importingClassMetadata) { + boolean webfluxPresent = ClassUtils.isPresent( + "org.springframework.web.reactive.function.client.WebClient", getClass().getClassLoader()); + + return webfluxPresent ? + new String[] { "org.springframework.security.config.annotation.web.configuration.OAuth2ResourceServerConfiguration.OAuth2ResourceServerWebFluxSecurityConfiguration" } : + new String[] {}; + } + } + + @Configuration(proxyBeanMethods = false) + static class OAuth2ResourceServerWebFluxSecurityConfiguration { + @Bean + BearerRequestContextSubscriberRegistrar bearerRequestContextSubscriberRegistrar() { + return new BearerRequestContextSubscriberRegistrar(); + } + + /** + * Registers a {@link CoreSubscriber} that provides the current {@link Authentication} + * to the correct {@link Context}. + * + * This is published as a {@code @Bean} automatically, so long as `spring-security-oauth2-resource-server` + * and `spring-webflux` are on the classpath. + */ + static class BearerRequestContextSubscriberRegistrar + implements InitializingBean, DisposableBean { + + private static final String REQUEST_CONTEXT_OPERATOR_KEY = BearerRequestContextSubscriber.class.getName(); + + @Override + public void afterPropertiesSet() throws Exception { + Hooks.onLastOperator(REQUEST_CONTEXT_OPERATOR_KEY, + Operators.liftPublisher((s, sub) -> createRequestContextSubscriber(sub))); + } + + @Override + public void destroy() throws Exception { + Hooks.resetOnLastOperator(REQUEST_CONTEXT_OPERATOR_KEY); + } + + private CoreSubscriber createRequestContextSubscriber(CoreSubscriber delegate) { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + return new BearerRequestContextSubscriber<>(delegate, authentication); + } + + static class BearerRequestContextSubscriber implements CoreSubscriber { + private CoreSubscriber delegate; + private final Context context; + + private BearerRequestContextSubscriber(CoreSubscriber delegate, + Authentication authentication) { + + this.delegate = delegate; + Context parentContext = this.delegate.currentContext(); + Context context; + if (authentication == null || parentContext.hasKey(Authentication.class)) { + context = parentContext; + } else { + context = parentContext.put(Authentication.class, authentication); + } + + this.context = context; + } + + @Override + public Context currentContext() { + return this.context; + } + + @Override + public void onSubscribe(Subscription s) { + this.delegate.onSubscribe(s); + } + + @Override + public void onNext(T t) { + this.delegate.onNext(t); + } + + @Override + public void onError(Throwable t) { + this.delegate.onError(t); + } + + @Override + public void onComplete() { + this.delegate.onComplete(); + } + } + } + } +} diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configuration/OAuth2ResourceServerConfigurationTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configuration/OAuth2ResourceServerConfigurationTests.java new file mode 100644 index 00000000000..e93155c41c3 --- /dev/null +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configuration/OAuth2ResourceServerConfigurationTests.java @@ -0,0 +1,165 @@ +/* + * 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. + * 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.annotation.web.configuration; + +import javax.annotation.PreDestroy; + +import okhttp3.mockwebserver.Dispatcher; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; +import org.apache.commons.lang.StringUtils; +import org.junit.Rule; +import org.junit.Test; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.config.test.SpringTestRule; +import org.springframework.security.oauth2.server.resource.authentication.BearerTokenAuthentication; +import org.springframework.security.oauth2.server.resource.web.reactive.function.client.ServletBearerExchangeFilterFunction; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.reactive.function.client.WebClient; + +import static org.springframework.security.oauth2.server.resource.authentication.TestBearerTokenAuthentications.bearer; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +/** + * Tests for {@link OAuth2ResourceServerConfiguration}. + * + * @author Josh Cummings + */ +public class OAuth2ResourceServerConfigurationTests { + @Rule + public final SpringTestRule spring = new SpringTestRule(); + + @Autowired + private MockMvc mockMvc; + + // gh-7418 + @Test + public void requestWhenUsingFilterThenBearerTokenPropagated() throws Exception { + BearerTokenAuthentication authentication = bearer(); + this.spring.register(BearerFilterConfig.class, WebServerConfig.class, Controller.class).autowire(); + + this.mockMvc.perform(get("/token") + .with(authentication(authentication))) + .andExpect(status().isOk()) + .andExpect(content().string("Bearer token")); + } + + // gh-7418 + @Test + public void requestWhenNotUsingFilterThenBearerTokenNotPropagated() throws Exception { + BearerTokenAuthentication authentication = bearer(); + this.spring.register(BearerFilterlessConfig.class, WebServerConfig.class, Controller.class).autowire(); + + this.mockMvc.perform(get("/token") + .with(authentication(authentication))) + .andExpect(status().isOk()) + .andExpect(content().string("")); + } + + + @EnableWebSecurity + static class BearerFilterConfig extends WebSecurityConfigurerAdapter { + @Override + protected void configure(HttpSecurity http) throws Exception { + } + + @Bean + WebClient rest() { + ServletBearerExchangeFilterFunction bearer = + new ServletBearerExchangeFilterFunction(); + return WebClient.builder() + .filter(bearer).build(); + } + } + + @EnableWebSecurity + static class BearerFilterlessConfig extends WebSecurityConfigurerAdapter { + @Override + protected void configure(HttpSecurity http) throws Exception { + } + + @Bean + WebClient rest() { + return WebClient.create(); + } + } + + @RestController + static class Controller { + private final WebClient rest; + private final String uri; + + @Autowired + Controller(MockWebServer server, WebClient rest) { + this.uri = server.url("/").toString(); + this.rest = rest; + } + + @GetMapping("/token") + public String token() { + return this.rest.get() + .uri(this.uri) + .retrieve() + .bodyToMono(String.class) + .flatMap(result -> this.rest.get() + .uri(this.uri) + .retrieve() + .bodyToMono(String.class)) + .block(); + } + } + + @Configuration + static class WebServerConfig { + private final MockWebServer server = new MockWebServer(); + + @Bean + MockWebServer server() throws Exception { + this.server.setDispatcher(new AuthorizationHeaderDispatcher()); + this.server.start(); + return this.server; + } + + @PreDestroy + void shutdown() throws Exception { + this.server.shutdown(); + } + } + + static class AuthorizationHeaderDispatcher extends Dispatcher { + + @Override + public MockResponse dispatch(RecordedRequest request) { + MockResponse response = new MockResponse().setResponseCode(200); + String header = request.getHeader("Authorization"); + if (StringUtils.isBlank(header)) { + return response; + + } + return response.setBody(header); + } + } +} diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/reactive/function/client/ServletBearerExchangeFilterFunction.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/reactive/function/client/ServletBearerExchangeFilterFunction.java index e85540761be..892ed534230 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/reactive/function/client/ServletBearerExchangeFilterFunction.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/reactive/function/client/ServletBearerExchangeFilterFunction.java @@ -17,9 +17,9 @@ package org.springframework.security.oauth2.server.resource.web.reactive.function.client; import reactor.core.publisher.Mono; +import reactor.util.context.Context; import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.oauth2.core.AbstractOAuth2Token; import org.springframework.web.reactive.function.client.ClientRequest; import org.springframework.web.reactive.function.client.ClientResponse; @@ -43,6 +43,13 @@ * } * * + * To locate the bearer token, this looks in the Reactor {@link Context} for a key of type {@link Authentication}. + * + * Registering + * {@see org.springframework.security.config.annotation.web.configuration.OAuth2ResourceServerConfiguration.OAuth2ResourceServerWebFluxSecurityConfiguration.BearerRequestContextSubscriberRegistrar}, + * as a {@code @Bean} will take care of this automatically, + * but certainly an application can supply a {@link Context} of its own to override. + * * @author Josh Cummings * @since 5.2 */ @@ -61,14 +68,16 @@ public Mono filter(ClientRequest request, ExchangeFunction next) } private Mono oauth2Token() { - return currentAuthentication() + return Mono.subscriberContext() + .flatMap(this::currentAuthentication) .filter(authentication -> authentication.getCredentials() instanceof AbstractOAuth2Token) .map(Authentication::getCredentials) .cast(AbstractOAuth2Token.class); } - private Mono currentAuthentication() { - return Mono.justOrEmpty(SecurityContextHolder.getContext().getAuthentication()); + private Mono currentAuthentication(Context ctx) { + Authentication authentication = ctx.getOrDefault(Authentication.class, null); + return Mono.justOrEmpty(authentication); } private ClientRequest bearer(ClientRequest request, AbstractOAuth2Token token) { diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/TestBearerTokenAuthentications.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/TestBearerTokenAuthentications.java new file mode 100644 index 00000000000..72a25840052 --- /dev/null +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/TestBearerTokenAuthentications.java @@ -0,0 +1,51 @@ +/* + * 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. + * 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.oauth2.server.resource.authentication; + +import java.time.Instant; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; + +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.oauth2.core.DefaultOAuth2AuthenticatedPrincipal; +import org.springframework.security.oauth2.core.OAuth2AccessToken; +import org.springframework.security.oauth2.core.OAuth2AuthenticatedPrincipal; + +/** + * Test instances of {@link BearerTokenAuthentication} + * + * @author Josh Cummings + */ +public class TestBearerTokenAuthentications { + public static BearerTokenAuthentication bearer() { + Collection authorities = + AuthorityUtils.createAuthorityList("SCOPE_USER"); + OAuth2AuthenticatedPrincipal principal = + new DefaultOAuth2AuthenticatedPrincipal( + Collections.singletonMap("sub", "user"), + authorities); + OAuth2AccessToken token = + new OAuth2AccessToken(OAuth2AccessToken.TokenType.BEARER, + "token", Instant.now(), Instant.now().plusSeconds(86400), + new HashSet<>(Arrays.asList("USER"))); + + return new BearerTokenAuthentication(principal, token, authorities); + } +} diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/reactive/function/client/ServletBearerExchangeFilterFunctionTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/reactive/function/client/ServletBearerExchangeFilterFunctionTests.java index adeb9be1852..e960a6e85f5 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/reactive/function/client/ServletBearerExchangeFilterFunctionTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/reactive/function/client/ServletBearerExchangeFilterFunctionTests.java @@ -22,15 +22,14 @@ import java.util.Collections; import java.util.Map; -import org.junit.After; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; +import reactor.util.context.Context; import org.springframework.http.HttpHeaders; import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.oauth2.core.OAuth2AccessToken; import org.springframework.security.oauth2.server.resource.authentication.AbstractOAuth2TokenAuthenticationToken; import org.springframework.security.oauth2.server.resource.web.MockExchangeFunction; @@ -62,11 +61,6 @@ public Map getTokenAttributes() { } }; - @After - public void cleanup() { - SecurityContextHolder.clearContext(); - } - @Test public void filterWhenUnauthenticatedThenAuthorizationHeaderNull() { ClientRequest request = ClientRequest.create(GET, URI.create("https://example.com")) @@ -80,41 +74,41 @@ public void filterWhenUnauthenticatedThenAuthorizationHeaderNull() { // gh-7353 @Test - public void filterWhenAuthenticatedWithOtherTokenThenAuthorizationHeaderNull() throws Exception { + public void filterWhenAuthenticatedWithOtherTokenThenAuthorizationHeaderNull() { TestingAuthenticationToken token = new TestingAuthenticationToken("user", "pass"); - SecurityContextHolder.getContext().setAuthentication(token); - ClientRequest request = ClientRequest.create(GET, URI.create("https://example.com")) .build(); - this.function.filter(request, this.exchange).block(); + this.function.filter(request, this.exchange) + .subscriberContext(Context.of(Authentication.class, token)) + .block(); assertThat(this.exchange.getRequest().headers().getFirst(HttpHeaders.AUTHORIZATION)) .isNull(); } @Test - public void filterWhenAuthenticatedThenAuthorizationHeader() throws Exception { - SecurityContextHolder.getContext().setAuthentication(this.authentication); - + public void filterWhenAuthenticatedThenAuthorizationHeader() { ClientRequest request = ClientRequest.create(GET, URI.create("https://example.com")) .build(); - this.function.filter(request, this.exchange).block(); + this.function.filter(request, this.exchange) + .subscriberContext(Context.of(Authentication.class, this.authentication)) + .block(); assertThat(this.exchange.getRequest().headers().getFirst(HttpHeaders.AUTHORIZATION)) .isEqualTo("Bearer " + this.accessToken.getTokenValue()); } @Test - public void filterWhenExistingAuthorizationThenSingleAuthorizationHeader() throws Exception { - SecurityContextHolder.getContext().setAuthentication(this.authentication); - + public void filterWhenExistingAuthorizationThenSingleAuthorizationHeader() { ClientRequest request = ClientRequest.create(GET, URI.create("https://example.com")) .header(HttpHeaders.AUTHORIZATION, "Existing") .build(); - this.function.filter(request, this.exchange).block(); + this.function.filter(request, this.exchange) + .subscriberContext(Context.of(Authentication.class, this.authentication)) + .block(); HttpHeaders headers = this.exchange.getRequest().headers(); assertThat(headers.get(HttpHeaders.AUTHORIZATION)).containsOnly("Bearer " + this.accessToken.getTokenValue());