From 200b7fecd347c29b4508351d4ee07cfc170888af Mon Sep 17 00:00:00 2001 From: Daniel Garnier-Moiroux Date: Sun, 2 Oct 2022 19:01:34 +0200 Subject: [PATCH 1/2] Add (Server)AuthenticationEntryPointFailureHandlerAdapter Issue gh-11932, gh-9429 (Server)AuthenticationEntryPointFailureHandler should produce HTTP 500 instead when an AuthenticationServiceException is thrown, instead of HTTP 401. This commit deprecates the current behavior and introduces an opt-in (Server)AuthenticationEntryPointFailureHandlerAdapter with the expected behavior. BearerTokenAuthenticationFilter uses the new adapter, but with a closure to keep the current behavior re: entrypoint. --- etc/checkstyle/checkstyle-suppressions.xml | 3 + .../BearerTokenAuthenticationFilter.java | 17 ++-- .../BearerTokenAuthenticationFilterTests.java | 15 ++++ ...uthenticationEntryPointFailureHandler.java | 4 +- ...cationEntryPointFailureHandlerAdapter.java | 56 ++++++++++++++ ...uthenticationEntryPointFailureHandler.java | 4 +- ...cationEntryPointFailureHandlerAdapter.java | 53 +++++++++++++ ...onEntryPointFailureHandlerAdapterTest.java | 69 +++++++++++++++++ ...onEntryPointFailureHandlerAdapterTest.java | 77 +++++++++++++++++++ 9 files changed, 289 insertions(+), 9 deletions(-) create mode 100644 web/src/main/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapter.java create mode 100644 web/src/main/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapter.java create mode 100644 web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapterTest.java create mode 100644 web/src/test/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapterTest.java diff --git a/etc/checkstyle/checkstyle-suppressions.xml b/etc/checkstyle/checkstyle-suppressions.xml index e42d8124ea..23b77adb50 100644 --- a/etc/checkstyle/checkstyle-suppressions.xml +++ b/etc/checkstyle/checkstyle-suppressions.xml @@ -53,4 +53,7 @@ + + + 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 53d56f6af8..c5b75781e7 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.AuthenticationEntryPointFailureHandlerAdapter; import org.springframework.security.web.authentication.AuthenticationFailureHandler; import org.springframework.security.web.authentication.WebAuthenticationDetailsSource; import org.springframework.security.web.context.NullSecurityContextRepository; @@ -73,12 +73,12 @@ 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 AuthenticationEntryPointFailureHandlerAdapter( + (request, response, authException) -> { + // This is a lambda and not a method reference so that the FailureHandler + // reflects entrypoint updates + this.authenticationEntryPoint.commence(request, response, authException); + }); private BearerTokenResolver bearerTokenResolver = new DefaultBearerTokenResolver(); @@ -192,7 +192,10 @@ public class BearerTokenAuthenticationFilter extends OncePerRequestFilter { * Set the {@link AuthenticationEntryPoint} to use. Defaults to * {@link BearerTokenAuthenticationEntryPoint}. * @param authenticationEntryPoint the {@code AuthenticationEntryPoint} to use + * @deprecated use + * {@link BearerTokenAuthenticationFilter#authenticationFailureHandler} instead */ + @Deprecated public void setAuthenticationEntryPoint(final AuthenticationEntryPoint authenticationEntryPoint) { Assert.notNull(authenticationEntryPoint, "authenticationEntryPoint cannot be null"); this.authenticationEntryPoint = authenticationEntryPoint; diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/authentication/BearerTokenAuthenticationFilterTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/authentication/BearerTokenAuthenticationFilterTests.java index 21edffdf36..f52c55fa07 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/authentication/BearerTokenAuthenticationFilterTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/authentication/BearerTokenAuthenticationFilterTests.java @@ -37,12 +37,14 @@ import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.AuthenticationManagerResolver; import org.springframework.security.authentication.AuthenticationServiceException; import org.springframework.security.authentication.TestingAuthenticationToken; +import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.security.core.context.SecurityContextImpl; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.server.resource.BearerTokenError; import org.springframework.security.oauth2.server.resource.BearerTokenErrorCodes; +import org.springframework.security.oauth2.server.resource.InvalidBearerTokenException; import org.springframework.security.oauth2.server.resource.authentication.BearerTokenAuthenticationToken; import org.springframework.security.oauth2.server.resource.web.BearerTokenResolver; import org.springframework.security.web.AuthenticationEntryPoint; @@ -199,6 +201,19 @@ public class BearerTokenAuthenticationFilterTests { .isThrownBy(() -> filter.doFilter(this.request, this.response, this.filterChain)); } + @Test + public void doFilterWhenCustomEntryPointAndAuthenticationErrorThenUses() throws ServletException, IOException { + AuthenticationException exception = new InvalidBearerTokenException("message"); + given(this.bearerTokenResolver.resolve(this.request)).willReturn("token"); + given(this.authenticationManager.authenticate(any())).willThrow(exception); + BearerTokenAuthenticationFilter filter = addMocks( + new BearerTokenAuthenticationFilter(this.authenticationManager)); + AuthenticationEntryPoint entrypoint = mock(AuthenticationEntryPoint.class); + filter.setAuthenticationEntryPoint(entrypoint); + filter.doFilter(this.request, this.response, this.filterChain); + verify(entrypoint).commence(any(), any(), any(InvalidBearerTokenException.class)); + } + @Test public void doFilterWhenCustomAuthenticationDetailsSourceThenUses() throws ServletException, IOException { given(this.bearerTokenResolver.resolve(this.request)).willReturn("token"); 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 0c6040f099..d96307a207 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 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. @@ -31,7 +31,9 @@ import org.springframework.util.Assert; * * @author Sergey Bespalov * @since 5.2.0 + * @deprecated Use {@link AuthenticationEntryPointFailureHandlerAdapter} instead */ +@Deprecated public class AuthenticationEntryPointFailureHandler implements AuthenticationFailureHandler { private final AuthenticationEntryPoint authenticationEntryPoint; diff --git a/web/src/main/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapter.java b/web/src/main/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapter.java new file mode 100644 index 0000000000..c7687fca7a --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapter.java @@ -0,0 +1,56 @@ +/* + * Copyright 2002-2022 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication; + +import java.io.IOException; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.springframework.security.authentication.AuthenticationServiceException; +import org.springframework.security.core.AuthenticationException; +import org.springframework.security.web.AuthenticationEntryPoint; +import org.springframework.util.Assert; + +/** + * Adapts a {@link AuthenticationEntryPoint} into a {@link AuthenticationFailureHandler}. + * When the failure is an {@link AuthenticationServiceException}, it re-throws, to produce + * an HTTP 500 error. + * + * @author Daniel Garnier-Moiroux + * @since 5.8 + */ +public final class AuthenticationEntryPointFailureHandlerAdapter implements AuthenticationFailureHandler { + + private final AuthenticationEntryPoint authenticationEntryPoint; + + public AuthenticationEntryPointFailureHandlerAdapter(AuthenticationEntryPoint authenticationEntryPoint) { + Assert.notNull(authenticationEntryPoint, "authenticationEntryPoint cannot be null"); + this.authenticationEntryPoint = authenticationEntryPoint; + } + + @Override + public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response, + AuthenticationException failure) throws IOException, ServletException { + if (AuthenticationServiceException.class.isAssignableFrom(failure.getClass())) { + throw failure; + } + this.authenticationEntryPoint.commence(request, response, failure); + } + +} 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 04061d91f9..4965969712 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2022 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. @@ -29,7 +29,9 @@ import org.springframework.util.Assert; * * @author Rob Winch * @since 5.0 + * @deprecated use {@link ServerAuthenticationEntryPointFailureHandlerAdapter} instead. */ +@Deprecated public class ServerAuthenticationEntryPointFailureHandler implements ServerAuthenticationFailureHandler { private final ServerAuthenticationEntryPoint authenticationEntryPoint; diff --git a/web/src/main/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapter.java b/web/src/main/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapter.java new file mode 100644 index 0000000000..76fe22de43 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapter.java @@ -0,0 +1,53 @@ +/* + * Copyright 2002-2022 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.server.authentication; + +import reactor.core.publisher.Mono; + +import org.springframework.security.authentication.AuthenticationServiceException; +import org.springframework.security.core.AuthenticationException; +import org.springframework.security.web.server.ServerAuthenticationEntryPoint; +import org.springframework.security.web.server.WebFilterExchange; +import org.springframework.util.Assert; + +/** + * Adapts a {@link ServerAuthenticationEntryPoint} into a + * {@link ServerAuthenticationFailureHandler}. When the failure is an + * {@link AuthenticationServiceException}, it re-throws, to produce an HTTP 500 error. + * + * @author Daniel Garnier-Moiroux + * @since 5.8 + */ +public class ServerAuthenticationEntryPointFailureHandlerAdapter implements ServerAuthenticationFailureHandler { + + private final ServerAuthenticationEntryPoint authenticationEntryPoint; + + public ServerAuthenticationEntryPointFailureHandlerAdapter( + ServerAuthenticationEntryPoint authenticationEntryPoint) { + Assert.notNull(authenticationEntryPoint, "authenticationEntryPoint cannot be null"); + this.authenticationEntryPoint = authenticationEntryPoint; + } + + @Override + public Mono onAuthenticationFailure(WebFilterExchange webFilterExchange, AuthenticationException exception) { + if (AuthenticationServiceException.class.isAssignableFrom(exception.getClass())) { + return Mono.error(exception); + } + return this.authenticationEntryPoint.commence(webFilterExchange.getExchange(), exception); + } + +} diff --git a/web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapterTest.java b/web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapterTest.java new file mode 100644 index 0000000000..0a1947ee58 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapterTest.java @@ -0,0 +1,69 @@ +/* + * Copyright 2002-2022 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication; + +import java.io.IOException; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.junit.jupiter.api.Test; + +import org.springframework.security.authentication.AuthenticationServiceException; +import org.springframework.security.core.AuthenticationException; +import org.springframework.security.web.AuthenticationEntryPoint; + +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; + +/** + * @author Daniel Garnier-Moiroux + * @since 5.8 + */ +class AuthenticationEntryPointFailureHandlerAdapterTest { + + private final AuthenticationEntryPoint authenticationEntryPoint = mock(AuthenticationEntryPoint.class); + + private final HttpServletRequest request = mock(HttpServletRequest.class); + + private final HttpServletResponse response = mock(HttpServletResponse.class); + + @Test + void onAuthenticationFailureThenCommenceAuthentication() throws ServletException, IOException { + AuthenticationEntryPointFailureHandlerAdapter failureHandler = new AuthenticationEntryPointFailureHandlerAdapter( + this.authenticationEntryPoint); + AuthenticationException failure = new AuthenticationException("failed") { + }; + failureHandler.onAuthenticationFailure(this.request, this.response, failure); + verify(this.authenticationEntryPoint).commence(this.request, this.response, failure); + } + + @Test + void onAuthenticationFailureWithAuthenticationServiceExceptionThenRethrows() { + AuthenticationEntryPointFailureHandlerAdapter failureHandler = new AuthenticationEntryPointFailureHandlerAdapter( + this.authenticationEntryPoint); + AuthenticationException failure = new AuthenticationServiceException("failed"); + assertThatExceptionOfType(AuthenticationServiceException.class) + .isThrownBy(() -> failureHandler.onAuthenticationFailure(this.request, this.response, failure)) + .isSameAs(failure); + verifyNoInteractions(this.authenticationEntryPoint); + } + +} diff --git a/web/src/test/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapterTest.java b/web/src/test/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapterTest.java new file mode 100644 index 0000000000..ae331a643e --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapterTest.java @@ -0,0 +1,77 @@ +/* + * Copyright 2002-2022 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.server.authentication; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Mono; + +import org.springframework.security.authentication.AuthenticationServiceException; +import org.springframework.security.core.AuthenticationException; +import org.springframework.security.web.server.ServerAuthenticationEntryPoint; +import org.springframework.security.web.server.WebFilterExchange; +import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.WebFilterChain; + +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; + +/** + * @author Daniel Garnier-Moiroux + * @since 5.8 + */ +class ServerAuthenticationEntryPointFailureHandlerAdapterTest { + + private final ServerAuthenticationEntryPoint serverAuthenticationEntryPoint = mock( + ServerAuthenticationEntryPoint.class); + + private final ServerWebExchange serverWebExchange = mock(ServerWebExchange.class); + + private final WebFilterExchange webFilterExchange = new WebFilterExchange(this.serverWebExchange, + mock(WebFilterChain.class)); + + @BeforeEach + void setUp() { + given(this.serverAuthenticationEntryPoint.commence(any(), any())).willReturn(Mono.empty()); + } + + @Test + void onAuthenticationFailureThenCommenceAuthentication() { + ServerAuthenticationEntryPointFailureHandlerAdapter failureHandler = new ServerAuthenticationEntryPointFailureHandlerAdapter( + this.serverAuthenticationEntryPoint); + AuthenticationException failure = new AuthenticationException("failed") { + }; + failureHandler.onAuthenticationFailure(this.webFilterExchange, failure).block(); + verify(this.serverAuthenticationEntryPoint).commence(this.serverWebExchange, failure); + } + + @Test + void onAuthenticationFailureWithAuthenticationServiceExceptionThenRethrows() { + ServerAuthenticationEntryPointFailureHandlerAdapter failureHandler = new ServerAuthenticationEntryPointFailureHandlerAdapter( + this.serverAuthenticationEntryPoint); + AuthenticationException failure = new AuthenticationServiceException("failed"); + assertThatExceptionOfType(AuthenticationServiceException.class) + .isThrownBy(() -> failureHandler.onAuthenticationFailure(this.webFilterExchange, failure).block()) + .isSameAs(failure); + verifyNoInteractions(this.serverWebExchange); + } + +} From 099aaa33ff3ad4151f338e110f5766547dca8a12 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Thu, 13 Oct 2022 12:07:07 -0600 Subject: [PATCH 2/2] Remove Deprecation Markers Since Spring Security still needs these methods and classes, we should wait on deprecating them if we can. Instead, this commit changes the original classes to have a boolean property that is currently false, but will switch to true in 6.0. At that time, BearerTokenAuthenticationFilter can change to use the handler. Closes gh-11932 --- .../BearerTokenAuthenticationFilter.java | 17 ++-- ...uthenticationEntryPointFailureHandler.java | 25 +++++- ...cationEntryPointFailureHandlerAdapter.java | 56 -------------- ...uthenticationEntryPointFailureHandler.java | 23 +++++- ...cationEntryPointFailureHandlerAdapter.java | 53 ------------- ...onEntryPointFailureHandlerAdapterTest.java | 69 ----------------- ...ticationEntryPointFailureHandlerTests.java | 48 ++++++++++++ ...onEntryPointFailureHandlerAdapterTest.java | 77 ------------------- ...ticationEntryPointFailureHandlerTests.java | 17 ++++ 9 files changed, 114 insertions(+), 271 deletions(-) delete mode 100644 web/src/main/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapter.java delete mode 100644 web/src/main/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapter.java delete mode 100644 web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapterTest.java create mode 100644 web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerTests.java delete mode 100644 web/src/test/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapterTest.java 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 c5b75781e7..53d56f6af8 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,6 +27,7 @@ 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; @@ -39,7 +40,6 @@ 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.AuthenticationEntryPointFailureHandlerAdapter; import org.springframework.security.web.authentication.AuthenticationFailureHandler; import org.springframework.security.web.authentication.WebAuthenticationDetailsSource; import org.springframework.security.web.context.NullSecurityContextRepository; @@ -73,12 +73,12 @@ public class BearerTokenAuthenticationFilter extends OncePerRequestFilter { private AuthenticationEntryPoint authenticationEntryPoint = new BearerTokenAuthenticationEntryPoint(); - private AuthenticationFailureHandler authenticationFailureHandler = new AuthenticationEntryPointFailureHandlerAdapter( - (request, response, authException) -> { - // This is a lambda and not a method reference so that the FailureHandler - // reflects entrypoint updates - this.authenticationEntryPoint.commence(request, response, authException); - }); + private AuthenticationFailureHandler authenticationFailureHandler = (request, response, exception) -> { + if (exception instanceof AuthenticationServiceException) { + throw exception; + } + this.authenticationEntryPoint.commence(request, response, exception); + }; private BearerTokenResolver bearerTokenResolver = new DefaultBearerTokenResolver(); @@ -192,10 +192,7 @@ public class BearerTokenAuthenticationFilter extends OncePerRequestFilter { * Set the {@link AuthenticationEntryPoint} to use. Defaults to * {@link BearerTokenAuthenticationEntryPoint}. * @param authenticationEntryPoint the {@code AuthenticationEntryPoint} to use - * @deprecated use - * {@link BearerTokenAuthenticationFilter#authenticationFailureHandler} instead */ - @Deprecated public void setAuthenticationEntryPoint(final AuthenticationEntryPoint authenticationEntryPoint) { Assert.notNull(authenticationEntryPoint, "authenticationEntryPoint cannot be null"); this.authenticationEntryPoint = authenticationEntryPoint; 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 d96307a207..3cf5cdd574 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 @@ -22,6 +22,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.springframework.security.authentication.AuthenticationServiceException; import org.springframework.security.core.AuthenticationException; import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.util.Assert; @@ -31,11 +32,11 @@ import org.springframework.util.Assert; * * @author Sergey Bespalov * @since 5.2.0 - * @deprecated Use {@link AuthenticationEntryPointFailureHandlerAdapter} instead */ -@Deprecated public class AuthenticationEntryPointFailureHandler implements AuthenticationFailureHandler { + private boolean rethrowAuthenticationServiceException = false; + private final AuthenticationEntryPoint authenticationEntryPoint; public AuthenticationEntryPointFailureHandler(AuthenticationEntryPoint authenticationEntryPoint) { @@ -46,7 +47,25 @@ public class AuthenticationEntryPointFailureHandler implements AuthenticationFai @Override public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response, AuthenticationException exception) throws IOException, ServletException { - this.authenticationEntryPoint.commence(request, response, exception); + if (!this.rethrowAuthenticationServiceException) { + this.authenticationEntryPoint.commence(request, response, exception); + return; + } + if (!AuthenticationServiceException.class.isAssignableFrom(exception.getClass())) { + this.authenticationEntryPoint.commence(request, response, exception); + return; + } + throw exception; + } + + /** + * Set whether to rethrow {@link AuthenticationServiceException}s (defaults to false) + * @param rethrowAuthenticationServiceException whether to rethrow + * {@link AuthenticationServiceException}s + * @since 5.8 + */ + public void setRethrowAuthenticationServiceException(boolean rethrowAuthenticationServiceException) { + this.rethrowAuthenticationServiceException = rethrowAuthenticationServiceException; } } diff --git a/web/src/main/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapter.java b/web/src/main/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapter.java deleted file mode 100644 index c7687fca7a..0000000000 --- a/web/src/main/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapter.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright 2002-2022 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.web.authentication; - -import java.io.IOException; - -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.springframework.security.authentication.AuthenticationServiceException; -import org.springframework.security.core.AuthenticationException; -import org.springframework.security.web.AuthenticationEntryPoint; -import org.springframework.util.Assert; - -/** - * Adapts a {@link AuthenticationEntryPoint} into a {@link AuthenticationFailureHandler}. - * When the failure is an {@link AuthenticationServiceException}, it re-throws, to produce - * an HTTP 500 error. - * - * @author Daniel Garnier-Moiroux - * @since 5.8 - */ -public final class AuthenticationEntryPointFailureHandlerAdapter implements AuthenticationFailureHandler { - - private final AuthenticationEntryPoint authenticationEntryPoint; - - public AuthenticationEntryPointFailureHandlerAdapter(AuthenticationEntryPoint authenticationEntryPoint) { - Assert.notNull(authenticationEntryPoint, "authenticationEntryPoint cannot be null"); - this.authenticationEntryPoint = authenticationEntryPoint; - } - - @Override - public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response, - AuthenticationException failure) throws IOException, ServletException { - if (AuthenticationServiceException.class.isAssignableFrom(failure.getClass())) { - throw failure; - } - this.authenticationEntryPoint.commence(request, response, failure); - } - -} 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 4965969712..74bcc1dbb2 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 @@ -18,6 +18,7 @@ package org.springframework.security.web.server.authentication; import reactor.core.publisher.Mono; +import org.springframework.security.authentication.AuthenticationServiceException; import org.springframework.security.core.AuthenticationException; import org.springframework.security.web.server.ServerAuthenticationEntryPoint; import org.springframework.security.web.server.WebFilterExchange; @@ -29,13 +30,13 @@ import org.springframework.util.Assert; * * @author Rob Winch * @since 5.0 - * @deprecated use {@link ServerAuthenticationEntryPointFailureHandlerAdapter} instead. */ -@Deprecated public class ServerAuthenticationEntryPointFailureHandler implements ServerAuthenticationFailureHandler { private final ServerAuthenticationEntryPoint authenticationEntryPoint; + private boolean rethrowAuthenticationServiceException = false; + public ServerAuthenticationEntryPointFailureHandler(ServerAuthenticationEntryPoint authenticationEntryPoint) { Assert.notNull(authenticationEntryPoint, "authenticationEntryPoint cannot be null"); this.authenticationEntryPoint = authenticationEntryPoint; @@ -43,7 +44,23 @@ public class ServerAuthenticationEntryPointFailureHandler implements ServerAuthe @Override public Mono onAuthenticationFailure(WebFilterExchange webFilterExchange, AuthenticationException exception) { - return this.authenticationEntryPoint.commence(webFilterExchange.getExchange(), exception); + if (!this.rethrowAuthenticationServiceException) { + return this.authenticationEntryPoint.commence(webFilterExchange.getExchange(), exception); + } + if (!AuthenticationServiceException.class.isAssignableFrom(exception.getClass())) { + return this.authenticationEntryPoint.commence(webFilterExchange.getExchange(), exception); + } + return Mono.error(exception); + } + + /** + * Set whether to rethrow {@link AuthenticationServiceException}s (defaults to false) + * @param rethrowAuthenticationServiceException whether to rethrow + * {@link AuthenticationServiceException}s + * @since 5.8 + */ + public void setRethrowAuthenticationServiceException(boolean rethrowAuthenticationServiceException) { + this.rethrowAuthenticationServiceException = rethrowAuthenticationServiceException; } } diff --git a/web/src/main/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapter.java b/web/src/main/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapter.java deleted file mode 100644 index 76fe22de43..0000000000 --- a/web/src/main/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapter.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright 2002-2022 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.web.server.authentication; - -import reactor.core.publisher.Mono; - -import org.springframework.security.authentication.AuthenticationServiceException; -import org.springframework.security.core.AuthenticationException; -import org.springframework.security.web.server.ServerAuthenticationEntryPoint; -import org.springframework.security.web.server.WebFilterExchange; -import org.springframework.util.Assert; - -/** - * Adapts a {@link ServerAuthenticationEntryPoint} into a - * {@link ServerAuthenticationFailureHandler}. When the failure is an - * {@link AuthenticationServiceException}, it re-throws, to produce an HTTP 500 error. - * - * @author Daniel Garnier-Moiroux - * @since 5.8 - */ -public class ServerAuthenticationEntryPointFailureHandlerAdapter implements ServerAuthenticationFailureHandler { - - private final ServerAuthenticationEntryPoint authenticationEntryPoint; - - public ServerAuthenticationEntryPointFailureHandlerAdapter( - ServerAuthenticationEntryPoint authenticationEntryPoint) { - Assert.notNull(authenticationEntryPoint, "authenticationEntryPoint cannot be null"); - this.authenticationEntryPoint = authenticationEntryPoint; - } - - @Override - public Mono onAuthenticationFailure(WebFilterExchange webFilterExchange, AuthenticationException exception) { - if (AuthenticationServiceException.class.isAssignableFrom(exception.getClass())) { - return Mono.error(exception); - } - return this.authenticationEntryPoint.commence(webFilterExchange.getExchange(), exception); - } - -} diff --git a/web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapterTest.java b/web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapterTest.java deleted file mode 100644 index 0a1947ee58..0000000000 --- a/web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerAdapterTest.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Copyright 2002-2022 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.web.authentication; - -import java.io.IOException; - -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.junit.jupiter.api.Test; - -import org.springframework.security.authentication.AuthenticationServiceException; -import org.springframework.security.core.AuthenticationException; -import org.springframework.security.web.AuthenticationEntryPoint; - -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; - -/** - * @author Daniel Garnier-Moiroux - * @since 5.8 - */ -class AuthenticationEntryPointFailureHandlerAdapterTest { - - private final AuthenticationEntryPoint authenticationEntryPoint = mock(AuthenticationEntryPoint.class); - - private final HttpServletRequest request = mock(HttpServletRequest.class); - - private final HttpServletResponse response = mock(HttpServletResponse.class); - - @Test - void onAuthenticationFailureThenCommenceAuthentication() throws ServletException, IOException { - AuthenticationEntryPointFailureHandlerAdapter failureHandler = new AuthenticationEntryPointFailureHandlerAdapter( - this.authenticationEntryPoint); - AuthenticationException failure = new AuthenticationException("failed") { - }; - failureHandler.onAuthenticationFailure(this.request, this.response, failure); - verify(this.authenticationEntryPoint).commence(this.request, this.response, failure); - } - - @Test - void onAuthenticationFailureWithAuthenticationServiceExceptionThenRethrows() { - AuthenticationEntryPointFailureHandlerAdapter failureHandler = new AuthenticationEntryPointFailureHandlerAdapter( - this.authenticationEntryPoint); - AuthenticationException failure = new AuthenticationServiceException("failed"); - assertThatExceptionOfType(AuthenticationServiceException.class) - .isThrownBy(() -> failureHandler.onAuthenticationFailure(this.request, this.response, failure)) - .isSameAs(failure); - verifyNoInteractions(this.authenticationEntryPoint); - } - -} 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 new file mode 100644 index 0000000000..af52e705d1 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/authentication/AuthenticationEntryPointFailureHandlerTests.java @@ -0,0 +1,48 @@ +/* + * Copyright 2002-2022 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication; + +import org.junit.jupiter.api.Test; + +import org.springframework.security.authentication.AuthenticationServiceException; +import org.springframework.security.web.AuthenticationEntryPoint; + +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.Mockito.mock; + +/** + * Tests for {@link AuthenticationEntryPointFailureHandler} + */ +public class AuthenticationEntryPointFailureHandlerTests { + + @Test + void onAuthenticationFailureWhenDefaultsThenAuthenticationServiceExceptionSwallowed() throws Exception { + AuthenticationEntryPoint entryPoint = mock(AuthenticationEntryPoint.class); + AuthenticationEntryPointFailureHandler handler = new AuthenticationEntryPointFailureHandler(entryPoint); + handler.onAuthenticationFailure(null, null, new AuthenticationServiceException("fail")); + } + + @Test + void handleWhenRethrowingThenAuthenticationServiceExceptionRethrown() { + 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/ServerAuthenticationEntryPointFailureHandlerAdapterTest.java b/web/src/test/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapterTest.java deleted file mode 100644 index ae331a643e..0000000000 --- a/web/src/test/java/org/springframework/security/web/server/authentication/ServerAuthenticationEntryPointFailureHandlerAdapterTest.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright 2002-2022 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.web.server.authentication; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import reactor.core.publisher.Mono; - -import org.springframework.security.authentication.AuthenticationServiceException; -import org.springframework.security.core.AuthenticationException; -import org.springframework.security.web.server.ServerAuthenticationEntryPoint; -import org.springframework.security.web.server.WebFilterExchange; -import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.server.WebFilterChain; - -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; - -/** - * @author Daniel Garnier-Moiroux - * @since 5.8 - */ -class ServerAuthenticationEntryPointFailureHandlerAdapterTest { - - private final ServerAuthenticationEntryPoint serverAuthenticationEntryPoint = mock( - ServerAuthenticationEntryPoint.class); - - private final ServerWebExchange serverWebExchange = mock(ServerWebExchange.class); - - private final WebFilterExchange webFilterExchange = new WebFilterExchange(this.serverWebExchange, - mock(WebFilterChain.class)); - - @BeforeEach - void setUp() { - given(this.serverAuthenticationEntryPoint.commence(any(), any())).willReturn(Mono.empty()); - } - - @Test - void onAuthenticationFailureThenCommenceAuthentication() { - ServerAuthenticationEntryPointFailureHandlerAdapter failureHandler = new ServerAuthenticationEntryPointFailureHandlerAdapter( - this.serverAuthenticationEntryPoint); - AuthenticationException failure = new AuthenticationException("failed") { - }; - failureHandler.onAuthenticationFailure(this.webFilterExchange, failure).block(); - verify(this.serverAuthenticationEntryPoint).commence(this.serverWebExchange, failure); - } - - @Test - void onAuthenticationFailureWithAuthenticationServiceExceptionThenRethrows() { - ServerAuthenticationEntryPointFailureHandlerAdapter failureHandler = new ServerAuthenticationEntryPointFailureHandlerAdapter( - this.serverAuthenticationEntryPoint); - AuthenticationException failure = new AuthenticationServiceException("failed"); - assertThatExceptionOfType(AuthenticationServiceException.class) - .isThrownBy(() -> failureHandler.onAuthenticationFailure(this.webFilterExchange, failure).block()) - .isSameAs(failure); - verifyNoInteractions(this.serverWebExchange); - } - -} 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 1ee69267a0..670f476c65 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 @@ -23,6 +23,7 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import reactor.core.publisher.Mono; +import org.springframework.security.authentication.AuthenticationServiceException; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.web.server.ServerAuthenticationEntryPoint; import org.springframework.security.web.server.WebFilterExchange; @@ -30,6 +31,7 @@ import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilterChain; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.mockito.BDDMockito.given; @@ -68,4 +70,19 @@ public class ServerAuthenticationEntryPointFailureHandlerTests { assertThat(this.handler.onAuthenticationFailure(this.filterExchange, e)).isEqualTo(result); } + @Test + void onAuthenticationFailureWhenDefaultsThenAuthenticationServiceExceptionSwallowed() { + AuthenticationServiceException e = new AuthenticationServiceException("fail"); + given(this.authenticationEntryPoint.commence(this.exchange, e)).willReturn(Mono.empty()); + this.handler.onAuthenticationFailure(this.filterExchange, e).block(); + } + + @Test + void handleWhenRethrowingThenAuthenticationServiceExceptionRethrown() { + AuthenticationServiceException e = new AuthenticationServiceException("fail"); + this.handler.setRethrowAuthenticationServiceException(true); + assertThatExceptionOfType(AuthenticationServiceException.class) + .isThrownBy(() -> this.handler.onAuthenticationFailure(this.filterExchange, e).block()); + } + }