added an extra validation for exp claims #2
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
jonnyzzz
requested changes
Dec 4, 2020
jonnyzzz
reviewed
Dec 7, 2020
| @@ -100,6 +100,12 @@ async function handler(request) { | |||
| return; | |||
| } | |||
| console.log(jwtKey.modeName + ": payload " + JSON.stringify(payload, null, ' ')); | |||
| // JBA id tokens are valid for an hour and JBT ones for 10 minutes, so the common threshold is set at 3600 seconds | |||
| if (Math.floor(Date.now() / 1000) + 3600 < payload.exp) { | |||
There was a problem hiding this comment.
If I do the match right - 1 hour = 3600 seconds. We a change (thanks to time drift) to see a token that is exactly 1 hour to live, so the test would not pass. Please tell me if you could set a longer time-bound here, just in case, say 2 hours?
There was a problem hiding this comment.
Is this code is safe for time zones? I'm not sure about Date.now() and payload.exp in that respect.
There was a problem hiding this comment.
I guess 2 hours is a large enough boundary, especially when Space's tokens are valid for 10 minutes only :)
Indeed, it is better to validate if the difference between iat and exp claims is greater then 7200 seconds. We will surely be time zone independent then.
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.
No description provided.