diff --git a/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/JwtIssuerValidator.java b/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/JwtIssuerValidator.java index 5558c272ac2..f49a60c2081 100644 --- a/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/JwtIssuerValidator.java +++ b/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/JwtIssuerValidator.java @@ -15,9 +15,6 @@ */ package org.springframework.security.oauth2.jwt; -import java.net.MalformedURLException; -import java.net.URL; - import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.OAuth2ErrorCodes; import org.springframework.security.oauth2.core.OAuth2TokenValidator; @@ -46,14 +43,7 @@ public final class JwtIssuerValidator implements OAuth2TokenValidator { */ public JwtIssuerValidator(String issuer) { Assert.notNull(issuer, "issuer cannot be null"); - - try { - this.issuer = new URL(issuer).toString(); - } catch (MalformedURLException ex) { - throw new IllegalArgumentException( - "Invalid Issuer URL " + issuer + " : " + ex.getMessage(), - ex); - } + this.issuer = issuer; } /** diff --git a/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverter.java b/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverter.java index 8082cd6442b..18c1ae8b7e2 100644 --- a/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverter.java +++ b/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverter.java @@ -16,7 +16,6 @@ package org.springframework.security.oauth2.jwt; -import java.net.MalformedURLException; import java.net.URI; import java.net.URL; import java.time.Instant; @@ -42,7 +41,7 @@ public final class MappedJwtClaimSetConverter implements Converter, Map> { private static final Converter> AUDIENCE_CONVERTER = new AudienceConverter(); - private static final Converter ISSUER_CONVERTER = new IssuerConverter(); + private static final Converter ISSUER_CONVERTER = new IssuerConverter(); private static final Converter STRING_CONVERTER = new StringConverter(); private static final Converter TEMPORAL_CONVERTER = new InstantConverter(); @@ -157,39 +156,27 @@ public Collection convert(Object source) { * Coerces an Issuer claim * into a {@link URL}, ignoring null values, and throwing an error if its coercion efforts fail. */ - private static class IssuerConverter implements Converter { + private static class IssuerConverter implements Converter { @Override - public URL convert(Object source) { + public String convert(Object source) { if (source == null) { return null; } if (source instanceof URL) { - return (URL) source; - } - - if (source instanceof URI) { - return toUrl((URI) source); + return ((URL) source).toExternalForm(); } - return toUrl(source.toString()); - } - - private URL toUrl(URI source) { - try { - return source.toURL(); - } catch (MalformedURLException e) { - throw new IllegalStateException("Could not coerce " + source + " into a URL", e); + if (source instanceof String && ((String) source).contains(":")) { + try { + return URI.create((String) source).toString(); + } catch (Exception e) { + throw new IllegalStateException("Could not coerce " + source + " into a URI String", e); + } } - } - private URL toUrl(String source) { - try { - return new URL(source); - } catch (MalformedURLException e) { - throw new IllegalStateException("Could not coerce " + source + " into a URL", e); - } + return source.toString(); } } diff --git a/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/JwtIssuerValidatorTests.java b/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/JwtIssuerValidatorTests.java index 5839f74d3b3..d108048b9f1 100644 --- a/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/JwtIssuerValidatorTests.java +++ b/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/JwtIssuerValidatorTests.java @@ -82,15 +82,24 @@ public void validateWhenJwtHasNoIssuerThenReturnsError() { assertThat(result.getErrors()).isNotEmpty(); } + // gh-6073 @Test - public void validateWhenJwtIsNullThenThrowsIllegalArgumentException() { - assertThatCode(() -> this.validator.validate(null)) - .isInstanceOf(IllegalArgumentException.class); + public void validateWhenIssuerMatchesAndIsNotAUriThenReturnsSuccess() { + Jwt jwt = new Jwt( + MOCK_TOKEN, + MOCK_ISSUED_AT, + MOCK_EXPIRES_AT, + MOCK_HEADERS, + Collections.singletonMap(JwtClaimNames.ISS, "issuer")); + JwtIssuerValidator validator = new JwtIssuerValidator("issuer"); + + assertThat(validator.validate(jwt)) + .isEqualTo(OAuth2TokenValidatorResult.success()); } @Test - public void constructorWhenMalformedIssuerIsGivenThenThrowsIllegalArgumentException() { - assertThatCode(() -> new JwtIssuerValidator("issuer")) + public void validateWhenJwtIsNullThenThrowsIllegalArgumentException() { + assertThatCode(() -> this.validator.validate(null)) .isInstanceOf(IllegalArgumentException.class); } diff --git a/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverterTests.java b/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverterTests.java index b210c96d7d3..a8e2200dad2 100644 --- a/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverterTests.java +++ b/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverterTests.java @@ -108,7 +108,7 @@ public void convertWhenUsingDefaultsThenCoercesAllAttributesInJwtSpec() throws E assertThat(target.get(JwtClaimNames.AUD)).isEqualTo(Arrays.asList("audience")); assertThat(target.get(JwtClaimNames.EXP)).isEqualTo(Instant.ofEpochSecond(2000000000L)); assertThat(target.get(JwtClaimNames.IAT)).isEqualTo(Instant.ofEpochSecond(1000000000L)); - assertThat(target.get(JwtClaimNames.ISS)).isEqualTo(new URL("https://any.url")); + assertThat(target.get(JwtClaimNames.ISS)).isEqualTo("https://any.url"); assertThat(target.get(JwtClaimNames.NBF)).isEqualTo(Instant.ofEpochSecond(1000000000L)); assertThat(target.get(JwtClaimNames.SUB)).isEqualTo("1234"); } @@ -135,7 +135,7 @@ public void convertWhenUsingCustomConverterThenAllOtherDefaultsAreStillUsed() th assertThat(target.get(JwtClaimNames.AUD)).isEqualTo(Arrays.asList("audience")); assertThat(target.get(JwtClaimNames.EXP)).isEqualTo(Instant.ofEpochSecond(2000000000L)); assertThat(target.get(JwtClaimNames.IAT)).isEqualTo(Instant.ofEpochSecond(1000000000L)); - assertThat(target.get(JwtClaimNames.ISS)).isEqualTo(new URL("https://any.url")); + assertThat(target.get(JwtClaimNames.ISS)).isEqualTo("https://any.url"); assertThat(target.get(JwtClaimNames.NBF)).isEqualTo(Instant.ofEpochSecond(1000000000L)); assertThat(target.get(JwtClaimNames.SUB)).isEqualTo("1234"); } @@ -196,7 +196,7 @@ public void convertWhenUsingDefaultsThenFailedConversionThrowsIllegalStateExcept MappedJwtClaimSetConverter converter = MappedJwtClaimSetConverter .withDefaults(Collections.emptyMap()); - Map badIssuer = Collections.singletonMap(JwtClaimNames.ISS, "badly-formed-iss"); + Map badIssuer = Collections.singletonMap(JwtClaimNames.ISS, "https://badly formed iss"); assertThatCode(() -> converter.convert(badIssuer)).isInstanceOf(IllegalStateException.class); Map badIssuedAt = Collections.singletonMap(JwtClaimNames.IAT, "badly-formed-iat"); @@ -209,6 +209,28 @@ public void convertWhenUsingDefaultsThenFailedConversionThrowsIllegalStateExcept assertThatCode(() -> converter.convert(badNotBefore)).isInstanceOf(IllegalStateException.class); } + // gh-6073 + @Test + public void convertWhenIssuerIsNotAUriThenConvertsToString() { + MappedJwtClaimSetConverter converter = MappedJwtClaimSetConverter + .withDefaults(Collections.emptyMap()); + + Map nonUriIssuer = Collections.singletonMap(JwtClaimNames.ISS, "issuer"); + Map target = converter.convert(nonUriIssuer); + assertThat(target.get(JwtClaimNames.ISS)).isEqualTo("issuer"); + } + + // gh-6073 + @Test + public void convertWhenIssuerIsOfTypeURLThenConvertsToString() throws Exception { + MappedJwtClaimSetConverter converter = MappedJwtClaimSetConverter + .withDefaults(Collections.emptyMap()); + + Map issuer = Collections.singletonMap(JwtClaimNames.ISS, new URL("https://issuer")); + Map target = converter.convert(issuer); + assertThat(target.get(JwtClaimNames.ISS)).isEqualTo("https://issuer"); + } + @Test public void constructWhenAnyParameterIsNullThenIllegalArgumentException() { assertThatCode(() -> new MappedJwtClaimSetConverter(null))