-
Notifications
You must be signed in to change notification settings - Fork 364
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
Improved OIDC compliance #248
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general looks good. Pay attention to the type checks (e.g. date claims should be numbers). Left a couple of comments.
Co-Authored-By: Luciano Balmaceda <balmacedaluciano@gmail.com>
8925c64
to
1eb123a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a small comment that is irrelevant to this PR (existed before too). I'm approving the PR but I think we need to improve our docs. I didn't see anywhere mentioned what I can pass as an option (e.g. max_age) and there is no mention that the default leeway is 60 seconds.
if I'm wrong and this is documented, please point me to the doc :) I've checked the readme and this link: https://auth0.com/docs/libraries/auth0-spa-js
EDIT: I found out that we document the different options here: https://auth0.github.io/auth0-spa-js/interfaces/auth0clientoptions.html
The only thing missing is that the default leeway is 60 seconds.
} | ||
|
||
const leeway = options.leeway || 60; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we set default leeway to 60 seconds? There is no mention about it in our docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We decided to set the default leeway to 60 across the board to keep the check consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Let's include this piece of information here: https://auth0.github.io/auth0-spa-js/interfaces/auth0clientoptions.html#leeway
Description
This update improves the SDK support for OpenID Connect. In particular, it modifies the sign in verification phase by substituting backchannel based checks with id_token validation.
Testing