Skip to content

Commit 42c18c8

Browse files
jgrandjajgrandja
committed
Fix client_secret_basic authentication failures and return challenge
Closes gh-468
1 parent e83adda commit 42c18c8

File tree

3 files changed

+62
-34
lines changed

3 files changed

+62
-34
lines changed

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

+45-15
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2020-2024 the original author or authors.
2+
* Copyright 2020-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -34,6 +34,7 @@
3434
import org.springframework.security.core.AuthenticationException;
3535
import org.springframework.security.core.context.SecurityContext;
3636
import org.springframework.security.core.context.SecurityContextHolder;
37+
import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
3738
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
3839
import org.springframework.security.oauth2.core.OAuth2Error;
3940
import org.springframework.security.oauth2.core.OAuth2ErrorCodes;
@@ -53,6 +54,7 @@
5354
import org.springframework.security.web.authentication.AuthenticationFailureHandler;
5455
import org.springframework.security.web.authentication.AuthenticationSuccessHandler;
5556
import org.springframework.security.web.authentication.WebAuthenticationDetailsSource;
57+
import org.springframework.security.web.authentication.www.BasicAuthenticationEntryPoint;
5658
import org.springframework.security.web.util.matcher.RequestMatcher;
5759
import org.springframework.util.Assert;
5860
import org.springframework.web.filter.OncePerRequestFilter;
@@ -90,6 +92,8 @@ public final class OAuth2ClientAuthenticationFilter extends OncePerRequestFilter
9092

9193
private final AuthenticationDetailsSource<HttpServletRequest, ?> authenticationDetailsSource = new WebAuthenticationDetailsSource();
9294

95+
private final BasicAuthenticationEntryPoint basicAuthenticationEntryPoint = new BasicAuthenticationEntryPoint();
96+
9397
private AuthenticationConverter authenticationConverter;
9498

9599
private AuthenticationSuccessHandler authenticationSuccessHandler = this::onAuthenticationSuccess;
@@ -110,6 +114,7 @@ public OAuth2ClientAuthenticationFilter(AuthenticationManager authenticationMana
110114
Assert.notNull(requestMatcher, "requestMatcher cannot be null");
111115
this.authenticationManager = authenticationManager;
112116
this.requestMatcher = requestMatcher;
117+
this.basicAuthenticationEntryPoint.setRealmName("default");
113118
// @formatter:off
114119
this.authenticationConverter = new DelegatingAuthenticationConverter(
115120
Arrays.asList(
@@ -130,8 +135,9 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
130135
return;
131136
}
132137

138+
Authentication authenticationRequest = null;
133139
try {
134-
Authentication authenticationRequest = this.authenticationConverter.convert(request);
140+
authenticationRequest = this.authenticationConverter.convert(request);
135141
if (authenticationRequest instanceof AbstractAuthenticationToken) {
136142
((AbstractAuthenticationToken) authenticationRequest)
137143
.setDetails(this.authenticationDetailsSource.buildDetails(request));
@@ -148,7 +154,14 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
148154
if (this.logger.isTraceEnabled()) {
149155
this.logger.trace(LogMessage.format("Client authentication failed: %s", ex.getError()), ex);
150156
}
151-
this.authenticationFailureHandler.onAuthenticationFailure(request, response, ex);
157+
if (authenticationRequest instanceof OAuth2ClientAuthenticationToken clientAuthentication) {
158+
this.authenticationFailureHandler.onAuthenticationFailure(request, response,
159+
new OAuth2ClientAuthenticationException(ex.getError(), ex, clientAuthentication));
160+
}
161+
else {
162+
this.authenticationFailureHandler.onAuthenticationFailure(request, response, ex);
163+
}
164+
152165
}
153166
}
154167

@@ -200,21 +213,21 @@ private void onAuthenticationSuccess(HttpServletRequest request, HttpServletResp
200213
}
201214

202215
private void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response,
203-
AuthenticationException exception) throws IOException {
216+
AuthenticationException authenticationException) throws IOException {
204217

205218
SecurityContextHolder.clearContext();
206219

207-
// TODO
208-
// The authorization server MAY return an HTTP 401 (Unauthorized) status code
209-
// to indicate which HTTP authentication schemes are supported.
210-
// If the client attempted to authenticate via the "Authorization" request header
211-
// field,
212-
// the authorization server MUST respond with an HTTP 401 (Unauthorized) status
213-
// code and
214-
// include the "WWW-Authenticate" response header field
215-
// matching the authentication scheme used by the client.
216-
217-
OAuth2Error error = ((OAuth2AuthenticationException) exception).getError();
220+
if (authenticationException instanceof OAuth2ClientAuthenticationException clientAuthenticationException) {
221+
OAuth2ClientAuthenticationToken clientAuthentication = clientAuthenticationException
222+
.getClientAuthentication();
223+
if (ClientAuthenticationMethod.CLIENT_SECRET_BASIC
224+
.equals(clientAuthentication.getClientAuthenticationMethod())) {
225+
this.basicAuthenticationEntryPoint.commence(request, response, authenticationException);
226+
return;
227+
}
228+
}
229+
230+
OAuth2Error error = ((OAuth2AuthenticationException) authenticationException).getError();
218231
ServletServerHttpResponse httpResponse = new ServletServerHttpResponse(response);
219232
if (OAuth2ErrorCodes.INVALID_CLIENT.equals(error.getErrorCode())) {
220233
httpResponse.setStatusCode(HttpStatus.UNAUTHORIZED);
@@ -249,4 +262,21 @@ private static void validateClientIdentifier(Authentication authentication) {
249262
}
250263
}
251264

265+
private static final class OAuth2ClientAuthenticationException extends OAuth2AuthenticationException {
266+
267+
private final OAuth2ClientAuthenticationToken clientAuthentication;
268+
269+
private OAuth2ClientAuthenticationException(OAuth2Error error, Throwable cause,
270+
OAuth2ClientAuthenticationToken clientAuthentication) {
271+
super(error, cause);
272+
Assert.notNull(clientAuthentication, "clientAuthentication cannot be null");
273+
this.clientAuthentication = clientAuthentication;
274+
}
275+
276+
private OAuth2ClientAuthenticationToken getClientAuthentication() {
277+
return this.clientAuthentication;
278+
}
279+
280+
}
281+
252282
}

‎oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2020-2024 the original author or authors.
2+
* Copyright 2020-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -538,7 +538,7 @@ public void requestWhenPublicClientWithPkceAndEmptyCodeThenBadRequest() throws E
538538
}
539539

540540
@Test
541-
public void requestWhenConfidentialClientWithPkceAndMissingCodeVerifierThenBadRequest() throws Exception {
541+
public void requestWhenConfidentialClientWithPkceAndMissingCodeVerifierThenUnauthorized() throws Exception {
542542
this.spring.register(AuthorizationServerConfiguration.class).autowire();
543543

544544
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
@@ -569,7 +569,7 @@ public void requestWhenConfidentialClientWithPkceAndMissingCodeVerifierThenBadRe
569569
.params(getTokenRequestParameters(registeredClient, authorizationCodeAuthorization))
570570
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId())
571571
.header(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(registeredClient)))
572-
.andExpect(status().isBadRequest());
572+
.andExpect(status().isUnauthorized());
573573
}
574574

575575
// gh-1011
@@ -601,7 +601,7 @@ public void requestWhenConfidentialClientWithPkceAndMissingCodeChallengeThenErro
601601
}
602602

603603
@Test
604-
public void requestWhenConfidentialClientWithPkceAndMissingCodeChallengeButCodeVerifierProvidedThenBadRequest()
604+
public void requestWhenConfidentialClientWithPkceAndMissingCodeChallengeButCodeVerifierProvidedThenUnauthorized()
605605
throws Exception {
606606
this.spring.register(AuthorizationServerConfiguration.class).autowire();
607607

@@ -631,7 +631,7 @@ public void requestWhenConfidentialClientWithPkceAndMissingCodeChallengeButCodeV
631631
.params(getTokenRequestParameters(registeredClient, authorizationCodeAuthorization))
632632
.param(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER)
633633
.header(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(registeredClient)))
634-
.andExpect(status().isBadRequest());
634+
.andExpect(status().isUnauthorized());
635635
}
636636

637637
@Test

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

+12-14
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2020-2022 the original author or authors.
2+
* Copyright 2020-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -26,6 +26,7 @@
2626
import org.junit.jupiter.api.Test;
2727
import org.mockito.ArgumentCaptor;
2828

29+
import org.springframework.http.HttpHeaders;
2930
import org.springframework.http.HttpMethod;
3031
import org.springframework.http.HttpStatus;
3132
import org.springframework.http.converter.HttpMessageConverter;
@@ -175,26 +176,25 @@ public void doFilterWhenRequestMatchesAndInvalidCredentialsThenInvalidRequestErr
175176

176177
// gh-889
177178
@Test
178-
public void doFilterWhenRequestMatchesAndClientIdContainsNonPrintableASCIIThenInvalidRequestError()
179-
throws Exception {
179+
public void doFilterWhenRequestMatchesAndClientIdContainsNonPrintableASCIIThenReturnChallenge() throws Exception {
180180
// Hex 00 -> null
181181
String clientId = new String(Hex.decode("00"), StandardCharsets.UTF_8);
182-
assertWhenInvalidClientIdThenInvalidRequestError(clientId);
182+
assertWhenInvalidClientIdThenReturnChallenge(clientId);
183183

184184
// Hex 0a61 -> line feed + a
185185
clientId = new String(Hex.decode("0a61"), StandardCharsets.UTF_8);
186-
assertWhenInvalidClientIdThenInvalidRequestError(clientId);
186+
assertWhenInvalidClientIdThenReturnChallenge(clientId);
187187

188188
// Hex 1b -> escape
189189
clientId = new String(Hex.decode("1b"), StandardCharsets.UTF_8);
190-
assertWhenInvalidClientIdThenInvalidRequestError(clientId);
190+
assertWhenInvalidClientIdThenReturnChallenge(clientId);
191191

192192
// Hex 1b61 -> escape + a
193193
clientId = new String(Hex.decode("1b61"), StandardCharsets.UTF_8);
194-
assertWhenInvalidClientIdThenInvalidRequestError(clientId);
194+
assertWhenInvalidClientIdThenReturnChallenge(clientId);
195195
}
196196

197-
private void assertWhenInvalidClientIdThenInvalidRequestError(String clientId) throws Exception {
197+
private void assertWhenInvalidClientIdThenReturnChallenge(String clientId) throws Exception {
198198
given(this.authenticationConverter.convert(any(HttpServletRequest.class)))
199199
.willReturn(new OAuth2ClientAuthenticationToken(clientId, ClientAuthenticationMethod.CLIENT_SECRET_BASIC,
200200
"secret", null));
@@ -210,13 +210,12 @@ private void assertWhenInvalidClientIdThenInvalidRequestError(String clientId) t
210210
verifyNoInteractions(this.authenticationManager);
211211

212212
assertThat(SecurityContextHolder.getContext().getAuthentication()).isNull();
213-
assertThat(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value());
214-
OAuth2Error error = readError(response);
215-
assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_REQUEST);
213+
assertThat(response.getStatus()).isEqualTo(HttpStatus.UNAUTHORIZED.value());
214+
assertThat(response.getHeader(HttpHeaders.WWW_AUTHENTICATE)).isEqualTo("Basic realm=\"default\"");
216215
}
217216

218217
@Test
219-
public void doFilterWhenRequestMatchesAndBadCredentialsThenInvalidClientError() throws Exception {
218+
public void doFilterWhenRequestMatchesAndBadCredentialsThenReturnChallenge() throws Exception {
220219
given(this.authenticationConverter.convert(any(HttpServletRequest.class)))
221220
.willReturn(new OAuth2ClientAuthenticationToken("clientId", ClientAuthenticationMethod.CLIENT_SECRET_BASIC,
222221
"invalid-secret", null));
@@ -235,8 +234,7 @@ public void doFilterWhenRequestMatchesAndBadCredentialsThenInvalidClientError()
235234

236235
assertThat(SecurityContextHolder.getContext().getAuthentication()).isNull();
237236
assertThat(response.getStatus()).isEqualTo(HttpStatus.UNAUTHORIZED.value());
238-
OAuth2Error error = readError(response);
239-
assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_CLIENT);
237+
assertThat(response.getHeader(HttpHeaders.WWW_AUTHENTICATE)).isEqualTo("Basic realm=\"default\"");
240238
}
241239

242240
@Test

0 commit comments

Comments
 (0)