8000 PyJWT >= 1.5.1 exception fixed by sharkykh · Pull Request #377 · twilio/twilio-python · GitHub
[go: up one dir, main page]
Skip to content

PyJWT >= 1.5.1 exception fixed #377

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send y 8000 ou account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Aug 10, 2017
Merged

PyJWT >= 1.5.1 exception fixed #377

merged 2 commits into from
Aug 10, 2017

Conversation

sharkykh
Copy link
Contributor
@sharkykh sharkykh commented Jul 22, 2017 •
edited
Loading

Hello. I took the time to trace the following error (caused with PyJWT >= 1.5.1).
This is the same error that resulted in this being committed.
I was able to find out the cause for the exception raised, and fix the test.

But you should verify that works for you, as I didn't check how it fits into your package.

Also, this fix is probably not backwards compatible for PyJWT.
As I don't use PyJWT, I can't really come up with that myself easily.
All I can come up with is checking pyjwt's version and deciding what call arguments to use according to that.
Edit: So, it is backwards compatible :)

Disclaimer: I don't actually use twilio or this package. Just thought this might help.

Original test error:

ERROR: test_decode_allows_skip_verification (tests.unit.jwt.test_jwt.JwtTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/twilio/twilio-python/tests/unit/jwt/test_jwt.py", line 264, in test_decode_allows_skip_verification
    decoded_jwt = Jwt.from_jwt(jwt.to_jwt(), key=None)
  File "/home/travis/build/twilio/twilio-python/twilio/jwt/__init__.py", line 144, in from_jwt
    raise JwtDecodeError(getattr(e, 'message', str(e)))
JwtDecodeError: Expected a string value

Call stack:

Click to expand/collapse call stack
  • JwtTest.test_decode_allows_skip_verification in tests/unit/jwt/test_jwt.py:
    source 
    decoded_jwt = Jwt.from_jwt(jwt.to_jwt(), key=None)
  • Jwt.from_jwt in twilio/jwt/__init__.py:
    source 
    payload = jwt_lib.decode(bytes(jwt), key, verify=verify, options={
  'verify_signature': True,
  'verify_exp': True,
  'verify_nbf': True,
})
  • PyJWT.decode in <pyjwt>/api_jwt.py:
    source 
    decoded = super(PyJWT, self).decode(
    jwt, key=key, algorithms=algorithms, options=options, **kwargs
)
  • PyJWS.decode in <pyjwt>/api_jws.py:
    source 
    self._verify_signature(payload, signing_input, header, signature,
                       key, algorithms)
  • PyJWS._verify_signature in <pyjwt>/api_jws.py:
    source 
    key = alg_obj.prepare_key(key)
  • HMACAlgorithm(HMACAlgorithm.SHA256).prepare_key in <pyjwt>/algorithms.py:
    source 
    key = force_bytes(key)
  • force_bytes in <pyjwt>/utils.py:
    source 
    def force_bytes(value):
    if isinstance(value, text_type):
        return value.encode('utf-8')
    elif isinstance(value, binary_type):
        return value
    else:
        raise TypeError('Expected a string value')
  • As key is None, it raises the exception.

Solution:

Now, it seems the verify argument usage has changed in the latest version, in a way that is not backwards compatible, as can be seen in the following diff for PyJWT:
source

-        decoded = super(PyJWT, self).decode(jwt, key, verify, algorithms,
-                                            options, **kwargs)
+        if options is None:
+            options = {'verify_signature': verify}
+        else:
+            options.setdefault('verify_signature', verify)
+
+        decoded = super(PyJWT, self).decode(
+            jwt, key=key, algorithms=algorithms, options=options, **kwargs
+        )

verify doesn't actually get passed.
Its value is set into 8000 verify_signature in options, but only if options is None or if options doesn't have verify_signature in it (see options.setdefault line)
Jwt.from_jwt sets options, and that's why verify wasn't being honoured in PyJWT.decode.

And so, by modifying the code as suggested in this PR, I was able to get the failing test to pass.

Hope this helps you solve the issues :)

@ilanbiala ilanbiala requested a review from codejudas July 22, 2017 04:13
codejudas
codejudas approved these changes Jul 25, 2017
View reviewed changes
Copy link
Contributor
@codejudas codejudas left a comment

Choose a reason for hiding this comment

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

@sharkykh Looks good thanks for taking the time to look into this for us!

@sharkykh
Copy link
Contributor Author
sharkykh commented Jul 25, 2017 •
edited
Loading

@efossier
Happy to help.
Are you sure that works for you as is? PyJWT still has the same arguments in previous versions?
I'm mostly referring to the options argument.

@codejudas
Copy link
Contributor

Yep I tested this on PyJWT versions 1.4.2 and 1.5.2 both worked whereas 1.5.2 would fail the unit tests without this change. Thanks again!

@sharkykh
Copy link
Contributor Author

Oh, that's great!

@jmctwilio jmctwilio merged commit 9321baf into twilio:master Aug 10, 2017
@sharkykh sharkykh deleted the pyjwt branch August 10, 2017 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codejudas codejudas codejudas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sharkykh @codejudas @jmctwilio
0