Skip to content

Commit

Permalink
Refactor: sign out button (#1534)
Browse files Browse the repository at this point in the history
  • Loading branch information
thekaveman authored Jul 14, 2023
2 parents 5efdcb8 + dd14e49 commit 6d24a70
Show file tree
Hide file tree
Showing 18 changed files with 115 additions and 76 deletions.
8 changes: 2 additions & 6 deletions benefits/core/context_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
The core application: context processors for enriching request context data.
"""
from django.conf import settings
from django.urls import reverse

from . import models, session

Expand Down Expand Up @@ -44,15 +43,12 @@ def authentication(request):

if verifier:
data = {
"required": verifier.is_auth_required,
"logged_in": session.logged_in(request),
"supports_sign_out": verifier.supports_sign_out,
"sign_out_route": reverse("oauth:logout"),
}

if verifier.is_auth_required:
auth_provider = verifier.auth_provider
data["sign_out_button_label"] = auth_provider.sign_out_button_label
data["sign_out_button_template"] = verifier.auth_provider.sign_out_button_template
data["sign_out_link_template"] = verifier.auth_provider.sign_out_link_template

return {"authentication": data}
else:
Expand Down
5 changes: 3 additions & 2 deletions benefits/core/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Generated by Django 4.2.3 on 2023-07-11 17:22
# Generated by Django 4.2.3 on 2023-07-14 04:51

from django.db import migrations, models
import django.db.models.deletion
Expand All @@ -14,7 +14,8 @@ class Migration(migrations.Migration):
name="AuthProvider",
fields=[
("id", models.AutoField(primary_key=True, serialize=False)),
("sign_out_button_label", models.TextField(null=True)),
("sign_out_button_template", models.TextField(null=True)),
("sign_out_link_template", models.TextField(null=True)),
("client_name", models.TextField()),
("client_id", models.TextField()),
("authority", models.TextField()),
Expand Down
3 changes: 2 additions & 1 deletion benefits/core/migrations/0002_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ def load_data(app, *args, **kwargs):
AuthProvider = app.get_model("core", "AuthProvider")

senior_auth_provider = AuthProvider.objects.create(
sign_out_button_label=_("eligibility.buttons.senior.signout"),
sign_out_button_template="core/includes/sign-out-button--senior.html",
sign_out_link_template="core/includes/sign-out-link--senior.html",
client_name=os.environ.get("SENIOR_AUTH_PROVIDER_CLIENT_NAME", "senior-benefits-oauth-client-name"),
client_id=os.environ.get("AUTH_PROVIDER_CLIENT_ID", "benefits-oauth-client-id"),
authority=os.environ.get("AUTH_PROVIDER_AUTHORITY", "https://example.com"),
Expand Down
17 changes: 11 additions & 6 deletions benefits/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,23 @@ class AuthProvider(models.Model):
"""An entity that provides authentication for eligibility verifiers."""

id = models.AutoField(primary_key=True)
sign_out_button_label = models.TextField(null=True)
sign_out_button_template = models.TextField(null=True)
sign_out_link_template = models.TextField(null=True)
client_name = models.TextField()
client_id = models.TextField()
authority = models.TextField()
scope = models.TextField(null=True)
claim = models.TextField(null=True)
scheme = models.TextField()

@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)


class EligibilityType(models.Model):
"""A single conditional eligibility type."""
Expand Down Expand Up @@ -138,11 +147,7 @@ def is_auth_required(self):
@property
def uses_auth_verification(self):
"""True if this Verifier verifies via the auth provider. False otherwise."""
return self.is_auth_required and self.auth_provider.scope and self.auth_provider.claim

@property
def supports_sign_out(self):
return bool(self.is_auth_required and self.auth_provider.sign_out_button_label)
return self.is_auth_required and self.auth_provider.supports_claims_verification

@staticmethod
def by_id(id):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{% load i18n %}

{% if authentication.logged_in %}
{% url "oauth:logout" as sign_out_url %}
<a id="login" href="{{ sign_out_url }}" class="p-0 btn btn-lg" role="button">
<span class="fallback-text color-logo">Login.gov</span>
</a>
{% endif %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{% extends "core/includes/sign-out-link.html" %}
{% load i18n %}

{% block button_text %}
{% translate "core.buttons.senior.signout" %}
{% endblock button_text %}
23 changes: 11 additions & 12 deletions benefits/core/templates/core/includes/sign-out-link.html
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@

{% load i18n %}

{% if authentication %}
{% if authentication.supports_sign_out and authentication.logged_in %}
<div class="w-100 position-absolute">
<div class="container">
<div class="row nav-button-row">
<div class="col-12 d-flex align-items-center justify-content-end">
<a class="signout-link" href="{{ authentication.sign_out_route }}">{% translate authentication.sign_out_button_label %}</a>
</div>
{% if authentication.logged_in %}
{% url "oauth:logout" as signout_url %}
<div class="w-100 position-absolute">
<div class="container">
<div class="row nav-button-row">
<div class="col-12 d-flex align-items-center justify-content-end">
<a class="signout-link" href="{{ signout_url }}">
{% block button_text %}
{% endblock button_text %}
</a>
</div>
</div>
</div>
{% endif %}
</div>
{% endif %}
7 changes: 0 additions & 7 deletions benefits/core/viewmodels.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
The core application: view model definitions for the root of the webapp.
"""
from django.utils.translation import pgettext, gettext_lazy as _
from django.urls import reverse

from benefits.core import models

Expand Down Expand Up @@ -51,12 +50,6 @@ def outline_primary(**kwargs):
classes.insert(0, "btn-outline-primary")
return Button(classes=classes, **kwargs)

@staticmethod
def logout(**kwargs):
"""Create a button that logs user out, with a login.gov button, with a login.gov logo and fallback text"""
btn = Button.primary(fallback_text="Login.gov", id="login", url=reverse("oauth:logout"), text="", **kwargs)
return btn

@staticmethod
def previous_page(url):
return Button(text=_("core.buttons.previous_page"), url=url)
Expand Down
4 changes: 3 additions & 1 deletion benefits/eligibility/templates/eligibility/unverified.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
{% extends "core/base.html" %}

{% block main-content %}
{% include "core/includes/sign-out-link.html" %}
{% if authentication and authentication.sign_out_link_template %}
{% include authentication.sign_out_link_template %}
{% endif %}

<div class="container">
{% include "core/includes/icon-title.html" with title=page.headline icon=page.icon %}
Expand Down
4 changes: 3 additions & 1 deletion benefits/enrollment/templates/enrollment/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
{% endblock page-title %}

{% block nav-buttons %}
{% include "core/includes/sign-out-link.html" %}
{% if authentication and authentication.sign_out_link_template %}
{% include authentication.sign_out_link_template %}
{% endif %}
{% endblock nav-buttons %}

{% block headline %}
Expand Down
4 changes: 3 additions & 1 deletion benefits/enrollment/templates/enrollment/retry.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
{% extends "core/base.html" %}

{% block main-content %}
{% include "core/includes/sign-out-link.html" %}
{% if authentication and authentication.sign_out_link_template %}
{% include authentication.sign_out_link_template %}
{% endif %}

<div class="container">
{% include "core/includes/icon-title.html" with title=page.headline icon=page.icon %}
Expand Down
11 changes: 3 additions & 8 deletions benefits/enrollment/templates/enrollment/success.html
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,12 @@ <h1 class="pb-lg-5 pb-4">{% translate "enrollment.pages.success.headline" %}</h1
{% endblock inner-content %}

{% block call-to-action %}
{% if page.buttons|length == 1 %}
{% if authentication and authentication.sign_out_button_template %}
<div class="row d-flex justify-content-start justify-content-lg-center">
<div class="col-12 col-lg-8 pt-lg-5 mt-lg-0 pt-4 mt-2">
<p>
{% translate "enrollment.pages.success.logout[0]" %}
{% for b in page.buttons %}
<a id="{{ b.id }}" class="d-inline-block p-0 {{ b.classes | join:" " }}" href="{{ b.url }}" role="button">
<span class="fallback-text color-logo">{{ b.fallback_text }}</span>
</a>
.
{% endfor %}
{% translate "enrollment.pages.success.logout" %}
{% include authentication.sign_out_button_template %}
</p>
</div>
</div>
Expand Down
7 changes: 2 additions & 5 deletions benefits/enrollment/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,9 @@ def success(request):

verifier = session.verifier(request)

page = viewmodels.Page()

if verifier.supports_sign_out and session.logged_in(request):
if session.logged_in(request) and verifier.auth_provider.supports_sign_out:
# overwrite origin for a logged in user
# if they click the logout button, they are taken to the new route
session.update(request, origin=reverse(ROUTE_LOGGED_OUT))
page.buttons = [viewmodels.Button.logout()]

return TemplateResponse(request, TEMPLATE_SUCCESS, page.context_dict())
return TemplateResponse(request, TEMPLATE_SUCCESS)
10 changes: 5 additions & 5 deletions benefits/locale/en/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,13 @@
msgid ""
msgstr ""
"Report-Msgid-Bugs-To: https://github.com/cal-itp/benefits/issues \n"
"POT-Creation-Date: 2023-07-13 23:04+0000\n"
"POT-Creation-Date: 2023-07-14 05:24+0000\n"
"Language: English\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8bit\n"
"Plural-Forms: nplurals=2; plural=(n != 1);\n"

msgid "eligibility.buttons.senior.signout"
msgstr "Sign out of Login.gov"

msgid "eligibility.pages.unverified.title"
msgstr "Eligibility Error"

Expand Down Expand Up @@ -267,6 +264,9 @@ msgstr ""
"To function properly, this website requires a browser that supports "
"JavaScript. Please enable JavaScript for this website and"

msgid "core.buttons.senior.signout"
msgstr "Sign out of Login.gov"

msgid "core.pages.index.button"
msgstr "Choose your Provider"

Expand Down Expand Up @@ -634,7 +634,7 @@ msgstr ""
msgid "enrollment.pages.success.thankyou"
msgstr "Thank you for using Cal-ITP Benefits!"

msgid "enrollment.pages.success.logout[0]"
msgid "enrollment.pages.success.logout"
msgstr ""
"If you are on a public or shared computer, don’t forget to sign out of "

Expand Down
10 changes: 5 additions & 5 deletions benefits/locale/es/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,13 @@
msgid ""
msgstr ""
"Report-Msgid-Bugs-To: https://github.com/cal-itp/benefits/issues \n"
"POT-Creation-Date: 2023-07-13 23:04+0000\n"
"POT-Creation-Date: 2023-07-14 05:24+0000\n"
"Language: Español\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8bit\n"
"Plural-Forms: nplurals=2; plural=(n != 1);\n"

msgid "eligibility.buttons.senior.signout"
msgstr "Cierre sesión de Login.gov"

msgid "eligibility.pages.unverified.title"
msgstr "Error de elegibilidad"

Expand Down Expand Up @@ -276,6 +273,9 @@ msgstr ""
"Para funcionar correctamente, este sitio web requiere un navegador que "
"admita JavaScript. Por favor, active JavaScript por este sitio web y"

msgid "core.buttons.senior.signout"
msgstr "Cierre sesión de Login.gov"

msgid "core.pages.index.button"
msgstr "Elija su Proveedor"

Expand Down Expand Up @@ -647,7 +647,7 @@ msgstr ""
msgid "enrollment.pages.success.thankyou"
msgstr "Gracias por usar los Beneficios de Cal-ITP."

msgid "enrollment.pages.success.logout[0]"
msgid "enrollment.pages.success.logout"
msgstr ""
"Si está en una computadora pública o compartida, no olvide cerrar sesión en "

Expand Down
27 changes: 19 additions & 8 deletions tests/pytest/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ def model_PemData():
@pytest.fixture
def model_AuthProvider():
auth_provider = AuthProvider.objects.create(
sign_out_button_label="Sign out",
sign_out_button_template="core/includes/sign-out-button--senior.html",
sign_out_link_template="core/includes/sign-out-link--senior.html",
client_name="Client",
client_id="1234",
authority="https://example.com",
Expand All @@ -66,7 +67,8 @@ def model_AuthProvider_with_verification(model_AuthProvider):
def model_AuthProvider_with_verification_no_sign_out(model_AuthProvider):
model_AuthProvider.scope = "scope"
model_AuthProvider.claim = "claim"
model_AuthProvider.sign_out_button_label = None
model_AuthProvider.sign_out_button_template = None
model_AuthProvider.sign_out_link_template = None
model_AuthProvider.save()

return model_AuthProvider
Expand All @@ -85,7 +87,8 @@ def model_AuthProvider_without_verification(model_AuthProvider):
def model_AuthProvider_without_verification_no_sign_out(model_AuthProvider):
model_AuthProvider.scope = None
model_AuthProvider.claim = None
model_AuthProvider.sign_out_button_label = None
model_AuthProvider.sign_out_button_template = None
model_AuthProvider.sign_out_link_template = None
model_AuthProvider.save()

return model_AuthProvider
Expand Down Expand Up @@ -237,12 +240,20 @@ def mocked_session_verifier_oauth(mocker, model_EligibilityVerifier_AuthProvider


@pytest.fixture
def mocked_session_verifier_auth_required(mocker, model_EligibilityVerifier, mocked_session_verifier):
mock_verifier = mocker.Mock(spec=model_EligibilityVerifier)
mock_verifier.name = model_EligibilityVerifier.name
def mocked_session_verifier_auth_required(
mocker, model_EligibilityVerifier_AuthProvider_with_verification, mocked_session_verifier_oauth
):
mock_verifier = mocker.Mock(spec=model_EligibilityVerifier_AuthProvider_with_verification)
mock_verifier.name = model_EligibilityVerifier_AuthProvider_with_verification.name
mock_verifier.is_auth_required = True
mocked_session_verifier.return_value = mock_verifier
return mocked_session_verifier
mock_verifier.auth_provider.sign_out_button_template = (
model_EligibilityVerifier_AuthProvider_with_verification.auth_provider.sign_out_button_template
)
mock_verifier.auth_provider.sign_out_link_template = (
model_EligibilityVerifier_AuthProvider_with_verification.auth_provider.sign_out_link_template
)
mocked_session_verifier_oauth.return_value = mock_verifier
return mocked_session_verifier_oauth


@pytest.fixture
Expand Down
Loading

0 comments on commit 6d24a70

Please sign in to comment.