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

Better error messages when missing cryptography package #231

Merged
merged 8 commits into from
Dec 1, 2016
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
-------------------------------------------------------------------------
### Changed
- Renamed commandline script `jwt` to `jwt-cli` to avoid issues with the script clobbering the `jwt` module in some circumstances.
- Better error messages when using an algorithm that requires the cryptography package, but it isn't available [#230][230]

### Fixed

Expand Down
1 change: 1 addition & 0 deletions jwt/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,5 +131,6 @@ def main():
else:
p.print_help()


Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you might have added an extra space here?

if __name__ == '__main__':
main()
4 changes: 4 additions & 0 deletions jwt/algorithms.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
except ImportError:
has_crypto = False

requires_cryptography = set(['RS256', 'RS384', 'RS512', 'ES256', 'ES384',
'ES521', 'ES512', 'PS256', 'PS384', 'PS512'])


def get_default_algorithms():
"""
Expand Down Expand Up @@ -171,6 +174,7 @@ def sign(self, msg, key):
def verify(self, msg, key, sig):
return constant_time_compare(sig, self.sign(msg, key))


Copy link
Contributor

Choose a reason for hiding this comment

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

Another extra space?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This (and the other extra space) was to fix flake8 errors reported by the tests. It seemed to be failing to pass without the changes (but maybe that was just a local configuration on my box)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

er. Extra newline, not extra space.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's possible our flake8 in CI just started caring about these. If so, carry on. As long as flake8 is happy, I'm happy.

if has_crypto:

class RSAAlgorithm(Algorithm):
Expand Down
13 changes: 11 additions & 2 deletions jwt/api_jws.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

from collections import Mapping

from .algorithms import Algorithm, get_default_algorithms # NOQA
from .algorithms import (
Algorithm, get_default_algorithms, has_crypto, requires_cryptography # NOQA
)
from .compat import binary_type, string_types, text_type
from .exceptions import DecodeError, InvalidAlgorithmError, InvalidTokenError
from .utils import base64url_decode, base64url_encode, force_bytes, merge_dict
Expand Down Expand Up @@ -101,7 +103,13 @@ def encode(self, payload, key, algorithm='HS256', headers=None,
signature = alg_obj.sign(signing_input, key)

except KeyError:
raise NotImplementedError('Algorithm not supported')
if not has_crypto and algorithm in requires_cryptography:
raise NotImplementedError(
"Algorithm '%s' could not be found. Do you have cryptography "
"installed?" % algorithm
)
else:
raise NotImplementedError('Algorithm not supported')

segments.append(base64url_encode(signature))

Expand Down Expand Up @@ -198,6 +206,7 @@ def _validate_kid(self, kid):
if not isinstance(kid, string_types):
raise InvalidTokenError('Key ID header parameter must be a string')


_jws_global_obj = PyJWS()
encode = _jws_global_obj.encode
decode = _jws_global_obj.decode
Expand Down
1 change: 1 addition & 0 deletions tests/keys/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def load_hmac_key():

return base64url_decode(force_bytes(keyobj['k']))


try:
from cryptography.hazmat.primitives.asymmetric import ec
from cryptography.hazmat.backends import default_backend
Expand Down
6 changes: 6 additions & 0 deletions tests/test_api_jws.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,12 @@ def test_invalid_crypto_alg(self, jws, payload):
with pytest.raises(NotImplementedError):
jws.encode(payload, 'secret', algorithm='HS1024')

@pytest.mark.skipif(has_crypto, reason='Scenario requires cryptography to not be installed')
def test_missing_crypto_library_better_error_messages(self, jws, payload):
with pytest.raises(NotImplementedError) as excinfo:
jws.encode(payload, 'secret', algorithm='RS256')
assert 'cryptography' in str(excinfo.value)

def test_unicode_secret(self, jws, payload):
secret = '\xc2'
jws_message = jws.encode(payload, secret)
Expand Down
1 change: 1 addition & 0 deletions tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ def key_path(key_name):
return os.path.join(os.path.dirname(os.path.realpath(__file__)),
'keys', key_name)


# Borrowed from `cryptography`
if hasattr(int, "from_bytes"):
int_from_bytes = int.from_bytes
Expand Down