polish gh-7996

Make defensive collection copy as Collections.unmodifiableCollection
does not protect from the source collection direct modification.
Use Mono#map instead of Mono#flatMap as it allocates less.
Use less operators to reduce allocations.
Use lambda parameter instead of outer method parameter
in authenticationManagers#computeIfAbsent()
to make it non capturing so it could be cached by JVM.
Propagate cause for InvalidBearerTokenException.
This commit is contained in:
Roman Matiushchenko 2020-02-21 12:25:46 +02:00 committed by Josh Cummings
parent 04e671fb4d
commit 9d66f2ccce

View File

@ -16,9 +16,9 @@
package org.springframework.security.oauth2.server.resource.authentication; package org.springframework.security.oauth2.server.resource.authentication;
import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collection; import java.util.Collection;
import java.util.Collections;
import java.util.Map; import java.util.Map;
import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate; import java.util.function.Predicate;
@ -54,6 +54,7 @@ import org.springframework.web.server.ServerWebExchange;
* <a href="https://tools.ietf.org/html/rfc6750#section-1.2" target="_blank">Bearer Token</a>. * <a href="https://tools.ietf.org/html/rfc6750#section-1.2" target="_blank">Bearer Token</a>.
* *
* @author Josh Cummings * @author Josh Cummings
* @author Roman Matiushchenko
* @since 5.3 * @since 5.3
*/ */
public final class JwtIssuerReactiveAuthenticationManagerResolver public final class JwtIssuerReactiveAuthenticationManagerResolver
@ -79,8 +80,7 @@ public final class JwtIssuerReactiveAuthenticationManagerResolver
public JwtIssuerReactiveAuthenticationManagerResolver(Collection<String> trustedIssuers) { public JwtIssuerReactiveAuthenticationManagerResolver(Collection<String> trustedIssuers) {
Assert.notEmpty(trustedIssuers, "trustedIssuers cannot be empty"); Assert.notEmpty(trustedIssuers, "trustedIssuers cannot be empty");
this.issuerAuthenticationManagerResolver = this.issuerAuthenticationManagerResolver =
new TrustedIssuerJwtAuthenticationManagerResolver new TrustedIssuerJwtAuthenticationManagerResolver(new ArrayList<>(trustedIssuers)::contains);
(Collections.unmodifiableCollection(trustedIssuers)::contains);
} }
/** /**
@ -133,26 +133,26 @@ public final class JwtIssuerReactiveAuthenticationManagerResolver
@Override @Override
public Mono<String> convert(@NonNull ServerWebExchange exchange) { public Mono<String> convert(@NonNull ServerWebExchange exchange) {
return this.converter.convert(exchange) return this.converter.convert(exchange).map(convertedToken -> {
.cast(BearerTokenAuthenticationToken.class) BearerTokenAuthenticationToken token = (BearerTokenAuthenticationToken) convertedToken;
.flatMap(this::issuer); try {
} String issuer = JWTParser.parse(token.getToken()).getJWTClaimsSet().getIssuer();
if (issuer == null) {
private Mono<String> issuer(BearerTokenAuthenticationToken token) { throw new InvalidBearerTokenException("Missing issuer");
try { } else {
String issuer = JWTParser.parse(token.getToken()).getJWTClaimsSet().getIssuer(); return issuer;
return Mono.justOrEmpty(issuer).switchIfEmpty( }
Mono.error(() -> new InvalidBearerTokenException("Missing issuer"))); } catch (Exception e) {
} catch (Exception e) { throw new InvalidBearerTokenException(e.getMessage(), e);
return Mono.error(new InvalidBearerTokenException(e.getMessage())); }
} });
} }
} }
private static class TrustedIssuerJwtAuthenticationManagerResolver private static class TrustedIssuerJwtAuthenticationManagerResolver
implements ReactiveAuthenticationManagerResolver<String> { implements ReactiveAuthenticationManagerResolver<String> {
private final Map<String, Mono<? extends ReactiveAuthenticationManager>> authenticationManagers = private final Map<String, Mono<ReactiveAuthenticationManager>> authenticationManagers =
new ConcurrentHashMap<>(); new ConcurrentHashMap<>();
private final Predicate<String> trustedIssuer; private final Predicate<String> trustedIssuer;
@ -162,15 +162,15 @@ public final class JwtIssuerReactiveAuthenticationManagerResolver
@Override @Override
public Mono<ReactiveAuthenticationManager> resolve(String issuer) { public Mono<ReactiveAuthenticationManager> resolve(String issuer) {
return Mono.just(issuer) if (!this.trustedIssuer.test(issuer)) {
.filter(this.trustedIssuer) return Mono.empty();
.flatMap(iss -> }
this.authenticationManagers.computeIfAbsent(iss, k -> return this.authenticationManagers.computeIfAbsent(issuer, k ->
Mono.fromCallable(() -> ReactiveJwtDecoders.fromIssuerLocation(iss)) Mono.<ReactiveAuthenticationManager>fromCallable(() ->
.subscribeOn(Schedulers.boundedElastic()) new JwtReactiveAuthenticationManager(ReactiveJwtDecoders.fromIssuerLocation(k))
.map(JwtReactiveAuthenticationManager::new) )
.cache()) .subscribeOn(Schedulers.boundedElastic())
); .cache());
} }
} }
} }