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

IdTokenVerifier does not verify the signature of ID Token #786

Closed
tamjidrahat opened this issue Dec 20, 2021 · 4 comments · Fixed by #861
Closed

IdTokenVerifier does not verify the signature of ID Token #786

tamjidrahat opened this issue Dec 20, 2021 · 4 comments · Fixed by #861
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@tamjidrahat
Copy link

The verify method in IdTokenVerifier does not validate the signature before verifying the claims (e.g., iss, aud, etc.). This is also a violation of the ID Token Validation steps mentioned in the current OpenID Connect Spec (https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation).

The spec requires to validate the signature of ID token for apps that cannot guarantee TLS communication, which is the case for this library. This library initiates a local server that can run on any client machine without TLS support. So, it is critical to validate the signature, before trusting the claims of an ID token, which can be received from a malicious service provider.

Thanks.

@Neenu1995 Neenu1995 added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Dec 20, 2021
@lesv lesv added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Dec 28, 2021
@lesv
Copy link
Contributor

lesv commented Dec 28, 2021

@silvolu FYI

@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Dec 28, 2021
@lesv lesv removed the 🚨 This issue needs some love. label Dec 30, 2021
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Dec 30, 2021
@TimurSadykov
Copy link
Contributor

The issue confirmed, investigating the best way to fix it

@Neenu1995 Neenu1995 removed the 🚨 This issue needs some love. label Jan 10, 2022
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Jan 10, 2022
@TimurSadykov TimurSadykov removed the 🚨 This issue needs some love. label Feb 7, 2022
@TimurSadykov
Copy link
Contributor

fix in progress

@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Feb 7, 2022
@TimurSadykov TimurSadykov added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Feb 28, 2022
@TimurSadykov
Copy link
Contributor

fix ETA Mar 1st, dropping priority according to definitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants