Skip to content

Commit

Permalink
refactor(model): move scope and claim config to EligibilityVerifier
Browse files Browse the repository at this point in the history
  • Loading branch information
lalver1 committed Aug 7, 2024
1 parent 35d8fce commit 5a64bb7
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 32 deletions.
Original file line number Diff line number Diff line change
@@ -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",
),
]
45 changes: 32 additions & 13 deletions benefits/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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"]
Expand All @@ -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."""
Expand All @@ -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."""
Expand Down
2 changes: 1 addition & 1 deletion benefits/eligibility/verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 []
22 changes: 11 additions & 11 deletions benefits/oauth/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
14 changes: 7 additions & 7 deletions benefits/oauth/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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")
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 5a64bb7

Please sign in to comment.