Token handling #38
Merged
Token handling #38
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pvizeli
reviewed
Feb 26, 2021
…so it can be reused by token verification.
JWTs are made of footguns. Follow the recommendations in Cognito's
developer guide and avoid some pitfalls with the defaults of the JWT
library in use (jose).
• Actually verify audience and issuer claims against expected values
instead of comparing the values to themselves.
• Require audience, issuer, and expiration claims to be present, as
otherwise the JWT library in use will silently skip verification.
• Check token_use claim matches after claim verification to avoid
using unverified claims at all.
Ensures that the token attributes are accessible, up-to-date, and valid.
It can be useful to call this method after creating a Cognito instance where you've provided externally-remembered token values.
Very handy to have these available so you can use the default claims like "cognito:groups" and any custom claims. Resolves NabuCasa#19.
tsibley
force-pushed
the
token-handling
branch
from
6f329c2 to
33ad7b8
Compare
As requested during review. I avoided using f-strings initially because I did not see them used elsewhere in the codebase. Co-authored-by: Pascal Vizeli <pascal.vizeli@syshack.ch>
tsibley
force-pushed
the
token-handling
branch
from
ab1de76 to
aa22fdb
Compare
|
Thanks! Repushed with change to f-strings. |
mdecuir
pushed a commit
to MDmetrix/pycognito
that referenced
this pull request
Jun 30, 2021
* Format exception descriptions which contained unformatted interpolations
* Refactor out user pool base URL templating
…so it can be reused by token verification.
* Fix several weaknesses in token verification
JWTs are made of footguns. Follow the recommendations in Cognito's
developer guide and avoid some pitfalls with the defaults of the JWT
library in use (jose).
• Actually verify audience and issuer claims against expected values
instead of comparing the values to themselves.
• Require audience, issuer, and expiration claims to be present, as
otherwise the JWT library in use will silently skip verification.
• Check token_use claim matches after claim verification to avoid
using unverified claims at all.
* Consistently set and verify tokens across assignment codepaths
Ensures that the token attributes are accessible, up-to-date, and valid.
* Add verify_tokens() method to manually (re-)check current tokens
It can be useful to call this method after creating a Cognito instance
where you've provided externally-remembered token values.
* Save the verified claims for the id and access token
Very handy to have these available so you can use the default claims
like "cognito:groups" and any custom claims.
Resolves NabuCasa#19.
* README: Correct two typos
* Switch to f-strings for interpolation
As requested during review. I avoided using f-strings initially because
I did not see them used elsewhere in the codebase.
Co-authored-by: Pascal Vizeli <pascal.vizeli@syshack.ch>
Co-authored-by: Pascal Vizeli <pascal.vizeli@syshack.ch>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please see the commit messages for details.