BUG FIX: Only require nonce in id_token when also passed in body

[DanMossa]

Context

Receiving a nonce is optional in the open id spec. Google has 2 apis, the older one does not return a nonce while the newer one does.
There's a risk of replay attack if there's no nonce, but that's why my PR makes passing the nonce optional ONLY if it's missing from the returned jwt

What kind of change does this PR introduce?

Fix(?) When using native auth on some platforms like Flutter's sign in with google, the idtoken that is returned does not contain a nonce.

What is the current behavior?

The current behavior requires a nonce inside the id_token as well as a nonce passed.

What is the new behavior?

If there is a nonce in the id_token, then a nonce is required in the body and vice versa.

Fixes and adds support for
#412
supabase/gotrue-dart#27
supabase/supabase-flutter#5

[kangmingtay]

Hey @DanMossa, unfortunately, i think we need to handle this check on a case-by-case basis for each OIDC provider. If we omit the check for the nonce when it's missing in the id_token for all providers, then this would be detrimental for some providers (e.g. apple) that requires nonce validation.

[DanMossa]

@kangmingtay , does that mean as it currently is now, you aren't able to use apple sign in either because a returned nonce is optional?

Looking more into how sign in with apple works via this link
https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_js/incorporating_sign_in_with_apple_into_other_platforms

The nonce field is present only when a nonce is set while making the initial authorization request. If it is not set at that point, it is absent from the JWT body of the id_token.

This is aligned with what I'm changing, right? And what I'm changing it to is also aligned with the OpenID spec:

https://openid.net/specs/openid-connect-core-1_0.html#IDToken

nonce
String value used to associate a Client session with an ID Token, and to mitigate replay attacks. The value is passed through unmodified from the Authentication Request to the ID Token. If present in the ID Token, Clients MUST verify that the nonce Claim Value is equal to the value of the nonce parameter sent in the Authentication Request. If present in the Authentication Request, Authorization Servers MUST include a nonce Claim in the ID Token with the Claim Value being the nonce value sent in the Authentication Request. Authorization Servers SHOULD perform no other processing on nonce values used. The nonce value is a case sensitive string.

• If a nonce is present in the authentication request, then the returned ID Token has a nonce.

• If a nonce is present in the ID Token, then we must verify the nonce, and we can do that by sending over the idToken and the nonce to this endpoint.

This means that for apple and google, if we pass a nonce to authenticate, then we'll send over the idtoken and nonce like we currently do, but if we don't pass a nonce to authenticate, then that means we don't get a nonce in the IDToken which means there's no nonce to validate.

[DanMossa]

@kangmingtay
Hey! I was wondering if you had any other thoughts on this.

[DanMossa]

Hey @kangmingtay ! I was wondering if it would be possible to take a look at this PR since this is a critical bug that is breaking the openID spec.

[kangmingtay]

Hey @DanMossa, sorry for the late reply, we just finished our launch week and i took some time off work after.

does that mean as it currently is now, you aren't able to use apple sign in either because a returned nonce is optional?

nope, you can still use apple sign-in but you need to ensure that you generate a nonce when you first sign the user into their apple account using Apple's authentication services framework. This is similar to how you would sign-in with apple with firebase using an id token.

The nonce field is present only when a nonce is set while making the initial authorization request. If it is not set at that point, it is absent from the JWT body of the id_token.

Yup you are correct here but it is important that we enforce validating the nonce for oidc providers that allow the developer to pass a nonce in the initial sign-in request.

but if we don't pass a nonce to authenticate, then that means we don't get a nonce in the IDToken which means there's no nonce to validate.

Hmm if the nonce is optional, how can we trust the client calling this endpoint with the user's id token? For example,

1. Client A performs sign-in request and gets back an id token.
2. Client A calls this endpoint on gotrue with the id token to sign in.
3. Client B (malicious) manages to obtain the id token in (1).
4. Client B calls this endpoint on gotrue with the same id token to sign in. (how do we detect this here?)

If (1) was performed with a nonce, the id token would contain the hashed nonce value. Steps 3 and 4 would be impossible because there is no way for Client B to know what the nonce is (they can get the hashed nonce from the id token but it's impossible to reverse a hash so they wouldn't know the original value of the nonce).

Tbh, I'm not too sure what should be the correct approach for oidc providers that don't allow you to send a nonce in the initial sign-in request. Maybe we shouldn't allow signing in with those providers on gotrue since it's insecure (?). I'll try to figure out how other auth services (auth0 and firebase) handle this but any suggestions you might have will be very much appreciated!

[DanMossa]

@kangmingtay No worries about the delay! I didn't realize you took off.

Yup you are correct here but it is important that we enforce validating the nonce for oidc providers that allow the developer to pass a nonce in the initial sign-in request

I disagree. I don't think it's the job of a third party package (supabase) to enforce how the developer should generate their idtoken. Especially since the spec allows nonce to be optional. It is the job of Supabase to enforce that if a nonce is present in either the token or argument, that it matches the other.

Hmm if the nonce is optional, how can we trust the client calling this endpoint with the user's id token?

You can't. But once again this is the job of the developer. A developer who generates a valid openID token should understand what a nonce is or should at least be able to follow the documentation of whatever package they're using to generate idTokens.

For example, Apple requires a nonce to be generated and this makes sense because they're the ones doing the authentication and they want to make nonce required.
Google has multiple APIs to create idTokens and one of them does not have a way to pass in a nonce. This makes sense because they're the ones doing the authentication.

Both of these are valid and implementing OpenID into Supabase should mean that Supabase accepts a valid openID token, not that it enforces what they think a valid openID token is.

Maybe we shouldn't allow signing in with those providers on gotrue since it's insecure

I think it would be silly to ban providers that offer a way to create openIDs without nonces. This would effectively prevent Flutter from using their native authentication until the package migrates to a different API which could take 6 months to a year.

I'll try to figure out how other auth services (auth0 and firebase) handle this

Sign in with Apple and authenticate with Firebase
A nonce is required for Firebase when using Sign in with Apple.

I couldn't find any mention of Firebase requiring a Nonce when using Android's sign in with google.

[kangmingtay]

I don't think it's the job of a third party package (supabase) to enforce how the developer should generate their idtoken. Especially since the spec allows nonce to be optional. It is the job of Supabase to enforce that if a nonce is present in either the token or argument, that it matches the other.

You are right, but just like how firebase requires a nonce for sign in with apple and not for google, the nonce requirement isn't optional for all providers, it's conditional depending on the provider. That's the reason why i think we should handle this by a case-by-case basis rather than making it optional for all providers.

For example: If you attempt to use sign-in with apple without a nonce, firebase would throw an error. However, based on your PR, as long as there is no nonce in the id token, gotrue would not require a nonce passed in the body so this would allow the sign-in with apple flow to succeed.

[DanMossa]

I could be wrong but it kind of sounds like you may be a little confused on where the nonce takes place?

Apple is the one that is requiring you to provide a nonce if you use their service. It is impossible to receive an idtoken from Apple without a nonce.

We aren't the one that have to make it case by case. We don't do any verification with any of the providers. We just make sure that the idtoken is valid following the openID spec.

We aren't the ones that have to make sure apple users provide us a nonce because it's apple that's the one providing the idToken. We're just verifying it.

If a provider requires a nonce then that means a nonce is in the idtoken and we should verify that.
If a provider does not require a nonce then that means a nonce is not in the idtoken and we don't need to verify that.

Supabase isn't the one that is creating this idToken that's why we don't require a nonce.

The flow is

1. We verify identity with provider. Some require nonce and some don't.
2. The provider decides to authenticate and returns back an idToken.
3. User sends authenticated idtoken to supabase.
4. Supabase should verify that this idToken is valid.

Supabase does many checks to verify the idtoken is valid. But one of the bugs is that supabase incorrectly requires a nonce in the idtoken.

[anggoran]

I think what Dan has said is true, and his PR is similar to what Firebase did:

Optional Nonce for Oauth in Flutter FirebaseAuth Android: Code Line 216
Optional Nonce for Oauth in Flutter FirebaseAuth iOS: Code Line 1137
The one that decide to return idToken or not seems should be the OAuth providers itself, because they are the one that sign JWT. It means that if provider requires nonce but we did not pass to them, gotrue will return an error because the provider does not return idToken, since idToken is required in gotrue

[kangmingtay]

Ah ok i see where you're coming from now, yeah i think i was slightly confused because i thought that it was optional (in apple's case) to provide a nonce on the initial authorisation request to obtain an id token based on #430 (comment) . But it seems like apple always require a nonce in order to obtain an id token.

Also, thanks for taking the time to clarify and explain this to me and the rest of the community and for continuing to follow up on the discussion even throughout my absence and delay in replies previously - it has been extremely helpful.

[github-actions]

🎉 This PR is included in version 2.6.20 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[DanMossa]

@kangmingtay you're incredible. Thank you for merging this in.
And of course! Nobody knows how computers work so I'm also figuring it out as we go and I was hope I didn't come off rude or anything!
Thanks again!

[DanMossa]

@kangmingtay
When do these changes propagate to the hosted supabase instances?

[k0shk0sh]

Does that mean, we can finally integrate social auth on mobile?

---

Edit: can confirm the fix working.