Merge pull request #74 from akatsoulas/pass-request

Pass the request object in the authenticate method.

mozilla_django_oidc/auth.py

@@ -86,13 +86,17 @@ def verify_token(self, token, **kwargs):
     def authenticate(self, **kwargs):
         """Authenticates a user based on the OIDC code flow."""
 
-        code = kwargs.pop('code', None)
-        state = kwargs.pop('state', None)
+        self.request = kwargs.pop('request', None)
+        if not self.request:
+            raise SuspiciousOperation('Request object not found.')
+
+        state = self.request.GET.get('state')
+        code = self.request.GET.get('code')
         nonce = kwargs.pop('nonce', None)
-        session = kwargs.pop('session', None)
+        session = self.request.session
 
         if not code or not state:
-            return None
+            raise SuspiciousOperation('Code or state not found.')
 
         token_payload = {
             'client_id': self.OIDC_RP_CLIENT_ID,

mozilla_django_oidc/views.py

@@ -44,10 +44,8 @@ def get(self, request):
 
         if 'code' in request.GET and 'state' in request.GET:
             kwargs = {
-                'code': request.GET['code'],
-                'state': request.GET['state'],
+                'request': request,
                 'nonce': nonce,
-                'session': request.session
             }
 
             if 'oidc_state' not in request.session:

tests/test_auth.py

@@ -4,7 +4,7 @@
 from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.core.exceptions import SuspiciousOperation
-from django.test import TestCase, override_settings
+from django.test import RequestFactory, TestCase, override_settings
 
 from mozilla_django_oidc.auth import OIDCAuthenticationBackend
 
@@ -26,6 +26,9 @@ def setUp(self):
     @patch('mozilla_django_oidc.auth.requests')
     def test_invalid_token(self, request_mock, token_mock):
         """Test authentication with an invalid token."""
+        auth_request = RequestFactory().get('/foo', {'code': 'foo',
+                                                     'state': 'bar'})
+        auth_request.session = {}
 
         token_mock.return_value = None
         get_json_mock = Mock()
@@ -40,7 +43,7 @@ def test_invalid_token(self, request_mock, token_mock):
             'accesss_token': 'access_token'
         }
         request_mock.post.return_value = post_json_mock
-        self.assertEqual(self.backend.authenticate(code='foo', state='bar'), None)
+        self.assertEqual(self.backend.authenticate(request=auth_request), None)
 
     def test_get_user(self):
         """Test get_user method with valid user."""
@@ -58,6 +61,9 @@ def test_get_invalid_user(self):
     @patch('mozilla_django_oidc.auth.OIDCAuthenticationBackend.verify_token')
     def test_successful_authentication_existing_user(self, token_mock, request_mock):
         """Test successful authentication for existing user."""
+        auth_request = RequestFactory().get('/foo', {'code': 'foo',
+                                                     'state': 'bar'})
+        auth_request.session = {}
 
         user = User.objects.create_user(username='a_username',
                                         email='email@example.com')
@@ -82,7 +88,7 @@ def test_successful_authentication_existing_user(self, token_mock, request_mock)
             'code': 'foo',
             'redirect_uri': 'http://site-url.com/callback/'
         }
-        self.assertEqual(self.backend.authenticate(code='foo', state='bar'), user)
+        self.assertEqual(self.backend.authenticate(request=auth_request), user)
         token_mock.assert_called_once_with('id_token', nonce=None)
         request_mock.post.assert_called_once_with('https://server.example.com/token',
                                                   data=post_data,
@@ -98,6 +104,9 @@ def test_successful_authentication_existing_user(self, token_mock, request_mock)
     @override_settings(SITE_URL='http://site-url.com')
     def test_successful_authentication_new_user(self, token_mock, request_mock, algo_mock):
         """Test successful authentication and user creation."""
+        auth_request = RequestFactory().get('/foo', {'code': 'foo',
+                                                     'state': 'bar'})
+        auth_request.session = {}
 
         algo_mock.return_value = 'username_algo'
         token_mock.return_value = True
@@ -121,7 +130,7 @@ def test_successful_authentication_new_user(self, token_mock, request_mock, algo
             'redirect_uri': 'http://site-url.com/callback/',
         }
         self.assertEqual(User.objects.all().count(), 0)
-        self.backend.authenticate(code='foo', state='bar')
+        self.backend.authenticate(request=auth_request)
         self.assertEqual(User.objects.all().count(), 1)
         user = User.objects.all()[0]
         self.assertEquals(user.email, 'email@example.com')
@@ -139,13 +148,20 @@ def test_successful_authentication_new_user(self, token_mock, request_mock, algo
     def test_authenticate_no_code_no_state(self):
         """Test authenticate with wrong parameters."""
 
-        self.assertEqual(self.backend.authenticate(code='', state=''), None)
+        # there are no GET params
+        request = RequestFactory().get('/foo')
+        request.session = {}
+        with self.assertRaisesMessage(SuspiciousOperation, 'Code or state not found'):
+            self.backend.authenticate(request=request)
 
     @override_settings(OIDC_USE_NONCE=False)
     @patch('mozilla_django_oidc.auth.jws.verify')
     @patch('mozilla_django_oidc.auth.requests')
     def test_jwt_decode_params(self, request_mock, jws_mock):
         """Test jwt verification signature."""
+        auth_request = RequestFactory().get('/foo', {'code': 'foo',
+                                                     'state': 'bar'})
+        auth_request.session = {}
 
         jws_mock.return_value = json.dumps({
             'aud': 'audience'
@@ -162,7 +178,7 @@ def test_jwt_decode_params(self, request_mock, jws_mock):
             'access_token': 'access_token'
         }
         request_mock.post.return_value = post_json_mock
-        self.backend.authenticate(code='foo', state='bar')
+        self.backend.authenticate(request=auth_request)
         calls = [
             call('token', 'client_secret', algorithms=['HS256'])
         ]
@@ -174,6 +190,9 @@ def test_jwt_decode_params(self, request_mock, jws_mock):
     @patch('mozilla_django_oidc.auth.requests')
     def test_jwt_decode_params_verify_false(self, request_mock, jws_mock):
         """Test jwt verification signature with verify False"""
+        auth_request = RequestFactory().get('/foo', {'code': 'foo',
+                                                     'state': 'bar'})
+        auth_request.session = {}
 
         jws_mock.return_value = json.dumps({
             'aud': 'audience'
@@ -194,7 +213,7 @@ def test_jwt_decode_params_verify_false(self, request_mock, jws_mock):
             call('token', 'client_secret', algorithms=['HS256'])
         ]
 
-        self.backend.authenticate(code='foo', state='bar')
+        self.backend.authenticate(request=auth_request)
         jws_mock.assert_has_calls(calls)
 
     @override_settings(OIDC_USE_NONCE=True)
@@ -208,16 +227,18 @@ def test_jwt_failed_nonce(self, jwt_mock):
             'aud': 'aud'
         })
         id_token = 'my_token'
-        with self.assertRaises(SuspiciousOperation) as context:
+        with self.assertRaisesMessage(SuspiciousOperation, 'JWT Nonce verification failed.'):
             self.backend.verify_token(id_token, **{'nonce': 'foo'})
-        self.assertEqual('JWT Nonce verification failed.', str(context.exception))
 
     @override_settings(OIDC_CREATE_USER=False)
     @override_settings(OIDC_USE_NONCE=False)
     @patch('mozilla_django_oidc.auth.jws.verify')
     @patch('mozilla_django_oidc.auth.requests')
     def test_create_user_disabled(self, request_mock, jws_mock):
         """Test with user creation disabled and no user found."""
+        auth_request = RequestFactory().get('/foo', {'code': 'foo',
+                                                     'state': 'bar'})
+        auth_request.session = {}
 
         jws_mock.return_value = json.dumps({
             'nonce': 'nonce'
@@ -234,13 +255,16 @@ def test_create_user_disabled(self, request_mock, jws_mock):
             'access_token': 'access_granted'
         }
         request_mock.post.return_value = post_json_mock
-        self.assertEqual(self.backend.authenticate(code='foo', state='bar'), None)
+        self.assertEqual(self.backend.authenticate(request=auth_request), None)
 
     @patch('mozilla_django_oidc.auth.jws.verify')
     @patch('mozilla_django_oidc.auth.requests')
     @override_settings(OIDC_USE_NONCE=False)
     def test_create_user_enabled(self, request_mock, jws_mock):
         """Test with user creation enabled and no user found."""
+        auth_request = RequestFactory().get('/foo', {'code': 'foo',
+                                                     'state': 'bar'})
+        auth_request.session = {}
 
         self.assertEqual(User.objects.filter(email='email@example.com').exists(), False)
         jws_mock.return_value = json.dumps({
@@ -258,7 +282,7 @@ def test_create_user_enabled(self, request_mock, jws_mock):
             'access_token': 'access_granted'
         }
         request_mock.post.return_value = post_json_mock
-        self.assertEqual(self.backend.authenticate(code='foo', state='bar'),
+        self.assertEqual(self.backend.authenticate(request=auth_request),
                          User.objects.get(email='email@example.com'))
 
     @patch.object(settings, 'OIDC_USERNAME_ALGO')
@@ -267,6 +291,9 @@ def test_create_user_enabled(self, request_mock, jws_mock):
     @patch('mozilla_django_oidc.auth.requests')
     def test_custom_username_algo(self, request_mock, jws_mock, algo_mock):
         """Test user creation with custom username algorithm."""
+        auth_request = RequestFactory().get('/foo', {'code': 'foo',
+                                                     'state': 'bar'})
+        auth_request.session = {}
 
         self.assertEqual(User.objects.filter(email='email@example.com').exists(), False)
         algo_mock.return_value = 'username_algo'
@@ -285,14 +312,17 @@ def test_custom_username_algo(self, request_mock, jws_mock, algo_mock):
             'access_token': 'access_granted'
         }
         request_mock.post.return_value = post_json_mock
-        self.assertEqual(self.backend.authenticate(code='foo', state='bar'),
+        self.assertEqual(self.backend.authenticate(request=auth_request),
                          User.objects.get(username='username_algo'))
 
     @override_settings(OIDC_USE_NONCE=False)
     @patch('mozilla_django_oidc.auth.jws.verify')
     @patch('mozilla_django_oidc.auth.requests')
     def test_duplicate_emails(self, request_mock, jws_mock):
         """Test auth with two users having the same email."""
+        auth_request = RequestFactory().get('/foo', {'code': 'foo',
+                                                     'state': 'bar'})
+        auth_request.session = {}
 
         User.objects.create(username='user1', email='email@example.com')
         User.objects.create(username='user2', email='email@example.com')
@@ -311,8 +341,4 @@ def test_duplicate_emails(self, request_mock, jws_mock):
             'access_token': 'access_granted'
         }
         request_mock.post.return_value = post_json_mock
-        auth_kwargs = {
-            'code': 'foo',
-            'state': 'bar'
-        }
-        self.assertEqual(self.backend.authenticate(**auth_kwargs), None)
+        self.assertEqual(self.backend.authenticate(request=auth_request), None)

tests/test_views.py

@@ -41,10 +41,8 @@ def test_get_auth_success(self):
                 mock_auth.return_value = user
                 response = callback_view(request)
 
-                mock_auth.assert_called_once_with(code='example_code',
-                                                  state='example_state',
-                                                  nonce=None,
-                                                  session=request.session)
+                mock_auth.assert_called_once_with(nonce=None,
+                                                  request=request)
                 mock_login.assert_called_once_with(request, user)
 
         self.assertEqual(response.status_code, 302)
@@ -72,10 +70,8 @@ def test_get_auth_success_next_url(self):
                 mock_auth.return_value = user
                 response = callback_view(request)
 
-                mock_auth.assert_called_once_with(code='example_code',
-                                                  state='example_state',
-                                                  nonce=None,
-                                                  session=request.session)
+                mock_auth.assert_called_once_with(nonce=None,
+                                                  request=request)
                 mock_login.assert_called_once_with(request, user)
 
         self.assertEqual(response.status_code, 302)
@@ -100,10 +96,8 @@ def test_get_auth_failure_nonexisting_user(self):
             mock_auth.return_value = None
             response = callback_view(request)
 
-            mock_auth.assert_called_once_with(code='example_code',
-                                              state='example_state',
-                                              nonce=None,
-                                              session=request.session)
+            mock_auth.assert_called_once_with(nonce=None,
+                                              request=request)
 
         self.assertEqual(response.status_code, 302)
         self.assertEqual(response.url, '/failure')
@@ -131,10 +125,8 @@ def test_get_auth_failure_inactive_user(self):
             mock_auth.return_value = user
             response = callback_view(request)
 
-            mock_auth.assert_called_once_with(code='example_code',
-                                              state='example_state',
-                                              nonce=None,
-                                              session=request.session)
+            mock_auth.assert_called_once_with(request=request,
+                                              nonce=None)
 
         self.assertEqual(response.status_code, 302)
         self.assertEqual(response.url, '/failure')
@@ -225,10 +217,8 @@ def test_nonce_is_deleted(self):
                 mock_auth.return_value = user
                 response = callback_view(request)
 
-                mock_auth.assert_called_once_with(code='example_code',
-                                                  state='example_state',
-                                                  nonce='example_nonce',
-                                                  session=request.session)
+                mock_auth.assert_called_once_with(nonce='example_nonce',
+                                                  request=request)
                 mock_login.assert_called_once_with(request, user)
 
         self.assertEqual(response.status_code, 302)