Skip to content

Commit acf56f2

Browse files
jgrandjajgrandja
committed
OAuth2AuthorizationCodeGrantWebFilter should handle OAuth2AuthorizationException
Fixes gh-8609
1 parent 7a7707b commit acf56f2

File tree

6 files changed

+110
-59
lines changed

6 files changed

+110
-59
lines changed

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@
2020
import org.springframework.security.core.AuthenticationException;
2121
import org.springframework.security.oauth2.client.endpoint.OAuth2AccessTokenResponseClient;
2222
import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest;
23+
import org.springframework.security.oauth2.core.OAuth2AuthorizationException;
24+
import org.springframework.security.oauth2.core.OAuth2Error;
2325
import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse;
26+
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
27+
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponse;
2428
import org.springframework.util.Assert;
2529

2630
/**
@@ -40,6 +44,7 @@
4044
* @see <a target="_blank" href="https://tools.ietf.org/html/rfc6749#section-4.1.4">Section 4.1.4 Access Token Response</a>
4145
*/
4246
public class OAuth2AuthorizationCodeAuthenticationProvider implements AuthenticationProvider {
47+
private static final String INVALID_STATE_PARAMETER_ERROR_CODE = "invalid_state_parameter";
4348
private final OAuth2AccessTokenResponseClient<OAuth2AuthorizationCodeGrantRequest> accessTokenResponseClient;
4449

4550
/**
@@ -59,8 +64,18 @@ public Authentication authenticate(Authentication authentication) throws Authent
5964
OAuth2AuthorizationCodeAuthenticationToken authorizationCodeAuthentication =
6065
(OAuth2AuthorizationCodeAuthenticationToken) authentication;
6166

62-
OAuth2AuthorizationExchangeValidator.validate(
63-
authorizationCodeAuthentication.getAuthorizationExchange());
67+
OAuth2AuthorizationResponse authorizationResponse = authorizationCodeAuthentication
68+
.getAuthorizationExchange().getAuthorizationResponse();
69+
if (authorizationResponse.statusError()) {
70+
throw new OAuth2AuthorizationException(authorizationResponse.getError());
71+
}
72+
73+
OAuth2AuthorizationRequest authorizationRequest = authorizationCodeAuthentication
74+
.getAuthorizationExchange().getAuthorizationRequest();
75+
if (!authorizationResponse.getState().equals(authorizationRequest.getState())) {
76+
OAuth2Error oauth2Error = new OAuth2Error(INVALID_STATE_PARAMETER_ERROR_CODE);
77+
throw new OAuth2AuthorizationException(oauth2Error);
78+
}
6479

6580
OAuth2AccessTokenResponse accessTokenResponse =
6681
this.accessTokenResponseClient.getTokenResponse(

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2AuthorizationCodeReactiveAuthenticationManager.java

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2020 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.
@@ -22,9 +22,13 @@
2222
import org.springframework.security.oauth2.client.registration.ClientRegistration;
2323
import org.springframework.security.oauth2.client.userinfo.ReactiveOAuth2UserService;
2424
import org.springframework.security.oauth2.core.OAuth2AccessToken;
25+
import org.springframework.security.oauth2.core.OAuth2AuthorizationException;
26+
import org.springframework.security.oauth2.core.OAuth2Error;
2527
import org.springframework.security.oauth2.core.OAuth2RefreshToken;
2628
import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse;
2729
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationExchange;
30+
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
31+
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponse;
2832
import org.springframework.security.oauth2.core.user.OAuth2User;
2933
import org.springframework.util.Assert;
3034
import reactor.core.publisher.Mono;
@@ -55,8 +59,8 @@
5559
* @see <a target="_blank" href="https://tools.ietf.org/html/rfc6749#section-4.1.3">Section 4.1.3 Access Token Request</a>
5660
* @see <a target="_blank" href="https://tools.ietf.org/html/rfc6749#section-4.1.4">Section 4.1.4 Access Token Response</a>
5761
*/
58-
public class OAuth2AuthorizationCodeReactiveAuthenticationManager implements
59-
ReactiveAuthenticationManager {
62+
public class OAuth2AuthorizationCodeReactiveAuthenticationManager implements ReactiveAuthenticationManager {
63+
private static final String INVALID_STATE_PARAMETER_ERROR_CODE = "invalid_state_parameter";
6064
private final ReactiveOAuth2AccessTokenResponseClient<OAuth2AuthorizationCodeGrantRequest> accessTokenResponseClient;
6165

6266
public OAuth2AuthorizationCodeReactiveAuthenticationManager(
@@ -70,7 +74,16 @@ public Mono<Authentication> authenticate(Authentication authentication) {
7074
return Mono.defer(() -> {
7175
OAuth2AuthorizationCodeAuthenticationToken token = (OAuth2AuthorizationCodeAuthenticationToken) authentication;
7276

73-
OAuth2AuthorizationExchangeValidator.validate(token.getAuthorizationExchange());
77+
OAuth2AuthorizationResponse authorizationResponse = token.getAuthorizationExchange().getAuthorizationResponse();
78+
if (authorizationResponse.statusError()) {
79+
return Mono.error(new OAuth2AuthorizationException(authorizationResponse.getError()));
80+
}
81+
82+
OAuth2AuthorizationRequest authorizationRequest = token.getAuthorizationExchange().getAuthorizationRequest();
83+
if (!authorizationResponse.getState().equals(authorizationRequest.getState())) {
84+
OAuth2Error oauth2Error = new OAuth2Error(INVALID_STATE_PARAMETER_ERROR_CODE);
85+
return Mono.error(new OAuth2AuthorizationException(oauth2Error));
86+
}
7487

7588
OAuth2AuthorizationCodeGrantRequest authzRequest = new OAuth2AuthorizationCodeGrantRequest(
7689
token.getClientRegistration(),

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2AuthorizationExchangeValidator.java

Lines changed: 0 additions & 47 deletions
This file was deleted.

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/OAuth2AuthorizationCodeGrantWebFilter.java

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import org.springframework.security.oauth2.client.authentication.OAuth2AuthorizationCodeAuthenticationToken;
2828
import org.springframework.security.oauth2.client.registration.ReactiveClientRegistrationRepository;
2929
import org.springframework.security.oauth2.client.web.AuthorizationRequestRepository;
30+
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
31+
import org.springframework.security.oauth2.core.OAuth2AuthorizationException;
3032
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
3133
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponse;
3234
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
@@ -173,15 +175,21 @@ private void updateDefaultAuthenticationConverter() {
173175
public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
174176
return this.requiresAuthenticationMatcher.matches(exchange)
175177
.filter(ServerWebExchangeMatcher.MatchResult::isMatch)
176-
.flatMap(matchResult -> this.authenticationConverter.convert(exchange))
178+
.flatMap(matchResult ->
179+
this.authenticationConverter.convert(exchange)
180+
.onErrorMap(OAuth2AuthorizationException.class, e -> new OAuth2AuthenticationException(
181+
e.getError(), e.getError().toString())))
177182
.switchIfEmpty(chain.filter(exchange).then(Mono.empty()))
178-
.flatMap(token -> authenticate(exchange, chain, token));
183+
.flatMap(token -> authenticate(exchange, chain, token))
184+
.onErrorResume(AuthenticationException.class, e -> this.authenticationFailureHandler
185+
.onAuthenticationFailure(new WebFilterExchange(exchange, chain), e));
179186
}
180187

181-
private Mono<Void> authenticate(ServerWebExchange exchange,
182-
WebFilterChain chain, Authentication token) {
188+
private Mono<Void> authenticate(ServerWebExchange exchange, WebFilterChain chain, Authentication token) {
183189
WebFilterExchange webFilterExchange = new WebFilterExchange(exchange, chain);
184190
return this.authenticationManager.authenticate(token)
191+
.onErrorMap(OAuth2AuthorizationException.class, e -> new OAuth2AuthenticationException(
192+
e.getError(), e.getError().toString()))
185193
.switchIfEmpty(Mono.defer(() -> Mono.error(new IllegalStateException("No provider found for " + token.getClass()))))
186194
.flatMap(authentication -> onAuthenticationSuccess(authentication, webFilterExchange))
187195
.onErrorResume(AuthenticationException.class, e -> this.authenticationFailureHandler

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/ServerOAuth2AuthorizationCodeAuthenticationTokenConverter.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import org.springframework.security.core.Authentication;
2020
import org.springframework.security.oauth2.client.authentication.OAuth2AuthorizationCodeAuthenticationToken;
21-
import org.springframework.security.oauth2.client.authentication.OAuth2LoginAuthenticationToken;
2221
import org.springframework.security.oauth2.client.registration.ReactiveClientRegistrationRepository;
2322
import org.springframework.security.oauth2.core.OAuth2AuthorizationException;
2423
import org.springframework.security.oauth2.core.OAuth2Error;
@@ -33,7 +32,7 @@
3332
import reactor.core.publisher.Mono;
3433

3534
/**
36-
* Converts from a {@link ServerWebExchange} to an {@link OAuth2LoginAuthenticationToken} that can be authenticated. The
35+
* Converts from a {@link ServerWebExchange} to an {@link OAuth2AuthorizationCodeAuthenticationToken} that can be authenticated. The
3736
* converter does not validate any errors it only performs a conversion.
3837
* @author Rob Winch
3938
* @since 5.1

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/OAuth2AuthorizationCodeGrantWebFilterTests.java

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
import org.springframework.security.oauth2.client.registration.ClientRegistration;
3030
import org.springframework.security.oauth2.client.registration.ReactiveClientRegistrationRepository;
3131
import org.springframework.security.oauth2.client.registration.TestClientRegistrations;
32+
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
33+
import org.springframework.security.oauth2.core.OAuth2AuthorizationException;
34+
import org.springframework.security.oauth2.core.OAuth2Error;
3235
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
3336
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
3437
import org.springframework.util.CollectionUtils;
@@ -41,6 +44,7 @@
4144
import java.util.Map;
4245

4346
import static org.assertj.core.api.Assertions.assertThatCode;
47+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
4448
import static org.mockito.ArgumentMatchers.any;
4549
import static org.mockito.Mockito.times;
4650
import static org.mockito.Mockito.verify;
@@ -233,6 +237,65 @@ public void filterWhenAuthorizationRequestRedirectUriParametersNotMatchThenNotPr
233237
verifyNoInteractions(this.authenticationManager);
234238
}
235239

240+
// gh-8609
241+
@Test
242+
public void filterWhenAuthenticationConverterThrowsOAuth2AuthorizationExceptionThenMappedToOAuth2AuthenticationException() {
243+
ClientRegistration clientRegistration = TestClientRegistrations.clientRegistration().build();
244+
when(this.clientRegistrationRepository.findByRegistrationId(any())).thenReturn(Mono.empty());
245+
246+
MockServerHttpRequest authorizationRequest =
247+
createAuthorizationRequest("/authorization/callback");
248+
OAuth2AuthorizationRequest oauth2AuthorizationRequest =
249+
createOAuth2AuthorizationRequest(authorizationRequest, clientRegistration);
250+
when(this.authorizationRequestRepository.loadAuthorizationRequest(any()))
251+
.thenReturn(Mono.just(oauth2AuthorizationRequest));
252+
when(this.authorizationRequestRepository.removeAuthorizationRequest(any()))
253+
.thenReturn(Mono.just(oauth2AuthorizationRequest));
254+
255+
MockServerHttpRequest authorizationResponse = createAuthorizationResponse(authorizationRequest);
256+
MockServerWebExchange exchange = MockServerWebExchange.from(authorizationResponse);
257+
DefaultWebFilterChain chain = new DefaultWebFilterChain(
258+
e -> e.getResponse().setComplete(), Collections.emptyList());
259+
260+
assertThatThrownBy(() -> this.filter.filter(exchange, chain).block())
261+
.isInstanceOf(OAuth2AuthenticationException.class)
262+
.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
263+
.extracting("errorCode")
264+
.isEqualTo("client_registration_not_found");
265+
verifyNoInteractions(this.authenticationManager);
266+
}
267+
268+
// gh-8609
269+
@Test
270+
public void filterWhenAuthenticationManagerThrowsOAuth2AuthorizationExceptionThenMappedToOAuth2AuthenticationException() {
271+
ClientRegistration clientRegistration = TestClientRegistrations.clientRegistration().build();
272+
when(this.clientRegistrationRepository.findByRegistrationId(any()))
273+
.thenReturn(Mono.just(clientRegistration));
274+
275+
MockServerHttpRequest authorizationRequest =
276+
createAuthorizationRequest("/authorization/callback");
277+
OAuth2AuthorizationRequest oauth2AuthorizationRequest =
278+
createOAuth2AuthorizationRequest(authorizationRequest, clientRegistration);
279+
when(this.authorizationRequestRepository.loadAuthorizationRequest(any()))
280+
.thenReturn(Mono.just(oauth2AuthorizationRequest));
281+
when(this.authorizationRequestRepository.removeAuthorizationRequest(any()))
282+
.thenReturn(Mono.just(oauth2AuthorizationRequest));
283+
284+
when(this.authenticationManager.authenticate(any()))
285+
.thenReturn(Mono.error(new OAuth2AuthorizationException(new OAuth2Error("authorization_error"))));
286+
287+
MockServerHttpRequest authorizationResponse = createAuthorizationResponse(authorizationRequest);
288+
MockServerWebExchange exchange = MockServerWebExchange.from(authorizationResponse);
289+
DefaultWebFilterChain chain = new DefaultWebFilterChain(
290+
e -> e.getResponse().setComplete(), Collections.emptyList());
291+
292+
assertThatThrownBy(() -> this.filter.filter(exchange, chain).block())
293+
.isInstanceOf(OAuth2AuthenticationException.class)
294+
.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
295+
.extracting("errorCode")
296+
.isEqualTo("authorization_error");
297+
}
298+
236299
private static OAuth2AuthorizationRequest createOAuth2AuthorizationRequest(
237300
MockServerHttpRequest authorizationRequest, ClientRegistration registration) {
238301
Map<String, Object> attributes = new HashMap<>();

0 commit comments

Comments
 (0)