diff --git a/benefits/core/migrations/0019_refactor_idg_config_eligibilityverifier.py b/benefits/core/migrations/0019_refactor_idg_config_eligibilityverifier.py new file mode 100644 index 000000000..56e1d8c03 --- /dev/null +++ b/benefits/core/migrations/0019_refactor_idg_config_eligibilityverifier.py @@ -0,0 +1,55 @@ +# Generated by Django 5.0.6 on 2024-08-07 19:24 + +from django.db import migrations, models + + +def migrate_data(apps, schema_editor): + # This shouldn't be needed since we can get scope and claim from EligibilityVerifier.claims_provider + # but it's here temporarily in case it's needed + # ClaimsProvider = apps.get_model("core", "ClaimsProvider") + EligibilityVerifier = apps.get_model("core", "EligibilityVerifier") + + for verifier in EligibilityVerifier.objects.all(): + # The data migration is not working with + # verifier.claims_scope = ClaimsProvider.objects.filter(id=verifier.claims_provider.id).scope + # or with + # verifier.claims_scope = verifier.claims_provider.scope + # the code below is a placeholder for troubleshooting + verifier.claims_scope = "claims_scope" + # To migrate claim data, repeat the code that is used above using `claim` instead of `scope` + verifier.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("core", "0018_rename_eligibility_api_fields"), + ] + + operations = [ + migrations.AddField( + model_name="eligibilityverifier", + name="claims_claim", + field=models.TextField( + blank=True, help_text="The name of the claim (name/value pair) that is used to verify eligibility", null=True + ), + ), + migrations.AddField( + model_name="eligibilityverifier", + name="claims_scope", + field=models.TextField( + blank=True, + help_text="A space-separated list of identifiers used to specify what access privileges are being requested", + null=True, + ), + ), + migrations.RunPython(migrate_data), + migrations.RemoveField( + model_name="claimsprovider", + name="claim", + ), + migrations.RemoveField( + model_name="claimsprovider", + name="scope", + ), + ] diff --git a/benefits/core/models.py b/benefits/core/models.py index 5c343dd56..29bd9b66a 100644 --- a/benefits/core/models.py +++ b/benefits/core/models.py @@ -86,20 +86,8 @@ class ClaimsProvider(models.Model): help_text="The name of the secret containing the client ID for this claims provider" ) authority = models.TextField(help_text="The fully qualified HTTPS domain name for an OAuth authority server") - scope = models.TextField( - null=True, - blank=True, - help_text="A space-separated list of identifiers used to specify what access privileges are being requested", - ) - claim = models.TextField( - null=True, blank=True, help_text="The name of the claim (name/value pair) that is used to verify eligibility" - ) scheme = models.TextField(help_text="The authentication scheme to use") - @property - def supports_claims_verification(self): - return bool(self.scope) and bool(self.claim) - @property def supports_sign_out(self): return bool(self.sign_out_button_template) or bool(self.sign_out_link_template) @@ -194,6 +182,14 @@ class EligibilityVerifier(models.Model): eligibility_form_class = models.TextField(null=True, blank=True) eligibility_unverified_template = models.TextField(default="eligibility/unverified.html") help_template = models.TextField(null=True, blank=True) + claims_scope = models.TextField( + null=True, + 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" + ) class Meta: ordering = ["display_order"] @@ -213,10 +209,14 @@ def eligibility_api_public_key_data(self): """This Verifier's public key as a string.""" return self.eligibility_api_public_key.data + @property + def supports_claims_verification(self): + return bool(self.claims_scope) and bool(self.claims_claim) + @property def uses_claims_verification(self): """True if this Verifier verifies via the claims provider. False otherwise.""" - return self.claims_provider is not None and self.claims_provider.supports_claims_verification + return self.claims_provider is not None and self.supports_claims_verification def eligibility_form_instance(self, *args, **kwargs): """Return an instance of this verifier's form, or None.""" @@ -235,6 +235,25 @@ def by_id(id): logger.debug(f"Get {EligibilityVerifier.__name__} by id: {id}") return EligibilityVerifier.objects.get(id=id) + # Acceptance criteria: EligibilityVerifier.claims_scheme added as optional (leave the field as required on ClaimsProvider); + # default to using the value on ClaimsProvider if no value on EligibilityVerifier + # + # 2 potential ways to do it: @property decorator or default Field option (using a callable) + + # @property + # def claims_scheme(self): + # """Authentication scheme (optional), defaults to using the value on ClaimsProvider if no value is set here""" + # pass + # # TO-DO + # + # def claims_scheme_default(): + # # return ClaimsProvider.objects.filter(claims_provider_id=self.claims_provider) + # # return getattr(ClaimsProvider.objects.filter(claims_provider_id=self.claims_provider), "scheme") + # return "default_claims_scheme" + # + # claims_scheme = models.TextField(help_text="The authentication scheme to use", default=claims_scheme_default) + # **find out how to use self in the callable** + class TransitProcessor(models.Model): """An entity that applies transit agency fare rules to rider transactions.""" diff --git a/benefits/eligibility/verify.py b/benefits/eligibility/verify.py index 9a4986d34..8808d3b8e 100644 --- a/benefits/eligibility/verify.py +++ b/benefits/eligibility/verify.py @@ -30,7 +30,7 @@ def eligibility_from_api(verifier, form, agency): def eligibility_from_oauth(verifier, oauth_claim, agency): - if verifier.uses_claims_verification and verifier.claims_provider.claim == oauth_claim: + if verifier.uses_claims_verification and verifier.claims_claim == oauth_claim: return agency.type_names_to_verify(verifier) else: return [] diff --git a/benefits/oauth/client.py b/benefits/oauth/client.py index 596be9ce0..6c466cc54 100644 --- a/benefits/oauth/client.py +++ b/benefits/oauth/client.py @@ -39,32 +39,32 @@ def _authorize_params(scheme): return params -def _register_provider(oauth_registry, provider): +def _register_provider(oauth_registry, verifier): """ - Register OAuth clients into the given registry, using configuration from ClaimsProvider model. + Register OAuth clients into the given registry, using configuration from ClaimsProvider and EligibilityVerifier models. Adapted from https://stackoverflow.com/a/64174413. """ - logger.debug(f"Registering OAuth client: {provider.client_name}") + logger.debug(f"Registering OAuth client: {verifier.claims_provider.client_name}") client = oauth_registry.register( - provider.client_name, - client_id=provider.client_id, - server_metadata_url=_server_metadata_url(provider.authority), - client_kwargs=_client_kwargs(provider.scope), - authorize_params=_authorize_params(provider.scheme), + verifier.claims_provider.client_name, + client_id=verifier.claims_provider.client_id, + server_metadata_url=_server_metadata_url(verifier.claims_provider.authority), + client_kwargs=_client_kwargs(verifier.claims_scope), + authorize_params=_authorize_params(verifier.claims_provider.scheme), ) return client -def create_client(oauth_registry, provider): +def create_client(oauth_registry, verifier): """ Returns an OAuth client, registering it if needed. """ - client = oauth_registry.create_client(provider.client_name) + client = oauth_registry.create_client(verifier.claims_provider.client_name) if client is None: - client = _register_provider(oauth_registry, provider) + client = _register_provider(oauth_registry, verifier) return client diff --git a/benefits/oauth/views.py b/benefits/oauth/views.py index aa0e32ef6..1171320ff 100644 --- a/benefits/oauth/views.py +++ b/benefits/oauth/views.py @@ -24,7 +24,7 @@ TEMPLATE_SYSTEM_ERROR = "oauth/system_error.html" -def _oauth_client_or_error_redirect(request, claims_provider): +def _oauth_client_or_error_redirect(request, verifier): """Calls `benefits.oauth.client.create_client()`. If a client is created successfully, return it; Otherwise, return a redirect response to the `oauth:system-error` route. @@ -34,12 +34,12 @@ def _oauth_client_or_error_redirect(request, claims_provider): exception = None try: - oauth_client = create_client(oauth, claims_provider) + oauth_client = create_client(oauth, verifier) except Exception as ex: exception = ex if not oauth_client and not exception: - exception = Exception(f"oauth_client not registered: {claims_provider.client_name}") + exception = Exception(f"oauth_client not registered: {verifier.claims_provider.client_name}") if exception: analytics.error(request, message=str(exception), operation="init") @@ -54,7 +54,7 @@ def login(request): """View implementing OIDC authorize_redirect.""" verifier = session.verifier(request) - oauth_client_result = _oauth_client_or_error_redirect(request, verifier.claims_provider) + oauth_client_result = _oauth_client_or_error_redirect(request, verifier) if hasattr(oauth_client_result, "authorize_redirect"): # this looks like an oauth_client since it has the method we need @@ -95,7 +95,7 @@ def authorize(request): """View implementing OIDC token authorization.""" verifier = session.verifier(request) - oauth_client_result = _oauth_client_or_error_redirect(request, verifier.claims_provider) + oauth_client_result = _oauth_client_or_error_redirect(request, verifier) if hasattr(oauth_client_result, "authorize_access_token"): # this looks like an oauth_client since it has the method we need @@ -128,7 +128,7 @@ def authorize(request): id_token = token["id_token"] # We store the returned claim in case it can be used later in eligibility verification. - verifier_claim = verifier.claims_provider.claim + verifier_claim = verifier.claims_claim stored_claim = None error_claim = None @@ -168,7 +168,7 @@ def logout(request): """View implementing OIDC and application sign out.""" verifier = session.verifier(request) - oauth_client_result = _oauth_client_or_error_redirect(request, verifier.claims_provider) + oauth_client_result = _oauth_client_or_error_redirect(request, verifier) if hasattr(oauth_client_result, "load_server_metadata"): # this looks like an oauth_client since it has the method we need