Skip to content

Commit 4ca9e15

Browse files
committed
Fix blocking in ServletOAuth2AuthorizedClientExchangeFilterFunction
Fixes gh-6589
1 parent c05b076 commit 4ca9e15

File tree

5 files changed

+317
-187
lines changed

5 files changed

+317
-187
lines changed

gradle/dependency-management.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ dependencyManagement {
7070
dependency 'commons-lang:commons-lang:2.6'
7171
dependency 'commons-logging:commons-logging:1.2'
7272
dependency 'dom4j:dom4j:1.6.1'
73+
dependency 'io.projectreactor.tools:blockhound:1.0.0.M4'
7374
dependency 'javax.activation:activation:1.1.1'
7475
dependency 'javax.annotation:jsr250-api:1.0'
7576
dependency 'javax.inject:javax.inject:1'

oauth2/oauth2-client/spring-security-oauth2-client.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ dependencies {
1717
testCompile 'com.fasterxml.jackson.core:jackson-databind'
1818
testCompile 'io.projectreactor.netty:reactor-netty'
1919
testCompile 'io.projectreactor:reactor-test'
20+
testCompile 'io.projectreactor.tools:blockhound'
2021

2122
provided 'javax.servlet:javax.servlet-api'
2223
}

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/reactive/function/client/ServletOAuth2AuthorizedClientExchangeFilterFunction.java

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import reactor.core.publisher.Hooks;
5151
import reactor.core.publisher.Mono;
5252
import reactor.core.publisher.Operators;
53+
import reactor.core.scheduler.Schedulers;
5354
import reactor.util.context.Context;
5455

5556
import javax.servlet.http.HttpServletRequest;
@@ -258,7 +259,6 @@ public Consumer<WebClient.RequestHeadersSpec<?>> defaultRequest() {
258259
spec.attributes(attrs -> {
259260
populateDefaultRequestResponse(attrs);
260261
populateDefaultAuthentication(attrs);
261-
populateDefaultOAuth2AuthorizedClient(attrs);
262262
});
263263
};
264264
}
@@ -349,20 +349,33 @@ public void setAccessTokenExpiresSkew(Duration accessTokenExpiresSkew) {
349349

350350
@Override
351351
public Mono<ClientResponse> filter(ClientRequest request, ExchangeFunction next) {
352-
return Mono.just(request)
353-
.filter(req -> req.attribute(OAUTH2_AUTHORIZED_CLIENT_ATTR_NAME).isPresent())
354-
.switchIfEmpty(mergeRequestAttributesFromContext(request))
352+
return mergeRequestAttributesIfNecessary(request)
355353
.filter(req -> req.attribute(OAUTH2_AUTHORIZED_CLIENT_ATTR_NAME).isPresent())
356354
.flatMap(req -> authorizedClient(getOAuth2AuthorizedClient(req.attributes()), req))
355+
.switchIfEmpty(Mono.defer(() ->
356+
mergeRequestAttributesIfNecessary(request)
357+
.filter(req -> resolveClientRegistrationId(req) != null)
358+
.flatMap(req -> authorizeClient(resolveClientRegistrationId(req), req))
359+
))
357360
.map(authorizedClient -> bearer(request, authorizedClient))
358361
.flatMap(next::exchange)
359-
.switchIfEmpty(next.exchange(request));
362+
.switchIfEmpty(Mono.defer(() -> next.exchange(request)));
363+
}
364+
365+
private Mono<ClientRequest> mergeRequestAttributesIfNecessary(ClientRequest request) {
366+
if (!request.attribute(HTTP_SERVLET_REQUEST_ATTR_NAME).isPresent() ||
367+
!request.attribute(HTTP_SERVLET_RESPONSE_ATTR_NAME).isPresent() ||
368+
!request.attribute(AUTHENTICATION_ATTR_NAME).isPresent()) {
369+
return mergeRequestAttributesFromContext(request);
370+
} else {
371+
return Mono.just(request);
372+
}
360373
}
361374

362375
private Mono<ClientRequest> mergeRequestAttributesFromContext(ClientRequest request) {
363-
return Mono.just(ClientRequest.from(request))
364-
.flatMap(builder -> Mono.subscriberContext()
365-
.map(ctx -> builder.attributes(attrs -> populateRequestAttributes(attrs, ctx))))
376+
ClientRequest.Builder builder = ClientRequest.from(request);
377+
return Mono.subscriberContext()
378+
.map(ctx -> builder.attributes(attrs -> populateRequestAttributes(attrs, ctx)))
366379
.map(ClientRequest.Builder::build);
367380
}
368381

@@ -376,7 +389,6 @@ private void populateRequestAttributes(Map<String, Object> attrs, Context ctx) {
376389
if (ctx.hasKey(AUTHENTICATION_ATTR_NAME)) {
377390
attrs.putIfAbsent(AUTHENTICATION_ATTR_NAME, ctx.get(AUTHENTICATION_ATTR_NAME));
378391
}
379-
populateDefaultOAuth2AuthorizedClient(attrs);
380392
}
381393

382394
private void populateDefaultRequestResponse(Map<String, Object> attrs) {
@@ -403,32 +415,38 @@ private void populateDefaultAuthentication(Map<String, Object> attrs) {
403415
attrs.putIfAbsent(AUTHENTICATION_ATTR_NAME, authentication);
404416
}
405417

406-
private void populateDefaultOAuth2AuthorizedClient(Map<String, Object> attrs) {
407-
if (this.authorizedClientManager == null ||
408-
attrs.containsKey(OAUTH2_AUTHORIZED_CLIENT_ATTR_NAME)) {
409-
return;
410-
}
411-
412-
Authentication authentication = getAuthentication(attrs);
418+
private String resolveClientRegistrationId(ClientRequest request) {
419+
Map<String, Object> attrs = request.attributes();
413420
String clientRegistrationId = getClientRegistrationId(attrs);
414421
if (clientRegistrationId == null) {
415422
clientRegistrationId = this.defaultClientRegistrationId;
416423
}
424+
Authentication authentication = getAuthentication(attrs);
417425
if (clientRegistrationId == null
418426
&& this.defaultOAuth2AuthorizedClient
419427
&& authentication instanceof OAuth2AuthenticationToken) {
420428
clientRegistrationId = ((OAuth2AuthenticationToken) authentication).getAuthorizedClientRegistrationId();
421429
}
422-
if (clientRegistrationId != null) {
423-
HttpServletRequest request = getRequest(attrs);
424-
if (authentication == null) {
425-
authentication = ANONYMOUS_AUTHENTICATION;
426-
}
427-
OAuth2AuthorizeRequest authorizeRequest = new OAuth2AuthorizeRequest(
428-
clientRegistrationId, authentication, request, getResponse(attrs));
429-
OAuth2AuthorizedClient authorizedClient = this.authorizedClientManager.authorize(authorizeRequest);
430-
oauth2AuthorizedClient(authorizedClient).accept(attrs);
430+
return clientRegistrationId;
431+
}
432+
433+
private Mono<OAuth2AuthorizedClient> authorizeClient(String clientRegistrationId, ClientRequest request) {
434+
if (this.authorizedClientManager == null) {
435+
return Mono.empty();
431436
}
437+
Map<String, Object> attrs = request.attributes();
438+
Authentication authentication = getAuthentication(attrs);
439+
if (authentication == null) {
440+
authentication = ANONYMOUS_AUTHENTICATION;
441+
}
442+
HttpServletRequest servletRequest = getRequest(attrs);
443+
HttpServletResponse servletResponse = getResponse(attrs);
444+
OAuth2AuthorizeRequest authorizeRequest = new OAuth2AuthorizeRequest(
445+
clientRegistrationId, authentication, servletRequest, servletResponse);
446+
447+
// NOTE: 'authorizedClientManager.authorize()' needs to be executed on a dedicated thread via subscribeOn(Schedulers.elastic())
448+
// since it performs a blocking I/O operation using RestTemplate internally
449+
return Mono.fromSupplier(() -> this.authorizedClientManager.authorize(authorizeRequest)).subscribeOn(Schedulers.elastic());
432450
}
433451

434452
private Mono<OAuth2AuthorizedClient> authorizedClient(OAuth2AuthorizedClient authorizedClient, ClientRequest request) {
@@ -444,7 +462,10 @@ private Mono<OAuth2AuthorizedClient> authorizedClient(OAuth2AuthorizedClient aut
444462
HttpServletResponse servletResponse = getResponse(attrs);
445463
OAuth2AuthorizeRequest reauthorizeRequest = new OAuth2AuthorizeRequest(
446464
authorizedClient, authentication, servletRequest, servletResponse);
447-
return Mono.fromSupplier(() -> this.authorizedClientManager.authorize(reauthorizeRequest));
465+
466+
// NOTE: 'authorizedClientManager.authorize()' needs to be executed on a dedicated thread via subscribeOn(Schedulers.elastic())
467+
// since it performs a blocking I/O operation using RestTemplate internally
468+
return Mono.fromSupplier(() -> this.authorizedClientManager.authorize(reauthorizeRequest)).subscribeOn(Schedulers.elastic());
448469
}
449470

450471
private ClientRequest bearer(ClientRequest request, OAuth2AuthorizedClient authorizedClient) {

0 commit comments

Comments
 (0)