From f4cc27c375ffb7b5d20e85198c4f34ed06ded279 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Thu, 13 Oct 2022 20:03:03 -0600 Subject: [PATCH] Change Default for (Server)AuthenticationEntryPointFailureHandler Closes gh-9429 --- docs/modules/ROOT/pages/whats-new.adoc | 1 + .../BearerTokenAuthenticationFilter.java | 10 +++------- .../AuthenticationEntryPointFailureHandler.java | 4 ++-- .../ServerAuthenticationEntryPointFailureHandler.java | 4 ++-- .../AuthenticationEntryPointFailureHandlerTests.java | 6 +++--- ...verAuthenticationEntryPointFailureHandlerTests.java | 6 +++--- 6 files changed, 14 insertions(+), 17 deletions(-) diff --git a/docs/modules/ROOT/pages/whats-new.adoc b/docs/modules/ROOT/pages/whats-new.adoc index 2ff27c67ed..8982d68ef7 100644 --- a/docs/modules/ROOT/pages/whats-new.adoc +++ b/docs/modules/ROOT/pages/whats-new.adoc @@ -32,6 +32,7 @@ Instead, use `requestMatchers` or `HttpSecurity#securityMatchers`. * https://github.com/spring-projects/spring-security/issues/11960[gh-11960] - Default to Xor CSRF protection for xref:servlet/exploits/csrf.adoc#servlet-csrf-configure-request-handler[servlet] and xref:reactive/exploits/csrf.adoc#webflux-csrf-configure-request-handler[reactive] * https://github.com/spring-projects/spring-security/issues/12019[gh-12019] - Remove deprecated method `setTokenFromMultipartDataEnabled` from `CsrfWebFilter` * https://github.com/spring-projects/spring-security/issues/12020[gh-12020] - Remove deprecated method `tokenFromMultipartDataEnabled` from Java Configuration +* https://github.com/spring-projects/spring-security/issues/9429[gh-9429] - `Authentication(Web)Filter` rethrows `AuthenticationServiceException`s == Observability diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/authentication/BearerTokenAuthenticationFilter.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/authentication/BearerTokenAuthenticationFilter.java index 6377899c87..6cdd3aadad 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/authentication/BearerTokenAuthenticationFilter.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/authentication/BearerTokenAuthenticationFilter.java @@ -27,7 +27,6 @@ import org.springframework.core.log.LogMessage; import org.springframework.security.authentication.AuthenticationDetailsSource; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.AuthenticationManagerResolver; -import org.springframework.security.authentication.AuthenticationServiceException; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.context.SecurityContext; @@ -40,6 +39,7 @@ import org.springframework.security.oauth2.server.resource.web.BearerTokenAuthen import org.springframework.security.oauth2.server.resource.web.BearerTokenResolver; import org.springframework.security.oauth2.server.resource.web.DefaultBearerTokenResolver; import org.springframework.security.web.AuthenticationEntryPoint; +import org.springframework.security.web.authentication.AuthenticationEntryPointFailureHandler; import org.springframework.security.web.authentication.AuthenticationFailureHandler; import org.springframework.security.web.authentication.WebAuthenticationDetailsSource; import org.springframework.security.web.context.RequestAttributeSecurityContextRepository; @@ -73,12 +73,8 @@ public class BearerTokenAuthenticationFilter extends OncePerRequestFilter { private AuthenticationEntryPoint authenticationEntryPoint = new BearerTokenAuthenticationEntryPoint(); - private AuthenticationFailureHandler authenticationFailureHandler = (request, response, exception) -> { - if (exception instanceof AuthenticationServiceException) { - throw exception; - } - this.authenticationEntryPoint.commence(request, response, exception); - }; + private AuthenticationFailureHandler authenticationFailureHandler = new AuthenticationEntryPointFailureHandler( + (request, response, exception) -> this.authenticationEntryPoint.commence(request, response, exception)); private BearerTokenResolver bearerTokenResolver = new DefaultBearerTokenResolver(); diff --git a/web/src/main/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandler.java b/web/src/main/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandler.java index e42cfe6a49..c846d66653 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandler.java +++ b/web/src/main/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandler.java @@ -35,7 +35,7 @@ import org.springframework.util.Assert; */ public class AuthenticationEntryPointFailureHandler implements AuthenticationFailureHandler { - private boolean rethrowAuthenticationServiceException = false; + private boolean rethrowAuthenticationServiceException = true; private final AuthenticationEntryPoint authenticationEntryPoint; @@ -59,7 +59,7 @@ public class AuthenticationEntryPointFailureHandler implements AuthenticationFai } /** - * Set whether to rethrow {@link AuthenticationServiceException}s (defaults to false) + * Set whether to rethrow {@link AuthenticationServiceException}s (defaults to true) * @param rethrowAuthenticationServiceException whether to rethrow * {@link AuthenticationServiceException}s * @since 5.8 diff --git a/web/src/main/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandler.java b/web/src/main/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandler.java index 74bcc1dbb2..aebddc9225 100644 --- a/web/src/main/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandler.java +++ b/web/src/main/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandler.java @@ -35,7 +35,7 @@ public class ServerAuthenticationEntryPointFailureHandler implements ServerAuthe private final ServerAuthenticationEntryPoint authenticationEntryPoint; - private boolean rethrowAuthenticationServiceException = false; + private boolean rethrowAuthenticationServiceException = true; public ServerAuthenticationEntryPointFailureHandler(ServerAuthenticationEntryPoint authenticationEntryPoint) { Assert.notNull(authenticationEntryPoint, "authenticationEntryPoint cannot be null"); @@ -54,7 +54,7 @@ public class ServerAuthenticationEntryPointFailureHandler implements ServerAuthe } /** - * Set whether to rethrow {@link AuthenticationServiceException}s (defaults to false) + * Set whether to rethrow {@link AuthenticationServiceException}s (defaults to true) * @param rethrowAuthenticationServiceException whether to rethrow * {@link AuthenticationServiceException}s * @since 5.8 diff --git a/web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerTests.java b/web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerTests.java index af52e705d1..28f46311db 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerTests.java @@ -30,17 +30,17 @@ import static org.mockito.Mockito.mock; public class AuthenticationEntryPointFailureHandlerTests { @Test - void onAuthenticationFailureWhenDefaultsThenAuthenticationServiceExceptionSwallowed() throws Exception { + void onAuthenticationFailureWhenRethrowingThenAuthenticationServiceExceptionSwallowed() throws Exception { AuthenticationEntryPoint entryPoint = mock(AuthenticationEntryPoint.class); AuthenticationEntryPointFailureHandler handler = new AuthenticationEntryPointFailureHandler(entryPoint); + handler.setRethrowAuthenticationServiceException(false); handler.onAuthenticationFailure(null, null, new AuthenticationServiceException("fail")); } @Test - void handleWhenRethrowingThenAuthenticationServiceExceptionRethrown() { + void handleWhenDefaultsThenAuthenticationServiceExceptionRethrown() { AuthenticationEntryPoint entryPoint = mock(AuthenticationEntryPoint.class); AuthenticationEntryPointFailureHandler handler = new AuthenticationEntryPointFailureHandler(entryPoint); - handler.setRethrowAuthenticationServiceException(true); assertThatExceptionOfType(AuthenticationServiceException.class).isThrownBy( () -> handler.onAuthenticationFailure(null, null, new AuthenticationServiceException("fail"))); } diff --git a/web/src/test/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerTests.java b/web/src/test/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerTests.java index 670f476c65..a0d4ed201c 100644 --- a/web/src/test/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerTests.java +++ b/web/src/test/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerTests.java @@ -71,16 +71,16 @@ public class ServerAuthenticationEntryPointFailureHandlerTests { } @Test - void onAuthenticationFailureWhenDefaultsThenAuthenticationServiceExceptionSwallowed() { + void onAuthenticationFailureWhenRethrownFalseThenAuthenticationServiceExceptionSwallowed() { AuthenticationServiceException e = new AuthenticationServiceException("fail"); + this.handler.setRethrowAuthenticationServiceException(false); given(this.authenticationEntryPoint.commence(this.exchange, e)).willReturn(Mono.empty()); this.handler.onAuthenticationFailure(this.filterExchange, e).block(); } @Test - void handleWhenRethrowingThenAuthenticationServiceExceptionRethrown() { + void handleWhenDefaultsThenAuthenticationServiceExceptionRethrown() { AuthenticationServiceException e = new AuthenticationServiceException("fail"); - this.handler.setRethrowAuthenticationServiceException(true); assertThatExceptionOfType(AuthenticationServiceException.class) .isThrownBy(() -> this.handler.onAuthenticationFailure(this.filterExchange, e).block()); }