Skip to content

Commit

Permalink
refactor: consolidate methods that basically represent the same thing
Browse files Browse the repository at this point in the history
the distinction between "requiring login" and "using claims to verify" is
unnecessary. this makes for one less method to rename.
  • Loading branch information
angela-tran committed Aug 2, 2024
1 parent 0fa98ee commit c3ada61
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 15 deletions.
2 changes: 1 addition & 1 deletion benefits/core/context_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def authentication(request):
"logged_in": session.logged_in(request),
}

if verifier.is_auth_required:
if verifier.uses_auth_verification:
data["sign_out_button_template"] = verifier.claims_provider.sign_out_button_template
data["sign_out_link_template"] = verifier.claims_provider.sign_out_link_template

Expand Down
2 changes: 1 addition & 1 deletion benefits/core/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class LoginRequired(MiddlewareMixin):
def process_view(self, request, view_func, view_args, view_kwargs):
# only require login if verifier requires it
verifier = session.verifier(request)
if not verifier or not verifier.is_auth_required or session.logged_in(request):
if not verifier or not verifier.uses_auth_verification or session.logged_in(request):
# pass through
return None

Expand Down
9 changes: 2 additions & 7 deletions benefits/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,15 +213,10 @@ def public_key_data(self):
"""This Verifier's public key as a string."""
return self.public_key.data

@property
def is_auth_required(self):
"""True if this Verifier requires authentication. False otherwise."""
return self.claims_provider is not None

@property
def uses_auth_verification(self):
"""True if this Verifier verifies via the auth provider. False otherwise."""
return self.is_auth_required and self.claims_provider.supports_claims_verification
"""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

def form_instance(self, *args, **kwargs):
"""Return an instance of this verifier's form, or None."""
Expand Down
2 changes: 1 addition & 1 deletion tests/pytest/core/test_middleware_login_required.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def test_login_auth_required(app_request, mocked_view, decorated_view):
@pytest.mark.django_db
def test_login_auth_not_required(app_request, model_EligibilityVerifier, mocked_view, decorated_view):
model_EligibilityVerifier.claims_provider = None
assert not model_EligibilityVerifier.is_auth_required
assert not model_EligibilityVerifier.uses_auth_verification
session.update(app_request, verifier=model_EligibilityVerifier)

decorated_view(app_request)
Expand Down
5 changes: 0 additions & 5 deletions tests/pytest/core/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,6 @@ def test_EligibilityVerifier_with_ClaimsProvider_with_verification(
):
model_EligibilityVerifier.claims_provider = model_ClaimsProvider_with_verification

assert model_EligibilityVerifier.is_auth_required
assert model_EligibilityVerifier.uses_auth_verification


Expand All @@ -307,7 +306,6 @@ def test_EligibilityVerifier_with_ClaimsProvider_with_verification_no_sign_out(
):
model_EligibilityVerifier.claims_provider = model_ClaimsProvider_with_verification_no_sign_out

assert model_EligibilityVerifier.is_auth_required
assert model_EligibilityVerifier.uses_auth_verification


Expand All @@ -317,7 +315,6 @@ def test_EligibilityVerifier_with_ClaimsProvider_without_verification(
):
model_EligibilityVerifier.claims_provider = model_ClaimsProvider_without_verification

assert model_EligibilityVerifier.is_auth_required
assert not model_EligibilityVerifier.uses_auth_verification


Expand All @@ -327,15 +324,13 @@ def test_EligibilityVerifier_with_ClaimsProvider_without_verification_no_sign_ou
):
model_EligibilityVerifier.claims_provider = model_ClaimsProvider_without_verification_no_sign_out

assert model_EligibilityVerifier.is_auth_required
assert not model_EligibilityVerifier.uses_auth_verification


@pytest.mark.django_db
def test_EligibilityVerifier_without_ClaimsProvider(model_EligibilityVerifier):
model_EligibilityVerifier.claims_provider = None

assert not model_EligibilityVerifier.is_auth_required
assert not model_EligibilityVerifier.uses_auth_verification


Expand Down

0 comments on commit c3ada61

Please sign in to comment.