Merge pull request #29 from johngian/multiple-fixes

Multiple fixes

Author: johngian

mozilla_django_oidc/auth.py

@@ -1,5 +1,7 @@
+import base64
 import jwt
 import requests
+
 try:
     from urllib import urlencode
 except ImportError:
@@ -8,7 +10,7 @@
 from django.contrib.auth import get_user_model
 from django.core.urlresolvers import reverse
 
-from mozilla_django_oidc.utils import import_from_settings
+from mozilla_django_oidc.utils import absolutify, import_from_settings
 
 
 class OIDCAuthenticationBackend(object):
@@ -26,9 +28,16 @@ def __init__(self, *args, **kwargs):
     def verify_token(self, token, **kwargs):
         """Validate the token signature."""
 
-        return jwt.decode(token,
-                          self.OIDC_OP_CLIENT_SECRET,
-                          verify=import_from_settings('OIDC_VERIFY_JWT', True))
+        # Get JWT audience without signature verification
+        audience = jwt.decode(token, verify=False)['aud']
+
+        secret = self.OIDC_OP_CLIENT_SECRET
+        if import_from_settings('OIDC_RP_CLIENT_SECRET_ENCODED', False):
+            secret = base64.urlsafe_b64decode(self.OIDC_OP_CLIENT_SECRET)
+
+        return jwt.decode(token, secret,
+                          verify=import_from_settings('OIDC_VERIFY_JWT', True),
+                          audience=audience)
 
     def authenticate(self, code=None, state=None):
         """Authenticates a user based on the OIDC code flow."""
@@ -39,30 +48,32 @@ def authenticate(self, code=None, state=None):
         token_payload = {
             'client_id': self.OIDC_OP_CLIENT_ID,
             'client_secret': self.OIDC_OP_CLIENT_SECRET,
-            'grand_type': 'authorization_code',
+            'grant_type': 'authorization_code',
             'code': code,
-            'redirect_url': reverse('oidc_authentication_callback')
+            'redirect_uri': absolutify(reverse('oidc_authentication_callback'))
         }
 
         # Get the token
         response = requests.post(self.OIDC_OP_TOKEN_ENDPOINT,
-                                 data=token_payload,
+                                 json=token_payload,
                                  verify=import_from_settings('VERIFY_SSL', True))
         # Validate the token
-        payload = self.verify_token(response.get('id_token'))
+        token_response = response.json()
+        payload = self.verify_token(token_response.get('id_token'))
 
         if payload:
             query = urlencode({
-                'access_token': response.get('access_token')
+                'access_token': token_response.get('access_token')
             })
-            user_info = requests.get('{url}?{query}'.format(url=self.OIDC_OP_USER_ENDPOINT,
-                                                            query=query))
+            user_response = requests.get('{url}?{query}'.format(url=self.OIDC_OP_USER_ENDPOINT,
+                                                                query=query))
+            user_info = user_response.json()
 
             try:
-                return self.UserModel.objects.get(email=user_info['verified_email'])
+                return self.UserModel.objects.get(email=user_info['email'])
             except self.UserModel.DoesNotExist:
-                return self.UserModel.objects.create_user(username=user_info['username'],
-                                                          email=user_info['verified_email'])
+                return self.UserModel.objects.create_user(username=user_info['nickname'],
+                                                          email=user_info['email'])
         return None
 
     def get_user(self, user_id):

mozilla_django_oidc/utils.py

@@ -1,3 +1,8 @@
+try:
+    from urlparse import urljoin
+except ImportError:
+    from urllib.parse import urljoin
+
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 
@@ -10,8 +15,15 @@ def import_from_settings(attr, default_val=None):
         ImproperlyConfigured
     """
     try:
-        if default_val:
+        if default_val is not None:
             return getattr(settings, attr, default_val)
         return getattr(settings, attr)
     except AttributeError:
         raise ImproperlyConfigured('Setting {0} not found'.format(attr))
+
+
+def absolutify(path):
+    """Return the absolute URL of url_path."""
+
+    site_url = import_from_settings('SITE_URL')
+    return urljoin(site_url, path)

mozilla_django_oidc/views.py

@@ -6,15 +6,16 @@
 from django.core.urlresolvers import reverse
 from django.contrib import auth
 from django.http import HttpResponseRedirect
+from django.utils.crypto import get_random_string
 from django.views.generic import View
 
-from mozilla_django_oidc.utils import import_from_settings
+from mozilla_django_oidc.utils import absolutify, import_from_settings
 
 
 class OIDCAuthenticationCallbackView(View):
     """OIDC client authentication callback HTTP endpoint"""
 
-    http_method_names = ['post']
+    http_method_names = ['get']
 
     @property
     def failure_url(self):
@@ -31,13 +32,13 @@ def login_success(self):
         auth.login(self.request, self.user)
         return HttpResponseRedirect(self.success_url)
 
-    def post(self, request):
+    def get(self, request):
         """Callback handler for OIDC authorization code flow"""
 
-        if 'code' in request.POST and 'state' in request.POST:
+        if 'code' in request.GET and 'state' in request.GET:
             kwargs = {
-                'code': request.POST['code'],
-                'state': request.POST['state']
+                'code': request.GET['code'],
+                'state': request.GET['state']
             }
             self.user = auth.authenticate(**kwargs)
 
@@ -63,7 +64,8 @@ def get(self, request):
             'response_type': 'code',
             'scope': 'openid',
             'client_id': self.OIDC_OP_CLIENT_ID,
-            'redirect_uri': reverse('oidc_authentication_callback')
+            'redirect_uri': absolutify(reverse('oidc_authentication_callback')),
+            'state': get_random_string(import_from_settings('OIDC_STATE_SIZE', 32))
         }
 
         query = urlencode(params)

requirements/requirements.txt

@@ -1,2 +1,3 @@
 django>=1.9.6
 PyJWT==1.4.2
+requests

runtests.py

@@ -20,6 +20,7 @@
             'mozilla_django_oidc',
         ],
         SITE_ID=1,
+        SITE_URL='http://example.com',
         MIDDLEWARE_CLASSES=(),
     )
 

setup.py

@@ -42,7 +42,7 @@
         'mozilla_django_oidc',
     ],
     include_package_data=True,
-    install_requires=[],
+    install_requires=['Django>1.7', 'PyJWT', 'requests'],
     license='MPL 2.0',
     zip_safe=False,
     keywords='mozilla-django-oidc',

tests/test_auth.py

@@ -1,7 +1,6 @@
-from mock import patch
+from mock import Mock, call, patch
 
 from django.contrib.auth import get_user_model
-from django.core.urlresolvers import reverse
 from django.test import TestCase, override_settings
 
 from mozilla_django_oidc.auth import OIDCAuthenticationBackend
@@ -26,14 +25,18 @@ def test_invalid_token(self, request_mock, token_mock):
         """Test authentication with an invalid token."""
 
         token_mock.return_value = None
-        request_mock.get.return_value = {
-            'username': 'username',
-            'verified_email': '[email protected]'
+        get_json_mock = Mock()
+        get_json_mock.json.return_value = {
+            'nickname': 'username',
+            'email': '[email protected]'
         }
-        request_mock.post.return_value = {
+        request_mock.get.return_value = get_json_mock
+        post_json_mock = Mock()
+        post_json_mock.json.return_value = {
             'id_token': 'id_token',
             'accesss_token': 'access_token'
         }
+        request_mock.post.return_value = post_json_mock
         self.assertEqual(self.backend.authenticate(code='foo', state='bar'), None)
 
     def test_get_user(self):
@@ -49,56 +52,67 @@ def test_get_invalid_user(self):
 
     @patch('mozilla_django_oidc.auth.requests')
     @patch('mozilla_django_oidc.auth.OIDCAuthenticationBackend.verify_token')
+    @override_settings(SITE_URL='http://site-url.com')
     def test_successful_authentication_existing_user(self, token_mock, request_mock):
         """Test successful authentication for existing user."""
 
         user = User.objects.create_user(username='a_username',
                                         email='[email protected]')
         token_mock.return_value = True
-        request_mock.get.return_value = {
-            'username': 'a_username',
-            'verified_email': '[email protected]'
+        get_json_mock = Mock()
+        get_json_mock.json.return_value = {
+            'nickname': 'a_username',
+            'email': '[email protected]'
         }
-        request_mock.post.return_value = {
+        request_mock.get.return_value = get_json_mock
+        post_json_mock = Mock()
+        post_json_mock.json.return_value = {
             'id_token': 'id_token',
             'access_token': 'access_granted'
         }
+        request_mock.post.return_value = post_json_mock
+
         post_data = {
             'client_id': 'example_id',
             'client_secret': 'example_secret',
-            'grand_type': 'authorization_code',
+            'grant_type': 'authorization_code',
             'code': 'foo',
-            'redirect_url': reverse('oidc_authentication_callback')
+            'redirect_uri': 'http://site-url.com/oidc/authentication_callback/'
         }
         self.assertEqual(self.backend.authenticate(code='foo', state='bar'), user)
         token_mock.assert_called_once_with('id_token')
         request_mock.post.assert_called_once_with('https://server.example.com/token',
-                                                  data=post_data,
+                                                  json=post_data,
                                                   verify=True)
         request_mock.get.assert_called_once_with(
             'https://server.example.com/user?access_token=access_granted'
         )
 
     @patch('mozilla_django_oidc.auth.requests')
     @patch('mozilla_django_oidc.auth.OIDCAuthenticationBackend.verify_token')
+    @override_settings(SITE_URL='http://site-url.com')
     def test_successful_authentication_new_user(self, token_mock, request_mock):
         """Test successful authentication and user creation."""
 
         token_mock.return_value = True
-        request_mock.get.return_value = {
-            'username': 'a_username',
-            'verified_email': '[email protected]'
+        get_json_mock = Mock()
+        get_json_mock.json.return_value = {
+            'nickname': 'a_username',
+            'email': '[email protected]'
         }
-        request_mock.post.return_value = {
+        request_mock.get.return_value = get_json_mock
+        post_json_mock = Mock()
+        post_json_mock.json.return_value = {
             'id_token': 'id_token',
             'access_token': 'access_granted'
         }
+        request_mock.post.return_value = post_json_mock
         post_data = {
             'client_id': 'example_id',
             'client_secret': 'example_secret',
-            'grand_type': 'authorization_code',
+            'grant_type': 'authorization_code',
             'code': 'foo',
-            'redirect_url': reverse('oidc_authentication_callback')
+            'redirect_uri': 'http://site-url.com/oidc/authentication_callback/',
         }
         self.assertEqual(User.objects.all().count(), 0)
         self.backend.authenticate(code='foo', state='bar')
@@ -109,7 +123,7 @@ def test_successful_authentication_new_user(self, token_mock, request_mock):
 
         token_mock.assert_called_once_with('id_token')
         request_mock.post.assert_called_once_with('https://server.example.com/token',
-                                                  data=post_data,
+                                                  json=post_data,
                                                   verify=True)
         request_mock.get.assert_called_once_with(
             'https://server.example.com/user?access_token=access_granted'
@@ -125,30 +139,53 @@ def test_authenticate_no_code_no_state(self):
     def test_jwt_decode_params(self, request_mock, jwt_mock):
         """Test jwt verification signature."""
 
-        request_mock.get.return_value = {
-            'username': 'username',
-            'verified_email': '[email protected]'
+        jwt_mock.decode.return_value = {
+            'aud': 'audience'
+        }
+        get_json_mock = Mock()
+        get_json_mock.json.return_value = {
+            'nickname': 'username',
+            'email': '[email protected]'
         }
-        request_mock.post.return_value = {
+        request_mock.get.return_value = get_json_mock
+        post_json_mock = Mock()
+        post_json_mock.json.return_value = {
             'id_token': 'token',
             'access_token': 'access_token'
         }
+        request_mock.post.return_value = post_json_mock
         self.backend.authenticate(code='foo', state='bar')
-        jwt_mock.decode.assert_called_once_with('token', 'example_secret', verify=True)
+        calls = [
+            call('token', verify=False),
+            call('token', 'example_secret', verify=True, audience='audience')
+        ]
+        jwt_mock.decode.assert_has_calls(calls)
 
     @override_settings(OIDC_VERIFY_JWT=False)
     @patch('mozilla_django_oidc.auth.jwt')
     @patch('mozilla_django_oidc.auth.requests')
     def test_jwt_decode_params_verify_false(self, request_mock, jwt_mock):
         """Test jwt verification signature with verify False"""
 
-        request_mock.get.return_value = {
-            'username': 'username',
-            'verified_email': '[email protected]'
+        jwt_mock.decode.return_value = {
+            'aud': 'audience'
         }
-        request_mock.post.return_value = {
+        get_json_mock = Mock()
+        get_json_mock.json.return_value = {
+            'nickname': 'username',
+            'email': '[email protected]'
+        }
+        request_mock.get.return_value = get_json_mock
+        post_json_mock = Mock()
+        post_json_mock.json.return_value = {
             'id_token': 'token',
             'access_token': 'access_token'
         }
+        request_mock.post.return_value = post_json_mock
+        calls = [
+            call('token', verify=False),
+            call('token', 'example_secret', verify=False, audience='audience')
+        ]
+
         self.backend.authenticate(code='foo', state='bar')
-        jwt_mock.decode.assert_called_once_with('token', 'example_secret', verify=False)
+        jwt_mock.decode.assert_has_calls(calls)

tests/test_utils.py

@@ -1,7 +1,7 @@
 from django.core.exceptions import ImproperlyConfigured
 from django.test import TestCase, override_settings
 
-from mozilla_django_oidc.utils import import_from_settings
+from mozilla_django_oidc.utils import absolutify, import_from_settings
 
 
 class SettingImportTestCase(TestCase):
@@ -18,3 +18,10 @@ def test_attr_nonexisting_no_default_value(self):
     def test_attr_nonexisting_default_value(self):
         s = import_from_settings('EXAMPLE_VARIABLE', 'example_default')
         self.assertEqual(s, 'example_default')
+
+
+class AbsolutifyTestCase(TestCase):
+    @override_settings(SITE_URL='http://site-url.com')
+    def test_absolutify(self):
+        url = absolutify('/foo/bar')
+        self.assertEqual(url, 'http://site-url.com/foo/bar')