Cwikius-Spring
/
spring-security
mirror of https://github.com/spring-projects/spring-security.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Amended treatment of OAuth2 'iss' claim 

Prior to this commit, the OAuth2 resource server code is failing any issuer
that is not a valid URL. This does not correspond to
https://datatracker.ietf.org/doc/html/rfc7662#page-7 which redirects to
https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.1, defining an
issuer as being a "StringOrURI", which is defined at
https://datatracker.ietf.org/doc/html/rfc7519#page-5 as being
an "arbitrary string value" that "MUST be a URI" only for
"any value containing a ':'".

The issue currently is that an issuer that is not a valid URL may be
provided, which will automatically result in the request being aborted
due to being invalid.

I have removed the check entirely, since while the claim could be invalid,
it is still a response that the OAuth2 introspection endpoint has provided.
In the liklihood that interpretations of this behaviour are different for
the OAuth2 server implementation in use, this currently stops Spring
Security from being able to be used at all without implementing a custom
introspector from scratch.

It is also worth noting that the spec does not specify whether it is
valid to normalize issuers or not if they are valid URLs. This may cause
other unintended side effects as a result of this change, so it is
safer to disable it entirely.
This commit is contained in:
Ashley Scopes 2021-08-15 14:38:13 +01:00 committed by Josh Cummings
parent fe274e7553
commit dd43d9198b
8 changed files with 80 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/NimbusOpaqueTokenIntrospector.java
View File

@ -17,7 +17,6 @@
package org.springframework.security.oauth2.server.resource.introspection;
import java.net.URI;
import java.net.URL;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
@ -207,7 +206,25 @@ public class NimbusOpaqueTokenIntrospector implements OpaqueTokenIntrospector {
claims.put(OAuth2TokenIntrospectionClaimNames.IAT, iat);
}
if (response.getIssuer() != null) {
claims.put(OAuth2TokenIntrospectionClaimNames.ISS, issuer(response.getIssuer().getValue()));
// RFC-7662 page 7 directs users to RFC-7519 for defining the values of these
// issuer fields.
// https://datatracker.ietf.org/doc/html/rfc7662#page-7
//
// RFC-7519 page 9 defines issuer fields as being 'case-sensitive' strings
// containing
// a 'StringOrURI', which is defined on page 5 as being any string, but
// strings containing ':'
// should be treated as valid URIs.
// https://datatracker.ietf.org/doc/html/rfc7519#section-2
//
// It is not defined however as to whether-or-not normalized URIs should be
// treated as the same literal
// value. It only defines validation itself, so to avoid potential ambiguity
// or unwanted side effects that
// may be awkward to debug, we do not want to manipulate this value. Previous
// versions of Spring Security
// would *only* allow valid URLs, which is not what we wish to achieve here.
claims.put(OAuth2TokenIntrospectionClaimNames.ISS, response.getIssuer().getValue());
}
if (response.getNotBeforeTime() != null) {
claims.put(OAuth2TokenIntrospectionClaimNames.NBF, response.getNotBeforeTime().toInstant());
@ -222,14 +239,4 @@ public class NimbusOpaqueTokenIntrospector implements OpaqueTokenIntrospector {
return new OAuth2IntrospectionAuthenticatedPrincipal(claims, authorities);
}
private URL issuer(String uri) {
try {
return new URL(uri);
}
catch (Exception ex) {
throw new OAuth2IntrospectionException(
"Invalid " + OAuth2TokenIntrospectionClaimNames.ISS + " value: " + uri);
}
}
}

31
oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/NimbusReactiveOpaqueTokenIntrospector.java
View File

@ -17,7 +17,6 @@
package org.springframework.security.oauth2.server.resource.introspection;
import java.net.URI;
import java.net.URL;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
@ -174,7 +173,25 @@ public class NimbusReactiveOpaqueTokenIntrospector implements ReactiveOpaqueToke
claims.put(OAuth2TokenIntrospectionClaimNames.IAT, iat);
}
if (response.getIssuer() != null) {
claims.put(OAuth2TokenIntrospectionClaimNames.ISS, issuer(response.getIssuer().getValue()));
// RFC-7662 page 7 directs users to RFC-7519 for defining the values of these
// issuer fields.
// https://datatracker.ietf.org/doc/html/rfc7662#page-7
//
// RFC-7519 page 9 defines issuer fields as being 'case-sensitive' strings
// containing
// a 'StringOrURI', which is defined on page 5 as being any string, but
// strings containing ':'
// should be treated as valid URIs.
// https://datatracker.ietf.org/doc/html/rfc7519#section-2
//
// It is not defined however as to whether-or-not normalized URIs should be
// treated as the same literal
// value. It only defines validation itself, so to avoid potential ambiguity
// or unwanted side effects that
// may be awkward to debug, we do not want to manipulate this value. Previous
// versions of Spring Security
// would *only* allow valid URLs, which is not what we wish to achieve here.
claims.put(OAuth2TokenIntrospectionClaimNames.ISS, response.getIssuer().getValue());
}
if (response.getNotBeforeTime() != null) {
claims.put(OAuth2TokenIntrospectionClaimNames.NBF, response.getNotBeforeTime().toInstant());
@ -190,16 +207,6 @@ public class NimbusReactiveOpaqueTokenIntrospector implements ReactiveOpaqueToke
return new OAuth2IntrospectionAuthenticatedPrincipal(claims, authorities);
}
private URL issuer(String uri) {
try {
return new URL(uri);
}
catch (Exception ex) {
throw new OAuth2IntrospectionException(
"Invalid " + OAuth2TokenIntrospectionClaimNames.ISS + " value: " + uri);
}
}
private OAuth2IntrospectionException onError(Throwable ex) {
return new OAuth2IntrospectionException(ex.getMessage(), ex);
}

31
oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospector.java
View File

@ -17,7 +17,6 @@
package org.springframework.security.oauth2.server.resource.introspection;
import java.net.URI;
import java.net.URL;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
@ -187,7 +186,25 @@ public class SpringOpaqueTokenIntrospector implements OpaqueTokenIntrospector {
(k, v) -> Instant.ofEpochSecond(((Number) v).longValue()));
claims.computeIfPresent(OAuth2TokenIntrospectionClaimNames.IAT,
(k, v) -> Instant.ofEpochSecond(((Number) v).longValue()));
claims.computeIfPresent(OAuth2TokenIntrospectionClaimNames.ISS, (k, v) -> issuer(v.toString()));
// RFC-7662 page 7 directs users to RFC-7519 for defining the values of these
// issuer fields.
// https://datatracker.ietf.org/doc/html/rfc7662#page-7
//
// RFC-7519 page 9 defines issuer fields as being 'case-sensitive' strings
// containing
// a 'StringOrURI', which is defined on page 5 as being any string, but strings
// containing ':'
// should be treated as valid URIs.
// https://datatracker.ietf.org/doc/html/rfc7519#section-2
//
// It is not defined however as to whether-or-not normalized URIs should be
// treated as the same literal
// value. It only defines validation itself, so to avoid potential ambiguity or
// unwanted side effects that
// may be awkward to debug, we do not want to manipulate this value. Previous
// versions of Spring Security
// would *only* allow valid URLs, which is not what we wish to achieve here.
claims.computeIfPresent(OAuth2TokenIntrospectionClaimNames.ISS, (k, v) -> v.toString());
claims.computeIfPresent(OAuth2TokenIntrospectionClaimNames.NBF,
(k, v) -> Instant.ofEpochSecond(((Number) v).longValue()));
Collection<GrantedAuthority> authorities = new ArrayList<>();
@ -204,14 +221,4 @@ public class SpringOpaqueTokenIntrospector implements OpaqueTokenIntrospector {
return new OAuth2IntrospectionAuthenticatedPrincipal(claims, authorities);
}
private URL issuer(String uri) {
try {
return new URL(uri);
}
catch (Exception ex) {
throw new OAuth2IntrospectionException(
"Invalid " + OAuth2TokenIntrospectionClaimNames.ISS + " value: " + uri);
}
}
}

31
oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospector.java
View File

@ -17,7 +17,6 @@
package org.springframework.security.oauth2.server.resource.introspection;
import java.net.URI;
import java.net.URL;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
@ -146,7 +145,25 @@ public class SpringReactiveOpaqueTokenIntrospector implements ReactiveOpaqueToke
(k, v) -> Instant.ofEpochSecond(((Number) v).longValue()));
claims.computeIfPresent(OAuth2IntrospectionClaimNames.ISSUED_AT,
(k, v) -> Instant.ofEpochSecond(((Number) v).longValue()));
claims.computeIfPresent(OAuth2IntrospectionClaimNames.ISSUER, (k, v) -> issuer(v.toString()));
// RFC-7662 page 7 directs users to RFC-7519 for defining the values of these
// issuer fields.
// https://datatracker.ietf.org/doc/html/rfc7662#page-7
//
// RFC-7519 page 9 defines issuer fields as being 'case-sensitive' strings
// containing
// a 'StringOrURI', which is defined on page 5 as being any string, but strings
// containing ':'
// should be treated as valid URIs.
// https://datatracker.ietf.org/doc/html/rfc7519#section-2
//
// It is not defined however as to whether-or-not normalized URIs should be
// treated as the same literal
// value. It only defines validation itself, so to avoid potential ambiguity or
// unwanted side effects that
// may be awkward to debug, we do not want to manipulate this value. Previous
// versions of Spring Security
// would *only* allow valid URLs, which is not what we wish to achieve here.
claims.computeIfPresent(OAuth2IntrospectionClaimNames.ISSUER, (k, v) -> v.toString());
claims.computeIfPresent(OAuth2IntrospectionClaimNames.NOT_BEFORE,
(k, v) -> Instant.ofEpochSecond(((Number) v).longValue()));
Collection<GrantedAuthority> authorities = new ArrayList<>();
@ -163,16 +180,6 @@ public class SpringReactiveOpaqueTokenIntrospector implements ReactiveOpaqueToke
return new OAuth2IntrospectionAuthenticatedPrincipal(claims, authorities);
}
private URL issuer(String uri) {
try {
return new URL(uri);
}
catch (Exception ex) {
throw new OAuth2IntrospectionException(
"Invalid " + OAuth2IntrospectionClaimNames.ISSUER + " value: " + uri);
}
}
private OAuth2IntrospectionException onError(Throwable ex) {
return new OAuth2IntrospectionException(ex.getMessage(), ex);
}

3
oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/NimbusOpaqueTokenIntrospectorTests.java
View File

@ -17,7 +17,6 @@
package org.springframework.security.oauth2.server.resource.introspection;
import java.io.IOException;
import java.net.URL;
import java.time.Instant;
import java.util.Arrays;
import java.util.Base64;
@ -146,7 +145,7 @@ public class NimbusOpaqueTokenIntrospectorTests {
Arrays.asList("https://protected.example.net/resource"))
.containsEntry(OAuth2TokenIntrospectionClaimNames.CLIENT_ID, "l238j323ds-23ij4")
.containsEntry(OAuth2TokenIntrospectionClaimNames.EXP, Instant.ofEpochSecond(1419356238))
.containsEntry(OAuth2TokenIntrospectionClaimNames.ISS, new URL("https://server.example.com/"))
.containsEntry(OAuth2TokenIntrospectionClaimNames.ISS, "https://server.example.com/")
.containsEntry(OAuth2TokenIntrospectionClaimNames.SCOPE, Arrays.asList("read", "write", "dolphin"))
.containsEntry(OAuth2TokenIntrospectionClaimNames.SUB, "Z5O3upPC88QrAjx00dis")
.containsEntry(OAuth2TokenIntrospectionClaimNames.USERNAME, "jdoe")

3
oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/NimbusReactiveOpaqueTokenIntrospectorTests.java
View File

@ -17,7 +17,6 @@
package org.springframework.security.oauth2.server.resource.introspection;
import java.io.IOException;
import java.net.URL;
import java.time.Instant;
import java.util.Arrays;
import java.util.Base64;
@ -117,7 +116,7 @@ public class NimbusReactiveOpaqueTokenIntrospectorTests {
Arrays.asList("https://protected.example.net/resource"))
.containsEntry(OAuth2TokenIntrospectionClaimNames.CLIENT_ID, "l238j323ds-23ij4")
.containsEntry(OAuth2TokenIntrospectionClaimNames.EXP, Instant.ofEpochSecond(1419356238))
.containsEntry(OAuth2TokenIntrospectionClaimNames.ISS, new URL("https://server.example.com/"))
.containsEntry(OAuth2TokenIntrospectionClaimNames.ISS, "https://server.example.com/")
.containsEntry(OAuth2TokenIntrospectionClaimNames.SCOPE, Arrays.asList("read", "write", "dolphin"))
.containsEntry(OAuth2TokenIntrospectionClaimNames.SUB, "Z5O3upPC88QrAjx00dis")
.containsEntry(OAuth2TokenIntrospectionClaimNames.USERNAME, "jdoe")

3
oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospectorTests.java
View File

@ -17,7 +17,6 @@
package org.springframework.security.oauth2.server.resource.introspection;
import java.io.IOException;
import java.net.URL;
import java.time.Instant;
import java.util.Arrays;
import java.util.Base64;
@ -150,7 +149,7 @@ public class SpringOpaqueTokenIntrospectorTests {
Arrays.asList("https://protected.example.net/resource"))
.containsEntry(OAuth2TokenIntrospectionClaimNames.CLIENT_ID, "l238j323ds-23ij4")
.containsEntry(OAuth2TokenIntrospectionClaimNames.EXP, Instant.ofEpochSecond(1419356238))
.containsEntry(OAuth2TokenIntrospectionClaimNames.ISS, new URL("https://server.example.com/"))
.containsEntry(OAuth2TokenIntrospectionClaimNames.ISS, "https://server.example.com/")
.containsEntry(OAuth2TokenIntrospectionClaimNames.SCOPE, Arrays.asList("read", "write", "dolphin"))
.containsEntry(OAuth2TokenIntrospectionClaimNames.SUB, "Z5O3upPC88QrAjx00dis")
.containsEntry(OAuth2TokenIntrospectionClaimNames.USERNAME, "jdoe")

3
oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospectorTests.java
View File

@ -17,7 +17,6 @@
package org.springframework.security.oauth2.server.resource.introspection;
import java.io.IOException;
import java.net.URL;
import java.time.Instant;
import java.util.Arrays;
import java.util.Base64;
@ -122,7 +121,7 @@ public class SpringReactiveOpaqueTokenIntrospectorTests {
Arrays.asList("https://protected.example.net/resource"))
.containsEntry(OAuth2IntrospectionClaimNames.CLIENT_ID, "l238j323ds-23ij4")
.containsEntry(OAuth2IntrospectionClaimNames.EXPIRES_AT, Instant.ofEpochSecond(1419356238))
.containsEntry(OAuth2IntrospectionClaimNames.ISSUER, new URL("https://server.example.com/"))
.containsEntry(OAuth2IntrospectionClaimNames.ISSUER, "https://server.example.com/")
.containsEntry(OAuth2IntrospectionClaimNames.SCOPE, Arrays.asList("read", "write", "dolphin"))
.containsEntry(OAuth2IntrospectionClaimNames.SUBJECT, "Z5O3upPC88QrAjx00dis")
.containsEntry(OAuth2IntrospectionClaimNames.USERNAME, "jdoe")