Przeglądaj źródła

TestOneTimeTokenGenerationSuccessHandler.lastToken to non-static variable

Previously there were race conditions on the static member lastToken of
TestOneTimeTokenGenerationSuccessHandler. This is because the tests run in
parallel and one test may override the other tests lastToken and thus
make the assertion on it incorrect.

This commit changes lastToken to be a non-static variable to ensure that
each test has it's own lastToken for asserting the expected value.

Closes gh-16471
Rob Winch 7 miesięcy temu
rodzic
commit
751b5580a1

+ 35 - 11
config/src/test/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurerTests.java

@@ -72,7 +72,7 @@ public class OneTimeTokenLoginConfigurerTests {
 		this.mvc.perform(post("/ott/generate").param("username", "user").with(csrf()))
 			.andExpectAll(status().isFound(), redirectedUrl("/login/ott"));
 
-		String token = TestOneTimeTokenGenerationSuccessHandler.lastToken.getTokenValue();
+		String token = getLastToken().getTokenValue();
 
 		this.mvc.perform(post("/login/ott").param("token", token).with(csrf()))
 			.andExpectAll(status().isFound(), redirectedUrl("/"), authenticated());
@@ -84,7 +84,7 @@ public class OneTimeTokenLoginConfigurerTests {
 		this.mvc.perform(post("/generateurl").param("username", "user").with(csrf()))
 			.andExpectAll(status().isFound(), redirectedUrl("/redirected"));
 
-		String token = TestOneTimeTokenGenerationSuccessHandler.lastToken.getTokenValue();
+		String token = getLastToken().getTokenValue();
 
 		this.mvc.perform(post("/loginprocessingurl").param("token", token).with(csrf()))
 			.andExpectAll(status().isFound(), redirectedUrl("/authenticated"), authenticated());
@@ -96,7 +96,7 @@ public class OneTimeTokenLoginConfigurerTests {
 		this.mvc.perform(post("/ott/generate").param("username", "user").with(csrf()))
 			.andExpectAll(status().isFound(), redirectedUrl("/login/ott"));
 
-		String token = TestOneTimeTokenGenerationSuccessHandler.lastToken.getTokenValue();
+		String token = getLastToken().getTokenValue();
 
 		this.mvc.perform(post("/login/ott").param("token", token).with(csrf()))
 			.andExpectAll(status().isFound(), redirectedUrl("/"), authenticated());
@@ -194,25 +194,37 @@ public class OneTimeTokenLoginConfigurerTests {
 					""");
 	}
 
+	private OneTimeToken getLastToken() {
+		OneTimeToken lastToken = this.spring.getContext()
+			.getBean(TestOneTimeTokenGenerationSuccessHandler.class).lastToken;
+		return lastToken;
+	}
+
 	@Configuration(proxyBeanMethods = false)
 	@EnableWebSecurity
 	@Import(UserDetailsServiceConfig.class)
 	static class OneTimeTokenDefaultConfig {
 
 		@Bean
-		SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
+		SecurityFilterChain securityFilterChain(HttpSecurity http,
+				OneTimeTokenGenerationSuccessHandler ottSuccessHandler) throws Exception {
 			// @formatter:off
 			http
 					.authorizeHttpRequests((authz) -> authz
 							.anyRequest().authenticated()
 					)
 					.oneTimeTokenLogin((ott) -> ott
-							.tokenGenerationSuccessHandler(new TestOneTimeTokenGenerationSuccessHandler())
+							.tokenGenerationSuccessHandler(ottSuccessHandler)
 					);
 			// @formatter:on
 			return http.build();
 		}
 
+		@Bean
+		TestOneTimeTokenGenerationSuccessHandler ottSuccessHandler() {
+			return new TestOneTimeTokenGenerationSuccessHandler();
+		}
+
 	}
 
 	@Configuration(proxyBeanMethods = false)
@@ -221,7 +233,8 @@ public class OneTimeTokenLoginConfigurerTests {
 	static class OneTimeTokenDifferentUrlsConfig {
 
 		@Bean
-		SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
+		SecurityFilterChain securityFilterChain(HttpSecurity http,
+				OneTimeTokenGenerationSuccessHandler ottSuccessHandler) throws Exception {
 			// @formatter:off
 			http
 					.authorizeHttpRequests((authz) -> authz
@@ -229,7 +242,7 @@ public class OneTimeTokenLoginConfigurerTests {
 					)
 					.oneTimeTokenLogin((ott) -> ott
 							.tokenGeneratingUrl("/generateurl")
-							.tokenGenerationSuccessHandler(new TestOneTimeTokenGenerationSuccessHandler("/redirected"))
+							.tokenGenerationSuccessHandler(ottSuccessHandler)
 							.loginProcessingUrl("/loginprocessingurl")
 							.authenticationSuccessHandler(new SimpleUrlAuthenticationSuccessHandler("/authenticated"))
 					);
@@ -237,6 +250,11 @@ public class OneTimeTokenLoginConfigurerTests {
 			return http.build();
 		}
 
+		@Bean
+		TestOneTimeTokenGenerationSuccessHandler ottSuccessHandler() {
+			return new TestOneTimeTokenGenerationSuccessHandler("/redirected");
+		}
+
 	}
 
 	@Configuration(proxyBeanMethods = false)
@@ -245,7 +263,8 @@ public class OneTimeTokenLoginConfigurerTests {
 	static class OneTimeTokenFormLoginConfig {
 
 		@Bean
-		SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
+		SecurityFilterChain securityFilterChain(HttpSecurity http,
+				OneTimeTokenGenerationSuccessHandler ottSuccessHandler) throws Exception {
 			// @formatter:off
 			http
 					.authorizeHttpRequests((authz) -> authz
@@ -253,12 +272,17 @@ public class OneTimeTokenLoginConfigurerTests {
 					)
 					.formLogin(Customizer.withDefaults())
 					.oneTimeTokenLogin((ott) -> ott
-							.tokenGenerationSuccessHandler(new TestOneTimeTokenGenerationSuccessHandler())
+							.tokenGenerationSuccessHandler(ottSuccessHandler)
 					);
 			// @formatter:on
 			return http.build();
 		}
 
+		@Bean
+		TestOneTimeTokenGenerationSuccessHandler ottSuccessHandler() {
+			return new TestOneTimeTokenGenerationSuccessHandler();
+		}
+
 	}
 
 	@Configuration(proxyBeanMethods = false)
@@ -282,7 +306,7 @@ public class OneTimeTokenLoginConfigurerTests {
 
 	static class TestOneTimeTokenGenerationSuccessHandler implements OneTimeTokenGenerationSuccessHandler {
 
-		private static OneTimeToken lastToken;
+		private OneTimeToken lastToken;
 
 		private final OneTimeTokenGenerationSuccessHandler delegate;
 
@@ -297,7 +321,7 @@ public class OneTimeTokenLoginConfigurerTests {
 		@Override
 		public void handle(HttpServletRequest request, HttpServletResponse response, OneTimeToken oneTimeToken)
 				throws IOException, ServletException {
-			lastToken = oneTimeToken;
+			this.lastToken = oneTimeToken;
 			this.delegate.handle(request, response, oneTimeToken);
 		}
 

+ 25 - 12
config/src/test/kotlin/org/springframework/security/config/annotation/web/OneTimeTokenLoginDslTests.kt

@@ -69,7 +69,7 @@ class OneTimeTokenLoginDslTests {
                 .redirectedUrl("/login/ott")
         )
 
-        val token = TestOneTimeTokenGenerationSuccessHandler.lastToken?.tokenValue
+        val token = getLastToken().tokenValue
 
         this.mockMvc.perform(
             MockMvcRequestBuilders.post("/login/ott").param("token", token)
@@ -91,7 +91,7 @@ class OneTimeTokenLoginDslTests {
         )
             .andExpectAll(MockMvcResultMatchers.status().isFound(), MockMvcResultMatchers.redirectedUrl("/redirected"))
 
-        val token = TestOneTimeTokenGenerationSuccessHandler.lastToken?.tokenValue
+        val token = getLastToken().tokenValue
 
         this.mockMvc.perform(
             MockMvcRequestBuilders.post("/loginprocessingurl").param("token", token)
@@ -104,25 +104,36 @@ class OneTimeTokenLoginDslTests {
             )
     }
 
+    private fun getLastToken(): OneTimeToken {
+        val lastToken: OneTimeToken = spring.context
+            .getBean(TestOneTimeTokenGenerationSuccessHandler::class.java).lastToken!!
+        return lastToken
+    }
+
     @Configuration
     @EnableWebSecurity
     @Import(UserDetailsServiceConfig::class)
     open class OneTimeTokenConfig {
 
         @Bean
-        open fun securityFilterChain(http: HttpSecurity): SecurityFilterChain {
+        open fun securityFilterChain(http: HttpSecurity, ottSuccessHandler: OneTimeTokenGenerationSuccessHandler): SecurityFilterChain {
             // @formatter:off
             http {
                 authorizeHttpRequests {
                     authorize(anyRequest, authenticated)
                 }
                 oneTimeTokenLogin {
-                    oneTimeTokenGenerationSuccessHandler = TestOneTimeTokenGenerationSuccessHandler()
+                    oneTimeTokenGenerationSuccessHandler = ottSuccessHandler
                 }
             }
             // @formatter:on
             return http.build()
         }
+
+        @Bean
+        open fun ottSuccessHandler(): TestOneTimeTokenGenerationSuccessHandler {
+            return TestOneTimeTokenGenerationSuccessHandler()
+        }
     }
 
     @EnableWebSecurity
@@ -130,7 +141,7 @@ class OneTimeTokenLoginDslTests {
     @Import(UserDetailsServiceConfig::class)
     open class OneTimeTokenDifferentUrlsConfig {
         @Bean
-        open fun securityFilterChain(http: HttpSecurity): SecurityFilterChain {
+        open fun securityFilterChain(http: HttpSecurity, ottSuccessHandler: OneTimeTokenGenerationSuccessHandler): SecurityFilterChain {
             // @formatter:off
             http {
                 authorizeHttpRequests {
@@ -138,7 +149,7 @@ class OneTimeTokenLoginDslTests {
                 }
                 oneTimeTokenLogin {
                     tokenGeneratingUrl = "/generateurl"
-                    oneTimeTokenGenerationSuccessHandler = TestOneTimeTokenGenerationSuccessHandler("/redirected")
+                    oneTimeTokenGenerationSuccessHandler = ottSuccessHandler
                     loginProcessingUrl = "/loginprocessingurl"
                     authenticationSuccessHandler = SimpleUrlAuthenticationSuccessHandler("/authenticated")
                 }
@@ -146,6 +157,11 @@ class OneTimeTokenLoginDslTests {
             // @formatter:on
             return http.build()
         }
+
+        @Bean
+        open fun ottSuccessHandler(): TestOneTimeTokenGenerationSuccessHandler {
+            return TestOneTimeTokenGenerationSuccessHandler("/redirected")
+        }
     }
 
     @Configuration(proxyBeanMethods = false)
@@ -156,9 +172,10 @@ class OneTimeTokenLoginDslTests {
             InMemoryUserDetailsManager(PasswordEncodedUser.user(), PasswordEncodedUser.admin())
     }
 
-    private class TestOneTimeTokenGenerationSuccessHandler :
+    class TestOneTimeTokenGenerationSuccessHandler :
         OneTimeTokenGenerationSuccessHandler {
         private val delegate: OneTimeTokenGenerationSuccessHandler
+        var lastToken: OneTimeToken? = null
 
         constructor() {
             this.delegate =
@@ -175,12 +192,8 @@ class OneTimeTokenLoginDslTests {
         }
 
         override fun handle(request: HttpServletRequest, response: HttpServletResponse, oneTimeToken: OneTimeToken) {
-            lastToken = oneTimeToken
+            this.lastToken = oneTimeToken
             delegate.handle(request, response, oneTimeToken)
         }
-
-        companion object {
-            var lastToken: OneTimeToken? = null
-        }
     }
 }