From f583668a9cf7f5317b99cedccfcadd17675ffd1c Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Mon, 22 Aug 2022 16:09:11 -0400 Subject: [PATCH] Make AuthorizationServerContextFilter private Closes gh-866 --- .../docs/asciidoc/configuration-model.adoc | 3 - .../AuthorizationServerContextFilter.java | 11 +- .../OAuth2AuthorizationServerConfigurer.java | 1 - .../AuthorizationServerContextHolder.java | 3 - ...Auth2AuthorizationServerMetadataTests.java | 22 +++- ...AuthorizationServerContextFilterTests.java | 101 ------------------ 6 files changed, 24 insertions(+), 117 deletions(-) rename oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/{web => config/annotation/web/configurers}/AuthorizationServerContextFilter.java (89%) delete mode 100644 oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/AuthorizationServerContextFilterTests.java diff --git a/docs/src/docs/asciidoc/configuration-model.adoc b/docs/src/docs/asciidoc/configuration-model.adoc index 011d33ee..5614a158 100644 --- a/docs/src/docs/asciidoc/configuration-model.adoc +++ b/docs/src/docs/asciidoc/configuration-model.adoc @@ -182,9 +182,6 @@ If the issuer identifier is not configured in `AuthorizationServerSettings.build [NOTE] The `AuthorizationServerContext` is accessible through the `AuthorizationServerContextHolder`, which associates it with the current request thread by using a `ThreadLocal`. -[NOTE] -The `AuthorizationServerContextFilter` associates the `AuthorizationServerContext` with the `AuthorizationServerContextHolder`. - [[configuring-client-authentication]] == Configuring Client Authentication diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/AuthorizationServerContextFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/AuthorizationServerContextFilter.java similarity index 89% rename from oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/AuthorizationServerContextFilter.java rename to oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/AuthorizationServerContextFilter.java index 36a9c095..0f3a7e4a 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/AuthorizationServerContextFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/AuthorizationServerContextFilter.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.security.oauth2.server.authorization.web; +package org.springframework.security.oauth2.server.authorization.config.annotation.web.configurers; import java.io.IOException; @@ -39,15 +39,10 @@ import org.springframework.web.util.UriComponentsBuilder; * @see AuthorizationServerContextHolder * @see AuthorizationServerSettings */ -public final class AuthorizationServerContextFilter extends OncePerRequestFilter { +final class AuthorizationServerContextFilter extends OncePerRequestFilter { private final AuthorizationServerSettings authorizationServerSettings; - /** - * Constructs an {@code AuthorizationServerContextFilter} using the provided parameters. - * - * @param authorizationServerSettings the authorization server settings - */ - public AuthorizationServerContextFilter(AuthorizationServerSettings authorizationServerSettings) { + AuthorizationServerContextFilter(AuthorizationServerSettings authorizationServerSettings) { Assert.notNull(authorizationServerSettings, "authorizationServerSettings cannot be null"); this.authorizationServerSettings = authorizationServerSettings; } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerConfigurer.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerConfigurer.java index 56371ef2..8bb36b2d 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerConfigurer.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerConfigurer.java @@ -33,7 +33,6 @@ import org.springframework.security.oauth2.server.authorization.OAuth2Authorizat import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository; import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings; import org.springframework.security.oauth2.server.authorization.token.OAuth2TokenGenerator; -import org.springframework.security.oauth2.server.authorization.web.AuthorizationServerContextFilter; import org.springframework.security.oauth2.server.authorization.web.NimbusJwkSetEndpointFilter; import org.springframework.security.oauth2.server.authorization.web.OAuth2AuthorizationServerMetadataEndpointFilter; import org.springframework.security.web.authentication.HttpStatusEntryPoint; diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/context/AuthorizationServerContextHolder.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/context/AuthorizationServerContextHolder.java index c15b4259..fa70ae44 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/context/AuthorizationServerContextHolder.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/context/AuthorizationServerContextHolder.java @@ -15,15 +15,12 @@ */ package org.springframework.security.oauth2.server.authorization.context; -import org.springframework.security.oauth2.server.authorization.web.AuthorizationServerContextFilter; - /** * A holder of the {@link AuthorizationServerContext} that associates it with the current thread using a {@code ThreadLocal}. * * @author Joe Grandja * @since 0.2.2 * @see AuthorizationServerContext - * @see AuthorizationServerContextFilter */ public final class AuthorizationServerContextHolder { private static final ThreadLocal holder = new ThreadLocal<>(); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerMetadataTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerMetadataTests.java index d7b181be..7750a77d 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerMetadataTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerMetadataTests.java @@ -92,7 +92,7 @@ public class OAuth2AuthorizationServerMetadataTests { } @Test - public void requestWhenAuthorizationServerMetadataRequestAndIssuerSetThenReturnMetadataResponse() throws Exception { + public void requestWhenAuthorizationServerMetadataRequestAndIssuerSetThenUsed() throws Exception { this.spring.register(AuthorizationServerConfiguration.class).autowire(); this.mvc.perform(get(DEFAULT_OAUTH2_AUTHORIZATION_SERVER_METADATA_ENDPOINT_URI)) @@ -101,6 +101,16 @@ public class OAuth2AuthorizationServerMetadataTests { .andReturn(); } + @Test + public void requestWhenAuthorizationServerMetadataRequestAndIssuerNotSetThenResolveFromRequest() throws Exception { + this.spring.register(AuthorizationServerConfigurationWithIssuerNotSet.class).autowire(); + + this.mvc.perform(get(DEFAULT_OAUTH2_AUTHORIZATION_SERVER_METADATA_ENDPOINT_URI)) + .andExpect(status().is2xxSuccessful()) + .andExpect(jsonPath("issuer").value("http://localhost")) + .andReturn(); + } + @EnableWebSecurity @Import(OAuth2AuthorizationServerConfiguration.class) static class AuthorizationServerConfiguration { @@ -129,4 +139,14 @@ public class OAuth2AuthorizationServerMetadataTests { } } + @EnableWebSecurity + @Import(OAuth2AuthorizationServerConfiguration.class) + static class AuthorizationServerConfigurationWithIssuerNotSet extends AuthorizationServerConfiguration { + + @Bean + AuthorizationServerSettings authorizationServerSettings() { + return AuthorizationServerSettings.builder().build(); + } + } + } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/AuthorizationServerContextFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/AuthorizationServerContextFilterTests.java deleted file mode 100644 index 9d4a9927..00000000 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/AuthorizationServerContextFilterTests.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * Copyright 2020-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.oauth2.server.authorization.web; - -import javax.servlet.FilterChain; - -import org.junit.After; -import org.junit.Test; - -import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.mock.web.MockHttpServletResponse; -import org.springframework.security.oauth2.server.authorization.context.AuthorizationServerContext; -import org.springframework.security.oauth2.server.authorization.context.AuthorizationServerContextHolder; -import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.mock; - -/** - * Tests for {@link AuthorizationServerContextFilter}. - * - * @author Joe Grandja - */ -public class AuthorizationServerContextFilterTests { - - @After - public void cleanup() { - AuthorizationServerContextHolder.resetContext(); - } - - @Test - public void constructorWhenAuthorizationServerSettingsNullThenThrowIllegalArgumentException() { - assertThatThrownBy(() -> new AuthorizationServerContextFilter(null)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("authorizationServerSettings cannot be null"); - } - - @Test - public void doFilterWhenIssuerConfiguredThenUsed() throws Exception { - String issuer = "https://provider.com"; - AuthorizationServerSettings authorizationServerSettings = AuthorizationServerSettings.builder().issuer(issuer).build(); - AuthorizationServerContextFilter filter = new AuthorizationServerContextFilter(authorizationServerSettings); - - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - request.setServletPath("/"); - MockHttpServletResponse response = new MockHttpServletResponse(); - FilterChain filterChain = mock(FilterChain.class); - - doAnswer(invocation -> { - AuthorizationServerContext authorizationServerContext = AuthorizationServerContextHolder.getContext(); - assertThat(authorizationServerContext).isNotNull(); - assertThat(authorizationServerContext.getAuthorizationServerSettings()).isSameAs(authorizationServerSettings); - assertThat(authorizationServerContext.getIssuer()).isEqualTo(issuer); - return null; - }).when(filterChain).doFilter(any(), any()); - - filter.doFilter(request, response, filterChain); - - assertThat(AuthorizationServerContextHolder.getContext()).isNull(); - } - - @Test - public void doFilterWhenIssuerNotConfiguredThenResolveFromRequest() throws Exception { - AuthorizationServerSettings authorizationServerSettings = AuthorizationServerSettings.builder().build(); - AuthorizationServerContextFilter filter = new AuthorizationServerContextFilter(authorizationServerSettings); - - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - request.setServletPath("/"); - MockHttpServletResponse response = new MockHttpServletResponse(); - FilterChain filterChain = mock(FilterChain.class); - - doAnswer(invocation -> { - AuthorizationServerContext authorizationServerContext = AuthorizationServerContextHolder.getContext(); - assertThat(authorizationServerContext).isNotNull(); - assertThat(authorizationServerContext.getAuthorizationServerSettings()).isSameAs(authorizationServerSettings); - assertThat(authorizationServerContext.getIssuer()).isEqualTo("http://localhost"); - return null; - }).when(filterChain).doFilter(any(), any()); - - filter.doFilter(request, response, filterChain); - - assertThat(AuthorizationServerContextHolder.getContext()).isNull(); - } - -}