From 26c3941a205038b5489b20870b109e6c864ece60 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Sun, 17 May 2020 19:34:53 -0400 Subject: [PATCH] Polish gh-31 --- build.gradle | 2 - ...ization-server-samples-boot-minimal.gradle | 6 - .../java/sample/JwkSetEndpointFilter.java | 37 +++--- .../src/main/java/sample/SecurityConfig.java | 15 +-- ...st.java => JwkSetEndpointFilterTests.java} | 107 +++++++++--------- ...alAuthorizationServerApplicationTests.java | 16 ++- 6 files changed, 81 insertions(+), 102 deletions(-) rename samples/boot/minimal/src/test/java/sample/{JwkSetEndpointFilterTest.java => JwkSetEndpointFilterTests.java} (65%) diff --git a/build.gradle b/build.gradle index 398c182..5a903ed 100644 --- a/build.gradle +++ b/build.gradle @@ -16,8 +16,6 @@ group = 'org.springframework.security.experimental' description = 'Spring Authorization Server' version = '0.0.1-SNAPSHOT' -ext['junit-jupiter.version'] = '5.4.0' - repositories { mavenCentral() } diff --git a/samples/boot/minimal/spring-authorization-server-samples-boot-minimal.gradle b/samples/boot/minimal/spring-authorization-server-samples-boot-minimal.gradle index a48ee07..70199fb 100644 --- a/samples/boot/minimal/spring-authorization-server-samples-boot-minimal.gradle +++ b/samples/boot/minimal/spring-authorization-server-samples-boot-minimal.gradle @@ -3,17 +3,11 @@ apply plugin: 'io.spring.convention.spring-sample-boot' dependencies { implementation 'org.springframework.boot:spring-boot-starter-web' implementation 'org.springframework.boot:spring-boot-starter-security' - implementation 'com.nimbusds:oauth2-oidc-sdk' testImplementation('org.springframework.boot:spring-boot-starter-test') { exclude group: 'org.junit.vintage', module: 'junit-vintage-engine' } - - testImplementation 'org.springframework.security:spring-security-test' - - testRuntime("org.junit.platform:junit-platform-runner") - testRuntime("org.junit.jupiter:junit-jupiter-engine") } test { diff --git a/samples/boot/minimal/src/main/java/sample/JwkSetEndpointFilter.java b/samples/boot/minimal/src/main/java/sample/JwkSetEndpointFilter.java index e8db90e..dc894ed 100644 --- a/samples/boot/minimal/src/main/java/sample/JwkSetEndpointFilter.java +++ b/samples/boot/minimal/src/main/java/sample/JwkSetEndpointFilter.java @@ -13,35 +13,27 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package sample; -import static org.springframework.http.HttpMethod.GET; -import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; - -import java.io.IOException; -import java.io.Writer; +import com.nimbusds.jose.jwk.JWKSet; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.util.Assert; +import org.springframework.web.filter.OncePerRequestFilter; import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.io.Writer; -import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.RequestMatcher; -import org.springframework.util.Assert; -import org.springframework.web.filter.OncePerRequestFilter; -import org.springframework.web.util.UrlPathHelper; - -import com.nimbusds.jose.jwk.JWKSet; +import static org.springframework.http.HttpMethod.GET; +import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; public class JwkSetEndpointFilter extends OncePerRequestFilter { - - static final String WELL_KNOWN_JWK_URIS = "/.well-known/jwk_uris"; - - private final RequestMatcher requestMatcher = new AntPathRequestMatcher(WELL_KNOWN_JWK_URIS, GET.name(), true, - new UrlPathHelper()); - + static final String DEFAULT_JWK_SET_URI = "/oauth2/jwks"; + private final RequestMatcher requestMatcher = new AntPathRequestMatcher(DEFAULT_JWK_SET_URI, GET.name()); private final JWKSet jwkSet; public JwkSetEndpointFilter(JWKSet jwkSet) { @@ -53,7 +45,7 @@ public class JwkSetEndpointFilter extends OncePerRequestFilter { protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { - if (ifRequestMatches(request)) { + if (matchesRequest(request)) { respond(response); } else { filterChain.doFilter(request, response); @@ -63,12 +55,11 @@ public class JwkSetEndpointFilter extends OncePerRequestFilter { private void respond(HttpServletResponse response) throws IOException { response.setContentType(APPLICATION_JSON_VALUE); try (Writer writer = response.getWriter()) { - writer.write(jwkSet.toPublicJWKSet().toJSONObject().toJSONString()); + writer.write(this.jwkSet.toPublicJWKSet().toJSONObject().toJSONString()); } } - private boolean ifRequestMatches(HttpServletRequest request) { + private boolean matchesRequest(HttpServletRequest request) { return this.requestMatcher.matches(request); } - } diff --git a/samples/boot/minimal/src/main/java/sample/SecurityConfig.java b/samples/boot/minimal/src/main/java/sample/SecurityConfig.java index 7c7bc6f..f1bc2c3 100644 --- a/samples/boot/minimal/src/main/java/sample/SecurityConfig.java +++ b/samples/boot/minimal/src/main/java/sample/SecurityConfig.java @@ -13,31 +13,28 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package sample; -import org.springframework.security.config.annotation.web.builders.HttpSecurity; -import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; -import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; -import org.springframework.security.web.access.channel.ChannelProcessingFilter; - import com.nimbusds.jose.JOSEException; import com.nimbusds.jose.jwk.JWK; import com.nimbusds.jose.jwk.JWKSet; import com.nimbusds.jose.jwk.KeyUse; import com.nimbusds.jose.jwk.gen.RSAKeyGenerator; +import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; +import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; +import org.springframework.security.web.authentication.logout.LogoutFilter; @EnableWebSecurity public class SecurityConfig extends WebSecurityConfigurerAdapter { @Override protected void configure(HttpSecurity http) throws Exception { - http.addFilterBefore(new JwkSetEndpointFilter(generateJwkSet()), ChannelProcessingFilter.class); + http.addFilterBefore(new JwkSetEndpointFilter(generateJwkSet()), LogoutFilter.class); } - protected JWKSet generateJwkSet() throws JOSEException { + private JWKSet generateJwkSet() throws JOSEException { JWK jwk = new RSAKeyGenerator(2048).keyID("minimal-ASA").keyUse(KeyUse.SIGNATURE).generate(); return new JWKSet(jwk); } - } diff --git a/samples/boot/minimal/src/test/java/sample/JwkSetEndpointFilterTest.java b/samples/boot/minimal/src/test/java/sample/JwkSetEndpointFilterTests.java similarity index 65% rename from samples/boot/minimal/src/test/java/sample/JwkSetEndpointFilterTest.java rename to samples/boot/minimal/src/test/java/sample/JwkSetEndpointFilterTests.java index aa71d2b..3f4ae0f 100644 --- a/samples/boot/minimal/src/test/java/sample/JwkSetEndpointFilterTest.java +++ b/samples/boot/minimal/src/test/java/sample/JwkSetEndpointFilterTests.java @@ -13,31 +13,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package sample; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.hamcrest.Matchers.is; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.only; -import static org.mockito.Mockito.verify; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; -import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; -import static sample.JwkSetEndpointFilter.WELL_KNOWN_JWK_URIS; - -import javax.servlet.FilterChain; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - +import com.nimbusds.jose.JOSEException; +import com.nimbusds.jose.jwk.JWK; +import com.nimbusds.jose.jwk.JWKSet; +import com.nimbusds.jose.jwk.KeyUse; +import com.nimbusds.jose.jwk.gen.RSAKeyGenerator; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.TestInstance.Lifecycle; -import org.mockito.Mockito; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.test.web.servlet.MockMvc; @@ -45,36 +31,48 @@ import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; -import com.nimbusds.jose.JOSEException; -import com.nimbusds.jose.jwk.JWK; -import com.nimbusds.jose.jwk.JWKSet; -import com.nimbusds.jose.jwk.KeyUse; -import com.nimbusds.jose.jwk.gen.RSAKeyGenerator; +import javax.servlet.FilterChain; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.hamcrest.Matchers.is; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.only; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import static sample.JwkSetEndpointFilter.DEFAULT_JWK_SET_URI; @TestInstance(Lifecycle.PER_CLASS) -public class JwkSetEndpointFilterTest { - - private MockMvc mvc; - private JWKSet jwkSet; +public class JwkSetEndpointFilterTests { private JWK jwk; + private JWKSet jwkSet; private JwkSetEndpointFilter filter; + private MockMvc mvc; @BeforeAll void setup() throws JOSEException { this.jwk = new RSAKeyGenerator(2048).keyID("endpoint-test").keyUse(KeyUse.SIGNATURE).generate(); - this.jwkSet = new JWKSet(jwk); - this.filter = new JwkSetEndpointFilter(jwkSet); - this.mvc = MockMvcBuilders.standaloneSetup(new FakeController()).addFilters(filter).alwaysDo(print()).build(); + this.jwkSet = new JWKSet(this.jwk); + this.filter = new JwkSetEndpointFilter(this.jwkSet); + this.mvc = MockMvcBuilders.standaloneSetup(new HelloController()).addFilters(this.filter).alwaysDo(print()).build(); } @Test - void constructorWhenJsonWebKeySetIsNullThrowIllegalArgumentException() { - assertThatThrownBy(() -> new JwkSetEndpointFilter(null)).isInstanceOf(IllegalArgumentException.class); + void constructorWhenJWKSetNullThenThrowIllegalArgumentException() { + assertThatThrownBy(() -> new JwkSetEndpointFilter(null)) + .isInstanceOf(IllegalArgumentException.class); } @Test - void doFilterWhenPathMatches() throws Exception { - String requestUri = WELL_KNOWN_JWK_URIS; + void doFilterWhenRequestMatchesThenProcess() throws Exception { + String requestUri = DEFAULT_JWK_SET_URI; MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); request.setServletPath(requestUri); @@ -83,13 +81,12 @@ public class JwkSetEndpointFilterTest { this.filter.doFilter(request, response, filterChain); - verify(filterChain, never()).doFilter(Mockito.any(HttpServletRequest.class), - Mockito.any(HttpServletResponse.class)); + verifyNoInteractions(filterChain); } @Test - void doFilterWhenPathDoesNotMatch() throws Exception { - String requestUri = "/stuff/" + WELL_KNOWN_JWK_URIS; + void doFilterWhenRequestDoesNotMatchThenContinueChain() throws Exception { + String requestUri = "/path"; MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); request.setServletPath(requestUri); @@ -98,30 +95,34 @@ public class JwkSetEndpointFilterTest { this.filter.doFilter(request, response, filterChain); - verify(filterChain, only()).doFilter(Mockito.any(HttpServletRequest.class), - Mockito.any(HttpServletResponse.class)); + verify(filterChain, only()).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class)); } @Test - void testResponseIfRequestMatches() throws Exception { - mvc.perform(get(WELL_KNOWN_JWK_URIS)).andDo(print()).andExpect(status().isOk()) - .andExpect(jsonPath("$.keys").isArray()).andExpect(jsonPath("$.keys").isNotEmpty()) - .andExpect(jsonPath("$.keys[0].kid").value(jwk.getKeyID())) - .andExpect(jsonPath("$.keys[0].kty").value(jwk.getKeyType().toString())); + void requestWhenMatchesThenResponseContainsKeys() throws Exception { + this.mvc.perform(get(DEFAULT_JWK_SET_URI)) + .andDo(print()) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.keys").isArray()) + .andExpect(jsonPath("$.keys").isNotEmpty()) + .andExpect(jsonPath("$.keys[0].kid").value(this.jwk.getKeyID())) + .andExpect(jsonPath("$.keys[0].kty").value(this.jwk.getKeyType().toString())); } @Test - void testResponseIfNotRequestMatches() throws Exception { - mvc.perform(get("/fake")).andDo(print()).andExpect(status().isOk()) - .andExpect(content().string(is("fake"))); + void requestWhenDoesNotMatchThenResponseContainsOther() throws Exception { + this.mvc.perform(get("/hello")) + .andDo(print()) + .andExpect(status().isOk()) + .andExpect(content().string(is("hello"))); } @RestController - class FakeController { + static class HelloController { - @RequestMapping("/fake") - public String hello() { - return "fake"; + @RequestMapping("/hello") + String hello() { + return "hello"; } } } diff --git a/samples/boot/minimal/src/test/java/sample/MinimalAuthorizationServerApplicationTests.java b/samples/boot/minimal/src/test/java/sample/MinimalAuthorizationServerApplicationTests.java index 9f0a859..58ce909 100644 --- a/samples/boot/minimal/src/test/java/sample/MinimalAuthorizationServerApplicationTests.java +++ b/samples/boot/minimal/src/test/java/sample/MinimalAuthorizationServerApplicationTests.java @@ -15,29 +15,27 @@ */ package sample; -import static org.assertj.core.api.Assertions.assertThat; -import static org.springframework.http.HttpStatus.OK; - import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.boot.web.server.LocalServerPort; +import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.client.RestTemplate; +import static org.assertj.core.api.Assertions.assertThat; + @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) public class MinimalAuthorizationServerApplicationTests { - private RestTemplate rest = new RestTemplate(); @LocalServerPort private int serverPort; @Test - void verifyJwkSetEndpointFilterAccessibleWithoutAuthentication() { - ResponseEntity responseEntity = rest.getForEntity( - "http://localhost:" + serverPort + JwkSetEndpointFilter.WELL_KNOWN_JWK_URIS, String.class); - assertThat(responseEntity.getStatusCode()).isEqualTo(OK); + void requestWhenNotAuthenticatedThenJwkSetEndpointStillAccessible() { + ResponseEntity responseEntity = this.rest.getForEntity( + "http://localhost:" + this.serverPort + JwkSetEndpointFilter.DEFAULT_JWK_SET_URI, String.class); + assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.OK); } - }