From f9eaa59d68b1dfb9eccb791bfc0bfe018095fde3 Mon Sep 17 00:00:00 2001 From: Luis Alvergue Date: Fri, 11 Oct 2024 19:06:20 +0000 Subject: [PATCH 1/3] feat: implement both main and extra claims for an enrollment flow --- .../core/migrations/0029_add_extra_claims.py | 30 +++++++++++++++++++ benefits/core/migrations/local_fixtures.json | 8 ++--- benefits/core/models.py | 14 +++++++-- benefits/eligibility/verify.py | 4 +-- tests/pytest/conftest.py | 2 +- tests/pytest/core/test_models.py | 15 ++++++++++ tests/pytest/eligibility/test_verify.py | 10 +++---- 7 files changed, 68 insertions(+), 15 deletions(-) create mode 100644 benefits/core/migrations/0029_add_extra_claims.py diff --git a/benefits/core/migrations/0029_add_extra_claims.py b/benefits/core/migrations/0029_add_extra_claims.py new file mode 100644 index 0000000000..1391d94753 --- /dev/null +++ b/benefits/core/migrations/0029_add_extra_claims.py @@ -0,0 +1,30 @@ +# Generated by Django 5.1.1 on 2024-10-18 19:38 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("core", "0028_remove_transitagency_enrollment_flows_and_more"), + ] + + operations = [ + migrations.RenameField( + model_name="enrollmentflow", + old_name="claims_claim", + new_name="claims_eligibility_claim", + ), + migrations.AddField( + model_name="enrollmentflow", + name="claims_extra_claims", + field=models.TextField(blank=True, help_text="A space-separated list of any additional claims", null=True), + ), + migrations.AlterField( + model_name="enrollmentflow", + name="claims_eligibility_claim", + field=models.TextField( + blank=True, help_text="The name of the claim that is used to verify eligibility", null=True + ), + ), + ] diff --git a/benefits/core/migrations/local_fixtures.json b/benefits/core/migrations/local_fixtures.json index 30f88e95bd..372267b34b 100644 --- a/benefits/core/migrations/local_fixtures.json +++ b/benefits/core/migrations/local_fixtures.json @@ -85,7 +85,7 @@ "selection_label_template": "eligibility/includes/selection-label--senior.html", "eligibility_start_template": "eligibility/start--senior.html", "claims_scope": "verify:senior", - "claims_claim": "senior", + "claims_eligibility_claim": "senior", "supported_enrollment_methods": ["digital", "in_person"], "transit_agency": 1 } @@ -103,7 +103,7 @@ "selection_label_template": "eligibility/includes/selection-label--veteran.html", "eligibility_start_template": "eligibility/start--veteran.html", "claims_scope": "verify:veteran", - "claims_claim": "veteran", + "claims_eligibility_claim": "veteran", "supported_enrollment_methods": ["digital", "in_person"], "transit_agency": 1 } @@ -152,7 +152,7 @@ "eligibility_start_template": "eligibility/start--calfresh.html", "help_template": "core/includes/help--calfresh.html", "claims_scope": "verify:calfresh", - "claims_claim": "calfresh", + "claims_eligibility_claim": "calfresh", "supported_enrollment_methods": ["digital", "in_person"], "transit_agency": 1 } @@ -171,7 +171,7 @@ "eligibility_start_template": "eligibility/start--medicare.html", "help_template": "core/includes/help--medicare.html", "claims_scope": "verify:medicare", - "claims_claim": "medicare", + "claims_eligibility_claim": "medicare", "supported_enrollment_methods": ["digital", "in_person"], "transit_agency": 1 } diff --git a/benefits/core/models.py b/benefits/core/models.py index 7b29a55f08..f6e5678e97 100644 --- a/benefits/core/models.py +++ b/benefits/core/models.py @@ -282,9 +282,10 @@ class EnrollmentFlow(models.Model): blank=True, help_text="A space-separated list of identifiers used to specify what access privileges are being requested", ) - claims_claim = models.TextField( - null=True, blank=True, help_text="The name of the claim (name/value pair) that is used to verify eligibility" + claims_eligibility_claim = models.TextField( + null=True, blank=True, help_text="The name of the claim that is used to verify eligibility" ) + claims_extra_claims = models.TextField(null=True, blank=True, help_text="A space-separated list of any additional claims") claims_scheme_override = models.TextField( help_text="The authentication scheme to use (Optional). If blank, defaults to the value in Claims providers", default=None, @@ -404,7 +405,7 @@ def eligibility_api_public_key_data(self): @property def uses_claims_verification(self): """True if this flow verifies via the claims provider and has a scope and claim. False otherwise.""" - return self.claims_provider is not None and bool(self.claims_scope) and bool(self.claims_claim) + return self.claims_provider is not None and bool(self.claims_scope) and bool(self.claims_eligibility_claim) @property def eligibility_verifier(self): @@ -459,6 +460,13 @@ def claims_scheme(self): return self.claims_provider.scheme return self.claims_scheme_override + @property + def claims_all_claims(self): + claims = [self.claims_eligibility_claim] + if self.claims_extra_claims is not None: + claims.extend(self.claims_extra_claims.split()) + return claims + class EnrollmentEvent(models.Model): """A record of a successful enrollment.""" diff --git a/benefits/eligibility/verify.py b/benefits/eligibility/verify.py index e0808b75de..e1052123c5 100644 --- a/benefits/eligibility/verify.py +++ b/benefits/eligibility/verify.py @@ -32,8 +32,8 @@ def eligibility_from_api(flow: models.EnrollmentFlow, form, agency: models.Trans return False -def eligibility_from_oauth(flow: models.EnrollmentFlow, oauth_claim, agency: models.TransitAgency): - if flow.uses_claims_verification and flow.claims_claim == oauth_claim: +def eligibility_from_oauth(flow: models.EnrollmentFlow, oauth_claims, agency: models.TransitAgency): + if flow.uses_claims_verification and flow.claims_eligibility_claim in oauth_claims: return True else: return False diff --git a/tests/pytest/conftest.py b/tests/pytest/conftest.py index 67fdda3dcb..a1ac35ab70 100644 --- a/tests/pytest/conftest.py +++ b/tests/pytest/conftest.py @@ -105,7 +105,7 @@ def model_EnrollmentFlow_with_eligibility_api(model_EnrollmentFlow, model_PemDat def model_EnrollmentFlow_with_scope_and_claim(model_EnrollmentFlow, model_ClaimsProvider): model_EnrollmentFlow.claims_provider = model_ClaimsProvider model_EnrollmentFlow.claims_scope = "scope" - model_EnrollmentFlow.claims_claim = "claim" + model_EnrollmentFlow.claims_eligibility_claim = "claim" model_EnrollmentFlow.save() return model_EnrollmentFlow diff --git a/tests/pytest/core/test_models.py b/tests/pytest/core/test_models.py index 50aa34d2af..8b81f3527c 100644 --- a/tests/pytest/core/test_models.py +++ b/tests/pytest/core/test_models.py @@ -193,6 +193,21 @@ def test_EnrollmentFlow_supported_enrollment_methods(model_TransitAgency): assert new_flow.supported_enrollment_methods == ["digital", "in_person"] +@pytest.mark.django_db +@pytest.mark.parametrize( + "extra_claims,all_claims", + [ + (None, ["claim"]), + ("extra_claim", ["claim", "extra_claim"]), + ("extra_claim_1 extra_claim_2", ["claim", "extra_claim_1", "extra_claim_2"]), + ], +) +def test_EnrollmentFlow_claims_all_claims(model_EnrollmentFlow_with_scope_and_claim, extra_claims, all_claims): + model_EnrollmentFlow_with_scope_and_claim.claims_extra_claims = extra_claims + model_EnrollmentFlow_with_scope_and_claim.save() + assert model_EnrollmentFlow_with_scope_and_claim.claims_all_claims == all_claims + + class SampleFormClass: """A class for testing EligibilityVerificationForm references.""" diff --git a/tests/pytest/eligibility/test_verify.py b/tests/pytest/eligibility/test_verify.py index 78f56eb989..fcb2c3d73b 100644 --- a/tests/pytest/eligibility/test_verify.py +++ b/tests/pytest/eligibility/test_verify.py @@ -60,7 +60,7 @@ def test_eligibility_from_oauth_does_not_use_claims_verification( # mocked_session_flow_does_not_use_claims_verification is Mocked version of the session.flow() function flow = mocked_session_flow_does_not_use_claims_verification.return_value - response = eligibility_from_oauth(flow, "claim", model_TransitAgency) + response = eligibility_from_oauth(flow, ["claim"], model_TransitAgency) assert response is False @@ -69,9 +69,9 @@ def test_eligibility_from_oauth_does_not_use_claims_verification( def test_eligibility_from_oauth_claim_mismatch(mocked_session_flow_uses_claims_verification, model_TransitAgency): # mocked_session_flow_uses_claims_verification is Mocked version of the session.flow() function flow = mocked_session_flow_uses_claims_verification.return_value - flow.claims_claim = "claim" + flow.claims_eligibility_claim = "claim" - response = eligibility_from_oauth(flow, "some_other_claim", model_TransitAgency) + response = eligibility_from_oauth(flow, ["some_other_claim"], model_TransitAgency) assert response is False @@ -80,8 +80,8 @@ def test_eligibility_from_oauth_claim_mismatch(mocked_session_flow_uses_claims_v def test_eligibility_from_oauth_claim_match(mocked_session_flow_uses_claims_verification, model_TransitAgency): # mocked_session_flow_uses_claims_verification is Mocked version of the session.flow() function flow = mocked_session_flow_uses_claims_verification.return_value - flow.claims_claim = "claim" + flow.claims_eligibility_claim = "claim" - response = eligibility_from_oauth(flow, "claim", model_TransitAgency) + response = eligibility_from_oauth(flow, ["claim"], model_TransitAgency) assert response is True From 9754e649a6834ce5752e36cf52a47d6858163464 Mon Sep 17 00:00:00 2001 From: Luis Alvergue Date: Thu, 17 Oct 2024 20:21:35 +0000 Subject: [PATCH 2/3] feat: store one or more returned claims --- benefits/oauth/views.py | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/benefits/oauth/views.py b/benefits/oauth/views.py index 102a864023..8b633556eb 100644 --- a/benefits/oauth/views.py +++ b/benefits/oauth/views.py @@ -123,27 +123,29 @@ def authorize(request): id_token = token["id_token"] # We store the returned claim in case it can be used later in eligibility verification. - flow_claim = flow.claims_claim - stored_claim = None + flow_claims = flow.claims_all_claims + stored_claims = [] error_claim = None - if flow_claim: + if flow_claims: userinfo = token.get("userinfo") if userinfo: - claim_value = userinfo.get(flow_claim) - # the claim comes back in userinfo like { "claim": "1" | "0" } - claim_value = int(claim_value) if claim_value else None - if claim_value is None: - logger.warning(f"userinfo did not contain: {flow_claim}") - elif claim_value == 1: - # if userinfo contains our claim and the flag is 1 (true), store the *claim* - stored_claim = flow_claim - elif claim_value >= 10: - error_claim = claim_value - - session.update(request, oauth_token=id_token, oauth_claim=stored_claim) + for claim in flow_claims: + claim_value = userinfo.get(claim) + # the claim comes back in userinfo like { "claim": "1" | "0" } + claim_value = int(claim_value) if claim_value else None + if claim_value is None: + logger.warning(f"userinfo did not contain: {claim}") + 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 + + session.update(request, oauth_token=id_token, oauth_claims=stored_claims) analytics.finished_sign_in(request, error=error_claim) return redirect(routes.ELIGIBILITY_CONFIRM) From c0a2516db02df7dea3467262321d145e5117d8b4 Mon Sep 17 00:00:00 2001 From: Luis Alvergue Date: Thu, 17 Oct 2024 20:23:18 +0000 Subject: [PATCH 3/3] feat: update session to handle one or more returned claims --- benefits/core/session.py | 18 +++++++++--------- benefits/eligibility/views.py | 2 +- tests/pytest/core/test_session.py | 10 +++++----- tests/pytest/oauth/test_views.py | 30 +++++++++++++++++++----------- 4 files changed, 34 insertions(+), 26 deletions(-) diff --git a/benefits/core/session.py b/benefits/core/session.py index e4531cb514..771054645f 100644 --- a/benefits/core/session.py +++ b/benefits/core/session.py @@ -26,7 +26,7 @@ _ENROLLMENT_EXP = "enrollment_expiry" _FLOW = "flow" _LANG = "lang" -_OAUTH_CLAIM = "oauth_claim" +_OAUTH_CLAIMS = "oauth_claims" _OAUTH_TOKEN = "oauth_token" _ORIGIN = "origin" _START = "start" @@ -60,7 +60,7 @@ def context_dict(request): _ENROLLMENT_TOKEN_EXP: enrollment_token_expiry(request), _LANG: language(request), _OAUTH_TOKEN: oauth_token(request), - _OAUTH_CLAIM: oauth_claim(request), + _OAUTH_CLAIMS: oauth_claims(request), _ORIGIN: origin(request), _START: start(request), _UID: uid(request), @@ -148,7 +148,7 @@ def logged_in(request): def logout(request): """Reset the session claims and tokens.""" - update(request, oauth_claim=False, oauth_token=False, enrollment_token=False) + update(request, oauth_claims=[], oauth_token=False, enrollment_token=False) def oauth_token(request): @@ -156,9 +156,9 @@ def oauth_token(request): return request.session.get(_OAUTH_TOKEN) -def oauth_claim(request): +def oauth_claims(request): """Get the oauth claim from the request's session, or None""" - return request.session.get(_OAUTH_CLAIM) + return request.session.get(_OAUTH_CLAIMS) def origin(request): @@ -177,7 +177,7 @@ def reset(request): request.session[_ENROLLMENT_TOKEN] = None request.session[_ENROLLMENT_TOKEN_EXP] = None request.session[_OAUTH_TOKEN] = None - request.session[_OAUTH_CLAIM] = None + request.session[_OAUTH_CLAIMS] = None if _UID not in request.session or not request.session[_UID]: logger.debug("Reset session time and uid") @@ -236,7 +236,7 @@ def update( enrollment_token=None, enrollment_token_exp=None, oauth_token=None, - oauth_claim=None, + oauth_claims=None, origin=None, ): """Update the request's session with non-null values.""" @@ -260,8 +260,8 @@ def update( request.session[_ENROLLMENT_TOKEN_EXP] = enrollment_token_exp if oauth_token is not None: request.session[_OAUTH_TOKEN] = oauth_token - if oauth_claim is not None: - request.session[_OAUTH_CLAIM] = oauth_claim + if oauth_claims is not None: + request.session[_OAUTH_CLAIMS] = oauth_claims if origin is not None: request.session[_ORIGIN] = origin if flow is not None and isinstance(flow, models.EnrollmentFlow): diff --git a/benefits/eligibility/views.py b/benefits/eligibility/views.py index 1b495fa43a..45e96481b8 100644 --- a/benefits/eligibility/views.py +++ b/benefits/eligibility/views.py @@ -85,7 +85,7 @@ def confirm(request): if request.method == "GET" and flow.uses_claims_verification: analytics.started_eligibility(request, flow) - is_verified = verify.eligibility_from_oauth(flow, session.oauth_claim(request), agency) + is_verified = verify.eligibility_from_oauth(flow, session.oauth_claims(request), agency) if is_verified: return verified(request) diff --git a/tests/pytest/core/test_session.py b/tests/pytest/core/test_session.py index 54ae320b0c..db6e6ee9e2 100644 --- a/tests/pytest/core/test_session.py +++ b/tests/pytest/core/test_session.py @@ -199,16 +199,16 @@ def test_logged_in_True(app_request): @pytest.mark.django_db def test_logout(app_request): - session.update(app_request, oauth_claim="oauth_claim", oauth_token="oauth_token", enrollment_token="enrollment_token") + session.update(app_request, oauth_claims=["oauth_claim"], oauth_token="oauth_token", enrollment_token="enrollment_token") assert session.logged_in(app_request) - assert session.oauth_claim(app_request) + assert session.oauth_claims(app_request) session.logout(app_request) assert not session.logged_in(app_request) assert not session.enrollment_token(app_request) assert not session.oauth_token(app_request) - assert not session.oauth_claim(app_request) + assert not session.oauth_claims(app_request) @pytest.mark.django_db @@ -269,12 +269,12 @@ def test_reset_enrollment(app_request): @pytest.mark.django_db def test_reset_oauth(app_request): app_request.session[session._OAUTH_TOKEN] = "oauthtoken456" - app_request.session[session._OAUTH_CLAIM] = "claim" + app_request.session[session._OAUTH_CLAIMS] = ["claim"] session.reset(app_request) assert session.oauth_token(app_request) is None - assert session.oauth_claim(app_request) is None + assert session.oauth_claims(app_request) is None @pytest.mark.django_db diff --git a/tests/pytest/oauth/test_views.py b/tests/pytest/oauth/test_views.py index 20246bc281..7678454906 100644 --- a/tests/pytest/oauth/test_views.py +++ b/tests/pytest/oauth/test_views.py @@ -213,11 +213,18 @@ def test_authorize_empty_token( @pytest.mark.django_db -@pytest.mark.usefixtures("mocked_session_flow_uses_claims_verification") -def test_authorize_success(mocked_oauth_client_or_error_redirect__client, mocked_analytics_module, app_request): +def test_authorize_success( + mocked_session_flow_uses_claims_verification, + mocked_oauth_client_or_error_redirect__client, + mocked_analytics_module, + app_request, +): mocked_oauth_client = mocked_oauth_client_or_error_redirect__client.return_value mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token"} + flow = mocked_session_flow_uses_claims_verification.return_value + flow.claims_extra_claims = "" + result = authorize(app_request) mocked_oauth_client.authorize_access_token.assert_called_with(app_request) @@ -234,14 +241,14 @@ def test_authorize_success_with_claim_true( app_request, mocked_session_flow_uses_claims_verification, mocked_oauth_client_or_error_redirect__client ): flow = mocked_session_flow_uses_claims_verification.return_value - flow.claims_claim = "claim" + flow.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"}} result = authorize(app_request) mocked_oauth_client.authorize_access_token.assert_called_with(app_request) - assert session.oauth_claim(app_request) == "claim" + assert session.oauth_claims(app_request) == ["claim"] assert result.status_code == 302 assert result.url == reverse(routes.ELIGIBILITY_CONFIRM) @@ -252,14 +259,14 @@ def test_authorize_success_with_claim_false( app_request, mocked_session_flow_uses_claims_verification, mocked_oauth_client_or_error_redirect__client ): flow = mocked_session_flow_uses_claims_verification.return_value - flow.claims_claim = "claim" + flow.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"}} result = authorize(app_request) mocked_oauth_client.authorize_access_token.assert_called_with(app_request) - assert session.oauth_claim(app_request) is None + assert session.oauth_claims(app_request) == [] assert result.status_code == 302 assert result.url == reverse(routes.ELIGIBILITY_CONFIRM) @@ -272,7 +279,7 @@ def test_authorize_success_with_claim_error( mocked_analytics_module, ): flow = mocked_session_flow_uses_claims_verification.return_value - flow.claims_claim = "claim" + flow.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"}} @@ -280,7 +287,7 @@ def test_authorize_success_with_claim_error( mocked_oauth_client.authorize_access_token.assert_called_with(app_request) mocked_analytics_module.finished_sign_in.assert_called_with(app_request, error=10) - assert session.oauth_claim(app_request) is None + assert session.oauth_claims(app_request) == [] assert result.status_code == 302 assert result.url == reverse(routes.ELIGIBILITY_CONFIRM) @@ -301,14 +308,15 @@ def test_authorize_success_without_claim_in_response( access_token_response, ): flow = mocked_session_flow_uses_claims_verification.return_value - flow.claims_claim = "claim" + flow.claims_eligibility_claim = "claim" + flow.claims_extra_claims = "" mocked_oauth_client = mocked_oauth_client_or_error_redirect__client.return_value mocked_oauth_client.authorize_access_token.return_value = access_token_response result = authorize(app_request) mocked_oauth_client.authorize_access_token.assert_called_with(app_request) - assert session.oauth_claim(app_request) is None + assert session.oauth_claims(app_request) == [] assert result.status_code == 302 assert result.url == reverse(routes.ELIGIBILITY_CONFIRM) @@ -374,7 +382,7 @@ def test_logout(app_request, mocker, mocked_oauth_client_or_error_redirect__clie assert not session.logged_in(app_request) assert session.enrollment_token(app_request) is False assert session.oauth_token(app_request) is False - assert session.oauth_claim(app_request) is False + assert session.oauth_claims(app_request) == [] @pytest.mark.django_db