Skip to content

Commit dd43d91

Browse files
ascopesjzheaux
ascopes
authored and
jzheaux
committed
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.
1 parent fe274e7 commit dd43d91

File tree

8 files changed

+80
-56
lines changed

8 files changed

+80
-56
lines changed

‎oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/NimbusOpaqueTokenIntrospector.java

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.springframework.security.oauth2.server.resource.introspection;
1818

1919
import java.net.URI;
20-
import java.net.URL;
2120
import java.time.Instant;
2221
import java.util.ArrayList;
2322
import java.util.Collection;
@@ -207,7 +206,25 @@ private OAuth2AuthenticatedPrincipal convertClaimsSet(TokenIntrospectionSuccessR
207206
claims.put(OAuth2TokenIntrospectionClaimNames.IAT, iat);
208207
}
209208
if (response.getIssuer() != null) {
210-
claims.put(OAuth2TokenIntrospectionClaimNames.ISS, issuer(response.getIssuer().getValue()));
209+
// RFC-7662 page 7 directs users to RFC-7519 for defining the values of these
210+
// issuer fields.
211+
// https://datatracker.ietf.org/doc/html/rfc7662#page-7
212+
//
213+
// RFC-7519 page 9 defines issuer fields as being 'case-sensitive' strings
214+
// containing
215+
// a 'StringOrURI', which is defined on page 5 as being any string, but
216+
// strings containing ':'
217+
// should be treated as valid URIs.
218+
// https://datatracker.ietf.org/doc/html/rfc7519#section-2
219+
//
220+
// It is not defined however as to whether-or-not normalized URIs should be
221+
// treated as the same literal
222+
// value. It only defines validation itself, so to avoid potential ambiguity
223+
// or unwanted side effects that
224+
// may be awkward to debug, we do not want to manipulate this value. Previous
225+
// versions of Spring Security
226+
// would *only* allow valid URLs, which is not what we wish to achieve here.
227+
claims.put(OAuth2TokenIntrospectionClaimNames.ISS, response.getIssuer().getValue());
211228
}
212229
if (response.getNotBeforeTime() != null) {
213230
claims.put(OAuth2TokenIntrospectionClaimNames.NBF, response.getNotBeforeTime().toInstant());
@@ -222,14 +239,4 @@ private OAuth2AuthenticatedPrincipal convertClaimsSet(TokenIntrospectionSuccessR
222239
return new OAuth2IntrospectionAuthenticatedPrincipal(claims, authorities);
223240
}
224241

225-
private URL issuer(String uri) {
226-
try {
227-
return new URL(uri);
228-
}
229-
catch (Exception ex) {
230-
throw new OAuth2IntrospectionException(
231-
"Invalid " + OAuth2TokenIntrospectionClaimNames.ISS + " value: " + uri);
232-
}
233-
}
234-
235242
}

‎oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/NimbusReactiveOpaqueTokenIntrospector.java

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.springframework.security.oauth2.server.resource.introspection;
1818

1919
import java.net.URI;
20-
import java.net.URL;
2120
import java.time.Instant;
2221
import java.util.ArrayList;
2322
import java.util.Collection;
@@ -174,7 +173,25 @@ private OAuth2AuthenticatedPrincipal convertClaimsSet(TokenIntrospectionSuccessR
174173
claims.put(OAuth2TokenIntrospectionClaimNames.IAT, iat);
175174
}
176175
if (response.getIssuer() != null) {
177-
claims.put(OAuth2TokenIntrospectionClaimNames.ISS, issuer(response.getIssuer().getValue()));
176+
// RFC-7662 page 7 directs users to RFC-7519 for defining the values of these
177+
// issuer fields.
178+
// https://datatracker.ietf.org/doc/html/rfc7662#page-7
179+
//
180+
// RFC-7519 page 9 defines issuer fields as being 'case-sensitive' strings
181+
// containing
182+
// a 'StringOrURI', which is defined on page 5 as being any string, but
183+
// strings containing ':'
184+
// should be treated as valid URIs.
185+
// https://datatracker.ietf.org/doc/html/rfc7519#section-2
186+
//
187+
// It is not defined however as to whether-or-not normalized URIs should be
188+
// treated as the same literal
189+
// value. It only defines validation itself, so to avoid potential ambiguity
190+
// or unwanted side effects that
191+
// may be awkward to debug, we do not want to manipulate this value. Previous
192+
// versions of Spring Security
193+
// would *only* allow valid URLs, which is not what we wish to achieve here.
194+
claims.put(OAuth2TokenIntrospectionClaimNames.ISS, response.getIssuer().getValue());
178195
}
179196
if (response.getNotBeforeTime() != null) {
180197
claims.put(OAuth2TokenIntrospectionClaimNames.NBF, response.getNotBeforeTime().toInstant());
@@ -190,16 +207,6 @@ private OAuth2AuthenticatedPrincipal convertClaimsSet(TokenIntrospectionSuccessR
190207
return new OAuth2IntrospectionAuthenticatedPrincipal(claims, authorities);
191208
}
192209

193-
private URL issuer(String uri) {
194-
try {
195-
return new URL(uri);
196-
}
197-
catch (Exception ex) {
198-
throw new OAuth2IntrospectionException(
199-
"Invalid " + OAuth2TokenIntrospectionClaimNames.ISS + " value: " + uri);
200-
}
201-
}
202-
203210
private OAuth2IntrospectionException onError(Throwable ex) {
204211
return new OAuth2IntrospectionException(ex.getMessage(), ex);
205212
}

‎oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospector.java

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.springframework.security.oauth2.server.resource.introspection;
1818

1919
import java.net.URI;
20-
import java.net.URL;
2120
import java.time.Instant;
2221
import java.util.ArrayList;
2322
import java.util.Arrays;
@@ -187,7 +186,25 @@ private OAuth2AuthenticatedPrincipal convertClaimsSet(Map<String, Object> claims
187186
(k, v) -> Instant.ofEpochSecond(((Number) v).longValue()));
188187
claims.computeIfPresent(OAuth2TokenIntrospectionClaimNames.IAT,
189188
(k, v) -> Instant.ofEpochSecond(((Number) v).longValue()));
190-
claims.computeIfPresent(OAuth2TokenIntrospectionClaimNames.ISS, (k, v) -> issuer(v.toString()));
189+
// RFC-7662 page 7 directs users to RFC-7519 for defining the values of these
190+
// issuer fields.
191+
// https://datatracker.ietf.org/doc/html/rfc7662#page-7
192+
//
193+
// RFC-7519 page 9 defines issuer fields as being 'case-sensitive' strings
194+
// containing
195+
// a 'StringOrURI', which is defined on page 5 as being any string, but strings
196+
// containing ':'
197+
// should be treated as valid URIs.
198+
// https://datatracker.ietf.org/doc/html/rfc7519#section-2
199+
//
200+
// It is not defined however as to whether-or-not normalized URIs should be
201+
// treated as the same literal
202+
// value. It only defines validation itself, so to avoid potential ambiguity or
203+
// unwanted side effects that
204+
// may be awkward to debug, we do not want to manipulate this value. Previous
205+
// versions of Spring Security
206+
// would *only* allow valid URLs, which is not what we wish to achieve here.
207+
claims.computeIfPresent(OAuth2TokenIntrospectionClaimNames.ISS, (k, v) -> v.toString());
191208
claims.computeIfPresent(OAuth2TokenIntrospectionClaimNames.NBF,
192209
(k, v) -> Instant.ofEpochSecond(((Number) v).longValue()));
193210
Collection<GrantedAuthority> authorities = new ArrayList<>();
@@ -204,14 +221,4 @@ private OAuth2AuthenticatedPrincipal convertClaimsSet(Map<String, Object> claims
204221
return new OAuth2IntrospectionAuthenticatedPrincipal(claims, authorities);
205222
}
206223

207-
private URL issuer(String uri) {
208-
try {
209-
return new URL(uri);
210-
}
211-
catch (Exception ex) {
212-
throw new OAuth2IntrospectionException(
213-
"Invalid " + OAuth2TokenIntrospectionClaimNames.ISS + " value: " + uri);
214-
}
215-
}
216-
217224
}

‎oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospector.java

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.springframework.security.oauth2.server.resource.introspection;
1818

1919
import java.net.URI;
20-
import java.net.URL;
2120
import java.time.Instant;
2221
import java.util.ArrayList;
2322
import java.util.Arrays;
@@ -146,7 +145,25 @@ private OAuth2AuthenticatedPrincipal convertClaimsSet(Map<String, Object> claims
146145
(k, v) -> Instant.ofEpochSecond(((Number) v).longValue()));
147146
claims.computeIfPresent(OAuth2IntrospectionClaimNames.ISSUED_AT,
148147
(k, v) -> Instant.ofEpochSecond(((Number) v).longValue()));
149-
claims.computeIfPresent(OAuth2IntrospectionClaimNames.ISSUER, (k, v) -> issuer(v.toString()));
148+
// RFC-7662 page 7 directs users to RFC-7519 for defining the values of these
149+
// issuer fields.
150+
// https://datatracker.ietf.org/doc/html/rfc7662#page-7
151+
//
152+
// RFC-7519 page 9 defines issuer fields as being 'case-sensitive' strings
153+
// containing
154+
// a 'StringOrURI', which is defined on page 5 as being any string, but strings
155+
// containing ':'
156+
// should be treated as valid URIs.
157+
// https://datatracker.ietf.org/doc/html/rfc7519#section-2
158+
//
159+
// It is not defined however as to whether-or-not normalized URIs should be
160+
// treated as the same literal
161+
// value. It only defines validation itself, so to avoid potential ambiguity or
162+
// unwanted side effects that
163+
// may be awkward to debug, we do not want to manipulate this value. Previous
164+
// versions of Spring Security
165+
// would *only* allow valid URLs, which is not what we wish to achieve here.
166+
claims.computeIfPresent(OAuth2IntrospectionClaimNames.ISSUER, (k, v) -> v.toString());
150167
claims.computeIfPresent(OAuth2IntrospectionClaimNames.NOT_BEFORE,
151168
(k, v) -> Instant.ofEpochSecond(((Number) v).longValue()));
152169
Collection<GrantedAuthority> authorities = new ArrayList<>();
@@ -163,16 +180,6 @@ private OAuth2AuthenticatedPrincipal convertClaimsSet(Map<String, Object> claims
163180
return new OAuth2IntrospectionAuthenticatedPrincipal(claims, authorities);
164181
}
165182

166-
private URL issuer(String uri) {
167-
try {
168-
return new URL(uri);
169-
}
170-
catch (Exception ex) {
171-
throw new OAuth2IntrospectionException(
172-
"Invalid " + OAuth2IntrospectionClaimNames.ISSUER + " value: " + uri);
173-
}
174-
}
175-
176183
private OAuth2IntrospectionException onError(Throwable ex) {
177184
return new OAuth2IntrospectionException(ex.getMessage(), ex);
178185
}

‎oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/NimbusOpaqueTokenIntrospectorTests.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.springframework.security.oauth2.server.resource.introspection;
1818

1919
import java.io.IOException;
20-
import java.net.URL;
2120
import java.time.Instant;
2221
import java.util.Arrays;
2322
import java.util.Base64;
@@ -146,7 +145,7 @@ public void introspectWhenActiveTokenThenOk() throws Exception {
146145
Arrays.asList("https://protected.example.net/resource"))
147146
.containsEntry(OAuth2TokenIntrospectionClaimNames.CLIENT_ID, "l238j323ds-23ij4")
148147
.containsEntry(OAuth2TokenIntrospectionClaimNames.EXP, Instant.ofEpochSecond(1419356238))
149-
.containsEntry(OAuth2TokenIntrospectionClaimNames.ISS, new URL("https://server.example.com/"))
148+
.containsEntry(OAuth2TokenIntrospectionClaimNames.ISS, "https://server.example.com/")
150149
.containsEntry(OAuth2TokenIntrospectionClaimNames.SCOPE, Arrays.asList("read", "write", "dolphin"))
151150
.containsEntry(OAuth2TokenIntrospectionClaimNames.SUB, "Z5O3upPC88QrAjx00dis")
152151
.containsEntry(OAuth2TokenIntrospectionClaimNames.USERNAME, "jdoe")

‎oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/NimbusReactiveOpaqueTokenIntrospectorTests.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.springframework.security.oauth2.server.resource.introspection;
1818

1919
import java.io.IOException;
20-
import java.net.URL;
2120
import java.time.Instant;
2221
import java.util.Arrays;
2322
import java.util.Base64;
@@ -117,7 +116,7 @@ public void authenticateWhenActiveTokenThenOk() throws Exception {
117116
Arrays.asList("https://protected.example.net/resource"))
118117
.containsEntry(OAuth2TokenIntrospectionClaimNames.CLIENT_ID, "l238j323ds-23ij4")
119118
.containsEntry(OAuth2TokenIntrospectionClaimNames.EXP, Instant.ofEpochSecond(1419356238))
120-
.containsEntry(OAuth2TokenIntrospectionClaimNames.ISS, new URL("https://server.example.com/"))
119+
.containsEntry(OAuth2TokenIntrospectionClaimNames.ISS, "https://server.example.com/")
121120
.containsEntry(OAuth2TokenIntrospectionClaimNames.SCOPE, Arrays.asList("read", "write", "dolphin"))
122121
.containsEntry(OAuth2TokenIntrospectionClaimNames.SUB, "Z5O3upPC88QrAjx00dis")
123122
.containsEntry(OAuth2TokenIntrospectionClaimNames.USERNAME, "jdoe")

‎oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospectorTests.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.springframework.security.oauth2.server.resource.introspection;
1818

1919
import java.io.IOException;
20-
import java.net.URL;
2120
import java.time.Instant;
2221
import java.util.Arrays;
2322
import java.util.Base64;
@@ -150,7 +149,7 @@ public void introspectWhenActiveTokenThenOk() throws Exception {
150149
Arrays.asList("https://protected.example.net/resource"))
151150
.containsEntry(OAuth2TokenIntrospectionClaimNames.CLIENT_ID, "l238j323ds-23ij4")
152151
.containsEntry(OAuth2TokenIntrospectionClaimNames.EXP, Instant.ofEpochSecond(1419356238))
153-
.containsEntry(OAuth2TokenIntrospectionClaimNames.ISS, new URL("https://server.example.com/"))
152+
.containsEntry(OAuth2TokenIntrospectionClaimNames.ISS, "https://server.example.com/")
154153
.containsEntry(OAuth2TokenIntrospectionClaimNames.SCOPE, Arrays.asList("read", "write", "dolphin"))
155154
.containsEntry(OAuth2TokenIntrospectionClaimNames.SUB, "Z5O3upPC88QrAjx00dis")
156155
.containsEntry(OAuth2TokenIntrospectionClaimNames.USERNAME, "jdoe")

‎oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospectorTests.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.springframework.security.oauth2.server.resource.introspection;
1818

1919
import java.io.IOException;
20-
import java.net.URL;
2120
import java.time.Instant;
2221
import java.util.Arrays;
2322
import java.util.Base64;
@@ -122,7 +121,7 @@ public void authenticateWhenActiveTokenThenOk() throws Exception {
122121
Arrays.asList("https://protected.example.net/resource"))
123122
.containsEntry(OAuth2IntrospectionClaimNames.CLIENT_ID, "l238j323ds-23ij4")
124123
.containsEntry(OAuth2IntrospectionClaimNames.EXPIRES_AT, Instant.ofEpochSecond(1419356238))
125-
.containsEntry(OAuth2IntrospectionClaimNames.ISSUER, new URL("https://server.example.com/"))
124+
.containsEntry(OAuth2IntrospectionClaimNames.ISSUER, "https://server.example.com/")
126125
.containsEntry(OAuth2IntrospectionClaimNames.SCOPE, Arrays.asList("read", "write", "dolphin"))
127126
.containsEntry(OAuth2IntrospectionClaimNames.SUBJECT, "Z5O3upPC88QrAjx00dis")
128127
.containsEntry(OAuth2IntrospectionClaimNames.USERNAME, "jdoe")

0 commit comments

Comments
 (0)