From f3695932de0bc2e9fda6ab7da9eaac87f19c3119 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Thu, 30 Jul 2020 16:22:49 -0600 Subject: [PATCH] Polish to Avoid NPE Issue gh-5648 Co-authored-by: MattyA --- .../security/oauth2/jwt/NimbusJwtDecoder.java | 15 ++++- .../oauth2/jwt/NimbusReactiveJwtDecoder.java | 62 +++++++++++-------- .../oauth2/jwt/NimbusJwtDecoderTests.java | 16 +++++ .../jwt/NimbusReactiveJwtDecoderTests.java | 16 +++++ 4 files changed, 80 insertions(+), 29 deletions(-) diff --git a/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java b/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java index 68c2318a1b..abf4af5062 100644 --- a/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java +++ b/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java @@ -22,6 +22,7 @@ import java.net.URL; import java.security.interfaces.RSAPublicKey; import java.text.ParseException; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -55,11 +56,13 @@ import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.http.RequestEntity; import org.springframework.http.ResponseEntity; +import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.OAuth2TokenValidator; import org.springframework.security.oauth2.core.OAuth2TokenValidatorResult; import org.springframework.security.oauth2.jose.jws.MacAlgorithm; import org.springframework.security.oauth2.jose.jws.SignatureAlgorithm; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import org.springframework.web.client.RestOperations; import org.springframework.web.client.RestTemplate; @@ -167,9 +170,17 @@ public final class NimbusJwtDecoder implements JwtDecoder { private Jwt validateJwt(Jwt jwt){ OAuth2TokenValidatorResult result = this.jwtValidator.validate(jwt); if (result.hasErrors()) { - String description = result.getErrors().iterator().next().getDescription(); + Collection errors = result.getErrors(); + String validationErrorString = "Unable to validate Jwt"; + for (OAuth2Error oAuth2Error : errors) { + if (!StringUtils.isEmpty(oAuth2Error.getDescription())) { + validationErrorString = String.format( + DECODING_ERROR_MESSAGE_TEMPLATE, oAuth2Error.getDescription()); + break; + } + } throw new JwtValidationException( - String.format(DECODING_ERROR_MESSAGE_TEMPLATE, description), + validationErrorString, result.getErrors()); } diff --git a/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusReactiveJwtDecoder.java b/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusReactiveJwtDecoder.java index c80bbb4a3a..6f394a9cae 100644 --- a/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusReactiveJwtDecoder.java +++ b/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusReactiveJwtDecoder.java @@ -15,17 +15,6 @@ */ package org.springframework.security.oauth2.jwt; -import java.security.interfaces.RSAPublicKey; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.Set; -import java.util.function.Consumer; -import java.util.function.Function; -import javax.crypto.SecretKey; - import com.nimbusds.jose.Header; import com.nimbusds.jose.JOSEException; import com.nimbusds.jose.JWSAlgorithm; @@ -47,28 +36,41 @@ import com.nimbusds.jwt.PlainJWT; import com.nimbusds.jwt.SignedJWT; import com.nimbusds.jwt.proc.DefaultJWTProcessor; import com.nimbusds.jwt.proc.JWTProcessor; -import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; - import org.springframework.core.convert.converter.Converter; +import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.OAuth2TokenValidator; import org.springframework.security.oauth2.core.OAuth2TokenValidatorResult; import org.springframework.security.oauth2.jose.jws.JwsAlgorithm; import org.springframework.security.oauth2.jose.jws.MacAlgorithm; import org.springframework.security.oauth2.jose.jws.SignatureAlgorithm; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import org.springframework.web.reactive.function.client.WebClient; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; + +import javax.crypto.SecretKey; +import java.security.interfaces.RSAPublicKey; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; +import java.util.function.Function; /** - * An implementation of a {@link ReactiveJwtDecoder} that "decodes" a - * JSON Web Token (JWT) and additionally verifies it's digital signature if the JWT is a - * JSON Web Signature (JWS). - * - *

- * NOTE: This implementation uses the Nimbus JOSE + JWT SDK internally. - * - * @author Rob Winch - * @author Joe Grandja +* An implementation of a {@link ReactiveJwtDecoder} that "decodes" a +* JSON Web Token (JWT) and additionally verifies it's digital signature if the JWT is a +* JSON Web Signature (JWS). +* +*

+* NOTE: This implementation uses the Nimbus JOSE + JWT SDK internally. +* +* @author Rob Winch +* @author Joe Grandja * @since 5.1 * @see ReactiveJwtDecoder * @see JSON Web Token (JWT) @@ -178,10 +180,16 @@ public final class NimbusReactiveJwtDecoder implements ReactiveJwtDecoder { private Jwt validateJwt(Jwt jwt) { OAuth2TokenValidatorResult result = this.jwtValidator.validate(jwt); - - if ( result.hasErrors() ) { - String message = result.getErrors().iterator().next().getDescription(); - throw new JwtValidationException(message, result.getErrors()); + if (result.hasErrors()) { + Collection errors = result.getErrors(); + String validationErrorString = "Unable to validate Jwt"; + for (OAuth2Error oAuth2Error : errors) { + if (!StringUtils.isEmpty(oAuth2Error.getDescription())) { + validationErrorString = oAuth2Error.getDescription(); + break; + } + } + throw new JwtValidationException(validationErrorString, errors); } return jwt; diff --git a/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoderTests.java b/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoderTests.java index 27ab00ff35..b2b9ace76a 100644 --- a/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoderTests.java +++ b/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoderTests.java @@ -193,6 +193,22 @@ public class NimbusJwtDecoderTests { .hasFieldOrPropertyWithValue("errors", Arrays.asList(firstFailure, secondFailure)); } + @Test + public void decodeWhenReadingErrorPickTheFirstErrorMessage() { + OAuth2TokenValidator jwtValidator = mock(OAuth2TokenValidator.class); + this.jwtDecoder.setJwtValidator(jwtValidator); + + OAuth2Error errorEmpty = new OAuth2Error("mock-error", "", "mock-uri"); + OAuth2Error error = new OAuth2Error("mock-error", "mock-description", "mock-uri"); + OAuth2Error error2 = new OAuth2Error("mock-error-second", "mock-description-second", "mock-uri-second"); + OAuth2TokenValidatorResult result = OAuth2TokenValidatorResult.failure(errorEmpty, error, error2); + when(jwtValidator.validate(any(Jwt.class))).thenReturn(result); + + Assertions.assertThatCode(() -> this.jwtDecoder.decode(SIGNED_JWT)) + .isInstanceOf(JwtValidationException.class) + .hasMessageContaining("mock-description"); + } + @Test public void decodeWhenUsingSignedJwtThenReturnsClaimsGivenByClaimSetConverter() { Converter, Map> claimSetConverter = mock(Converter.class); diff --git a/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusReactiveJwtDecoderTests.java b/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusReactiveJwtDecoderTests.java index 74a9d8f671..7da68f25a0 100644 --- a/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusReactiveJwtDecoderTests.java +++ b/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusReactiveJwtDecoderTests.java @@ -221,6 +221,22 @@ public class NimbusReactiveJwtDecoderTests { .hasMessageContaining("mock-description"); } + @Test + public void decodeWhenReadingErrorPickTheFirstErrorMessage() { + OAuth2TokenValidator jwtValidator = mock(OAuth2TokenValidator.class); + this.decoder.setJwtValidator(jwtValidator); + + OAuth2Error errorEmpty = new OAuth2Error("mock-error", "", "mock-uri"); + OAuth2Error error = new OAuth2Error("mock-error", "mock-description", "mock-uri"); + OAuth2Error error2 = new OAuth2Error("mock-error-second", "mock-description-second", "mock-uri-second"); + OAuth2TokenValidatorResult result = OAuth2TokenValidatorResult.failure(errorEmpty, error, error2); + when(jwtValidator.validate(any(Jwt.class))).thenReturn(result); + + assertThatCode(() -> this.decoder.decode(this.messageReadToken).block()) + .isInstanceOf(JwtValidationException.class) + .hasMessageContaining("mock-description"); + } + @Test public void decodeWhenUsingSignedJwtThenReturnsClaimsGivenByClaimSetConverter() { Converter, Map> claimSetConverter = mock(Converter.class);