Skip to content

Commit

Permalink
Leave Issuer As String
Browse files Browse the repository at this point in the history 
Since StringOrURI is a valid issuer, MappedJwtClaimSetConverter and
JwtIssuerValidator no longer assume it.

Issue: spring-projectsgh-6073
  • Loading branch information
@jzheaux
jzheaux authored and jer051 committed Nov 21, 2018
1 parent c16f2a8 commit 3335df1
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 43 deletions.
12 changes: 1 addition & 11 deletions ...oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/JwtIssuerValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -46,14 +43,7 @@ public final class JwtIssuerValidator implements OAuth2TokenValidator<Jwt> {
*/
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;
}

/**
Expand Down
35 changes: 11 additions & 24 deletions ...ose/src/main/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -42,7 +41,7 @@ public final class MappedJwtClaimSetConverter
implements Converter<Map<String, Object>, Map<String, Object>> {

private static final Converter<Object, Collection<String>> AUDIENCE_CONVERTER = new AudienceConverter();
private static final Converter<Object, URL> ISSUER_CONVERTER = new IssuerConverter();
private static final Converter<Object, String> ISSUER_CONVERTER = new IssuerConverter();
private static final Converter<Object, String> STRING_CONVERTER = new StringConverter();
private static final Converter<Object, Instant> TEMPORAL_CONVERTER = new InstantConverter();

Expand Down Expand Up @@ -157,39 +156,27 @@ public Collection<String> convert(Object source) {
* Coerces an <a target="_blank" href="https://tools.ietf.org/html/rfc7519#section-4.1.1">Issuer</a> claim
* into a {@link URL}, ignoring null values, and throwing an error if its coercion efforts fail.
*/
private static class IssuerConverter implements Converter<Object, URL> {
private static class IssuerConverter implements Converter<Object, String> {

@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();
}
}

Expand Down
19 changes: 14 additions & 5 deletions ...2-jose/src/test/java/org/springframework/security/oauth2/jwt/JwtIssuerValidatorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
28 changes: 25 additions & 3 deletions ...rc/test/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverterTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand All @@ -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");
}
Expand Down Expand Up @@ -196,7 +196,7 @@ public void convertWhenUsingDefaultsThenFailedConversionThrowsIllegalStateExcept
MappedJwtClaimSetConverter converter = MappedJwtClaimSetConverter
.withDefaults(Collections.emptyMap());

Map<String, Object> badIssuer = Collections.singletonMap(JwtClaimNames.ISS, "badly-formed-iss");
Map<String, Object> badIssuer = Collections.singletonMap(JwtClaimNames.ISS, "https://badly formed iss");
assertThatCode(() -> converter.convert(badIssuer)).isInstanceOf(IllegalStateException.class);

Map<String, Object> badIssuedAt = Collections.singletonMap(JwtClaimNames.IAT, "badly-formed-iat");
Expand All @@ -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<String, Object> nonUriIssuer = Collections.singletonMap(JwtClaimNames.ISS, "issuer");
Map<String, Object> 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<String, Object> issuer = Collections.singletonMap(JwtClaimNames.ISS, new URL("https://issuer"));
Map<String, Object> target = converter.convert(issuer);
assertThat(target.get(JwtClaimNames.ISS)).isEqualTo("https://issuer");
}

@Test
public void constructWhenAnyParameterIsNullThenIllegalArgumentException() {
assertThatCode(() -> new MappedJwtClaimSetConverter(null))
Expand Down

0 comments on commit 3335df1

Please sign in to comment.