diff --git a/benefits/core/migrations/0030_enrollmentevent_extra_claims.py b/benefits/core/migrations/0030_enrollmentevent_extra_claims.py new file mode 100644 index 0000000000..4f3a54308f --- /dev/null +++ b/benefits/core/migrations/0030_enrollmentevent_extra_claims.py @@ -0,0 +1,18 @@ +# Generated by Django 5.1.2 on 2024-10-29 22:11 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("core", "0029_add_extra_claims"), + ] + + operations = [ + migrations.AddField( + model_name="enrollmentevent", + name="extra_claims", + field=models.TextField(blank=True, null=True), + ), + ] diff --git a/benefits/core/models.py b/benefits/core/models.py index f6e5678e97..3b8b930797 100644 --- a/benefits/core/models.py +++ b/benefits/core/models.py @@ -483,6 +483,7 @@ class EnrollmentEvent(models.Model): verified_by = models.TextField() enrollment_datetime = models.DateTimeField(default=timezone.now) expiration_datetime = models.DateTimeField(blank=True, null=True) + extra_claims = models.TextField(blank=True, null=True) def __str__(self): dt = timezone.localtime(self.enrollment_datetime) diff --git a/benefits/core/session.py b/benefits/core/session.py index 771054645f..d5d5a07750 100644 --- a/benefits/core/session.py +++ b/benefits/core/session.py @@ -157,10 +157,23 @@ def oauth_token(request): def oauth_claims(request): - """Get the oauth claim from the request's session, or None""" + """Get the oauth claims from the request's session, or None""" return request.session.get(_OAUTH_CLAIMS) +def oauth_extra_claims(request): + """Get the extra oauth claims from the request's session, or None""" + claims = oauth_claims(request) + if claims: + f = flow(request) + if f and f.uses_claims_verification: + claims.remove(f.claims_eligibility_claim) + return claims + raise Exception("Oauth claims but no flow") + else: + return None + + def origin(request): """Get the origin for the request's session, or default to the index route.""" return request.session.get(_ORIGIN, reverse(routes.INDEX)) diff --git a/benefits/enrollment/analytics.py b/benefits/enrollment/analytics.py index da6b1bf9e6..0859ea8be3 100644 --- a/benefits/enrollment/analytics.py +++ b/benefits/enrollment/analytics.py @@ -8,10 +8,18 @@ class ReturnedEnrollmentEvent(core.Event): """Analytics event representing the end of transit processor enrollment request.""" - def __init__(self, request, status, error=None, enrollment_group=None, enrollment_method=models.EnrollmentMethods.DIGITAL): + def __init__( + self, + request, + status, + error=None, + enrollment_group=None, + enrollment_method=models.EnrollmentMethods.DIGITAL, + extra_claims=None, + ): super().__init__(request, "returned enrollment", enrollment_method) if str(status).lower() in ("error", "retry", "success"): - self.update_event_properties(status=status, error=error) + self.update_event_properties(status=status, error=error, extra_claims=extra_claims) if enrollment_group is not None: self.update_event_properties(enrollment_group=enrollment_group) @@ -35,11 +43,15 @@ def returned_retry(request, enrollment_method: str = models.EnrollmentMethods.DI core.send_event(ReturnedEnrollmentEvent(request, status="retry", enrollment_method=enrollment_method)) -def returned_success(request, enrollment_group, enrollment_method: str = models.EnrollmentMethods.DIGITAL): +def returned_success(request, enrollment_group, enrollment_method: str = models.EnrollmentMethods.DIGITAL, extra_claims=None): """Send the "returned enrollment" analytics event with a success status.""" core.send_event( ReturnedEnrollmentEvent( - request, status="success", enrollment_group=enrollment_group, enrollment_method=enrollment_method + request, + status="success", + enrollment_group=enrollment_group, + enrollment_method=enrollment_method, + extra_claims=extra_claims, ) ) diff --git a/benefits/enrollment/views.py b/benefits/enrollment/views.py index f0da5a8289..45fa7dfb17 100644 --- a/benefits/enrollment/views.py +++ b/benefits/enrollment/views.py @@ -71,15 +71,22 @@ def index(request): agency = session.agency(request) flow = session.flow(request) expiry = session.enrollment_expiry(request) + oauth_extra_claims = session.oauth_extra_claims(request) + # EnrollmentEvent expects a string value for extra_claims + if oauth_extra_claims: + str_extra_claims = ", ".join(oauth_extra_claims) + else: + str_extra_claims = "" event = models.EnrollmentEvent.objects.create( transit_agency=agency, enrollment_flow=flow, enrollment_method=models.EnrollmentMethods.DIGITAL, verified_by=flow.eligibility_verifier, expiration_datetime=expiry, + extra_claims=str_extra_claims, ) event.save() - analytics.returned_success(request, flow.group_id) + analytics.returned_success(request, flow.group_id, extra_claims=oauth_extra_claims) return success(request) case Status.SYSTEM_ERROR: diff --git a/benefits/oauth/views.py b/benefits/oauth/views.py index 8b633556eb..dc0ccd2aa7 100644 --- a/benefits/oauth/views.py +++ b/benefits/oauth/views.py @@ -126,7 +126,7 @@ def authorize(request): flow_claims = flow.claims_all_claims stored_claims = [] - error_claim = None + error_claim = {} if flow_claims: userinfo = token.get("userinfo") @@ -141,9 +141,8 @@ def authorize(request): elif claim_value == 1: # if userinfo contains our claim and the flag is 1 (true), store the *claim* stored_claims.append(claim) - elif claim_value >= 10 and claim == flow.claims_eligibility_claim: - # error_claim is only set if claim is the eligibility claim - error_claim = claim_value + elif claim_value >= 10: + error_claim[claim] = claim_value session.update(request, oauth_token=id_token, oauth_claims=stored_claims) analytics.finished_sign_in(request, error=error_claim) diff --git a/tests/pytest/conftest.py b/tests/pytest/conftest.py index a1ac35ab70..fbee1add68 100644 --- a/tests/pytest/conftest.py +++ b/tests/pytest/conftest.py @@ -274,3 +274,8 @@ def mocked_session_reset(mocker): @pytest.fixture def mocked_session_update(mocker): return mocker.patch("benefits.core.session.update") + + +@pytest.fixture +def mocked_session_oauth_extra_claims(mocker): + return mocker.patch("benefits.core.session.oauth_extra_claims") diff --git a/tests/pytest/core/test_session.py b/tests/pytest/core/test_session.py index db6e6ee9e2..086cd67c2b 100644 --- a/tests/pytest/core/test_session.py +++ b/tests/pytest/core/test_session.py @@ -446,3 +446,36 @@ def test_update_flow(app_request): @pytest.mark.django_db def test_flow_default(app_request): assert session.flow(app_request) is None + + +@pytest.mark.django_db +def test_oauth_extra_claims(app_request, model_EnrollmentFlow_with_scope_and_claim): + + app_request.session[session._FLOW] = model_EnrollmentFlow_with_scope_and_claim.id + app_request.session[session._OAUTH_CLAIMS] = [ + model_EnrollmentFlow_with_scope_and_claim.claims_eligibility_claim, + "extra_claim", + ] + + assert session.oauth_extra_claims(app_request) == ["extra_claim"] + + +@pytest.mark.django_db +def test_oauth_extra_claims_no_claims(app_request, model_EnrollmentFlow_with_scope_and_claim): + + app_request.session[session._FLOW] = model_EnrollmentFlow_with_scope_and_claim.id + app_request.session[session._OAUTH_CLAIMS] = [] + + assert session.oauth_extra_claims(app_request) is None + + +@pytest.mark.django_db +def test_oauth_extra_claims_claims_no_flow(app_request): + + app_request.session[session._OAUTH_CLAIMS] = [ + "eligibility_claim", + "extra_claim", + ] + + with pytest.raises(Exception, match="Oauth claims but no flow"): + session.oauth_extra_claims(app_request) diff --git a/tests/pytest/enrollment/test_analytics.py b/tests/pytest/enrollment/test_analytics.py index d774586a96..fba93050e0 100644 --- a/tests/pytest/enrollment/test_analytics.py +++ b/tests/pytest/enrollment/test_analytics.py @@ -1,6 +1,6 @@ import pytest -from benefits.enrollment.analytics import FailedAccessTokenRequestEvent +from benefits.enrollment.analytics import FailedAccessTokenRequestEvent, ReturnedEnrollmentEvent @pytest.mark.django_db @@ -8,3 +8,21 @@ def test_FailedAccessTokenRequestEvent_sets_status_code(app_request): event = FailedAccessTokenRequestEvent(app_request, 500) assert event.event_properties["status_code"] == 500 + + +@pytest.mark.django_db +def test_ReturnedEnrollmentEvent_without_error(app_request, mocker): + + key1 = "enrollment_flows" + key2 = "extra_claims" + mock_flow = mocker.Mock() + mock_flow.system_name = "flow_1" + mocker.patch("benefits.core.session.flow", return_value=mock_flow) + + mock_claims = ["eligibility_claim", "extra_claim"] + mocker.patch("benefits.core.session.oauth_claims", return_value=mock_claims) + + event = ReturnedEnrollmentEvent(app_request, status="success") + assert "error_code" not in event.event_properties + assert key1 in event.event_properties + assert key2 in event.event_properties diff --git a/tests/pytest/enrollment/test_views.py b/tests/pytest/enrollment/test_views.py index 81b027aa03..bc620f12ba 100644 --- a/tests/pytest/enrollment/test_views.py +++ b/tests/pytest/enrollment/test_views.py @@ -305,7 +305,9 @@ def test_index_eligible_post_valid_form_success_claims( mocked_analytics_module, model_TransitAgency, model_EnrollmentFlow_with_scope_and_claim, + mocked_session_oauth_extra_claims, ): + mocked_session_oauth_extra_claims.return_value = ["claim_1", "claim_2"] mocker.patch("benefits.enrollment.views.enroll", return_value=(Status.SUCCESS, None)) spy = mocker.spy(benefits.enrollment.views.models.EnrollmentEvent.objects, "create") @@ -318,6 +320,7 @@ def test_index_eligible_post_valid_form_success_claims( enrollment_method=models.EnrollmentMethods.DIGITAL, verified_by=model_EnrollmentFlow_with_scope_and_claim.claims_provider.client_name, expiration_datetime=None, + extra_claims="claim_1, claim_2", ) assert response.status_code == 200 @@ -335,7 +338,9 @@ def test_index_eligible_post_valid_form_success_eligibility_api( mocked_analytics_module, model_TransitAgency, model_EnrollmentFlow_with_eligibility_api, + mocked_session_oauth_extra_claims, ): + mocked_session_oauth_extra_claims.return_value = ["claim_1", "claim_2"] mocker.patch("benefits.enrollment.views.enroll", return_value=(Status.SUCCESS, None)) spy = mocker.spy(benefits.enrollment.views.models.EnrollmentEvent.objects, "create") @@ -348,6 +353,7 @@ def test_index_eligible_post_valid_form_success_eligibility_api( enrollment_method=models.EnrollmentMethods.DIGITAL, verified_by=model_EnrollmentFlow_with_eligibility_api.eligibility_api_url, expiration_datetime=None, + extra_claims="claim_1, claim_2", ) assert response.status_code == 200 diff --git a/tests/pytest/oauth/test_views.py b/tests/pytest/oauth/test_views.py index 7678454906..26fa98ee37 100644 --- a/tests/pytest/oauth/test_views.py +++ b/tests/pytest/oauth/test_views.py @@ -237,31 +237,62 @@ def test_authorize_success( @pytest.mark.django_db @pytest.mark.usefixtures("mocked_analytics_module") +@pytest.mark.parametrize( + "extra_claims,userinfo,oauth_claims", + [ + (None, {"claim": 1}, ["claim"]), + ("", {"claim": 1}, ["claim"]), + ("extra_claim", {"claim": 1, "extra_claim": 1}, ["claim", "extra_claim"]), + ( + "extra_claim_1 extra_claim_2", + {"claim": 1, "extra_claim_1": 1, "extra_claim_2": 1}, + ["claim", "extra_claim_1", "extra_claim_2"], + ), + ], +) def test_authorize_success_with_claim_true( - app_request, mocked_session_flow_uses_claims_verification, mocked_oauth_client_or_error_redirect__client + app_request, + mocked_session_flow_uses_claims_verification, + mocked_oauth_client_or_error_redirect__client, + extra_claims, + userinfo, + oauth_claims, ): flow = mocked_session_flow_uses_claims_verification.return_value - flow.claims_extra_claims = "" + flow.claims_extra_claims = extra_claims mocked_oauth_client = mocked_oauth_client_or_error_redirect__client.return_value - mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token", "userinfo": {"claim": "1"}} + mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token", "userinfo": userinfo} result = authorize(app_request) mocked_oauth_client.authorize_access_token.assert_called_with(app_request) - assert session.oauth_claims(app_request) == ["claim"] + assert session.oauth_claims(app_request) == oauth_claims assert result.status_code == 302 assert result.url == reverse(routes.ELIGIBILITY_CONFIRM) @pytest.mark.django_db @pytest.mark.usefixtures("mocked_analytics_module") +@pytest.mark.parametrize( + "extra_claims,userinfo", + [ + (None, {"claim": 0}), + ("", {"claim": 0}), + ("extra_claim", {"claim": 0, "extra_claim": 0}), + ("extra_claim_1 extra_claim_2", {"claim": 0, "extra_claim_1": 0, "extra_claim_2": 0}), + ], +) def test_authorize_success_with_claim_false( - app_request, mocked_session_flow_uses_claims_verification, mocked_oauth_client_or_error_redirect__client + app_request, + mocked_session_flow_uses_claims_verification, + mocked_oauth_client_or_error_redirect__client, + extra_claims, + userinfo, ): flow = mocked_session_flow_uses_claims_verification.return_value - flow.claims_extra_claims = "" + flow.claims_extra_claims = extra_claims mocked_oauth_client = mocked_oauth_client_or_error_redirect__client.return_value - mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token", "userinfo": {"claim": "0"}} + mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token", "userinfo": userinfo} result = authorize(app_request) @@ -272,21 +303,32 @@ def test_authorize_success_with_claim_false( @pytest.mark.django_db +@pytest.mark.parametrize( + "extra_claims,userinfo", + [ + (None, {"claim": 10}), + ("", {"claim": 10}), + ("extra_claim", {"claim": 10, "extra_claim": 10}), + ("extra_claim_1 extra_claim_2", {"claim": 10, "extra_claim_1": 10, "extra_claim_2": 10}), + ], +) def test_authorize_success_with_claim_error( app_request, mocked_session_flow_uses_claims_verification, mocked_oauth_client_or_error_redirect__client, mocked_analytics_module, + extra_claims, + userinfo, ): flow = mocked_session_flow_uses_claims_verification.return_value - flow.claims_extra_claims = "" + flow.claims_extra_claims = extra_claims mocked_oauth_client = mocked_oauth_client_or_error_redirect__client.return_value - mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token", "userinfo": {"claim": "10"}} + mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token", "userinfo": userinfo} result = authorize(app_request) mocked_oauth_client.authorize_access_token.assert_called_with(app_request) - mocked_analytics_module.finished_sign_in.assert_called_with(app_request, error=10) + mocked_analytics_module.finished_sign_in.assert_called_with(app_request, error=userinfo) assert session.oauth_claims(app_request) == [] assert result.status_code == 302 assert result.url == reverse(routes.ELIGIBILITY_CONFIRM)