From 4dd79e2d61eb84ec523807392eccf5f9883e625a Mon Sep 17 00:00:00 2001 From: ruabtmh Date: Tue, 13 Feb 2024 11:47:23 +0300 Subject: [PATCH 1/2] Add ContinueOnError Support For Failed Authentications Closes gh-14521 --- ...legatingReactiveAuthenticationManager.java | 25 +++++++---- ...ingReactiveAuthenticationManagerTests.java | 41 ++++++++++++++++++- 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/springframework/security/authentication/DelegatingReactiveAuthenticationManager.java b/core/src/main/java/org/springframework/security/authentication/DelegatingReactiveAuthenticationManager.java index 7a06a0695b5..aca5b97dfd0 100644 --- a/core/src/main/java/org/springframework/security/authentication/DelegatingReactiveAuthenticationManager.java +++ b/core/src/main/java/org/springframework/security/authentication/DelegatingReactiveAuthenticationManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2024 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. @@ -18,7 +18,10 @@ import java.util.Arrays; import java.util.List; +import java.util.function.Function; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -28,7 +31,7 @@ /** * A {@link ReactiveAuthenticationManager} that delegates to other * {@link ReactiveAuthenticationManager} instances using the result from the first non - * empty result. + * empty result. Errors from delegates will be ignored if continueOnError is true. * * @author Rob Winch * @since 5.1 @@ -37,6 +40,10 @@ public class DelegatingReactiveAuthenticationManager implements ReactiveAuthenti private final List delegates; + private boolean continueOnError = false; + + private final Log logger = LogFactory.getLog(getClass()); + public DelegatingReactiveAuthenticationManager(ReactiveAuthenticationManager... entryPoints) { this(Arrays.asList(entryPoints)); } @@ -48,11 +55,15 @@ public DelegatingReactiveAuthenticationManager(List authenticate(Authentication authentication) { - // @formatter:off - return Flux.fromIterable(this.delegates) - .concatMap((m) -> m.authenticate(authentication)) - .next(); - // @formatter:on + Flux result = Flux.fromIterable(this.delegates); + Function> logging = (m) -> m.authenticate(authentication) + .doOnError(this.logger::debug); + + return ((this.continueOnError) ? result.concatMapDelayError(logging) : result.concatMap(logging)).next(); + } + + public void setContinueOnError(boolean continueOnError) { + this.continueOnError = continueOnError; } } diff --git a/core/src/test/java/org/springframework/security/authentication/DelegatingReactiveAuthenticationManagerTests.java b/core/src/test/java/org/springframework/security/authentication/DelegatingReactiveAuthenticationManagerTests.java index dd89bd7c89e..2d4b2c7a158 100644 --- a/core/src/test/java/org/springframework/security/authentication/DelegatingReactiveAuthenticationManagerTests.java +++ b/core/src/test/java/org/springframework/security/authentication/DelegatingReactiveAuthenticationManagerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2024 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. @@ -77,4 +77,43 @@ public void authenticateWhenBadCredentialsThenDelegate2NotInvokedAndError() { .verify(); } + @Test + public void authenticateWhenContinueOnErrorAndFirstBadCredentialsThenTriesSecond() { + given(this.delegate1.authenticate(any())).willReturn(Mono.error(new BadCredentialsException("Test"))); + given(this.delegate2.authenticate(any())).willReturn(Mono.just(this.authentication)); + + DelegatingReactiveAuthenticationManager manager = managerWithContinueOnError(); + + assertThat(manager.authenticate(this.authentication).block()).isEqualTo(this.authentication); + } + + @Test + public void authenticateWhenContinueOnErrorAndBothDelegatesBadCredentialsThenError() { + given(this.delegate1.authenticate(any())).willReturn(Mono.error(new BadCredentialsException("Test"))); + given(this.delegate2.authenticate(any())).willReturn(Mono.error(new BadCredentialsException("Test"))); + + DelegatingReactiveAuthenticationManager manager = managerWithContinueOnError(); + + StepVerifier.create(manager.authenticate(this.authentication)) + .expectError(BadCredentialsException.class) + .verify(); + } + + @Test + public void authenticateWhenContinueOnErrorAndDelegate1NotEmptyThenReturnsNotEmpty() { + given(this.delegate1.authenticate(any())).willReturn(Mono.just(this.authentication)); + + DelegatingReactiveAuthenticationManager manager = managerWithContinueOnError(); + + assertThat(manager.authenticate(this.authentication).block()).isEqualTo(this.authentication); + } + + private DelegatingReactiveAuthenticationManager managerWithContinueOnError() { + DelegatingReactiveAuthenticationManager manager = new DelegatingReactiveAuthenticationManager(this.delegate1, + this.delegate2); + manager.setContinueOnError(true); + + return manager; + } + } From 353bef7d9ad851d95737ba687c559b3a922c4138 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Fri, 23 Feb 2024 15:39:03 -0700 Subject: [PATCH 2/2] Polish JavaDoc Issue gh-14521 --- .../DelegatingReactiveAuthenticationManager.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/springframework/security/authentication/DelegatingReactiveAuthenticationManager.java b/core/src/main/java/org/springframework/security/authentication/DelegatingReactiveAuthenticationManager.java index aca5b97dfd0..2fdc2d48c42 100644 --- a/core/src/main/java/org/springframework/security/authentication/DelegatingReactiveAuthenticationManager.java +++ b/core/src/main/java/org/springframework/security/authentication/DelegatingReactiveAuthenticationManager.java @@ -30,8 +30,9 @@ /** * A {@link ReactiveAuthenticationManager} that delegates to other - * {@link ReactiveAuthenticationManager} instances using the result from the first non - * empty result. Errors from delegates will be ignored if continueOnError is true. + * {@link ReactiveAuthenticationManager} instances. When {@code continueOnError} is + * {@code true}, will continue until the first non-empty, non-error result; otherwise, + * will continue only until the first non-empty result. * * @author Rob Winch * @since 5.1 @@ -62,6 +63,11 @@ public Mono authenticate(Authentication authentication) { return ((this.continueOnError) ? result.concatMapDelayError(logging) : result.concatMap(logging)).next(); } + /** + * Continue iterating when a delegate errors, defaults to {@code false} + * @param continueOnError whether to continue when a delegate errors + * @since 6.3 + */ public void setContinueOnError(boolean continueOnError) { this.continueOnError = continueOnError; }