openid tests

src/djangooidc/tests/test_views.py

@@ -4,13 +4,13 @@ from django.http import HttpResponse
 from django.test import Client, TestCase, RequestFactory
 from django.urls import reverse
 
-from djangooidc.exceptions import NoStateDefined
+from djangooidc.exceptions import NoStateDefined, InternalError
-from ..views import login_callback
+from ..views import login_callback, CLIENT
 
 from .common import less_console_noise
 
 
-@patch("djangooidc.views.CLIENT", autospec=True)
+@patch("djangooidc.views.CLIENT", new_callable=MagicMock)
 class ViewsTest(TestCase):
     def setUp(self):
         self.client = Client()
@@ -35,56 +35,135 @@ class ViewsTest(TestCase):
         pass
 
     def test_openid_sets_next(self, mock_client):
+        """ Test that the openid method properly sets next in the session."""
         with less_console_noise():
-            # setup
+            # SETUP
+            # set up the callback url that will be tested in assertions against
+            # session[next]
             callback_url = reverse("openid_login_callback")
-            # mock
+            # MOCK
+            # when login is called, response from create_authn_request should
+            # be returned to user, so let's mock it and test it
             mock_client.create_authn_request.side_effect = self.say_hi
+            # in this case, we need to mock the get_default_acr_value so that
+            # openid method will execute properly, but the acr_value itself
+            # is not important for this test
             mock_client.get_default_acr_value.side_effect = self.create_acr
-            # test
+            # TEST
+            # test the login url, passing a callback url
             response = self.client.get(reverse("login"), {"next": callback_url})
-            # assert
+            # ASSERTIONS
             session = mock_client.create_authn_request.call_args[0][0]
+            # assert the session[next] is set to the callback_url
             self.assertEqual(session["next"], callback_url)
+            # assert that openid returned properly the response from
+            # create_authn_request
             self.assertEqual(response.status_code, 200)
             self.assertContains(response, "Hi")
 
     def test_openid_raises(self, mock_client):
+        """Test that errors in openid raise 500 error for the user.
+        This test specifically tests for any exceptions that might be raised from
+        create_authn_request. This includes scenarios where CLIENT exists, but
+        is no longer functioning properly."""
         with less_console_noise():
-            # mock
+            # MOCK
+            # when login is called, exception thrown from create_authn_request
+            # should present 500 error page to user
             mock_client.create_authn_request.side_effect = Exception("Test")
-            # test
+            # TEST
+            # test when login url is called
             response = self.client.get(reverse("login"))
-            # assert
+            # ASSERTIONS
+            # assert that the 500 error page is raised
             self.assertEqual(response.status_code, 500)
             self.assertTemplateUsed(response, "500.html")
             self.assertIn("Server error", response.content.decode("utf-8"))
 
-    def test_callback_with_no_session_state(self, mock_client):
+    def test_openid_raises_when_client_is_none_and_cant_init(self, mock_client):
+        """Test that errors in openid raise 500 error for the user.
+        This test specifically tests for the condition where the CLIENT
+        is None and the client initialization attempt raises an exception."""
+        with less_console_noise():
+            # MOCK
+            # mock that CLIENT is None
+            # mock that Client() raises an exception (by mocking _initialize_client)
+            # Patch CLIENT to None for this specific test
+            with patch("djangooidc.views.CLIENT", None):
+                # Patch _initialize_client() to raise an exception
+                with patch("djangooidc.views._initialize_client") as mock_init:
+                    mock_init.side_effect = InternalError
+                    # TEST
+                    # test when login url is called
+                    response = self.client.get(reverse("login"))
+                    # ASSERTIONS
+                    # assert that the 500 error page is raised
+                    self.assertEqual(response.status_code, 500)
+                    self.assertTemplateUsed(response, "500.html")
+                    self.assertIn("Server error", response.content.decode("utf-8"))
+
+    def test_openid_initializes_client_and_calls_create_authn_request(self, mock_client):
+        """Test that openid re-initializes the client when the client had not
+        been previously initiated."""
+        with less_console_noise():
+            # MOCK
+            # response from create_authn_request should
+            # be returned to user, so let's mock it and test it
+            mock_client.create_authn_request.side_effect = self.say_hi
+            # in this case, we need to mock the get_default_acr_value so that
+            # openid method will execute properly, but the acr_value itself
+            # is not important for this test
+            mock_client.get_default_acr_value.side_effect = self.create_acr
+            with patch("djangooidc.views._initialize_client") as mock_init_client:
+                with patch("djangooidc.views._client_is_none") as mock_client_is_none:
+                    # mock the client to initially be None
+                    mock_client_is_none.return_value = True
+                    # TEST
+                    # test when login url is called
+                    response = self.client.get(reverse("login"))
+                    # ASSERTIONS
+                    # assert that _initialize_client was called
+                    mock_init_client.assert_called_once()
+                    # assert that the response is the mocked response from create_authn_request
+                    self.assertEqual(response.status_code, 200)
+                    self.assertContains(response, "Hi")
+
+    def test_login_callback_with_no_session_state(self, mock_client):
         """If the local session is None (ie the server restarted while user was logged out),
         we do not throw an exception. Rather, we attempt to login again."""
         with less_console_noise():
-            # mock
+            # MOCK
+            # mock the acr_value to some string
+            # mock the callback function to raise the NoStateDefined Exception
             mock_client.get_default_acr_value.side_effect = self.create_acr
             mock_client.callback.side_effect = NoStateDefined()
-            # test
+            # TEST
+            # test the login callback
             response = self.client.get(reverse("openid_login_callback"))
-            # assert
+            # ASSERTIONS
+            # assert that the user is redirected to the start of the login process
             self.assertEqual(response.status_code, 302)
             self.assertEqual(response.url, "/")
 
     def test_login_callback_reads_next(self, mock_client):
+        """If the next value is set in the session, test that login_callback returns
+        a redirect to the 'next' url."""
         with less_console_noise():
-            # setup
+            # SETUP
             session = self.client.session
+            # set 'next' to the logout url
             session["next"] = reverse("logout")
             session.save()
-            # mock
+            # MOCK
+            # mock that callback returns user_info; this is the expected behavior
             mock_client.callback.side_effect = self.user_info
-            # test
+            # patch that the request does not require step up auth
-            with patch("djangooidc.views._requires_step_up_auth", return_value=False), less_console_noise():
+            # TEST
+            # test the login callback url
+            with patch("djangooidc.views._requires_step_up_auth", return_value=False):
                 response = self.client.get(reverse("openid_login_callback"))
-            # assert
+            # ASSERTIONS
+            # assert the redirect url is the same as the 'next' value set in session
             self.assertEqual(response.status_code, 302)
             self.assertEqual(response.url, reverse("logout"))
 
@@ -92,15 +171,19 @@ class ViewsTest(TestCase):
         """Walk through login_callback when _requires_step_up_auth returns False
         and assert that we have a redirect to /"""
         with less_console_noise():
-            # setup
+            # SETUP
             session = self.client.session
             session.save()
-            # mock
+            # MOCK
+            # mock that callback returns user_info; this is the expected behavior
             mock_client.callback.side_effect = self.user_info
-            # test
+            # patch that the request does not require step up auth
-            with patch("djangooidc.views._requires_step_up_auth", return_value=False), less_console_noise():
+            # TEST
+            # test the login callback url
+            with patch("djangooidc.views._requires_step_up_auth", return_value=False):
                 response = self.client.get(reverse("openid_login_callback"))
-            # assert
+            # ASSERTIONS
+            # assert that redirect is to / when no 'next' is set
             self.assertEqual(response.status_code, 302)
             self.assertEqual(response.url, "/")
 
@@ -108,6 +191,7 @@ class ViewsTest(TestCase):
         """Invoke login_callback passing it a request when _requires_step_up_auth returns True
         and assert that session is updated and create_authn_request (mock) is called."""
         with less_console_noise():
+            # MOCK
             # Configure the mock to return an expected value for get_step_up_acr_value
             mock_client.return_value.get_step_up_acr_value.return_value = "step_up_acr_value"
             # Create a mock request
@@ -118,7 +202,10 @@ class ViewsTest(TestCase):
             with patch("djangooidc.views._requires_step_up_auth", return_value=True), patch(
                 "djangooidc.views.CLIENT.create_authn_request", return_value=MagicMock()
             ) as mock_create_authn_request:
+                # TEST
+                # test the login callback
                 login_callback(request)
+            # ASSERTIONS
             # create_authn_request only gets called when _requires_step_up_auth is True
             # and it changes this acr_value in request.session
             # Assert that acr_value is no longer empty string
@@ -132,6 +219,7 @@ class ViewsTest(TestCase):
 
         Possibly redundant with test_login_callback_requires_step_up_auth"""
         with less_console_noise():
+            # MOCK
             # Create a mock request
             request = self.factory.get("/some-url")
             request.session = {"acr_value": ""}
@@ -140,7 +228,10 @@ class ViewsTest(TestCase):
             with patch("djangooidc.views._requires_step_up_auth", return_value=False), patch(
                 "djangooidc.views.CLIENT.create_authn_request", return_value=MagicMock()
             ) as mock_create_authn_request:
+                # TEST
+                # test the login callback
                 login_callback(request)
+            # ASSERTIONS
             # create_authn_request only gets called when _requires_step_up_auth is True
             # and it changes this acr_value in request.session
             # Assert that acr_value is NOT updated by testing that it is still an empty string
@@ -150,33 +241,36 @@ class ViewsTest(TestCase):
 
     @patch("djangooidc.views.authenticate")
     def test_login_callback_raises(self, mock_auth, mock_client):
+        """Test that login callback raises a 401 when user is unauthorized"""
         with less_console_noise():
-            # mock
+            # MOCK
+            # mock that callback returns user_info; this is the expected behavior
             mock_client.callback.side_effect = self.user_info
             mock_auth.return_value = None
-            # test
+            # TEST
-            with patch("djangooidc.views._requires_step_up_auth", return_value=False), less_console_noise():
+            with patch("djangooidc.views._requires_step_up_auth", return_value=False):
                 response = self.client.get(reverse("openid_login_callback"))
-            # assert
+            # ASSERTIONS
             self.assertEqual(response.status_code, 401)
             self.assertTemplateUsed(response, "401.html")
             self.assertIn("Unauthorized", response.content.decode("utf-8"))
 
     def test_logout_redirect_url(self, mock_client):
+        """Test that logout redirects to the configured post_logout_redirect_uris."""
         with less_console_noise():
-            # setup
+            # SETUP
             session = self.client.session
             session["state"] = "TEST"  # nosec B105
             session.save()
-            # mock
+            # MOCK
             mock_client.callback.side_effect = self.user_info
             mock_client.registration_response = {"post_logout_redirect_uris": ["http://example.com/back"]}
             mock_client.provider_info = {"end_session_endpoint": "http://example.com/log_me_out"}
             mock_client.client_id = "TEST"
-            # test
+            # TEST
             with less_console_noise():
                 response = self.client.get(reverse("logout"))
-            # assert
+            # ASSERTIONS
             expected = (
                 "http://example.com/log_me_out?client_id=TEST&state"
                 "=TEST&post_logout_redirect_uri=http%3A%2F%2Fexample.com%2Fback"
@@ -187,20 +281,23 @@ class ViewsTest(TestCase):
 
     @patch("djangooidc.views.auth_logout")
     def test_logout_always_logs_out(self, mock_logout, _):
-        # Without additional mocking, logout will always fail.
+        """Without additional mocking, logout will always fail.
-        # Here we test that auth_logout is called regardless
+        Here we test that auth_logout is called regardless"""
+        # TEST
         with less_console_noise():
             self.client.get(reverse("logout"))
+        # ASSERTIONS
         self.assertTrue(mock_logout.called)
 
     def test_logout_callback_redirects(self, _):
+        """Test that the logout_callback redirects properly"""
         with less_console_noise():
-            # setup
+            # SETUP
             session = self.client.session
             session["next"] = reverse("logout")
             session.save()
-            # test
+            # TEST
             response = self.client.get(reverse("openid_logout_callback"))
-            # assert
+            # ASSERTIONS
             self.assertEqual(response.status_code, 302)
             self.assertEqual(response.url, reverse("logout"))

src/djangooidc/views.py

@@ -27,6 +27,10 @@ def _initialize_client():
     CLIENT = Client(OP)
     logger.debug("Client initialized: %s" % CLIENT)
 
+def _client_is_none():
+    """ Return if the CLIENT is currently None."""
+    global CLIENT
+    return CLIENT is None
 
 # Initialize CLIENT
 try:
@@ -71,7 +75,7 @@ def openid(request):
     global CLIENT
     try:
         # If the CLIENT is none, attempt to reinitialize before handling the request
-        if CLIENT is None:
+        if _client_is_none():
             _initialize_client()
         request.session["acr_value"] = CLIENT.get_default_acr_value()
         request.session["next"] = request.GET.get("next", "/")
@@ -86,7 +90,7 @@ def login_callback(request):
     global CLIENT
     try:
         # If the CLIENT is none, attempt to reinitialize before handling the request
-        if CLIENT is None:
+        if _client_is_none():
             _initialize_client()
         query = parse_qs(request.GET.urlencode())
         userinfo = CLIENT.callback(query, request.session)