Add Servlet and ServerBearerExchangeFilterFunction#7330
Add Servlet and ServerBearerExchangeFilterFunction#7330jzheaux merged 1 commit intospring-projects:masterfrom
Conversation
jzheaux
added
in: oauth2
There was a problem hiding this comment.
maybe it would be better not to create empty RequestContextDataHolder?
by
if (authentication == null || parentContext.hasKey(REQUEST_CONTEXT_DATA_HOLDER_ATTR_NAME)) {}
There was a problem hiding this comment.
Operators.liftPublisher() will not create unnecessary transformations into Scannable of a publisher
There was a problem hiding this comment.
Thanks for the tip! I'll update that.
There was a problem hiding this comment.
maybe would be better to write
return oauth2Token(request.attributes())
.map(oauth2Token -> bearer(request, oauth2Token))
.defaultIfEmpty(request)
.flatMap(next::exchange);This should create less Publishers/functions and it does not duplicate next::exchange
There was a problem hiding this comment.
I wonder what case does it cover?
Isn't it covered by Hooks.onLastOperator() from afterPropertiesSet() - it does the same.
It seems to me that this is just legacy solution copied from
https://github.com/spring-projects/spring-security/blob/master/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/reactive/function/client/ServletOAuth2AuthorizedClientExchangeFilterFunction.java
Or is it just an altarnative for case when Hook is not enabled and chaining is not necessary?
There was a problem hiding this comment.
This is an alternative for when more sophisticated chaining is wanted by the application, so oauth2Configuration would not suffice. Order obviously matters with filters, so, for example, I might need to default the request, add my own custom filter, and then add this filter after that.
| .map(req -> getOAuth2Token(req.attributes())) | ||
| .map(token -> bearer(request, token)) | ||
| .flatMap(next::exchange) | ||
| .switchIfEmpty(Mono.defer(() -> next.exchange(request))); |
There was a problem hiding this comment.
I know there are activities to reduce number of Optional usage and replacement of filter/map/flatMap() to the handle() in the spring core
so it could be rewritten to
may be it is a little bit more imperative, but it creates less operators and does not use create Optional to check attribute presence
public Mono<ClientResponse> filter(ClientRequest request, ExchangeFunction next) {
return mergeRequestAttributesIfNecessary(request)
.handle((req, sink) -> {
Map<String, Object> attributes = req.attributes();
if (attributes.get(AUTHENTICATION_ATTR_NAME) == null) {
sink.next(request);
} else {
AbstractOAuth2Token token = getOAuth2Token(attributes);
sink.next(bearer(req, token));
}
})
.flatMap(next::exchange);
}
private Mono<ClientRequest> mergeRequestAttributesIfNecessary(ClientRequest request) {
if (request.attributes().get(AUTHENTICATION_ATTR_NAME) != null) {
return Mono.just(request);
}
return mergeRequestAttributesFromContext(request);
}
private Mono<ClientRequest> mergeRequestAttributesFromContext(ClientRequest request) {
return Mono.subscriberContext()
.map(ctx -> ClientRequest.from(request)
.attributes(attrs -> populateRequestAttributes(attrs, ctx))
.build()
);
}
2 participants
Fixes gh-5334
Fixes gh-7284