Selaa lähdekoodia

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.
Daniel Garnier-Moiroux 2 vuotta sitten
vanhempi
commit
200b7fecd3

+ 3 - 0
etc/checkstyle/checkstyle-suppressions.xml

@@ -53,4 +53,7 @@
 	<suppress files="WithSecurityContextTestExecutionListenerTests\.java" checks="SpringMethodVisibility"/>
 	<suppress files="AbstractOAuth2AuthorizationGrantRequestEntityConverter\.java" checks="SpringMethodVisibility"/>
 	<suppress files="JoseHeader\.java" checks="SpringMethodVisibility"/>
+
+	<!-- Lambdas that we can't replace with a method reference because a closure is required -->
+	<suppress files="BearerTokenAuthenticationFilter\.java" checks="SpringLambda"/>
 </suppressions>

+ 10 - 7
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;

+ 15 - 0
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");

+ 3 - 1
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;

+ 56 - 0
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);
+	}
+
+}

+ 3 - 1
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;

+ 53 - 0
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<Void> onAuthenticationFailure(WebFilterExchange webFilterExchange, AuthenticationException exception) {
+		if (AuthenticationServiceException.class.isAssignableFrom(exception.getClass())) {
+			return Mono.error(exception);
+		}
+		return this.authenticationEntryPoint.commence(webFilterExchange.getExchange(), exception);
+	}
+
+}

+ 69 - 0
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);
+	}
+
+}

+ 77 - 0
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);
+	}
+
+}