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

Update pyjwt to 1.5.0 #5203

Merged
merged 3 commits into from
Apr 26, 2017
Merged

Update pyjwt to 1.5.0 #5203

merged 3 commits into from
Apr 26, 2017

Conversation

pyup-bot
Copy link
Contributor

There's a new version of PyJWT available.
You are currently using 1.4.2. I have updated it to 1.5.0

These links might come in handy: PyPI | Changelog | Repo

Changelog

1.5.0

1.4.21.4.2


Fixed

  • A PEM-formatted key encoded as bytes could cause a TypeError to be raised 213

1.4.11.4.1


Fixed

  • Newer versions of Pytest could not detect warnings properly 182
  • Non-string 'kid' value now raises InvalidTokenError 174
  • jwt.decode(None) now gracefully fails with InvalidTokenError 183

Got merge conflicts? Close this PR and delete the branch. I'll create a new PR for you.

Happy merging! 🤖

Also add a new test for none-number `iat` values.
@EnTeQuAk
Copy link
Contributor

Summoning @kumar303 and @diox for opinions on this. PyJWT removed validation for future iat values in jpadilla/pyjwt#252 with quite a bit of discussion in jpadilla/pyjwt#190 resulting in the decision that it's not defined in the JWT standard so it's applications responsibility to implement that check.

I do see points of both argument-sides but think validating iat for future-dates kinda makes sense. So I added that check and updated the tests a bit to also check for new pyjwt functionality validating that iat is an actual number.

r?

@EnTeQuAk EnTeQuAk requested a review from diox April 26, 2017 13:45
@diox
Copy link
Member

diox commented Apr 26, 2017

I don't have a strong opinion regarding future iat values. @kumar303 ?

@kumar303
Copy link
Contributor

If these tests still pass then maybe it's ok? We might do our own iat validation.

I haven't read the discussion but as I recall, protecting against iat manipulation does prevent some time-oriented attacks. I don't recall exactly what those attacks are but before removing iat validation (maybe this patch doesn't remove it) we should make sure it's ok to do so.

# https://github.com/jpadilla/pyjwt/pull/252/
# `verify_iat` is still in options because pyjwt still validates
# that `iat` is a proper number.
if int(payload['iat']) > (now + api_settings.JWT_LEEWAY):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I see that you moved it over explicitly. I think that's a good thing to do, at least until we know can assess the usefulness of iat validation with security folks.

r+

@EnTeQuAk
Copy link
Contributor

Perfect, thanks for the comments! I'll go ahead and merge this then so we simply keep our current functionality in tact but upgrade to the new version to stay up-to-date.

@EnTeQuAk EnTeQuAk merged commit ee286c4 into master Apr 26, 2017
@EnTeQuAk EnTeQuAk deleted the pyup-update-pyjwt-1.4.2-to-1.5.0 branch April 26, 2017 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants