Skip to content

Commit

Permalink
Allow special characters in issuer value
Browse files Browse the repository at this point in the history
The recommended value for issuer is a URL with an https:// prefix. The
current strategy of defining keys using an uppercase issuer as the
variable suffix does not allow for special characters in the name. This
change moves the key values to a dictionary keyed by the issuer
string under the new `JWT_KEYS` setting while deprecating and
maintaining backward compatibility for the original setting.
  • Loading branch information
com4 committed Dec 23, 2020
1 parent 2ef8352 commit 29cbb18
Show file tree
Hide file tree
Showing 7 changed files with 162 additions and 38 deletions.
40 changes: 27 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ AUTHENTICATION_BACKENDS = (

Now we need to set up a `JWT_ISSUER` variable in our config, which will be the
name of the issuer. Take the private key that we generated before
and store it in a `JWT_PRIVATE_KEY_<JWT_ISSUER>` variable. Finally, define the
ID attribute you wish to use in `JWT_ID_ATTRIBUTE`. While this can be any
and store it in a `JWT_KEYS[<JWT_ISSUER>]['private']` variable. Finally, define
the ID attribute you wish to use in `JWT_ID_ATTRIBUTE`. While this can be any
attribute on your user model it should be unique.

Also you have to set your JWT-encoding Algorithm if it's different than
Expand All @@ -119,26 +119,39 @@ For example:

```python
# settings.py

JWT_ISSUER = 'OneIssuer'
JWT_ID_ATTRIBUTE = 'email'
JWT_PRIVATE_KEY_ONEISSUER = """
jwt_private_key = """
-----BEGIN RSA PRIVATE KEY-----
MIIBOAIBAAJAbCmbRUsLrsv0/Cq7DVDpUooPS1V2sr0EhTZAZmJhid2o/+ya/28m
...
6D0+csaGDlZ9GbrTpTJUObNENNHqfrHGfqzDxQ==
-----END RSA PRIVATE KEY-----
"""
```
jwt_public_key = """
-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAmeym/g734dlLr3bEeVSR
...
6wIDAQAB
-----END PUBLIC KEY-----
"""
JWT_ISSUER = 'OneIssuer'
JWT_ID_ATTRIBUTE = 'email'

\* *Note that storing hardcoded secrets in the settings is a bad practice and
JWT_KEYS = {
'OneIssuer': { # Or JWT_ISSUER
'private': jwt_private_key
'public': jwt_public_key
}
}

```
* NOTE: storing hardcoded secrets in the settings is a bad practice and
can lead to severe security breaches in your code. We recommend using
environment variables for this purpose.*
environment variables for this purpose.

\** *Note that you can configure only **one** JWT-Encoding Algorithm in
* NOTE: you can configure only **one** JWT-Encoding Algorithm in
`JWT_ENC_ALGORITHM`. But you can set multiple allowed decoding(verifying)
Algorithms in `JWT_JWS_ALGORITHMS` as an array of Strings. It is only useful
if the JWT is from a 3rd Party and you don't know which Algorithm is used.*
if the JWT is from a 3rd Party and you don't know which Algorithm is used.

The payload of messages will be by default something like:

Expand Down Expand Up @@ -216,11 +229,12 @@ REST_FRAMEWORK = {
```

Also, you will need to add to the settings every public key of all the
possible token issuers, if configured, using a variable `JWT_PUBLIC_KEY_<JWT_ISSUER>`:
possible token issuers, if configured, using a variable
`JWT_KEYS[<JWT_ISSUER>]['public']`:

```python
# settings.py
JWT_PUBLIC_KEY_ONEISSUER = """
JWT_KEYS['OneIssuer']['public'] = """
-----BEGIN PUBLIC KEY-----
MFswDQYJKoZIhvcNAQEBBQADSgAwRwJAbCmbRUsLrsv0/Cq7DVDpUooPS1V2sr0E
hTZAZmJhid2o/+ya/28muuoQgknEoJz32bKeWuYZrFkRKUrGFnlxHwIDAQAB
Expand Down
49 changes: 41 additions & 8 deletions oauth2_provider_jwt/utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import base64
from datetime import datetime, timedelta
import json
import warnings

from django.conf import settings
from django.core.exceptions import ImproperlyConfigured
Expand Down Expand Up @@ -48,11 +49,27 @@ def encode_jwt(payload, issuer=None, headers=None):
# RS256 in default, because hardcoded legacy
algorithm = getattr(settings, 'JWT_ENC_ALGORITHM', 'RS256')

private_key_name = 'JWT_PRIVATE_KEY_{}'.format(issuer.upper())
private_key = getattr(settings, private_key_name, None)
try:
private_key = getattr(settings, 'JWT_KEYS', {})[issuer]['private']
except KeyError:
private_key = None

if not private_key:
raise ImproperlyConfigured('Missing setting {}'.format(
private_key_name))
# Check the legacy setting
private_key_name = 'JWT_PRIVATE_KEY_{}'.format(issuer.upper())
try:
private_key = getattr(settings, private_key_name)
if private_key.strip() == '':
# Little hack to pick up the test case where the key is an
# empty string.
raise AttributeError
warnings.warn(
f'{private_key_name} has been deprecated in favor of JWT_KEYS',
DeprecationWarning
)
except AttributeError:
raise ImproperlyConfigured(f'Missing private key for {issuer}')

encoded = jwt.encode(payload, private_key, algorithm=algorithm,
headers=headers)
return encoded.decode("utf-8")
Expand All @@ -78,11 +95,27 @@ def decode_jwt(jwt_value, issuer=None):
'Unable to determine issuer. Token missing iss claim')

algorithms = getattr(settings, 'JWT_JWS_ALGORITHMS', ['HS256', 'RS256'])
public_key_name = 'JWT_PUBLIC_KEY_{}'.format(issuer.upper())
public_key = getattr(settings, public_key_name, None)

try:
public_key = getattr(settings, 'JWT_KEYS', {})[issuer]['public']
except KeyError:
public_key = None

if not public_key:
raise ImproperlyConfigured('Missing setting {}'.format(
public_key_name))
# Check the legacy setting
public_key_name = 'JWT_PUBLIC_KEY_{}'.format(issuer.upper())
try:
public_key = getattr(settings, public_key_name)
if public_key.strip() == '':
# Little hack to pick up the test case where the key is an
# empty string.
raise AttributeError
warnings.warn(
f'{public_key_name} has been deprecated in favor of JWT_KEYS',
DeprecationWarning
)
except AttributeError:
raise ImproperlyConfigured(f'Missing public key for {issuer}')

decoded = jwt.decode(jwt_value, public_key, algorithms=algorithms)
return decoded
12 changes: 10 additions & 2 deletions oauth2_provider_jwt/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,16 @@ def _get_access_token_jwt(self, request, content):
@staticmethod
def _is_jwt_config_set():
issuer = getattr(settings, 'JWT_ISSUER', '')
private_key_name = 'JWT_PRIVATE_KEY_{}'.format(issuer.upper())
private_key = getattr(settings, private_key_name, None)
try:
private_key = getattr(settings, 'JWT_KEYS', {})[issuer]['private']
except KeyError:
private_key = None

# Deprecated legacy setting
if not private_key:
private_key_name = 'JWT_PRIVATE_KEY_{}'.format(issuer.upper())
private_key = getattr(settings, private_key_name, None)

id_attribute = getattr(settings, 'JWT_ID_ATTRIBUTE', None)
if issuer and private_key and id_attribute:
return True
Expand Down
11 changes: 9 additions & 2 deletions tests/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@

# JWT_ENC_ALGORITHM = 'HS256' # 'RS256' is still default

JWT_PRIVATE_KEY_ISSUER = """
priv_key = """
-----BEGIN RSA PRIVATE KEY-----
MIIBOAIBAAJAbCmbRUsLrsv0/Cq7DVDpUooPS1V2sr0EhTZAZmJhid2o/+ya/28m
uuoQgknEoJz32bKeWuYZrFkRKUrGFnlxHwIDAQABAkBILcO2DAxxyx1jIcjNbA8n
Expand All @@ -141,13 +141,20 @@
-----END RSA PRIVATE KEY-----
"""

JWT_PUBLIC_KEY_ISSUER = """
pub_key = """
-----BEGIN PUBLIC KEY-----
MFswDQYJKoZIhvcNAQEBBQADSgAwRwJAbCmbRUsLrsv0/Cq7DVDpUooPS1V2sr0E
hTZAZmJhid2o/+ya/28muuoQgknEoJz32bKeWuYZrFkRKUrGFnlxHwIDAQAB
-----END PUBLIC KEY-----
"""

JWT_KEYS = {
JWT_ISSUER: {
'private': priv_key,
'public': pub_key,
}
}

REST_FRAMEWORK = {
'DEFAULT_AUTHENTICATION_CLASSES': (
'oauth2_provider_jwt.authentication.JWTAuthentication',
Expand Down
10 changes: 6 additions & 4 deletions tests/test_authentication.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
from datetime import datetime, timedelta
import json

from django.test import TestCase
from django.conf import settings
from django.contrib.auth import get_user_model
from django.test import TestCase
from django.test.utils import override_settings
from rest_framework.test import APIClient

Expand Down Expand Up @@ -134,16 +135,17 @@ def test_post_valid_jwt_with_auth_and_scope_is_valid(self):

class ECDSAJWTAuth(JWTAuthenticationTests):
@override_settings(JWT_ENC_ALGORITHM="ES256")
@override_settings(JWT_PRIVATE_KEY_ISSUER="""-----BEGIN OPENSSH PRIVATE KEY-----
@override_settings(JWT_KEYS={settings.JWT_ISSUER: {
'private': """-----BEGIN OPENSSH PRIVATE KEY-----
b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAaAAAABNlY2RzYS
1zaGEyLW5pc3RwMjU2AAAACG5pc3RwMjU2AAAAQQTRAzrSN7dFmGa05mrQaKfP2xplEFo/
1InJc3P+pcy1zgN427Y96mmPMMkx1zBk9S0uDFX5WlEJ9UuRxmf+yceVAAAAsJLFlSOSxZ
UjAAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBNEDOtI3t0WYZrTm
atBop8/bGmUQWj/Uiclzc/6lzLXOA3jbtj3qaY8wyTHXMGT1LS4MVflaUQn1S5HGZ/7Jx5
UAAAAhAJlPEfG7s+7zces2RHc1txeyyjwZxCqUqs25kvtO36nrAAAAFnRob21hc0B3b3Jr
c3RhdGlvbi1kZWIB
-----END OPENSSH PRIVATE KEY-----""")
@override_settings(JWT_PUBLIC_KEY_ISSUER="ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBNEDOtI3t0WYZrTmatBop8/bGmUQWj/Uiclzc/6lzLXOA3jbtj3qaY8wyTHXMGT1LS4MVflaUQn1S5HGZ/7Jx5U= thomas@workstation-deb") # noqa: E501
-----END OPENSSH PRIVATE KEY-----""",
'public': 'ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBNEDOtI3t0WYZrTmatBop8/bGmUQWj/Uiclzc/6lzLXOA3jbtj3qaY8wyTHXMGT1LS4MVflaUQn1S5HGZ/7Jx5U= thomas@workstation-deb'}}) # noqa: E501
def setUp(self):
self.client = APIClient(enforce_csrf_checks=True)
User = get_user_model()
Expand Down
65 changes: 60 additions & 5 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@
from unittest.mock import patch
from unittest import TestCase as PythonTestCase

from django.conf import settings
from django.core.exceptions import ImproperlyConfigured
from django.test import override_settings
import jwt
from oauth2_provider_jwt import utils

issuer_special_chars = 'https://api.service.cloud'
privkey = settings.JWT_KEYS[settings.JWT_ISSUER]['private']
pubkey = settings.JWT_KEYS[settings.JWT_ISSUER]['public']


class GeneratePayloadTest(PythonTestCase):
def _get_payload_args(self):
Expand Down Expand Up @@ -71,7 +76,7 @@ def _get_payload(self):
'org': 'some_org',
}

@override_settings(JWT_PRIVATE_KEY_ISSUER='')
@override_settings(JWT_KEYS={})
def test_encode_jwt_no_private_key_in_setting(self):
payload = self._get_payload()
self.assertRaises(ImproperlyConfigured,
Expand All @@ -90,6 +95,22 @@ def test_encode_jwt_rs256(self):
json.loads(base64.b64decode(payload).decode("utf-8")),
payload_in)

@override_settings(JWT_ISSUER=issuer_special_chars)
@override_settings(JWT_KEYS={issuer_special_chars: {'private': privkey}})
def test_encode_jwt__issuer_special_chars(self):
payload_in = self._get_payload()
payload_in['iss'] = issuer_special_chars
encoded = utils.encode_jwt(payload_in)
self.assertIn(type(encoded).__name__, ('unicode', 'str'))
headers, payload, verify_signature = encoded.split(".")
self.assertDictEqual(
json.loads(base64.b64decode(headers)),
{"typ": "JWT", "alg": "RS256"})
payload += '=' * (-len(payload) % 4) # add padding
self.assertEqual(
json.loads(base64.b64decode(payload).decode("utf-8")),
payload_in)

def test_encode_jwt_explicit_issuer(self):
payload_in = self._get_payload()
payload_in['iss'] = 'different-issuer'
Expand All @@ -104,7 +125,7 @@ def test_encode_jwt_explicit_issuer(self):
json.loads(base64.b64decode(payload).decode("utf-8")),
payload_in)

@override_settings(JWT_PRIVATE_KEY_ISSUER='test')
@override_settings(JWT_KEYS={settings.JWT_ISSUER: {'private': 'test'}})
@override_settings(JWT_ENC_ALGORITHM='HS256')
def test_encode_jwt_hs256(self):
payload_in = self._get_payload()
Expand All @@ -119,6 +140,21 @@ def test_encode_jwt_hs256(self):
json.loads(base64.b64decode(payload).decode('utf-8')),
payload_in)

@override_settings(JWT_KEYS={})
@override_settings(JWT_PRIVATE_KEY_ISSUER=privkey)
def test_encode_jwt__legacy_settings_private_key(self):
payload_in = self._get_payload()
encoded = utils.encode_jwt(payload_in)
self.assertIn(type(encoded).__name__, ('unicode', 'str'))
headers, payload, verify_signature = encoded.split(".")
self.assertDictEqual(
json.loads(base64.b64decode(headers)),
{"typ": "JWT", "alg": "RS256"})
payload += '=' * (-len(payload) % 4) # add padding
self.assertEqual(
json.loads(base64.b64decode(payload).decode("utf-8")),
payload_in)


class DecodeJWTTest(PythonTestCase):
def _get_payload(self):
Expand All @@ -135,7 +171,7 @@ def _get_payload(self):
def test_decode_jwt_invalid(self):
self.assertRaises(jwt.InvalidTokenError, utils.decode_jwt, 'abc')

@override_settings(JWT_PUBLIC_KEY_ISSUER='')
@override_settings(JWT_KEYS={settings.JWT_ISSUER: {'private': privkey}})
def test_decode_jwt_public_key_not_found(self):
payload = self._get_payload()
jwt_value = utils.encode_jwt(payload)
Expand All @@ -157,18 +193,37 @@ def test_decode_jwt_rs256(self):
payload_out = utils.decode_jwt(jwt_value)
self.assertDictEqual(payload, payload_out)

@override_settings(JWT_ISSUER=issuer_special_chars)
@override_settings(JWT_KEYS={
issuer_special_chars: {'private': privkey, 'public': pubkey}})
def test_decode_jwt__issuer_special_chars(self):
payload = self._get_payload()
payload['iss'] = issuer_special_chars
jwt_value = utils.encode_jwt(payload)
payload_out = utils.decode_jwt(jwt_value)
self.assertDictEqual(payload, payload_out)

def test_decode_jwt_explicit_issuer(self):
payload = self._get_payload()
payload['iss'] = 'different-issuer'
jwt_value = utils.encode_jwt(payload, 'issuer')
payload_out = utils.decode_jwt(jwt_value, 'issuer')
self.assertDictEqual(payload, payload_out)

@override_settings(JWT_PRIVATE_KEY_ISSUER='test')
@override_settings(JWT_PUBLIC_KEY_ISSUER='test')
@override_settings(JWT_KEYS={
settings.JWT_ISSUER: {'private': 'test', 'public': 'test'}})
@override_settings(JWT_ENC_ALGORITHM='HS256')
def test_decode_jwt_hs256(self):
payload = self._get_payload()
jwt_value = utils.encode_jwt(payload)
payload_out = utils.decode_jwt(jwt_value)
self.assertDictEqual(payload, payload_out)

@override_settings(JWT_KEYS={})
@override_settings(JWT_PRIVATE_KEY_ISSUER=privkey)
@override_settings(JWT_PUBLIC_KEY_ISSUER=pubkey)
def test_decode_jwt__legacy_settings_public_key(self):
payload = self._get_payload()
jwt_value = utils.encode_jwt(payload)
payload_out = utils.decode_jwt(jwt_value)
self.assertDictEqual(payload, payload_out)
13 changes: 9 additions & 4 deletions tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,23 +80,28 @@ def tearDown(self):
self.dev_user.delete()

@override_settings(JWT_ISSUER='api')
@override_settings(JWT_PRIVATE_KEY_API='somevalue')
@override_settings(JWT_KEYS={'api': {'private': 'somevalue'}})
def test_is_jwt_config_set(self):
self.assertTrue(TokenView._is_jwt_config_set())

@override_settings(JWT_ISSUER='')
@override_settings(JWT_ISSUER='api')
@override_settings(JWT_PRIVATE_KEY_API='somevalue')
def test_is_jwt_config_set_legacy(self):
self.assertTrue(TokenView._is_jwt_config_set())

@override_settings(JWT_ISSUER='')
@override_settings(JWT_KEYS={'api': {'private': 'somevalue'}})
def test_is_jwt_config_not_set_missing_issuer(self):
self.assertFalse(TokenView._is_jwt_config_set())

@override_settings()
@override_settings(JWT_PRIVATE_KEY_API='somevalue')
@override_settings(JWT_KEYS={'api': {'private': 'somevalue'}})
def test_is_jwt_config_not_set_none_issuer(self):
del settings.JWT_ISSUER
self.assertFalse(TokenView._is_jwt_config_set())

@override_settings(JWT_ISSUER='api')
@override_settings(JWT_PRIVATE_KEY_API='')
@override_settings(JWT_KEYS={'api': {'private': ''}})
def test_is_jwt_config_not_set_missing_private_key(self):
self.assertFalse(TokenView._is_jwt_config_set())

Expand Down

0 comments on commit 29cbb18

Please sign in to comment.