Skip to content

Commit c6a7df9

Browse files
akatsoulasakatsoulas
authored
Merge pull request #54 from akatsoulas/validate-token
Improve token validation.
2 parents dc68193 + e8caa43 commit c6a7df9

File tree

3 files changed

+52
-46
lines changed

3 files changed

+52
-46
lines changed

‎mozilla_django_oidc/auth.py‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import base64
22
import hashlib
3-
import jwt
3+
import json
44
import logging
55
import requests
66

@@ -12,6 +12,8 @@
1212
from django.core.exceptions import SuspiciousOperation
1313
from django.core.urlresolvers import reverse
1414

15+
from jose import jws
16+
1517
from mozilla_django_oidc.utils import absolutify, import_from_settings
1618

1719

@@ -68,21 +70,18 @@ def create_user(self, claims):
6870
def verify_token(self, token, **kwargs):
6971
"""Validate the token signature."""
7072
nonce = kwargs.get('nonce')
71-
# Get JWT audience without signature verification
72-
audience = jwt.decode(token, verify=False)['aud']
7373

7474
secret = self.OIDC_RP_CLIENT_SECRET
7575
if import_from_settings('OIDC_RP_CLIENT_SECRET_ENCODED', False):
7676
secret = base64.urlsafe_b64decode(self.OIDC_RP_CLIENT_SECRET)
77+
# Verify the token
78+
verified_token = jws.verify(token, secret, algorithms=['HS256'])
79+
token_nonce = json.loads(verified_token).get('nonce')
7780

78-
id_token = jwt.decode(token, secret,
79-
verify=import_from_settings('OIDC_VERIFY_JWT', True),
80-
audience=audience)
81-
82-
if import_from_settings('OIDC_USE_NONCE', True) and nonce != id_token['nonce']:
81+
if import_from_settings('OIDC_USE_NONCE', True) and nonce != token_nonce:
8382
msg = 'JWT Nonce verification failed.'
8483
raise SuspiciousOperation(msg)
85-
return id_token
84+
return True
8685

8786
def authenticate(self, **kwargs):
8887
"""Authenticates a user based on the OIDC code flow."""
@@ -110,15 +109,14 @@ def authenticate(self, **kwargs):
110109

111110
# Validate the token
112111
token_response = response.json()
113-
payload = self.verify_token(token_response.get('id_token'), nonce=nonce)
114-
115-
if payload:
112+
if self.verify_token(token_response.get('id_token'), nonce=nonce):
116113
access_token = token_response.get('access_token')
117114
user_response = requests.get(self.OIDC_OP_USER_ENDPOINT,
118115
headers={
119116
'Authorization': 'Bearer {0}'.format(access_token)
120117
})
121118
user_response.raise_for_status()
119+
122120
user_info = user_response.json()
123121
email = user_info.get('email')
124122

‎requirements/requirements.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
django>=1.9.6
2-
PyJWT==1.4.2
32
requests
3+
python-jose>=1.3.2

‎tests/test_auth.py‎

Lines changed: 41 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import json
12
from mock import Mock, call, patch
23

34
from django.conf import settings
@@ -17,7 +18,7 @@ class OIDCAuthenticationBackendTestCase(TestCase):
1718
@override_settings(OIDC_OP_TOKEN_ENDPOINT='https://server.example.com/token')
1819
@override_settings(OIDC_OP_USER_ENDPOINT='https://server.example.com/user')
1920
@override_settings(OIDC_RP_CLIENT_ID='example_id')
20-
@override_settings(OIDC_RP_CLIENT_SECRET='example_secret')
21+
@override_settings(OIDC_RP_CLIENT_SECRET='client_secret')
2122
def setUp(self):
2223
self.backend = OIDCAuthenticationBackend()
2324

@@ -52,9 +53,9 @@ def test_get_invalid_user(self):
5253

5354
self.assertEqual(self.backend.get_user(user_id=1), None)
5455

56+
@override_settings(SITE_URL='http://site-url.com')
5557
@patch('mozilla_django_oidc.auth.requests')
5658
@patch('mozilla_django_oidc.auth.OIDCAuthenticationBackend.verify_token')
57-
@override_settings(SITE_URL='http://site-url.com')
5859
def test_successful_authentication_existing_user(self, token_mock, request_mock):
5960
"""Test successful authentication for existing user."""
6061

@@ -76,7 +77,7 @@ def test_successful_authentication_existing_user(self, token_mock, request_mock)
7677

7778
post_data = {
7879
'client_id': 'example_id',
79-
'client_secret': 'example_secret',
80+
'client_secret': 'client_secret',
8081
'grant_type': 'authorization_code',
8182
'code': 'foo',
8283
'redirect_uri': 'http://site-url.com/callback/'
@@ -114,7 +115,7 @@ def test_successful_authentication_new_user(self, token_mock, request_mock, algo
114115
request_mock.post.return_value = post_json_mock
115116
post_data = {
116117
'client_id': 'example_id',
117-
'client_secret': 'example_secret',
118+
'client_secret': 'client_secret',
118119
'grant_type': 'authorization_code',
119120
'code': 'foo',
120121
'redirect_uri': 'http://site-url.com/callback/',
@@ -141,14 +142,14 @@ def test_authenticate_no_code_no_state(self):
141142
self.assertEqual(self.backend.authenticate(code='', state=''), None)
142143

143144
@override_settings(OIDC_USE_NONCE=False)
144-
@patch('mozilla_django_oidc.auth.jwt')
145+
@patch('mozilla_django_oidc.auth.jws.verify')
145146
@patch('mozilla_django_oidc.auth.requests')
146-
def test_jwt_decode_params(self, request_mock, jwt_mock):
147+
def test_jwt_decode_params(self, request_mock, jws_mock):
147148
"""Test jwt verification signature."""
148149

149-
jwt_mock.decode.return_value = {
150+
jws_mock.return_value = json.dumps({
150151
'aud': 'audience'
151-
}
152+
})
152153
get_json_mock = Mock()
153154
get_json_mock.json.return_value = {
154155
'nickname': 'username',
@@ -163,21 +164,20 @@ def test_jwt_decode_params(self, request_mock, jwt_mock):
163164
request_mock.post.return_value = post_json_mock
164165
self.backend.authenticate(code='foo', state='bar')
165166
calls = [
166-
call('token', verify=False),
167-
call('token', 'example_secret', verify=True, audience='audience')
167+
call('token', 'client_secret', algorithms=['HS256'])
168168
]
169-
jwt_mock.decode.assert_has_calls(calls)
169+
jws_mock.assert_has_calls(calls)
170170

171171
@override_settings(OIDC_VERIFY_JWT=False)
172172
@override_settings(OIDC_USE_NONCE=False)
173-
@patch('mozilla_django_oidc.auth.jwt')
173+
@patch('mozilla_django_oidc.auth.jws.verify')
174174
@patch('mozilla_django_oidc.auth.requests')
175-
def test_jwt_decode_params_verify_false(self, request_mock, jwt_mock):
175+
def test_jwt_decode_params_verify_false(self, request_mock, jws_mock):
176176
"""Test jwt verification signature with verify False"""
177177

178-
jwt_mock.decode.return_value = {
178+
jws_mock.return_value = json.dumps({
179179
'aud': 'audience'
180-
}
180+
})
181181
get_json_mock = Mock()
182182
get_json_mock.json.return_value = {
183183
'nickname': 'username',
@@ -191,35 +191,37 @@ def test_jwt_decode_params_verify_false(self, request_mock, jwt_mock):
191191
}
192192
request_mock.post.return_value = post_json_mock
193193
calls = [
194-
call('token', verify=False),
195-
call('token', 'example_secret', verify=False, audience='audience')
194+
call('token', 'client_secret', algorithms=['HS256'])
196195
]
197196

198197
self.backend.authenticate(code='foo', state='bar')
199-
jwt_mock.decode.assert_has_calls(calls)
198+
jws_mock.assert_has_calls(calls)
200199

201200
@override_settings(OIDC_USE_NONCE=True)
202-
@patch('mozilla_django_oidc.auth.jwt')
201+
@override_settings(OIDC_RP_CLIENT_SECRET_ENCODED=False)
202+
@patch('mozilla_django_oidc.auth.jws')
203203
def test_jwt_failed_nonce(self, jwt_mock):
204204
"""Test Nonce verification."""
205205

206-
jwt_mock.decode.return_value = {
206+
jwt_mock.verify.return_value = json.dumps({
207207
'nonce': 'foobar',
208208
'aud': 'aud'
209-
}
209+
})
210210
id_token = 'my_token'
211211
with self.assertRaises(SuspiciousOperation) as context:
212212
self.backend.verify_token(id_token, **{'nonce': 'foo'})
213213
self.assertEqual('JWT Nonce verification failed.', str(context.exception))
214214

215215
@override_settings(OIDC_CREATE_USER=False)
216216
@override_settings(OIDC_USE_NONCE=False)
217-
@patch('mozilla_django_oidc.auth.jwt')
217+
@patch('mozilla_django_oidc.auth.jws.verify')
218218
@patch('mozilla_django_oidc.auth.requests')
219-
def test_create_user_disabled(self, request_mock, jwt_mock):
219+
def test_create_user_disabled(self, request_mock, jws_mock):
220220
"""Test with user creation disabled and no user found."""
221221

222-
jwt_mock.return_value = True
222+
jws_mock.return_value = json.dumps({
223+
'nonce': 'nonce'
224+
})
223225
get_json_mock = Mock()
224226
get_json_mock.json.return_value = {
225227
'nickname': 'a_username',
@@ -234,14 +236,16 @@ def test_create_user_disabled(self, request_mock, jwt_mock):
234236
request_mock.post.return_value = post_json_mock
235237
self.assertEqual(self.backend.authenticate(code='foo', state='bar'), None)
236238

237-
@patch('mozilla_django_oidc.auth.jwt')
239+
@patch('mozilla_django_oidc.auth.jws.verify')
238240
@patch('mozilla_django_oidc.auth.requests')
239241
@override_settings(OIDC_USE_NONCE=False)
240-
def test_create_user_enabled(self, request_mock, jwt_mock):
242+
def test_create_user_enabled(self, request_mock, jws_mock):
241243
"""Test with user creation enabled and no user found."""
242244

243245
self.assertEqual(User.objects.filter(email='[email protected]').exists(), False)
244-
jwt_mock.return_value = True
246+
jws_mock.return_value = json.dumps({
247+
'nonce': 'nonce'
248+
})
245249
get_json_mock = Mock()
246250
get_json_mock.json.return_value = {
247251
'nickname': 'a_username',
@@ -259,14 +263,16 @@ def test_create_user_enabled(self, request_mock, jwt_mock):
259263

260264
@patch.object(settings, 'OIDC_USERNAME_ALGO')
261265
@override_settings(OIDC_USE_NONCE=False)
262-
@patch('mozilla_django_oidc.auth.jwt')
266+
@patch('mozilla_django_oidc.auth.jws.verify')
263267
@patch('mozilla_django_oidc.auth.requests')
264-
def test_custom_username_algo(self, request_mock, jwt_mock, algo_mock):
268+
def test_custom_username_algo(self, request_mock, jws_mock, algo_mock):
265269
"""Test user creation with custom username algorithm."""
266270

267271
self.assertEqual(User.objects.filter(email='[email protected]').exists(), False)
268272
algo_mock.return_value = 'username_algo'
269-
jwt_mock.return_value = True
273+
jws_mock.return_value = json.dumps({
274+
'nonce': 'nonce'
275+
})
270276
get_json_mock = Mock()
271277
get_json_mock.json.return_value = {
272278
'nickname': 'a_username',
@@ -283,14 +289,16 @@ def test_custom_username_algo(self, request_mock, jwt_mock, algo_mock):
283289
User.objects.get(username='username_algo'))
284290

285291
@override_settings(OIDC_USE_NONCE=False)
286-
@patch('mozilla_django_oidc.auth.jwt')
292+
@patch('mozilla_django_oidc.auth.jws.verify')
287293
@patch('mozilla_django_oidc.auth.requests')
288-
def test_duplicate_emails(self, request_mock, jwt_mock):
294+
def test_duplicate_emails(self, request_mock, jws_mock):
289295
"""Test auth with two users having the same email."""
290296

291297
User.objects.create(username='user1', email='[email protected]')
292298
User.objects.create(username='user2', email='[email protected]')
293-
jwt_mock.return_value = True
299+
jws_mock.return_value = json.dumps({
300+
'nonce': 'nonce'
301+
})
294302
get_json_mock = Mock()
295303
get_json_mock.json.return_value = {
296304
'nickname': 'a_username',

0 commit comments

Comments
 (0)