DelegatingServerAuthenticationSuccessHandler Executes Sequentially

Fixes gh-7728

Author: rwinch

web/src/main/java/org/springframework/security/web/server/authentication/DelegatingServerAuthenticationSuccessHandler.java

@@ -19,11 +19,11 @@
 import org.springframework.security.core.Authentication;
 import org.springframework.security.web.server.WebFilterExchange;
 import org.springframework.util.Assert;
+import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
 import java.util.Arrays;
 import java.util.List;
-import java.util.ArrayList;
 
 /**
  * Delegates to a collection of {@link ServerAuthenticationSuccessHandler} implementations.
@@ -42,10 +42,8 @@ public DelegatingServerAuthenticationSuccessHandler(ServerAuthenticationSuccessH
 	@Override
 	public Mono<Void> onAuthenticationSuccess(WebFilterExchange exchange,
 			Authentication authentication) {
-		List<Mono<Void>> results = new ArrayList<>();
-		for (ServerAuthenticationSuccessHandler delegate : delegates) {
-			results.add(delegate.onAuthenticationSuccess(exchange, authentication));
-		}
-		return Mono.when(results);
+		return Flux.fromIterable(this.delegates)
+			.concatMap(delegate -> delegate.onAuthenticationSuccess(exchange, authentication))
+			.then();
 	}
 }

web/src/test/java/org/springframework/security/web/server/authentication/DelegatingServerAuthenticationSuccessHandlerTests.java

@@ -16,20 +16,26 @@
 
 package org.springframework.security.web.server.authentication;
 
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.when;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.web.server.WebFilterExchange;
-
+import reactor.core.publisher.Mono;
 import reactor.test.publisher.PublisherProbe;
 
+import java.time.Duration;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+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.when;
+
 /**
  * @author Rob Winch
  * @since 5.1
@@ -88,4 +94,26 @@ public void onAuthenticationSuccessWhenMultipleThenExecuted() {
 		this.delegate1Result.assertWasSubscribed();
 		this.delegate2Result.assertWasSubscribed();
 	}
+
+	@Test
+	public void onAuthenticationSuccessSequential() throws Exception {
+		AtomicBoolean slowDone = new AtomicBoolean();
+		CountDownLatch latch = new CountDownLatch(1);
+		ServerAuthenticationSuccessHandler slow = (exchange, authentication) ->
+				Mono.delay(Duration.ofMillis(100))
+						.doOnSuccess(__ -> slowDone.set(true))
+						.then();
+		ServerAuthenticationSuccessHandler second = (exchange, authentication) ->
+				Mono.fromRunnable(() -> {
+					latch.countDown();
+					assertThat(slowDone.get())
+							.describedAs("ServerAuthenticationSuccessHandler should be executed sequentially")
+							.isTrue();
+				});
+		DelegatingServerAuthenticationSuccessHandler handler = new DelegatingServerAuthenticationSuccessHandler(slow, second);
+
+		handler.onAuthenticationSuccess(this.exchange, this.authentication).block();
+
+		assertThat(latch.await(3, TimeUnit.SECONDS)).isTrue();
+	}
 }