Skip to content

Commit 8c08d7b

Browse files
jgrandjajgrandja
committed
Polish gh-1384 
1 parent 96c90dd commit 8c08d7b

File tree

5 files changed

+59
-56
lines changed

5 files changed

+59
-56
lines changed

‎docs/modules/ROOT/pages/protocol-endpoints.adoc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h
167167
* `*AuthenticationConverter*` -- An `OAuth2DeviceAuthorizationRequestAuthenticationConverter`.
168168
* `*AuthenticationManager*` -- An `AuthenticationManager` composed of `OAuth2DeviceAuthorizationRequestAuthenticationProvider`.
169169
* `*AuthenticationSuccessHandler*` -- An internal implementation that handles an "`authenticated`" `OAuth2DeviceAuthorizationRequestAuthenticationToken` and returns the `OAuth2DeviceAuthorizationResponse`.
170-
* `*AuthenticationFailureHandler*` -- An `OAuth2ErrorAuthenticationFailureHandler` instance that handles the `OAuth2Error` associated with the `OAuth2AuthenticationException` and returns the `OAuth2Error` response.
170+
* `*AuthenticationFailureHandler*` -- An `OAuth2ErrorAuthenticationFailureHandler`.
171171

172172
[[oauth2-device-verification-endpoint]]
173173
== OAuth2 Device Verification Endpoint
@@ -264,7 +264,7 @@ The supported https://datatracker.ietf.org/doc/html/rfc6749#section-1.3[authoriz
264264
* `*AuthenticationConverter*` -- A `DelegatingAuthenticationConverter` composed of `OAuth2AuthorizationCodeAuthenticationConverter`, `OAuth2RefreshTokenAuthenticationConverter`, `OAuth2ClientCredentialsAuthenticationConverter`, and `OAuth2DeviceCodeAuthenticationConverter`.
265265
* `*AuthenticationManager*` -- An `AuthenticationManager` composed of `OAuth2AuthorizationCodeAuthenticationProvider`, `OAuth2RefreshTokenAuthenticationProvider`, `OAuth2ClientCredentialsAuthenticationProvider`, and `OAuth2DeviceCodeAuthenticationProvider`.
266266
* `*AuthenticationSuccessHandler*` -- An internal implementation that handles an `OAuth2AccessTokenAuthenticationToken` and returns the `OAuth2AccessTokenResponse`.
267-
* `*AuthenticationFailureHandler*` -- An `OAuth2ErrorAuthenticationFailureHandler` instance that handles the `OAuth2Error` associated with the `OAuth2AuthenticationException` and returns the `OAuth2Error` response.
267+
* `*AuthenticationFailureHandler*` -- An `OAuth2ErrorAuthenticationFailureHandler`.
268268

269269
[[oauth2-token-introspection-endpoint]]
270270
== OAuth2 Token Introspection Endpoint
@@ -311,7 +311,7 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h
311311
* `*AuthenticationConverter*` -- An `OAuth2TokenIntrospectionAuthenticationConverter`.
312312
* `*AuthenticationManager*` -- An `AuthenticationManager` composed of `OAuth2TokenIntrospectionAuthenticationProvider`.
313313
* `*AuthenticationSuccessHandler*` -- An internal implementation that handles an "`authenticated`" `OAuth2TokenIntrospectionAuthenticationToken` and returns the `OAuth2TokenIntrospection` response.
314-
* `*AuthenticationFailureHandler*` -- An `OAuth2ErrorAuthenticationFailureHandler` instance that handles the `OAuth2Error` associated with the `OAuth2AuthenticationException` and returns the `OAuth2Error` response.
314+
* `*AuthenticationFailureHandler*` -- An `OAuth2ErrorAuthenticationFailureHandler`.
315315

316316
[[oauth2-token-revocation-endpoint]]
317317
== OAuth2 Token Revocation Endpoint
@@ -358,7 +358,7 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h
358358
* `*AuthenticationConverter*` -- An `OAuth2TokenRevocationAuthenticationConverter`.
359359
* `*AuthenticationManager*` -- An `AuthenticationManager` composed of `OAuth2TokenRevocationAuthenticationProvider`.
360360
* `*AuthenticationSuccessHandler*` -- An internal implementation that handles an "`authenticated`" `OAuth2TokenRevocationAuthenticationToken` and returns the OAuth2 revocation response.
361-
* `*AuthenticationFailureHandler*` -- An `OAuth2ErrorAuthenticationFailureHandler` instance that handles the `OAuth2Error` associated with the `OAuth2AuthenticationException` and returns the `OAuth2Error` response.
361+
* `*AuthenticationFailureHandler*` -- An `OAuth2ErrorAuthenticationFailureHandler`.
362362

363363
[[oauth2-authorization-server-metadata-endpoint]]
364364
== OAuth2 Authorization Server Metadata Endpoint

‎oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceAuthorizationEndpointFilter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@
4141
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2DeviceAuthorizationRequestAuthenticationProvider;
4242
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2DeviceAuthorizationRequestAuthenticationToken;
4343
import org.springframework.security.oauth2.server.authorization.context.AuthorizationServerContextHolder;
44-
import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2ErrorAuthenticationFailureHandler;
4544
import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2DeviceAuthorizationRequestAuthenticationConverter;
45+
import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2ErrorAuthenticationFailureHandler;
4646
import org.springframework.security.web.authentication.AuthenticationConverter;
4747
import org.springframework.security.web.authentication.AuthenticationFailureHandler;
4848
import org.springframework.security.web.authentication.AuthenticationSuccessHandler;

‎oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2TokenEndpointFilter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@
4848
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientCredentialsAuthenticationProvider;
4949
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2DeviceCodeAuthenticationProvider;
5050
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2RefreshTokenAuthenticationProvider;
51-
import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2ErrorAuthenticationFailureHandler;
5251
import org.springframework.security.oauth2.server.authorization.web.authentication.DelegatingAuthenticationConverter;
5352
import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2AuthorizationCodeAuthenticationConverter;
5453
import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2ClientCredentialsAuthenticationConverter;
5554
import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2DeviceCodeAuthenticationConverter;
55+
import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2ErrorAuthenticationFailureHandler;
5656
import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2RefreshTokenAuthenticationConverter;
5757
import org.springframework.security.web.authentication.AuthenticationConverter;
5858
import org.springframework.security.web.authentication.AuthenticationFailureHandler;

‎oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2ErrorAuthenticationFailureHandler.java

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@
2020
import jakarta.servlet.ServletException;
2121
import jakarta.servlet.http.HttpServletRequest;
2222
import jakarta.servlet.http.HttpServletResponse;
23+
2324
import org.apache.commons.logging.Log;
2425
import org.apache.commons.logging.LogFactory;
25-
import org.springframework.core.log.LogMessage;
26+
2627
import org.springframework.http.HttpStatus;
2728
import org.springframework.http.converter.HttpMessageConverter;
2829
import org.springframework.http.server.ServletServerHttpResponse;
@@ -31,42 +32,47 @@
3132
import org.springframework.security.oauth2.core.OAuth2Error;
3233
import org.springframework.security.oauth2.core.http.converter.OAuth2ErrorHttpMessageConverter;
3334
import org.springframework.security.web.authentication.AuthenticationFailureHandler;
35+
import org.springframework.util.Assert;
3436

3537
/**
36-
* A default implementation of an {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException}
37-
* and returning the {@link OAuth2Error Error Response}.
38+
* An implementation of an {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException}
39+
* and returning the {@link OAuth2Error OAuth 2.0 Error Response}.
3840
*
3941
* @author Dmitriy Dubson
4042
* @see AuthenticationFailureHandler
41-
* @since 1.2.0
43+
* @see OAuth2ErrorHttpMessageConverter
44+
* @since 1.2
4245
*/
4346
public final class OAuth2ErrorAuthenticationFailureHandler implements AuthenticationFailureHandler {
44-
4547
private final Log logger = LogFactory.getLog(getClass());
46-
47-
private HttpMessageConverter<OAuth2Error> errorHttpResponseConverter = new OAuth2ErrorHttpMessageConverter();
48+
private HttpMessageConverter<OAuth2Error> errorResponseConverter = new OAuth2ErrorHttpMessageConverter();
4849

4950
@Override
50-
public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response, AuthenticationException authenticationException) throws IOException, ServletException {
51+
public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response,
52+
AuthenticationException authenticationException) throws IOException, ServletException {
5153
ServletServerHttpResponse httpResponse = new ServletServerHttpResponse(response);
5254
httpResponse.setStatusCode(HttpStatus.BAD_REQUEST);
5355

5456
if (authenticationException instanceof OAuth2AuthenticationException) {
5557
OAuth2Error error = ((OAuth2AuthenticationException) authenticationException).getError();
56-
this.errorHttpResponseConverter.write(error, null, httpResponse);
58+
this.errorResponseConverter.write(error, null, httpResponse);
5759
} else {
5860
if (this.logger.isWarnEnabled()) {
59-
this.logger.warn(LogMessage.format("Authentication exception must be of type 'org.springframework.security.oauth2.core.OAuth2AuthenticationException'. Provided exception was '%s'", authenticationException.getClass().getName()));
61+
this.logger.warn(AuthenticationException.class.getSimpleName() + " must be of type " +
62+
OAuth2AuthenticationException.class.getName() +
63+
" but was " + authenticationException.getClass().getName());
6064
}
6165
}
6266
}
6367

6468
/**
65-
* Sets OAuth error HTTP message converter to write to upon authentication failure
69+
* Sets the {@link HttpMessageConverter} used for converting an {@link OAuth2Error} to an HTTP response.
6670
*
67-
* @param errorHttpResponseConverter the error HTTP message converter to set
71+
* @param errorResponseConverter the {@link HttpMessageConverter} used for converting an {@link OAuth2Error} to an HTTP response
6872
*/
69-
public void setErrorHttpResponseConverter(HttpMessageConverter<OAuth2Error> errorHttpResponseConverter) {
70-
this.errorHttpResponseConverter = errorHttpResponseConverter;
73+
public void setErrorResponseConverter(HttpMessageConverter<OAuth2Error> errorResponseConverter) {
74+
Assert.notNull(errorResponseConverter, "errorResponseConverter cannot be null");
75+
this.errorResponseConverter = errorResponseConverter;
7176
}
77+
7278
}

‎oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2ErrorAuthenticationFailureHandlerTests.java

Lines changed: 33 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -15,75 +15,72 @@
1515
*/
1616
package org.springframework.security.oauth2.server.authorization.web.authentication;
1717

18-
import java.io.IOException;
19-
20-
import jakarta.servlet.ServletException;
21-
import jakarta.servlet.http.HttpServletRequest;
22-
import jakarta.servlet.http.HttpServletResponse;
23-
import org.junit.jupiter.api.BeforeEach;
2418
import org.junit.jupiter.api.Test;
19+
2520
import org.springframework.http.HttpStatus;
2621
import org.springframework.http.converter.HttpMessageConverter;
27-
import org.springframework.http.server.ServletServerHttpResponse;
2822
import org.springframework.mock.web.MockHttpServletRequest;
2923
import org.springframework.mock.web.MockHttpServletResponse;
3024
import org.springframework.security.authentication.BadCredentialsException;
3125
import org.springframework.security.core.AuthenticationException;
3226
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
3327
import org.springframework.security.oauth2.core.OAuth2Error;
3428
import org.springframework.security.oauth2.core.OAuth2ErrorCodes;
29+
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
3530

3631
import static org.assertj.core.api.Assertions.assertThat;
37-
import static org.mockito.ArgumentMatchers.any;
38-
import static org.mockito.ArgumentMatchers.eq;
39-
import static org.mockito.ArgumentMatchers.isNull;
32+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
4033
import static org.mockito.Mockito.mock;
41-
import static org.mockito.Mockito.verify;
4234
import static org.mockito.Mockito.verifyNoInteractions;
4335

4436
/**
45-
* Tests for {@link OAuth2ErrorAuthenticationFailureHandler}
37+
* Tests for {@link OAuth2ErrorAuthenticationFailureHandler}.
4638
*
4739
* @author Dmitriy Dubson
4840
*/
4941
public class OAuth2ErrorAuthenticationFailureHandlerTests {
42+
private final OAuth2ErrorAuthenticationFailureHandler authenticationFailureHandler = new OAuth2ErrorAuthenticationFailureHandler();
5043

51-
private HttpMessageConverter<OAuth2Error> errorHttpMessageConverter;
52-
53-
private HttpServletRequest request;
54-
55-
private HttpServletResponse response;
56-
57-
@BeforeEach
58-
@SuppressWarnings("unchecked")
59-
public void setUp() {
60-
errorHttpMessageConverter = (HttpMessageConverter<OAuth2Error>) mock(HttpMessageConverter.class);
61-
request = new MockHttpServletRequest();
62-
response = new MockHttpServletResponse();
44+
@Test
45+
public void setErrorResponseConverterWhenNullThenThrowIllegalArgumentException() {
46+
// @formatter:off
47+
assertThatThrownBy(() -> this.authenticationFailureHandler.setErrorResponseConverter(null))
48+
.isInstanceOf(IllegalArgumentException.class)
49+
.hasMessage("errorResponseConverter cannot be null");
50+
// @formatter:on
6351
}
6452

6553
@Test
66-
public void onAuthenticationFailure() throws IOException, ServletException {
67-
OAuth2Error invalidRequestError = new OAuth2Error(OAuth2ErrorCodes.INVALID_REQUEST);
68-
AuthenticationException authenticationException = new OAuth2AuthenticationException(invalidRequestError);
69-
OAuth2ErrorAuthenticationFailureHandler handler = new OAuth2ErrorAuthenticationFailureHandler();
70-
handler.setErrorHttpResponseConverter(errorHttpMessageConverter);
54+
public void onAuthenticationFailureWhenValidExceptionThenErrorResponse() throws Exception {
55+
MockHttpServletRequest request = new MockHttpServletRequest();
56+
MockHttpServletResponse response = new MockHttpServletResponse();
57+
OAuth2Error error = new OAuth2Error(OAuth2ErrorCodes.INVALID_REQUEST, "error description", "error uri");
58+
AuthenticationException authenticationException = new OAuth2AuthenticationException(error);
7159

72-
handler.onAuthenticationFailure(request, response, authenticationException);
60+
this.authenticationFailureHandler.onAuthenticationFailure(request, response, authenticationException);
7361

74-
verify(errorHttpMessageConverter).write(eq(invalidRequestError), isNull(), any(ServletServerHttpResponse.class));
7562
assertThat(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value());
63+
assertThat(response.getContentAsString()).contains("invalid_request");
64+
assertThat(response.getContentAsString()).contains("error description");
65+
assertThat(response.getContentAsString()).contains("error uri");
7666
}
7767

7868
@Test
79-
public void onAuthenticationFailure_ifExceptionProvidedIsNotOAuth2AuthenticationException() throws ServletException, IOException {
80-
OAuth2ErrorAuthenticationFailureHandler handler = new OAuth2ErrorAuthenticationFailureHandler();
81-
handler.setErrorHttpResponseConverter(errorHttpMessageConverter);
69+
public void onAuthenticationFailureWhenInvalidExceptionThenStatusResponse() throws Exception {
70+
MockHttpServletRequest request = new MockHttpServletRequest();
71+
MockHttpServletResponse response = new MockHttpServletResponse();
72+
AuthenticationException authenticationException = new BadCredentialsException("Not a valid exception.");
73+
74+
HttpMessageConverter<OAuth2Error> errorResponseConverter = mock(HttpMessageConverter.class);
75+
this.authenticationFailureHandler.setErrorResponseConverter(errorResponseConverter);
8276

83-
handler.onAuthenticationFailure(request, response, new BadCredentialsException("Not a valid exception."));
77+
this.authenticationFailureHandler.onAuthenticationFailure(request, response, authenticationException);
8478

85-
verifyNoInteractions(errorHttpMessageConverter);
79+
verifyNoInteractions(errorResponseConverter);
8680
assertThat(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value());
81+
assertThat(response.getContentAsString()).doesNotContain(OAuth2ParameterNames.ERROR);
82+
assertThat(response.getContentAsString()).doesNotContain(OAuth2ParameterNames.ERROR_DESCRIPTION);
83+
assertThat(response.getContentAsString()).doesNotContain(OAuth2ParameterNames.ERROR_URI);
8784
}
8885

8986
}

0 commit comments

Comments
 (0)