From 7cbd1665a6ba5df01ab676db7d945b6599c2000a Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Fri, 22 Nov 2019 12:38:01 -0700 Subject: [PATCH] Isolate Jwt Test Support Isolating Jwt test support inside JwtRequestPostProcessor and JwtMutator. Fixes gh-7641 --- .../asciidoc/_includes/reactive/test.adoc | 6 +- .../_includes/servlet/test/mockmvc.adoc | 6 +- .../OAuth2ResourceServerControllerTests.java | 4 +- .../OAuth2ResourceServerControllerTests.java | 8 +- .../server/SecurityMockServerConfigurers.java | 85 +++++++------ .../SecurityMockMvcRequestPostProcessors.java | 114 +++++++----------- ...SecurityMockServerConfigurersJwtTests.java | 10 +- ...yMockMvcRequestPostProcessorsJwtTests.java | 10 +- 8 files changed, 107 insertions(+), 136 deletions(-) diff --git a/docs/manual/src/docs/asciidoc/_includes/reactive/test.adoc b/docs/manual/src/docs/asciidoc/_includes/reactive/test.adoc index 8c1a378553..bbd1241150 100644 --- a/docs/manual/src/docs/asciidoc/_includes/reactive/test.adoc +++ b/docs/manual/src/docs/asciidoc/_includes/reactive/test.adoc @@ -203,7 +203,7 @@ Any headers or claims can be configured with their corresponding methods: [source,java] ---- client - .mutateWith(jwt(jwt -> jwt.header("kid", "one") + .mutateWith(mockJwt().jwt(jwt -> jwt.header("kid", "one") .claim("iss", "https://idp.example.org"))) .get().uri("/endpoint").exchange(); ---- @@ -211,7 +211,7 @@ client [source,java] ---- client - .mutateWith(jwt(jwt -> jwt.claims(claims -> claims.remove("scope")))) + .mutateWith(mockJwt().jwt(jwt -> jwt.claims(claims -> claims.remove("scope")))) .get().uri("/endpoint").exchange(); ---- @@ -244,7 +244,7 @@ Jwt jwt = Jwt.withTokenValue("token") .claim("scope", "read"); client - .mutateWith(jwt(jwt)) + .mutateWith(mockJwt().jwt(jwt)) .get().uri("/endpoint").exchange(); ---- diff --git a/docs/manual/src/docs/asciidoc/_includes/servlet/test/mockmvc.adoc b/docs/manual/src/docs/asciidoc/_includes/servlet/test/mockmvc.adoc index dde0def631..ecf64a5d8d 100644 --- a/docs/manual/src/docs/asciidoc/_includes/servlet/test/mockmvc.adoc +++ b/docs/manual/src/docs/asciidoc/_includes/servlet/test/mockmvc.adoc @@ -335,14 +335,14 @@ Any headers or claims can be configured with their corresponding methods: ---- mvc .perform(get("/endpoint") - .with(jwt(jwt -> jwt.header("kid", "one").claim("iss", "https://idp.example.org")))); + .with(jwt().jwt(jwt -> jwt.header("kid", "one").claim("iss", "https://idp.example.org")))); ---- [source,java] ---- mvc .perform(get("/endpoint") - .with(jwt(jwt -> jwt.claims(claims -> claims.remove("scope"))))); + .with(jwt().jwt(jwt -> jwt.claims(claims -> claims.remove("scope"))))); ---- The `scope` and `scp` claims are processed the same way here as they are in a normal bearer token request. @@ -375,7 +375,7 @@ Jwt jwt = Jwt.withTokenValue("token") mvc .perform(get("/endpoint") - .with(jwt(jwt))); + .with(jwt().jwt(jwt))); ---- ===== `authentication()` `RequestPostProcessor` diff --git a/samples/boot/oauth2resourceserver-webflux/src/test/java/sample/OAuth2ResourceServerControllerTests.java b/samples/boot/oauth2resourceserver-webflux/src/test/java/sample/OAuth2ResourceServerControllerTests.java index 663f95792f..3f1291c6fa 100644 --- a/samples/boot/oauth2resourceserver-webflux/src/test/java/sample/OAuth2ResourceServerControllerTests.java +++ b/samples/boot/oauth2resourceserver-webflux/src/test/java/sample/OAuth2ResourceServerControllerTests.java @@ -50,14 +50,14 @@ public class OAuth2ResourceServerControllerTests { @Test public void indexGreetsAuthenticatedUser() { - this.rest.mutateWith(mockJwt(jwt -> jwt.subject("test-subject"))) + this.rest.mutateWith(mockJwt().jwt(jwt -> jwt.subject("test-subject"))) .get().uri("/").exchange() .expectBody(String.class).isEqualTo("Hello, test-subject!"); } @Test public void messageCanBeReadWithScopeMessageReadAuthority() { - this.rest.mutateWith(mockJwt(jwt -> jwt.claim("scope", "message:read"))) + this.rest.mutateWith(mockJwt().jwt(jwt -> jwt.claim("scope", "message:read"))) .get().uri("/message").exchange() .expectBody(String.class).isEqualTo("secret message"); diff --git a/samples/boot/oauth2resourceserver/src/test/java/sample/OAuth2ResourceServerControllerTests.java b/samples/boot/oauth2resourceserver/src/test/java/sample/OAuth2ResourceServerControllerTests.java index b55c6ed53c..46ec794827 100644 --- a/samples/boot/oauth2resourceserver/src/test/java/sample/OAuth2ResourceServerControllerTests.java +++ b/samples/boot/oauth2resourceserver/src/test/java/sample/OAuth2ResourceServerControllerTests.java @@ -47,13 +47,13 @@ public class OAuth2ResourceServerControllerTests { @Test public void indexGreetsAuthenticatedUser() throws Exception { - mockMvc.perform(get("/").with(jwt(jwt -> jwt.subject("ch4mpy")))) + mockMvc.perform(get("/").with(jwt().jwt(jwt -> jwt.subject("ch4mpy")))) .andExpect(content().string(is("Hello, ch4mpy!"))); } @Test public void messageCanBeReadWithScopeMessageReadAuthority() throws Exception { - mockMvc.perform(get("/message").with(jwt(jwt -> jwt.claim("scope", "message:read")))) + mockMvc.perform(get("/message").with(jwt().jwt(jwt -> jwt.claim("scope", "message:read")))) .andExpect(content().string(is("secret message"))); mockMvc.perform(get("/message") @@ -79,7 +79,7 @@ public class OAuth2ResourceServerControllerTests { public void messageCanNotBeCreatedWithScopeMessageReadAuthority() throws Exception { mockMvc.perform(post("/message") .content("Hello message") - .with(jwt(jwt -> jwt.claim("scope", "message:read")))) + .with(jwt().jwt(jwt -> jwt.claim("scope", "message:read")))) .andExpect(status().isForbidden()); } @@ -88,7 +88,7 @@ public class OAuth2ResourceServerControllerTests { throws Exception { mockMvc.perform(post("/message") .content("Hello message") - .with(jwt(jwt -> jwt.claim("scope", "message:write")))) + .with(jwt().jwt(jwt -> jwt.claim("scope", "message:write")))) .andExpect(status().isOk()) .andExpect(content().string(is("Message was created. Content: Hello message"))); } diff --git a/test/src/main/java/org/springframework/security/test/web/reactive/server/SecurityMockServerConfigurers.java b/test/src/main/java/org/springframework/security/test/web/reactive/server/SecurityMockServerConfigurers.java index 312d511749..89ba9fb964 100644 --- a/test/src/main/java/org/springframework/security/test/web/reactive/server/SecurityMockServerConfigurers.java +++ b/test/src/main/java/org/springframework/security/test/web/reactive/server/SecurityMockServerConfigurers.java @@ -127,42 +127,7 @@ public class SecurityMockServerConfigurers { * @since 5.2 */ public static JwtMutator mockJwt() { - return mockJwt(jwt -> {}); - } - - /** - * Updates the ServerWebExchange to establish a {@link SecurityContext} that has a - * {@link JwtAuthenticationToken} for the - * {@link Authentication} and a {@link Jwt} for the - * {@link Authentication#getPrincipal()}. All details are - * declarative and do not require the JWT to be valid. - * - * @param jwtBuilderConsumer For configuring the underlying {@link Jwt} - * @return the {@link JwtMutator} to further configure or use - * @since 5.2 - */ - public static JwtMutator mockJwt(Consumer jwtBuilderConsumer) { - Jwt.Builder jwtBuilder = Jwt.withTokenValue("token") - .header("alg", "none") - .claim(SUB, "user") - .claim("scope", "read"); - jwtBuilderConsumer.accept(jwtBuilder); - return new JwtMutator(jwtBuilder.build()); - } - - /** - * Updates the ServerWebExchange to establish a {@link SecurityContext} that has a - * {@link JwtAuthenticationToken} for the - * {@link Authentication} and a {@link Jwt} for the - * {@link Authentication#getPrincipal()}. All details are - * declarative and do not require the JWT to be valid. - * - * @param jwt The preliminary constructed {@link Jwt} - * @return the {@link JwtMutator} to further configure or use - * @since 5.2 - */ - public static JwtMutator mockJwt(Jwt jwt) { - return new JwtMutator(jwt); + return new JwtMutator(); } public static CsrfMutator csrf() { @@ -361,11 +326,45 @@ public class SecurityMockServerConfigurers { */ public static class JwtMutator implements WebTestClientConfigurer, MockServerConfigurer { private Jwt jwt; - private Collection authorities; + private Converter> authoritiesConverter = + new JwtGrantedAuthoritiesConverter(); - private JwtMutator(Jwt jwt) { + private JwtMutator() { + jwt((jwt) -> {}); + } + + /** + * Use the given {@link Jwt.Builder} {@link Consumer} to configure the underlying {@link Jwt} + * + * This method first creates a default {@link Jwt.Builder} instance with default values for + * the {@code alg}, {@code sub}, and {@code scope} claims. The {@link Consumer} can then modify + * these or provide additional configuration. + * + * Calling {@link SecurityMockServerConfigurers#mockJwt()} is the equivalent of calling + * {@code SecurityMockMvcRequestPostProcessors.mockJwt().jwt(() -> {})}. + * + * @param jwtBuilderConsumer For configuring the underlying {@link Jwt} + * @return the {@link JwtMutator} for further configuration + */ + public JwtMutator jwt(Consumer jwtBuilderConsumer) { + Jwt.Builder jwtBuilder = Jwt.withTokenValue("token") + .header("alg", "none") + .claim(SUB, "user") + .claim("scope", "read"); + jwtBuilderConsumer.accept(jwtBuilder); + this.jwt = jwtBuilder.build(); + return this; + } + + /** + * Use the given {@link Jwt} + * + * @param jwt The {@link Jwt} to use + * @return the {@link JwtMutator} for further configuration + */ + public JwtMutator jwt(Jwt jwt) { this.jwt = jwt; - this.authorities = new JwtGrantedAuthoritiesConverter().convert(jwt); + return this; } /** @@ -375,7 +374,7 @@ public class SecurityMockServerConfigurers { */ public JwtMutator authorities(Collection authorities) { Assert.notNull(authorities, "authorities cannot be null"); - this.authorities = authorities; + this.authoritiesConverter = jwt -> authorities; return this; } @@ -386,7 +385,7 @@ public class SecurityMockServerConfigurers { */ public JwtMutator authorities(GrantedAuthority... authorities) { Assert.notNull(authorities, "authorities cannot be null"); - this.authorities = Arrays.asList(authorities); + this.authoritiesConverter = jwt -> Arrays.asList(authorities); return this; } @@ -400,7 +399,7 @@ public class SecurityMockServerConfigurers { */ public JwtMutator authorities(Converter> authoritiesConverter) { Assert.notNull(authoritiesConverter, "authoritiesConverter cannot be null"); - this.authorities = authoritiesConverter.convert(this.jwt); + this.authoritiesConverter = authoritiesConverter; return this; } @@ -427,7 +426,7 @@ public class SecurityMockServerConfigurers { } private T configurer() { - return mockAuthentication(new JwtAuthenticationToken(this.jwt, this.authorities)); + return mockAuthentication(new JwtAuthenticationToken(this.jwt, this.authoritiesConverter.convert(this.jwt))); } } } diff --git a/test/src/main/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessors.java b/test/src/main/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessors.java index f66a9ddc34..017567930a 100644 --- a/test/src/main/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessors.java +++ b/test/src/main/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessors.java @@ -227,70 +227,7 @@ public final class SecurityMockMvcRequestPostProcessors { * @return the {@link JwtRequestPostProcessor} for additional customization */ public static JwtRequestPostProcessor jwt() { - return jwt(jwt -> {}); - } - - /** - * Establish a {@link SecurityContext} that has a - * {@link JwtAuthenticationToken} for the - * {@link Authentication} and a {@link Jwt} for the - * {@link Authentication#getPrincipal()}. All details are - * declarative and do not require the JWT to be valid. - * - *

- * The support works by associating the authentication to the HttpServletRequest. To associate - * the request to the SecurityContextHolder you need to ensure that the - * SecurityContextPersistenceFilter is associated with the MockMvc instance. A few - * ways to do this are: - *

- * - *
    - *
  • Invoking apply {@link SecurityMockMvcConfigurers#springSecurity()}
    • - *
  • Adding Spring Security's FilterChainProxy to MockMvc
    • - *
  • Manually adding {@link SecurityContextPersistenceFilter} to the MockMvc - * instance may make sense when using MockMvcBuilders standaloneSetup
    • - *
- * - * @param jwtBuilderConsumer For configuring the underlying {@link Jwt} - * @return the {@link JwtRequestPostProcessor} for additional customization - * @since 5.2 - */ - public static JwtRequestPostProcessor jwt(Consumer jwtBuilderConsumer) { - Jwt.Builder jwtBuilder = Jwt.withTokenValue("token") - .header("alg", "none") - .claim(SUB, "user") - .claim("scope", "read"); - jwtBuilderConsumer.accept(jwtBuilder); - return new JwtRequestPostProcessor(jwtBuilder.build()); - } - - /** - * Establish a {@link SecurityContext} that has a - * {@link JwtAuthenticationToken} for the - * {@link Authentication} and a {@link Jwt} for the - * {@link Authentication#getPrincipal()}. All details are - * declarative and do not require the JWT to be valid. - * - *

- * The support works by associating the authentication to the HttpServletRequest. To associate - * the request to the SecurityContextHolder you need to ensure that the - * SecurityContextPersistenceFilter is associated with the MockMvc instance. A few - * ways to do this are: - *

- * - *
    - *
  • Invoking apply {@link SecurityMockMvcConfigurers#springSecurity()}
    • - *
  • Adding Spring Security's FilterChainProxy to MockMvc
    • - *
  • Manually adding {@link SecurityContextPersistenceFilter} to the MockMvc - * instance may make sense when using MockMvcBuilders standaloneSetup
    • - *
- * - * @param jwt The preliminary constructed {@link Jwt} - * @return the {@link JwtRequestPostProcessor} for additional customization - * @since 5.2 - */ - public static JwtRequestPostProcessor jwt(Jwt jwt) { - return new JwtRequestPostProcessor(jwt); + return new JwtRequestPostProcessor(); } /** @@ -1000,11 +937,45 @@ public final class SecurityMockMvcRequestPostProcessors { */ public final static class JwtRequestPostProcessor implements RequestPostProcessor { private Jwt jwt; - private Collection authorities; + private Converter> authoritiesConverter = + new JwtGrantedAuthoritiesConverter(); - private JwtRequestPostProcessor(Jwt jwt) { + private JwtRequestPostProcessor() { + this.jwt((jwt) -> {}); + } + + /** + * Use the given {@link Jwt.Builder} {@link Consumer} to configure the underlying {@link Jwt} + * + * This method first creates a default {@link Jwt.Builder} instance with default values for + * the {@code alg}, {@code sub}, and {@code scope} claims. The {@link Consumer} can then modify + * these or provide additional configuration. + * + * Calling {@link SecurityMockMvcRequestPostProcessors#jwt()} is the equivalent of calling + * {@code SecurityMockMvcRequestPostProcessors.jwt().jwt(() -> {})}. + * + * @param jwtBuilderConsumer For configuring the underlying {@link Jwt} + * @return the {@link JwtRequestPostProcessor} for additional customization + */ + public JwtRequestPostProcessor jwt(Consumer jwtBuilderConsumer) { + Jwt.Builder jwtBuilder = Jwt.withTokenValue("token") + .header("alg", "none") + .claim(SUB, "user") + .claim("scope", "read"); + jwtBuilderConsumer.accept(jwtBuilder); + this.jwt = jwtBuilder.build(); + return this; + } + + /** + * Use the given {@link Jwt} + * + * @param jwt The {@link Jwt} to use + * @return the {@link JwtRequestPostProcessor} for additional customization + */ + public JwtRequestPostProcessor jwt(Jwt jwt) { this.jwt = jwt; - this.authorities = new JwtGrantedAuthoritiesConverter().convert(jwt); + return this; } /** @@ -1014,7 +985,7 @@ public final class SecurityMockMvcRequestPostProcessors { */ public JwtRequestPostProcessor authorities(Collection authorities) { Assert.notNull(authorities, "authorities cannot be null"); - this.authorities = authorities; + this.authoritiesConverter = jwt -> authorities; return this; } @@ -1025,7 +996,7 @@ public final class SecurityMockMvcRequestPostProcessors { */ public JwtRequestPostProcessor authorities(GrantedAuthority... authorities) { Assert.notNull(authorities, "authorities cannot be null"); - this.authorities = Arrays.asList(authorities); + this.authoritiesConverter = jwt -> Arrays.asList(authorities); return this; } @@ -1039,14 +1010,15 @@ public final class SecurityMockMvcRequestPostProcessors { */ public JwtRequestPostProcessor authorities(Converter> authoritiesConverter) { Assert.notNull(authoritiesConverter, "authoritiesConverter cannot be null"); - this.authorities = authoritiesConverter.convert(this.jwt); + this.authoritiesConverter = authoritiesConverter; return this; } @Override public MockHttpServletRequest postProcessRequest(MockHttpServletRequest request) { CsrfFilter.skipRequest(request); - JwtAuthenticationToken token = new JwtAuthenticationToken(this.jwt, this.authorities); + JwtAuthenticationToken token = new JwtAuthenticationToken(this.jwt, + this.authoritiesConverter.convert(this.jwt)); return new AuthenticationRequestPostProcessor(token).postProcessRequest(request); } diff --git a/test/src/test/java/org/springframework/security/test/web/reactive/server/SecurityMockServerConfigurersJwtTests.java b/test/src/test/java/org/springframework/security/test/web/reactive/server/SecurityMockServerConfigurersJwtTests.java index 70aebf81d1..e92100fa39 100644 --- a/test/src/test/java/org/springframework/security/test/web/reactive/server/SecurityMockServerConfigurersJwtTests.java +++ b/test/src/test/java/org/springframework/security/test/web/reactive/server/SecurityMockServerConfigurersJwtTests.java @@ -85,7 +85,7 @@ public class SecurityMockServerConfigurersJwtTests extends AbstractMockServerCon public void mockJwtWhenProvidingBuilderConsumerThenProducesJwtAuthentication() { String name = new String("user"); client - .mutateWith(mockJwt(jwt -> jwt.subject(name))) + .mutateWith(mockJwt().jwt(jwt -> jwt.subject(name))) .get() .exchange() .expectStatus().isOk(); @@ -100,7 +100,7 @@ public class SecurityMockServerConfigurersJwtTests extends AbstractMockServerCon @Test public void mockJwtWhenProvidingCustomAuthoritiesThenProducesJwtAuthentication() { client - .mutateWith(mockJwt(jwt -> jwt.claim("scope", "ignored authorities")) + .mutateWith(mockJwt().jwt(jwt -> jwt.claim("scope", "ignored authorities")) .authorities(this.authority1, this.authority2)) .get() .exchange() @@ -114,7 +114,7 @@ public class SecurityMockServerConfigurersJwtTests extends AbstractMockServerCon @Test public void mockJwtWhenProvidingScopedAuthoritiesThenProducesJwtAuthentication() { client - .mutateWith(mockJwt(jwt -> jwt.claim("scope", "scoped authorities"))) + .mutateWith(mockJwt().jwt(jwt -> jwt.claim("scope", "scoped authorities"))) .get() .exchange() .expectStatus().isOk(); @@ -128,7 +128,7 @@ public class SecurityMockServerConfigurersJwtTests extends AbstractMockServerCon @Test public void mockJwtWhenProvidingGrantedAuthoritiesThenProducesJwtAuthentication() { client - .mutateWith(mockJwt(jwt -> jwt.claim("scope", "ignored authorities")) + .mutateWith(mockJwt().jwt(jwt -> jwt.claim("scope", "ignored authorities")) .authorities(jwt -> Arrays.asList(this.authority1))) .get() .exchange() @@ -146,7 +146,7 @@ public class SecurityMockServerConfigurersJwtTests extends AbstractMockServerCon .subject("some_user") .build(); this.client - .mutateWith(mockJwt(originalToken)) + .mutateWith(mockJwt().jwt(originalToken)) .get() .exchange() .expectStatus().isOk(); diff --git a/test/src/test/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessorsJwtTests.java b/test/src/test/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessorsJwtTests.java index fd69f4b02c..24441de7dc 100644 --- a/test/src/test/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessorsJwtTests.java +++ b/test/src/test/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessorsJwtTests.java @@ -107,7 +107,7 @@ public class SecurityMockMvcRequestPostProcessorsJwtTests { @Test public void jwtWhenProvidingBuilderConsumerThenProducesJwtAuthentication() { String name = new String("user"); - jwt(jwt -> jwt.subject(name)).postProcessRequest(this.request); + jwt().jwt(jwt -> jwt.subject(name)).postProcessRequest(this.request); verify(this.repository).saveContext(this.contextCaptor.capture(), eq(this.request), any(HttpServletResponse.class)); @@ -120,7 +120,7 @@ public class SecurityMockMvcRequestPostProcessorsJwtTests { @Test public void jwtWhenProvidingCustomAuthoritiesThenProducesJwtAuthentication() { - jwt(jwt -> jwt.claim("scope", "ignored authorities")) + jwt().jwt(jwt -> jwt.claim("scope", "ignored authorities")) .authorities(this.authority1, this.authority2) .postProcessRequest(this.request); @@ -133,7 +133,7 @@ public class SecurityMockMvcRequestPostProcessorsJwtTests { @Test public void jwtWhenProvidingScopedAuthoritiesThenProducesJwtAuthentication() { - jwt(jwt -> jwt.claim("scope", "scoped authorities")) + jwt().jwt(jwt -> jwt.claim("scope", "scoped authorities")) .postProcessRequest(this.request); verify(this.repository).saveContext(this.contextCaptor.capture(), eq(this.request), @@ -146,7 +146,7 @@ public class SecurityMockMvcRequestPostProcessorsJwtTests { @Test public void jwtWhenProvidingGrantedAuthoritiesThenProducesJwtAuthentication() { - jwt(jwt -> jwt.claim("scope", "ignored authorities")) + jwt().jwt(jwt -> jwt.claim("scope", "ignored authorities")) .authorities(jwt -> Arrays.asList(this.authority1)) .postProcessRequest(this.request); @@ -163,7 +163,7 @@ public class SecurityMockMvcRequestPostProcessorsJwtTests { .header("header1", "value1") .subject("some_user") .build(); - jwt(originalToken).postProcessRequest(this.request); + jwt().jwt(originalToken).postProcessRequest(this.request); verify(this.repository).saveContext(this.contextCaptor.capture(), eq(this.request), any(HttpServletResponse.class));