More oidc tests test_login_callback_requires_step_up_auth and test_login_callback_no_step_up_auth, lint

src/djangooidc/oidc.py

@@ -147,7 +147,7 @@ class Client(oic.Client):
             raise o_e.InternalError(locator=state)
 
         return response
-    
+
     def callback(self, unparsed_response, session):
         """Step 3: Receive OP's response, request an access token, and user info."""
         logger.debug("Processing the OpenID Connect callback response...")
@@ -276,7 +276,7 @@ class Client(oic.Client):
         """returns the step_up_acr_value from settings
         this helper function is called from djangooidc views"""
         return self.behaviour.get("step_up_acr_value")
-    
+
     def __repr__(self):
         return "Client {} {} {}".format(
             self.client_id,

src/djangooidc/tests/test_views.py

@@ -1,9 +1,9 @@
 from unittest.mock import MagicMock, patch
 
-from django.http import HttpResponse, HttpResponseRedirect
+from django.http import HttpResponse
 from django.test import Client, TestCase, RequestFactory
 from django.urls import reverse
-from ..views import login_callback, requires_step_up_auth
+from ..views import login_callback
 
 from .common import less_console_noise
 
@@ -61,46 +61,32 @@ class ViewsTest(TestCase):
         # mock
         mock_client.callback.side_effect = self.user_info
         # test
-        with patch("djangooidc.views.requires_step_up_auth", return_value=False), \
+        with patch("djangooidc.views.requires_step_up_auth", return_value=False), less_console_noise():
-        less_console_noise():
             response = self.client.get(reverse("openid_login_callback"))
         # assert
         self.assertEqual(response.status_code, 302)
         self.assertEqual(response.url, reverse("logout"))
-        
+
     def test_login_callback_no_step_up_auth(self, mock_client):
+        """Walk through login_callback when requires_step_up_auth returns False
+        and assert that we have a redirect to /"""
         # setup
         session = self.client.session
         session.save()
         # mock
         mock_client.callback.side_effect = self.user_info
         # test
-        with patch("djangooidc.views.requires_step_up_auth", return_value=False), \
+        with patch("djangooidc.views.requires_step_up_auth", return_value=False), less_console_noise():
-        less_console_noise():
             response = self.client.get(reverse("openid_login_callback"))
         # assert
         self.assertEqual(response.status_code, 302)
         self.assertEqual(response.url, "/")
-    
+
-    @patch.object(requires_step_up_auth, return_value=True)
-    def test_login_callback_requires_step_up_auth(self, mock_client):
-        # setup
-        callback_url = reverse("openid_login_callback")
-        # session = self.client.session
-        # session.save()
-        # mock
-        # mock_client.callback.side_effect = self.user_info
-        # mock_client.create_authn_request.side_effect = self.say_hi
-        # test
-        # with patch("djangooidc.views.requires_step_up_auth", return_value=True):
-            
-        response = self.client.get(reverse("openid_login_callback"))
-        
-        # assert
-        # self.assertEqual(response.status_code, 200)
-        # self.assertContains(response, "Hi")
-    
     def test_requires_step_up_auth(self, mock_client):
+        """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.
+
+        Possibly redundant with test_login_callback_no_step_up_auth"""
         # 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"
 
@@ -110,29 +96,35 @@ class ViewsTest(TestCase):
 
         # Ensure that the CLIENT instance used in login_callback is the mock
         # patch requires_step_up_auth to return True
-        with patch("djangooidc.views.requires_step_up_auth", return_value=True), \
+        with patch("djangooidc.views.requires_step_up_auth", return_value=True), patch(
-        patch("djangooidc.views.CLIENT.create_authn_request", return_value=MagicMock()) as mock_create_authn_request:
+            "djangooidc.views.CLIENT.create_authn_request", return_value=MagicMock()
+        ) as mock_create_authn_request:
             login_callback(request)
 
         # Assert that get_step_up_acr_value was called and session was updated
         self.assertNotEqual(request.session["acr_value"], "")
         # And create_authn_request was called again
         mock_create_authn_request.assert_called_once()
-        
+
     def test_does_not_requires_step_up_auth(self, mock_client):
+        """Invoke login_callback passing it a request when requires_step_up_auth returns False
+        and assert that session is not updated and create_authn_request (mock) is not called.
+
+        Possibly redundant with test_login_callback_requires_step_up_auth"""
         # Create a mock request
         request = self.factory.get("/some-url")
         request.session = {"acr_value": ""}
 
         # Ensure that the CLIENT instance used in login_callback is the mock
         # patch requires_step_up_auth to return False
-        with patch("djangooidc.views.requires_step_up_auth", return_value=False), \
+        with patch("djangooidc.views.requires_step_up_auth", return_value=False), patch(
-        patch("djangooidc.views.CLIENT.create_authn_request", return_value=MagicMock()) as mock_create_authn_request:
+            "djangooidc.views.CLIENT.create_authn_request", return_value=MagicMock()
+        ) as mock_create_authn_request:
             login_callback(request)
 
         # Assert that get_step_up_acr_value was NOT called and session was NOT updated
         self.assertEqual(request.session["acr_value"], "")
-        # create_authn_request was not called 
+        # create_authn_request was not called
         mock_create_authn_request.assert_not_called()
 
     @patch("djangooidc.views.authenticate")
@@ -141,8 +133,7 @@ class ViewsTest(TestCase):
         mock_client.callback.side_effect = self.user_info
         mock_auth.return_value = None
         # test
-        with patch("djangooidc.views.requires_step_up_auth", return_value=False), \
+        with patch("djangooidc.views.requires_step_up_auth", return_value=False), less_console_noise():
-        less_console_noise():
             response = self.client.get(reverse("openid_login_callback"))
         # assert
         self.assertEqual(response.status_code, 401)
@@ -189,3 +180,34 @@ class ViewsTest(TestCase):
         # assert
         self.assertEqual(response.status_code, 302)
         self.assertEqual(response.url, reverse("logout"))
+
+
+class ViewsTestUnpatched(TestCase):
+    def setUp(self):
+        self.client = Client()
+        self.factory = RequestFactory()
+
+    def say_hi(*args):
+        return HttpResponse("Hi")
+
+    def user_info(*args):
+        return {
+            "sub": "TEST",
+            "email": "test@example.com",
+            "first_name": "Testy",
+            "last_name": "Tester",
+            "phone": "814564000",
+        }
+
+    def test_login_callback_requires_step_up_auth(self):
+        """Walk through login_callback when requires_step_up_auth returns True
+        and assert that create_authn_request is returned."""
+
+        with patch("djangooidc.views.requires_step_up_auth", return_value=True), patch(
+            "djangooidc.views.Client.callback", return_value=self.user_info
+        ), patch("djangooidc.views.Client.create_authn_request", side_effect=self.say_hi):
+            response = self.client.get(reverse("openid_login_callback"))
+
+        # assert
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, "Hi")

src/djangooidc/views.py

@@ -70,13 +70,10 @@ def login_callback(request):
         userinfo = CLIENT.callback(query, request.session)
         # test for need for identity verification and if it is satisfied
         # if not satisfied, redirect user to login with stepped up acr_value
-        logger.info('login_callback start')
         if requires_step_up_auth(userinfo):
             # add acr_value to request.session
-            logger.info('login_callback inside requires_step_up_auth')
             request.session["acr_value"] = CLIENT.get_step_up_acr_value()
-            logger.info('login_callback after get_step_up_acr_value')
+            return CLIENT.create_authn_request(request.session)
-            # return CLIENT.create_authn_request(request.session)
         user = authenticate(request=request, **userinfo)
         if user:
             login(request, user)
@@ -87,15 +84,17 @@ def login_callback(request):
     except Exception as err:
         return error_page(request, err)
 
+
 def requires_step_up_auth(userinfo):
-    """ if User.needs_identity_verification and step_up_acr_value not in
+    """if User.needs_identity_verification and step_up_acr_value not in
-    ial returned from callback, return True """
+    ial returned from callback, return True"""
     step_up_acr_value = CLIENT.get_step_up_acr_value()
     acr_value = userinfo.get("ial", "")
     uuid = userinfo.get("sub", "")
     email = userinfo.get("email", "")
     return User.needs_identity_verification(email, uuid) and acr_value != step_up_acr_value
 
+
 def logout(request, next_page=None):
     """Redirect the user to the authentication provider (OP) logout page."""
     try:
@@ -125,6 +124,7 @@ def logout(request, next_page=None):
         if next_page:
             request.session["next"] = next_page
 
+
 def logout_callback(request):
     """Simple redirection view: after logout, redirect to `next`."""
     next = request.session.get("next", "/")

src/registrar/models/user.py

@@ -70,29 +70,32 @@ class User(AbstractUser):
     def needs_identity_verification(cls, email, uuid):
         """A method used by our oidc classes to test whether a user needs email/uuid verification
         or the full identity PII verification"""
-                
+
-         # An existing user who is a domain manager of a domain (that is,
+        # An existing user who is a domain manager of a domain (that is,
-         # they have an entry in UserDomainRole for their User)
+        # they have an entry in UserDomainRole for their User)
-        try:    
+        try:
             existing_user = cls.objects.get(username=uuid)
             if existing_user and UserDomainRole.objects.filter(user=existing_user).exists():
                 return False
-        except:
+        except cls.DoesNotExist:
+            # Do nothing when the user is not found, as we're checking for existence.
             pass
-                
+        except Exception as err:
+            raise err
+
         # A new incoming user who is a domain manager for one of the domains
         # that we inputted from Verisign (that is, their email address appears
-        # in the username field of a TransitionDomain) 
+        # in the username field of a TransitionDomain)
         if TransitionDomain.objects.filter(username=email).exists():
             return False
-            
+
         # A new incoming user who is being invited to be a domain manager (that is,
         # their email address is in DomainInvitation for an invitation that is not yet "retrieved").
         if DomainInvitation.objects.filter(email=email, status=DomainInvitation.INVITED):
             return False
-                
+
         return True
-    
+
     def check_domain_invitations_on_login(self):
         """When a user first arrives on the site, we need to retrieve any domain
         invitations that match their email address."""

src/registrar/tests/test_models.py

@@ -613,7 +613,7 @@ class TestUser(TestCase):
         self.email = "mayor@igorville.gov"
         self.domain_name = "igorvilleInTransition.gov"
         self.domain, _ = Domain.objects.get_or_create(name="igorville.gov")
-        self.user, _ = User.objects.get_or_create(email=self.email)        
+        self.user, _ = User.objects.get_or_create(email=self.email)
 
     def tearDown(self):
         super().tearDown()
@@ -632,25 +632,25 @@ class TestUser(TestCase):
         are created."""
         self.user.on_each_login()
         self.assertFalse(Domain.objects.filter(name=self.domain_name).exists())
-        
+
     def test_identity_verification_with_domain_manager(self):
         """A domain manager should return False when tested with class
         method needs_identity_verification"""
         UserDomainRole.objects.get_or_create(user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER)
         self.assertFalse(User.needs_identity_verification(self.user.email, self.user.username))
-        
+
     def test_identity_verification_with_transition_user(self):
         """A user from the Verisign transition should return False
         when tested with class method needs_identity_verification"""
         TransitionDomain.objects.get_or_create(username=self.user.email, domain_name=self.domain_name)
         self.assertFalse(User.needs_identity_verification(self.user.email, self.user.username))
-        
+
     def test_identity_verification_with_invited_user(self):
         """An invited user should return False when tested with class
         method needs_identity_verification"""
         DomainInvitation.objects.get_or_create(email=self.user.email, domain=self.domain)
         self.assertFalse(User.needs_identity_verification(self.user.email, self.user.username))
-        
+
     def test_identity_verification_with_new_user(self):
         """A new user who's neither transitioned nor invited should
         return True when tested with class method needs_identity_verification"""