openid / AppAuth-Android

Android client SDK for communicating with OAuth 2.0 and OpenID Connect providers.
https://openid.github.io/AppAuth-Android
Apache License 2.0
2.82k stars 881 forks source link

ID Token Issuer is only validated if Discovery was used #458

Open KarlBusse opened 5 years ago

KarlBusse commented 5 years ago

Currently, ID Token Issuer is only validated if Discovery was used... https://github.com/openid/AppAuth-Android/blob/f12592517140ecfe91a411090601b4264ddb641f/library/java/net/openid/appauth/IdToken.java#L119

My understanding of the spec is that Discovery is optional...

This information is normally obtained via Discovery, as described in OpenID Connect Discovery 1.0 [OpenID.Discovery], or may be obtained via other mechanisms.

The following is what the OpenID Connect Core spec says in section 3.1.3.7 about verifying the Issuer in the ID Token...

The Issuer Identifier for the OpenID Provider (which is typically obtained during Discovery) MUST exactly match the value of the iss (issuer) Claim.

Since Discovery is optional, I believe "typically obtained" may be just a helpful hint? It seems to me that using Discovery was not intended to be a required precondition for performing ID Token Issuer validation.

carstenhag commented 3 years ago

~~Just stumbled across this because I tried to upgrade to 0.8.0.
For us a specific IDM provider (GCDM, the IDM from the BMW Group) does not seem to have this field filled out. This means that we can't upgrade to 0.8.0 as there is no way to disable the ID Token validation either (going to create a ticket for that).~~

Edit: Sorry, had not looked at the following code lines. If discoveryUrl is null, it gets skipped. For us the if (!TextUtils.equals(this.nonce, expectedNonce)) { check fails.