From 9d66f2ccce0203ae04c48dbf00942d044762a031 Mon Sep 17 00:00:00 2001 From: Roman Matiushchenko Date: Fri, 21 Feb 2020 12:25:46 +0200 Subject: [PATCH] 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. --- ...ReactiveAuthenticationManagerResolver.java | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerReactiveAuthenticationManagerResolver.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerReactiveAuthenticationManagerResolver.java index a30359176b..c2f415378b 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerReactiveAuthenticationManagerResolver.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerReactiveAuthenticationManagerResolver.java @@ -16,9 +16,9 @@ package org.springframework.security.oauth2.server.resource.authentication; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Predicate; @@ -54,6 +54,7 @@ import org.springframework.web.server.ServerWebExchange; * Bearer Token. * * @author Josh Cummings + * @author Roman Matiushchenko * @since 5.3 */ public final class JwtIssuerReactiveAuthenticationManagerResolver @@ -79,8 +80,7 @@ public final class JwtIssuerReactiveAuthenticationManagerResolver public JwtIssuerReactiveAuthenticationManagerResolver(Collection trustedIssuers) { Assert.notEmpty(trustedIssuers, "trustedIssuers cannot be empty"); this.issuerAuthenticationManagerResolver = - new TrustedIssuerJwtAuthenticationManagerResolver - (Collections.unmodifiableCollection(trustedIssuers)::contains); + new TrustedIssuerJwtAuthenticationManagerResolver(new ArrayList<>(trustedIssuers)::contains); } /** @@ -133,26 +133,26 @@ public final class JwtIssuerReactiveAuthenticationManagerResolver @Override public Mono convert(@NonNull ServerWebExchange exchange) { - return this.converter.convert(exchange) - .cast(BearerTokenAuthenticationToken.class) - .flatMap(this::issuer); - } - - private Mono issuer(BearerTokenAuthenticationToken token) { - try { - String issuer = JWTParser.parse(token.getToken()).getJWTClaimsSet().getIssuer(); - return Mono.justOrEmpty(issuer).switchIfEmpty( - Mono.error(() -> new InvalidBearerTokenException("Missing issuer"))); - } catch (Exception e) { - return Mono.error(new InvalidBearerTokenException(e.getMessage())); - } + return this.converter.convert(exchange).map(convertedToken -> { + BearerTokenAuthenticationToken token = (BearerTokenAuthenticationToken) convertedToken; + try { + String issuer = JWTParser.parse(token.getToken()).getJWTClaimsSet().getIssuer(); + if (issuer == null) { + throw new InvalidBearerTokenException("Missing issuer"); + } else { + return issuer; + } + } catch (Exception e) { + throw new InvalidBearerTokenException(e.getMessage(), e); + } + }); } } private static class TrustedIssuerJwtAuthenticationManagerResolver implements ReactiveAuthenticationManagerResolver { - private final Map> authenticationManagers = + private final Map> authenticationManagers = new ConcurrentHashMap<>(); private final Predicate trustedIssuer; @@ -162,15 +162,15 @@ public final class JwtIssuerReactiveAuthenticationManagerResolver @Override public Mono resolve(String issuer) { - return Mono.just(issuer) - .filter(this.trustedIssuer) - .flatMap(iss -> - this.authenticationManagers.computeIfAbsent(iss, k -> - Mono.fromCallable(() -> ReactiveJwtDecoders.fromIssuerLocation(iss)) - .subscribeOn(Schedulers.boundedElastic()) - .map(JwtReactiveAuthenticationManager::new) - .cache()) - ); + if (!this.trustedIssuer.test(issuer)) { + return Mono.empty(); + } + return this.authenticationManagers.computeIfAbsent(issuer, k -> + Mono.fromCallable(() -> + new JwtReactiveAuthenticationManager(ReactiveJwtDecoders.fromIssuerLocation(k)) + ) + .subscribeOn(Schedulers.boundedElastic()) + .cache()); } } }