Fix: OAuth session checks and events (#1537)

benefits/enrollment/templates/enrollment/success.html

@@ -36,7 +36,7 @@ <h1 class="pb-lg-5 pb-4">{% translate "enrollment.pages.success.headline" %}</h1
 {% endblock inner-content %}
 
 {% block call-to-action %}
-  {% if authentication and authentication.sign_out_button_template %}
+  {% if authentication and authentication.logged_in and authentication.sign_out_button_template %}
     <div class="row d-flex justify-content-start justify-content-lg-center">
       <div class="col-12 col-lg-8 pt-lg-5 mt-lg-0 pt-4 mt-2">
         <p>

benefits/oauth/analytics.py

@@ -10,7 +10,8 @@ class OAuthEvent(core.Event):
     def __init__(self, request, event_type):
         super().__init__(request, event_type)
         verifier = session.verifier(request)
-        self.update_event_properties(auth_provider=verifier.auth_provider.client_name)
+        if verifier and verifier.uses_auth_verification:
+            self.update_event_properties(auth_provider=verifier.auth_provider.client_name)
 
 
 class StartedSignInEvent(OAuthEvent):

benefits/oauth/middleware.py

@@ -0,0 +1,23 @@
+import logging
+
+from benefits.core import session
+from benefits.core.middleware import VerifierSessionRequired, user_error
+
+
+logger = logging.getLogger(__name__)
+
+
+class VerifierUsesAuthVerificationSessionRequired(VerifierSessionRequired):
+    """Middleware raises an exception for sessions lacking an eligibility verifier that uses auth verification."""
+
+    def process_request(self, request):
+        result = super().process_request(request)
+        if result:
+            # from the base middleware class, the session didn't have a verifier
+            return result
+
+        if session.verifier(request).uses_auth_verification:
+            return None
+        else:
+            logger.debug("Session not configured with eligibility verifier that uses auth verification")
+            return user_error(request)

benefits/oauth/views.py

@@ -5,9 +5,9 @@
 from django.utils.decorators import decorator_from_middleware
 
 from benefits.core import session
-from benefits.core.middleware import VerifierSessionRequired
 from . import analytics, redirects
 from .client import oauth
+from .middleware import VerifierUsesAuthVerificationSessionRequired
 
 
 logger = logging.getLogger(__name__)
@@ -20,7 +20,7 @@
 ROUTE_POST_LOGOUT = "oauth:post_logout"
 
 
-@decorator_from_middleware(VerifierSessionRequired)
+@decorator_from_middleware(VerifierUsesAuthVerificationSessionRequired)
 def login(request):
     """View implementing OIDC authorize_redirect."""
     verifier = session.verifier(request)
@@ -39,7 +39,7 @@ def login(request):
     return oauth_client.authorize_redirect(request, redirect_uri)
 
 
-@decorator_from_middleware(VerifierSessionRequired)
+@decorator_from_middleware(VerifierUsesAuthVerificationSessionRequired)
 def authorize(request):
     """View implementing OIDC token authorization."""
     verifier = session.verifier(request)
@@ -83,6 +83,7 @@ def authorize(request):
     return redirect(ROUTE_CONFIRM)
 
 
+@decorator_from_middleware(VerifierUsesAuthVerificationSessionRequired)
 def cancel(request):
     """View implementing cancellation of OIDC authorization."""
 
@@ -91,7 +92,7 @@ def cancel(request):
     return redirect(ROUTE_UNVERIFIED)
 
 
-@decorator_from_middleware(VerifierSessionRequired)
+@decorator_from_middleware(VerifierUsesAuthVerificationSessionRequired)
 def logout(request):
     """View implementing OIDC and application sign out."""
     verifier = session.verifier(request)
@@ -116,6 +117,7 @@ def logout(request):
     return redirects.deauthorize_redirect(oauth_client, token, redirect_uri)
 
 
+@decorator_from_middleware(VerifierUsesAuthVerificationSessionRequired)
 def post_logout(request):
     """View routes the user to their origin after sign out."""
 

tests/pytest/core/test_middleware_login_required.py

@@ -5,7 +5,6 @@
 
 from benefits.core import session
 from benefits.core.middleware import LoginRequired
-from benefits.core.models import EligibilityVerifier
 
 ROUTE_LOGIN = "oauth:login"
 
@@ -15,15 +14,8 @@ def decorated_view(mocked_view):
     return decorator_from_middleware(LoginRequired)(mocked_view)
 
 
-@pytest.fixture
-def require_login(mocker):
-    mock_verifier = mocker.Mock(spec=EligibilityVerifier)
-    mock_verifier.is_auth_required = True
-    mocker.patch("benefits.core.session.verifier", return_value=mock_verifier)
-
-
 @pytest.mark.django_db
-@pytest.mark.usefixtures("require_login")
+@pytest.mark.usefixtures("mocked_session_verifier_oauth")
 def test_login_auth_required(app_request, mocked_view, decorated_view):
     response = decorated_view(app_request)
 
@@ -45,7 +37,7 @@ def test_login_auth_not_required(app_request, model_EligibilityVerifier, mocked_
 
 
 @pytest.mark.django_db
-@pytest.mark.usefixtures("require_login")
+@pytest.mark.usefixtures("mocked_session_verifier_oauth")
 def test_logged_in(app_request, mocked_view, decorated_view):
     # log in
     session.update(app_request, oauth_token="something")

tests/pytest/core/test_middleware_verifier_required.py

@@ -0,0 +1,27 @@
+from django.utils.decorators import decorator_from_middleware
+
+import pytest
+
+from benefits.core.middleware import VerifierSessionRequired, TEMPLATE_USER_ERROR
+
+
+@pytest.fixture
+def decorated_view(mocked_view):
+    return decorator_from_middleware(VerifierSessionRequired)(mocked_view)
+
+
+@pytest.mark.django_db
+def test_verifier_required_no_verifier(app_request, mocked_view, decorated_view):
+    response = decorated_view(app_request)
+
+    mocked_view.assert_not_called()
+    assert response.status_code == 200
+    assert response.template_name == TEMPLATE_USER_ERROR
+
+
+@pytest.mark.django_db
+@pytest.mark.usefixtures("mocked_session_verifier_oauth")
+def test_verifier_required_verifier(app_request, mocked_view, decorated_view):
+    decorated_view(app_request)
+
+    mocked_view.assert_called_once()

tests/pytest/oauth/test_analytics.py

@@ -0,0 +1,31 @@
+import pytest
+
+from benefits.oauth.analytics import OAuthEvent
+
+
+@pytest.mark.django_db
+def test_OAuthEvent_checks_verifier_uses_auth_verification(app_request, mocked_session_verifier_auth_required):
+    mocked_verifier = mocked_session_verifier_auth_required(app_request)
+
+    OAuthEvent(app_request, "event type")
+
+    mocked_verifier.uses_auth_verification.assert_called_once
+
+
+@pytest.mark.django_db
+def test_OAuthEvent_verifier_client_name_when_uses_auth_verification(app_request, mocked_session_verifier_auth_required):
+    mocked_verifier = mocked_session_verifier_auth_required(app_request)
+    mocked_verifier.auth_provider.client_name = "ClientName"
+
+    event = OAuthEvent(app_request, "event type")
+
+    assert "auth_provider" in event.event_properties
+    assert event.event_properties["auth_provider"] == mocked_verifier.auth_provider.client_name
+
+
+@pytest.mark.django_db
+@pytest.mark.usefixtures("mocked_session_verifier_auth_not_required")
+def test_OAuthEvent_verifier_no_client_name_when_does_not_use_auth_verification(app_request):
+    event = OAuthEvent(app_request, "event type")
+
+    assert "auth_provider" not in event.event_properties

tests/pytest/oauth/test_middleware_authverifier_required.py

@@ -0,0 +1,38 @@
+from django.utils.decorators import decorator_from_middleware
+
+import pytest
+
+from benefits.core.middleware import TEMPLATE_USER_ERROR
+from benefits.oauth.middleware import VerifierUsesAuthVerificationSessionRequired
+
+
+@pytest.fixture
+def decorated_view(mocked_view):
+    return decorator_from_middleware(VerifierUsesAuthVerificationSessionRequired)(mocked_view)
+
+
+@pytest.mark.django_db
+def test_authverifier_required_no_verifier(app_request, mocked_view, decorated_view):
+    response = decorated_view(app_request)
+
+    mocked_view.assert_not_called()
+    assert response.status_code == 200
+    assert response.template_name == TEMPLATE_USER_ERROR
+
+
+@pytest.mark.django_db
+@pytest.mark.usefixtures("mocked_session_verifier_auth_not_required")
+def test_authverifier_required_no_authverifier(app_request, mocked_view, decorated_view):
+    response = decorated_view(app_request)
+
+    mocked_view.assert_not_called()
+    assert response.status_code == 200
+    assert response.template_name == TEMPLATE_USER_ERROR
+
+
+@pytest.mark.django_db
+@pytest.mark.usefixtures("mocked_session_verifier_oauth")
+def test_authverifier_required_authverifier(app_request, mocked_view, decorated_view):
+    decorated_view(app_request)
+
+    mocked_view.assert_called_once()

tests/pytest/oauth/test_views.py

@@ -4,7 +4,7 @@
 import pytest
 
 from benefits.core import session
-from benefits.core.middleware import ROUTE_INDEX
+from benefits.core.middleware import ROUTE_INDEX, TEMPLATE_USER_ERROR
 
 from benefits.oauth.views import ROUTE_START, ROUTE_CONFIRM, ROUTE_UNVERIFIED, login, authorize, cancel, logout, post_logout
 import benefits.oauth.views
@@ -24,6 +24,14 @@ def test_login_no_oauth_client(mocked_oauth_create_client, app_request):
         login(app_request)
 
 
+@pytest.mark.django_db
+def test_login_no_session_verifier(app_request):
+    result = login(app_request)
+
+    assert result.status_code == 200
+    assert result.template_name == TEMPLATE_USER_ERROR
+
+
 @pytest.mark.django_db
 def test_login(mocked_oauth_create_client, mocked_session_verifier_auth_required, mocked_analytics_module, app_request):
     assert not session.logged_in(app_request)
@@ -49,6 +57,14 @@ def test_authorize_no_oauth_client(mocked_oauth_create_client, app_request):
         authorize(app_request)
 
 
+@pytest.mark.django_db
+def test_authorize_no_session_verifier(app_request):
+    result = authorize(app_request)
+
+    assert result.status_code == 200
+    assert result.template_name == TEMPLATE_USER_ERROR
+
+
 @pytest.mark.django_db
 @pytest.mark.usefixtures("mocked_session_verifier_auth_required")
 def test_authorize_fail(mocked_oauth_create_client, app_request):
@@ -162,6 +178,8 @@ def test_authorize_success_without_claim_in_response(
     assert result.url == reverse(ROUTE_CONFIRM)
 
 
+@pytest.mark.django_db
+@pytest.mark.usefixtures("mocked_session_verifier_auth_required")
 def test_cancel(mocked_analytics_module, app_request):
     unverified_route = reverse(ROUTE_UNVERIFIED)
 
@@ -172,6 +190,14 @@ def test_cancel(mocked_analytics_module, app_request):
     assert result.url == unverified_route
 
 
+@pytest.mark.django_db
+def test_cancel_no_session_verifier(app_request):
+    result = cancel(app_request)
+
+    assert result.status_code == 200
+    assert result.template_name == TEMPLATE_USER_ERROR
+
+
 @pytest.mark.django_db
 @pytest.mark.usefixtures("mocked_session_verifier_auth_required")
 def test_logout_no_oauth_client(mocked_oauth_create_client, app_request):
@@ -181,6 +207,14 @@ def test_logout_no_oauth_client(mocked_oauth_create_client, app_request):
         logout(app_request)
 
 
+@pytest.mark.django_db
+def test_logout_no_session_verifier(app_request):
+    result = logout(app_request)
+
+    assert result.status_code == 200
+    assert result.template_name == TEMPLATE_USER_ERROR
+
+
 @pytest.mark.django_db
 @pytest.mark.usefixtures("mocked_session_verifier_auth_required")
 def test_logout(mocker, mocked_oauth_create_client, mocked_analytics_module, app_request):
@@ -208,6 +242,8 @@ def test_logout(mocker, mocked_oauth_create_client, mocked_analytics_module, app
     assert session.oauth_claim(app_request) is False
 
 
+@pytest.mark.django_db
+@pytest.mark.usefixtures("mocked_session_verifier_auth_required")
 def test_post_logout(app_request, mocked_analytics_module):
     origin = reverse(ROUTE_INDEX)
     session.update(app_request, origin=origin)
@@ -217,3 +253,11 @@ def test_post_logout(app_request, mocked_analytics_module):
     assert result.status_code == 302
     assert result.url == origin
     mocked_analytics_module.finished_sign_out.assert_called_once()
+
+
+@pytest.mark.django_db
+def test_post_logout_no_session_verifier(app_request):
+    result = post_logout(app_request)
+
+    assert result.status_code == 200
+    assert result.template_name == TEMPLATE_USER_ERROR