Fixed ID token nonce claim validation

[jo-carter]

Previously, ID token nonce claim validation was skipped in all cases due to lack of detection of if the session was a new session.

[lcrilly]

Presumably the global variable is not persisting across internal redirects (as was the intention)?
Perhaps js_var is worth considering to avoid performing the newSession check twice?

[jo-carter]

Presumably the global variable is not persisting across internal redirects (as was the intention)?

@lcrilly That's right.

Perhaps js_var is worth considering to avoid performing the newSession check twice?

Yep, makes sense. Please see the changes in the latest push.

[jo-carter]

@lcrilly Actually, as was pointed out by interested parties out of band - that would not work, as the js_var will not persist beyond the 302 redirect to the IDP that occurs later in the auth function.

Reverting the commit back the original, and also converting newSession to a local variable in id_token_validation function, as it no longer needs to be global.

[ag-TJNII]

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

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.

Should the nonce check even be wrapped in the newSession if()? Skimming the docs I can't seem to find a definitive answer whether the renewal response contains the original nonce. I'm assuming there's cases where it doesn't. Looking at the spec I'm wondering if this should:

• Always validate nonce if it's present
• Ensure it is present on new sessions
• Tolerate missing on renewals

https://learn.microsoft.com/en-us/entra/identity-platform/id-tokens says Nonce: the nonce claim in the payload must match the nonce parameter passed into the /authorize endpoint during the initial request. implying that it will be returned on renewal, but I haven't confirmed that.

[jo-carter]

@ag-TJNII

Should the nonce check even be wrapped in the newSession if()? Skimming the docs I can't seem to find a definitive answer whether the renewal response contains the original nonce. I'm assuming there's cases where it doesn't.

It is ambiguous in the spec, but I believe you are correct that it is intended. I see from issues on other projects that some do include nonce, and some do not include nonce claim in this case, so keeping it as a warning makes sense. (see resolved conversation above)