Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] MSAL should not attempt to validate the ID Token #656

Closed
bgavrilMS opened this issue Jan 25, 2024 · 4 comments · Fixed by #657
Closed

[Bug] MSAL should not attempt to validate the ID Token #656

bgavrilMS opened this issue Jan 25, 2024 · 4 comments · Fixed by #657

Comments

@bgavrilMS
Copy link
Member

bgavrilMS commented Jan 25, 2024

Describe the bug

WSL + Azure CLI uses MSAL for auth in a public client scenario.
Upon receiving an ID Token, MSAL validates it and fails due to invalid nbf with error "The ID token is not yet valid"

Root cause

Looks like MSAL Py tries to validate the id token as follows:

However, due to a clock skew, MSAL Py raises an error.

Clients should not validate the id token's nbf and exp claim. In case of access tokens, the STS returns an expiry duration in the token response to help clients cache the access tokens correctly.

At a minimum remove nbf and exp validation.
Ideally remove all token validation logic - other MSALs do not perform this and rely on having had an SSL connection with the token issuer.

@bgavrilMS
Copy link
Member Author

CC @jiasli

@rayluo
Copy link
Collaborator

rayluo commented Jan 25, 2024

That was not a bug. It was a feature, required by the JWT specs. Also, the OIDC specs defines ID token validation requirements which mentions SSL connection can only be used to replace the issuer check in step 6 but not the other steps.
Those specs even used the term "MUST" which has its specific meaning.

That being said, it is technically OK to choose a larger time skew allowance and still be specs-compliant. It is perhaps also fine if we choose to drop this feature.

Out of curiosity, what was the context of this ask? We have a reasonably small built-in clock skew allowance that works in the wild for years now. Occasionally I did run into that "ID token is not yet valid" error but that's typically when I was using an out-of-sync VM.

Note that, a machine with huge time skew is a problem in itself. The SSL stack would usually fail validation somewhere, without even providing a meaningful error.

Lastly, the context here is about how or whether to perform ID token validation. Access token's specs (OAuth2) is completely different, thus not applicable in the ID token context.

@rayluo
Copy link
Collaborator

rayluo commented Jan 25, 2024

How about change the current error message to use local time, such as "The ID token is not yet valid. Make sure your computer's time and time zone are both correct. Current time on your machine is 12:34PM. The id_token has an issue time at 13:45PM".

@rayluo
Copy link
Collaborator

rayluo commented Jan 26, 2024

UPDATE: After some thought experiment, the next version of MSAL Python will not validate ID token's time.

@rayluo rayluo linked a pull request Jan 29, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants