Validate ID Token Issuer #8357
Validate ID Token Issuer #8357
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
| @@ -143,6 +146,13 @@ public void setClock(Clock clock) { | |||
| this.clock = clock; | |||
| } | |||
|
|
|||
| private boolean issuerMatchesMetadata(Jwt idToken) { | |||
| String metadataIssuer = (String) clientRegistration.getProviderDetails().getConfigurationMetadata() | |||
| .get("issuer"); | |||
There was a problem hiding this comment.
The issuer claim is iss. You can use JwtClaimNames.ISS.
There was a problem hiding this comment.
Are you sure? AFAIK the claim is called issuer in the provider metadata and iss in the id token (https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata).
There was a problem hiding this comment.
Of course, my mistake. Thanks for correcting me @mengelbrecht !
| @@ -68,7 +69,9 @@ public OAuth2TokenValidatorResult validate(Jwt idToken) { | |||
|
|
|||
| // 2. The Issuer Identifier for the OpenID Provider (which is typically obtained during Discovery) | |||
| // MUST exactly match the value of the iss (issuer) Claim. | |||
| // TODO Depends on gh-4413 | |||
| if (!issuerMatchesMetadata(idToken)) { | |||
There was a problem hiding this comment.
Can you move the validation check here and remove the method.
| @@ -143,6 +146,13 @@ public void setClock(Clock clock) { | |||
| this.clock = clock; | |||
| } | |||
|
|
|||
| private boolean issuerMatchesMetadata(Jwt idToken) { | |||
| String metadataIssuer = (String) clientRegistration.getProviderDetails().getConfigurationMetadata() | |||
There was a problem hiding this comment.
We prefix this. for private members -> this.clientRegistration
| * the validation must fail | ||
| */ | ||
| Map<String, Object> configurationMetadata = new HashMap<>(); | ||
| configurationMetadata.put("issuer", "https://issuer.somethingelse.com"); |
| * the validation must fail | ||
| */ | ||
| Map<String, Object> configurationMetadata = new HashMap<>(); | ||
| configurationMetadata.put("issuer", "https://issuer.example.com"); |
| public void validateWhenMetadataIssuerMatchThenNoErrors() { | ||
| /* | ||
| * When the issuer is set in the provider metadata, and it does not match the issuer in the ID Token, | ||
| * the validation must fail |
There was a problem hiding this comment.
validation must fail -> validation will pass
jgrandja
added
in: oauth2
When the issuer is set in the provider metadata, we validate the iss field of the ID Token against it. The OpenID Connect Specification says this must always be validated. But this would be a breaking change for applications configured other than with ClientRegistrations.fromOidcIssuerLocation(issuer). This will be done later with spring-projects#8326 fixes spring-projectsgh-8321
furti
force-pushed
the
gh-8321-issuer-validation
branch
from
b9d029e to
f15e84b
Compare
|
@jgrandja I implemented your changes and pushed the commit again. I moved the check inline, added the "this." prefix and fixed the wrong javadoc comment in the Test (copy paste error). |
|
Thanks for the updates @furti. This is now in master! |
When the issuer is set in the provider metadata, we validate the iss
field of the ID Token against it.
The OpenID Connect Specification says this must always be validated.
But this would be a breaking change for applications configured other
than with ClientRegistrations.fromOidcIssuerLocation(issuer). This will
be done later with #8326
fixes gh-8321
Running gradlew
I tried to run
./gradlew buildbut 3 Tests failed in classes I did not even touch. But the tests for the class I changed passed, so I think the changes should be fine.If you want me to give the build another try, please let me know.
CLA
I signed the CLA.