From bf86654e724be8697355a090fc961fbbdeb4e0a7 Mon Sep 17 00:00:00 2001 From: Bernard Siebens Date: Mon, 13 Jul 2026 23:53:35 +0200 Subject: [PATCH] Give every auth button an icon, and lay out the password and MFA screens The button element now takes an `icon`, so a page gets one by passing icon="name" rather than by hand-rolling its own button markup. Every button on the account and MFA screens carries one; a test walks each page and asserts no button is left bare. Change password: labels dropped (allauth already sets a placeholder on each field), the current password set apart from the new pair, help text kept on the new password, and Forgot Password promoted from a bare link to an accent button. MFA management: recovery-code actions are now ranked -- View is primary, Download and Generate are outline. Generate silently invalidates the codes you already hold, so it must not read as the obvious thing to click. Panel actions get breathing room from the body text (card-actions mt-4). Viewing recovery codes: Download and Generate sit side by side instead of stacking. TOTP activate: the code box loses its heading -- an otp field never takes a visible label, the boxes say what they are -- and the authenticator secret gets margin around it, since it is copied out by hand. Co-Authored-By: Claude Opus 4.8 --- authentication/tests.py | 80 + static/css/app.css | 4135 ++++++++++++++++- templates/_base.html | 2 +- templates/account/password_change.html | 40 + templates/allauth/elements/button.html | 5 +- templates/allauth/elements/field.html | 2 +- templates/allauth/elements/fields.html | 6 +- templates/allauth/elements/panel.html | 5 +- templates/mfa/index.html | 128 + templates/mfa/recovery_codes/generate.html | 35 + templates/mfa/recovery_codes/index.html | 65 + templates/mfa/totp/activate_form.html | 45 + templates/mfa/totp/deactivate_form.html | 30 + templates/mfa/webauthn/add_form.html | 39 + .../authenticator_confirm_delete.html | 25 + .../mfa/webauthn/authenticator_list.html | 72 + 16 files changed, 4705 insertions(+), 9 deletions(-) create mode 100644 templates/account/password_change.html create mode 100644 templates/mfa/index.html create mode 100644 templates/mfa/recovery_codes/generate.html create mode 100644 templates/mfa/recovery_codes/index.html create mode 100644 templates/mfa/totp/activate_form.html create mode 100644 templates/mfa/totp/deactivate_form.html create mode 100644 templates/mfa/webauthn/add_form.html create mode 100644 templates/mfa/webauthn/authenticator_confirm_delete.html create mode 100644 templates/mfa/webauthn/authenticator_list.html diff --git a/authentication/tests.py b/authentication/tests.py index 14d54ff..799a18a 100644 --- a/authentication/tests.py +++ b/authentication/tests.py @@ -1,8 +1,10 @@ +import re import uuid from urllib.parse import parse_qs, urlparse from allauth.core import context from allauth.mfa.models import Authenticator +from allauth.mfa.recovery_codes.internal.auth import RecoveryCodes from django.contrib.auth import get_user_model from django.contrib.auth.models import AnonymousUser from django.db import IntegrityError @@ -388,3 +390,81 @@ class SignOutPageTests(TestCase): self.client.post(reverse("account_logout")) self.assertIsNone(self.client.session.get("_auth_user_id")) + + +class ChangePasswordPageTests(TestCase): + def setUp(self): + User.objects.create_user(email="mfa@example.com", password="pw-secret-123") + self.client.post(reverse("account_login"), {"login": "mfa@example.com", "password": "pw-secret-123"}, follow=True) + self.response = self.client.get(reverse("account_change_password")) + + def test_the_fields_have_no_visible_labels(self): + # allauth gives each a placeholder, so the label would only repeat it. + self.assertNotContains(self.response, 'Current Password') + self.assertContains(self.response, 'name="oldpassword"') + self.assertContains(self.response, 'name="password1"') + + def test_the_new_password_keeps_its_help_text(self): + self.assertContains(self.response, "id_password1_helptext") + + def test_the_current_password_is_set_apart_from_the_new_one(self): + self.assertContains(self.response, "mt-10") + + def test_forgot_password_is_an_accent_button_and_both_actions_have_icons(self): + html = self.response.content.decode() + forgot = html[html.index('class="btn btn-accent gap-2"') :] + submit = html[html.index('class="btn btn-primary gap-2"') :] + + self.assertIn("")]) + self.assertIn("")]) + + +class MfaButtonIconTests(TestCase): + """Every button on the MFA screens carries an icon, and the recovery-code actions are + ranked: View is primary, Download and Generate are outline. Generate throws away the + codes you already have, so it must not read as the obvious thing to click.""" + + def setUp(self): + self.user = User.objects.create_user(email="mfa@example.com", password="pw-secret-123") + # Sign in *before* enrolling: a user who already holds a second factor is stopped at + # the 2FA challenge and never reaches these pages. + self.client.post(reverse("account_login"), {"login": "mfa@example.com", "password": "pw-secret-123"}, follow=True) + enrol_mfa(self.user) + RecoveryCodes.activate(self.user).instance.save() + + def buttons(self, url): + """Every / + + +{% endblock content %} diff --git a/templates/allauth/elements/button.html b/templates/allauth/elements/button.html index 8e24b3f..5c648a6 100644 --- a/templates/allauth/elements/button.html +++ b/templates/allauth/elements/button.html @@ -1,4 +1,4 @@ -{% load allauth %} +{% load allauth lucide %} {% comment %} allauth tags every button it renders, and mapping those tags is what styles the MFA @@ -9,13 +9,14 @@ {% endcomment %} {% comment %} djlint:off {% endcomment %} <{% if attrs.href %}a href="{{ attrs.href }}"{% else %}button{% endif %} -class="btn {% if attrs.tags and 'danger' in attrs.tags %}btn-error{% elif attrs.tags and 'link' in attrs.tags %}btn-link{% elif attrs.tags and 'secondary' in attrs.tags %}btn-outline btn-neutral{% elif attrs.tags and 'outline' in attrs.tags %}btn-outline btn-primary{% else %}btn-primary{% endif %}" +class="btn gap-2 {% if attrs.tags and 'danger' in attrs.tags %}btn-error{% elif attrs.tags and 'link' in attrs.tags %}btn-link{% elif attrs.tags and 'secondary' in attrs.tags %}btn-outline btn-neutral{% elif attrs.tags and 'outline' in attrs.tags %}btn-outline btn-primary{% else %}btn-primary{% endif %}" {% if attrs.form %}form="{{ attrs.form }}"{% endif %} {% if attrs.id %}id="{{ attrs.id }}"{% endif %} {% if attrs.name %}name="{{ attrs.name }}"{% endif %} {% if attrs.value %}value="{{ attrs.value }}"{% endif %} {% if attrs.type %}type="{{ attrs.type }}"{% endif %} > +{% if attrs.icon %}{% lucide attrs.icon size=16 %}{% endif %} {% slot %} {% endslot %} diff --git a/templates/allauth/elements/field.html b/templates/allauth/elements/field.html index 19aa365..5e6bd51 100644 --- a/templates/allauth/elements/field.html +++ b/templates/allauth/elements/field.html @@ -6,7 +6,7 @@ character by character and must not misread. {% endcomment %} {{ attrs.errors }} -
+
{% if slots.label %} {% else %} - {% if not attrs.unlabeled %} + {# An otp box never takes a visible label — the boxes say what they are, and a + heading above them just crowds the field. It keeps an sr-only one below. #} + {% if not attrs.unlabeled and not field|is_otp %} @@ -53,7 +55,7 @@ For the same reason the sr-only label lives outside the container: another child would throw the count off again. {% endcomment %} - {% if attrs.unlabeled %}{% endif %} +
diff --git a/templates/allauth/elements/panel.html b/templates/allauth/elements/panel.html index f295669..44c3931 100644 --- a/templates/allauth/elements/panel.html +++ b/templates/allauth/elements/panel.html @@ -1,8 +1,9 @@ {% load allauth %} -
+
{% if slots.title %}

{% slot title %}{% endslot %}

{% endif %}
{% slot body %}{% endslot %}
- {% if slots.actions %}
{% slot actions %}{% endslot %}
{% endif %} + {% if slots.actions %} +
{% slot actions %}{% endslot %}
{% endif %}
diff --git a/templates/mfa/index.html b/templates/mfa/index.html new file mode 100644 index 0000000..704b91b --- /dev/null +++ b/templates/mfa/index.html @@ -0,0 +1,128 @@ +{% extends "mfa/base_manage.html" %} +{% load allauth i18n %} + +{% comment %} + allauth's own index, with an icon on every button and the recovery-code actions given + the ranking they deserve: View is the primary action, Download and Generate are outline + (Generate silently invalidates the codes you already have, so it must not look like the + obvious thing to click). +{% endcomment %} + +{% block head_title %} + {% trans "Two-Factor Authentication" %} +{% endblock head_title %} + +{% block content %} + {% element h1 tags="mfa,index" %} + {% trans "Two-Factor Authentication" %} + {% endelement %} + + {% if "totp" in MFA_SUPPORTED_TYPES %} + {% element panel %} + {% slot title %} + {% translate "Authenticator App" %} + {% endslot %} + {% slot body %} + {% if authenticators.totp %} + {% element p %} + {% translate "Authentication using an authenticator app is active." %} + {% endelement %} + {% else %} + {% element p %} + {% translate "An authenticator app is not active." %} + {% endelement %} + {% endif %} + {% endslot %} + {% slot actions %} + {% url 'mfa_deactivate_totp' as deactivate_url %} + {% url 'mfa_activate_totp' as activate_url %} + {% if authenticators.totp %} + {% element button href=deactivate_url icon="power" tags="danger,delete,panel" %} + {% translate "Deactivate" %} + {% endelement %} + {% else %} + {% element button href=activate_url icon="smartphone" tags="panel" %} + {% translate "Activate" %} + {% endelement %} + {% endif %} + {% endslot %} + {% endelement %} + {% endif %} + + {% if "webauthn" in MFA_SUPPORTED_TYPES %} + {% element panel %} + {% slot title %} + {% translate "Security Keys" %} + {% endslot %} + {% slot body %} + {% if authenticators.webauthn|length %} + {% element p %} + {% blocktranslate count count=authenticators.webauthn|length %}You have added {{ count }} security key.{% plural %}You have added {{ count }} security keys.{% endblocktranslate %} + {% endelement %} + {% else %} + {% element p %} + {% translate "No security keys have been added." %} + {% endelement %} + {% endif %} + {% endslot %} + {% slot actions %} + {% if authenticators.webauthn|length %} + {% url 'mfa_list_webauthn' as webauthn_list_url %} + {% element button href=webauthn_list_url icon="settings" %} + {% translate "Manage" %} + {% endelement %} + {% else %} + {% url 'mfa_add_webauthn' as webauthn_add_url %} + {% element button href=webauthn_add_url icon="usb" %} + {% translate "Add" %} + {% endelement %} + {% endif %} + {% endslot %} + {% endelement %} + {% endif %} + + {% if "recovery_codes" in MFA_SUPPORTED_TYPES %} + {% with total_count=authenticators.recovery_codes.generate_codes|length unused_count=authenticators.recovery_codes.get_unused_codes|length %} + {% element panel %} + {% slot title %} + {% translate "Recovery Codes" %} + {% endslot %} + {% slot body %} + {% if authenticators.recovery_codes %} + {% element p %} + {% blocktranslate count unused_count=unused_count %}There is {{ unused_count }} out of {{ total_count }} recovery codes available.{% plural %}There are {{ unused_count }} out of {{ total_count }} recovery codes available.{% endblocktranslate %} + {% endelement %} + {% else %} + {% element p %} + {% translate "No recovery codes set up." %} + {% endelement %} + {% endif %} + {% endslot %} + {% if is_mfa_enabled %} + {% if authenticators.recovery_codes %} + {% if unused_count > 0 and not MFA_RECOVERY_CODES_SHOW_ONCE %} + {% slot actions %} + {% url 'mfa_view_recovery_codes' as view_url %} + {% element button href=view_url icon="eye" tags="panel" %} + {% translate "View" %} + {% endelement %} + {% endslot %} + {% slot actions %} + {% url 'mfa_download_recovery_codes' as download_url %} + {% element button href=download_url icon="download" tags="outline,panel" %} + {% translate "Download" %} + {% endelement %} + {% endslot %} + {% endif %} + {% endif %} + {% slot actions %} + {% url 'mfa_generate_recovery_codes' as generate_url %} + {% element button href=generate_url icon="refresh-cw" tags="outline,panel" %} + {% translate "Generate" %} + {% endelement %} + {% endslot %} + {% endif %} + {% endelement %} + {% endwith %} + {% endif %} +{% endblock content %} diff --git a/templates/mfa/recovery_codes/generate.html b/templates/mfa/recovery_codes/generate.html new file mode 100644 index 0000000..25c8dfd --- /dev/null +++ b/templates/mfa/recovery_codes/generate.html @@ -0,0 +1,35 @@ +{% extends "mfa/recovery_codes/base.html" %} +{% load i18n allauth %} + +{% block content %} + {% element h1 %} + {% translate "Recovery Codes" %} + {% endelement %} + {% element p %} + {% blocktranslate %}You are about to generate a new set of recovery codes for your account.{% endblocktranslate %} + {% if unused_code_count %} + {% blocktranslate %}This action will invalidate your existing codes.{% endblocktranslate %} + {% endif %} + {% blocktranslate %}Are you sure?{% endblocktranslate %} + {% endelement %} + + {% url 'mfa_generate_recovery_codes' as action_url %} + {% element form method="post" action=action_url no_visible_fields=True %} + {% slot body %} + {% csrf_token %} + {{ form.as_p }} + {% endslot %} + {% slot actions %} + {# Destructive when codes already exist: generating throws the old ones away. #} + {% setvar tags %} + {% if unused_code_count %} + danger + {% else %} + {% endif %} + {% endsetvar %} + {% element button type="submit" icon="refresh-cw" tags=tags %} + {% trans "Generate" %} + {% endelement %} + {% endslot %} + {% endelement %} +{% endblock content %} diff --git a/templates/mfa/recovery_codes/index.html b/templates/mfa/recovery_codes/index.html new file mode 100644 index 0000000..f17d8c3 --- /dev/null +++ b/templates/mfa/recovery_codes/index.html @@ -0,0 +1,65 @@ +{% extends "mfa/recovery_codes/base.html" %} +{% load i18n allauth static %} + +{% comment %} + Overridden for layout: Download and Generate sit side by side under the codes rather + than stacking. Generate is outline-only — it silently invalidates the codes on screen, + so it must not be the button your eye lands on. +{% endcomment %} + +{% block content %} + {% element h1 %} + {% translate "Recovery Codes" %} + {% endelement %} + {% element p %} + {% blocktranslate count unused_count=unused_codes|length %}There is {{ unused_count }} out of {{ total_count }} recovery codes available.{% plural %}There are {{ unused_count }} out of {{ total_count }} recovery codes available.{% endblocktranslate %} + {% endelement %} + + {% if can_view_codes %} + {% element field id="recovery_codes" type="textarea" rows=unused_codes|length readonly=True %} + {% slot label %} + {% translate "Unused codes" %} + {% endslot %} + {# djlint:off #} +{% slot value %}{% for code in unused_codes %}{% if forloop.counter0 %} +{% endif %}{{ code }}{% endfor %}{% endslot %} + {# djlint:on #} + {% endelement %} + {% endif %} + +
+ {% if can_download_codes %} + {% url 'mfa_download_recovery_codes' as download_url %} + {% element button href=download_url icon="download" %} + {% translate "Download codes" %} + {% endelement %} + {% endif %} + {% if can_generate_codes %} + {% url 'mfa_generate_recovery_codes' as generate_url %} + {% element button href=generate_url icon="refresh-cw" tags="outline" %} + {% translate "Generate new codes" %} + {% endelement %} + {% endif %} +
+ + {% if MFA_RECOVERY_CODES_SHOW_ONCE and can_view_codes %} + {% element field type="checkbox" id="codes_saved" %} + {% slot label %} + {% translate "I have saved my recovery codes" %} + {% endslot %} + {% endelement %} + {% endif %} +{% endblock content %} + +{% block extra_body %} + {{ block.super }} + + + +{% endblock extra_body %} diff --git a/templates/mfa/totp/activate_form.html b/templates/mfa/totp/activate_form.html new file mode 100644 index 0000000..c81c39a --- /dev/null +++ b/templates/mfa/totp/activate_form.html @@ -0,0 +1,45 @@ +{% extends "mfa/totp/base.html" %} +{% load allauth i18n %} + +{% comment %} + The code field is boxed and label-less (the `fields` element does that for any `code` + field), and the secret sits in a mono field with room around it — it is copied by hand, + character by character. +{% endcomment %} + +{% block head_title %} + {% translate "Activate Authenticator App" %} +{% endblock head_title %} + +{% block content %} + {% element h1 %} + {% translate "Activate Authenticator App" %} + {% endelement %} + {% element p %} + {% blocktranslate %}To protect your account with two-factor authentication, scan the QR code below with your authenticator app. Then, input the verification code generated by the app below.{% endblocktranslate %} + {% endelement %} + + {% url 'mfa_activate_totp' as action_url %} + {% element form form=form method="post" action=action_url %} + {% slot body %} + {% element img src=totp_svg_data_uri alt=form.secret tags="mfa,totp,qr" %} + {% endelement %} + {% csrf_token %} + {% element field id="authenticator_secret" type="text" value=form.secret disabled=True %} + {% slot label %} + {% translate "Authenticator secret" %} + {% endslot %} + {% slot help_text %} + {% translate "You can store this secret and use it to reinstall your authenticator app at a later time." %} + {% endslot %} + {% endelement %} + {% element fields form=form %} + {% endelement %} + {% endslot %} + {% slot actions %} + {% element button type="submit" icon="check" %} + {% trans "Activate" %} + {% endelement %} + {% endslot %} + {% endelement %} +{% endblock content %} diff --git a/templates/mfa/totp/deactivate_form.html b/templates/mfa/totp/deactivate_form.html new file mode 100644 index 0000000..36736bb --- /dev/null +++ b/templates/mfa/totp/deactivate_form.html @@ -0,0 +1,30 @@ +{% extends "mfa/totp/base.html" %} +{% load allauth i18n %} + +{% block head_title %} + {% trans "Deactivate Authenticator App" %} +{% endblock head_title %} + +{% block content %} + {% element h1 %} + {% trans "Deactivate Authenticator App" %} + {% endelement %} + {% element p %} + {% blocktranslate %}You are about to deactivate authenticator app based authentication. Are you sure?{% endblocktranslate %} + {% endelement %} + + {% url 'mfa_deactivate_totp' as action_url %} + {% element form form=form method="post" action=action_url no_visible_fields=True %} + {% slot body %} + {% csrf_token %} + {% element fields form=form %} + {{ form.as_p }} + {% endelement %} + {% endslot %} + {% slot actions %} + {% element button type="submit" icon="power" tags="danger,delete" %} + {% trans "Deactivate" %} + {% endelement %} + {% endslot %} + {% endelement %} +{% endblock content %} diff --git a/templates/mfa/webauthn/add_form.html b/templates/mfa/webauthn/add_form.html new file mode 100644 index 0000000..68af5c6 --- /dev/null +++ b/templates/mfa/webauthn/add_form.html @@ -0,0 +1,39 @@ +{% extends "mfa/webauthn/base.html" %} +{% load allauth i18n static %} + +{% block head_title %} + {% trans "Add Security Key" %} +{% endblock head_title %} + +{% block content %} + {% element h1 %} + {% trans "Add Security Key" %} + {% endelement %} + + {% url 'mfa_add_webauthn' as action_url %} + {% element form form=form method="post" action=action_url %} + {% slot body %} + {% csrf_token %} + {% element fields form=form %} + {% endelement %} + {% endslot %} + {% slot actions %} + {# type="button": the webauthn script takes over the click and posts the form itself. #} + {% element button id="mfa_webauthn_add" type="button" icon="usb" %} + {% trans "Add" %} + {% endelement %} + {% endslot %} + {% endelement %} + + {% include "mfa/webauthn/snippets/scripts.html" %} + {{ js_data|json_script:"js_data" }} + +{% endblock content %} diff --git a/templates/mfa/webauthn/authenticator_confirm_delete.html b/templates/mfa/webauthn/authenticator_confirm_delete.html new file mode 100644 index 0000000..82e9945 --- /dev/null +++ b/templates/mfa/webauthn/authenticator_confirm_delete.html @@ -0,0 +1,25 @@ +{% extends "mfa/webauthn/base.html" %} +{% load allauth i18n %} + +{% block head_title %} + {% trans "Remove Security Key" %} +{% endblock head_title %} + +{% block content %} + {% element h1 %} + {% trans "Remove Security Key" %} + {% endelement %} + {% element p %} + {% blocktranslate %}Are you sure you want to remove this security key?{% endblocktranslate %} + {% endelement %} + + {% url 'mfa_remove_webauthn' pk=authenticator.pk as action_url %} + {% element form method="post" action=action_url no_visible_fields=True %} + {% slot actions %} + {% csrf_token %} + {% element button type="submit" icon="trash-2" tags="danger" %} + {% translate "Remove" %} + {% endelement %} + {% endslot %} + {% endelement %} +{% endblock content %} diff --git a/templates/mfa/webauthn/authenticator_list.html b/templates/mfa/webauthn/authenticator_list.html new file mode 100644 index 0000000..872afbf --- /dev/null +++ b/templates/mfa/webauthn/authenticator_list.html @@ -0,0 +1,72 @@ +{% extends "mfa/webauthn/base.html" %} +{% load i18n %} +{% load static %} +{% load allauth %} +{% load humanize %} +{% block content %} + {% element h1 %} + {% trans "Security Keys" %} + {% endelement %} + {% if authenticators|length == 0 %} + {% element p %} + {% blocktranslate %}No security keys have been added.{% endblocktranslate %} + {% endelement %} + {% else %} + {% element table %} + {% element thead %} + {% element th %} + {% translate "Key" %} + {% endelement %} + {% element th %} + {% translate "Usage" %} + {% endelement %} + {% element th %} + {% endelement %} + {% endelement %} + {% element tbody %} + {% for authenticator in authenticators %} + {% element tr %} + {% element td %} + {{ authenticator }} + {% if authenticator.wrap.is_passwordless is True %} + {% element badge tags="mfa,key,primary" %} + {% translate "Passkey" %} + {% endelement %} + {% elif authenticator.wrap.is_passwordless is False %} + {% element badge tags="mfa,key,secondary" %} + {% translate "Security key" %} + {% endelement %} + {% else %} + {% element badge title=_("This key does not indicate whether it is a passkey.") tags="mfa,key,warning" %} + {% translate "Unspecified" %} + {% endelement %} + {% endif %} + {% endelement %} + {% element td %} + {% blocktranslate with created_at=authenticator.created_at|date:"SHORT_DATE_FORMAT" %}Added on {{ created_at }}{% endblocktranslate %}. + {% if authenticator.last_used_at %} + {% blocktranslate with last_used=authenticator.last_used_at|naturaltime %}Last used {{ last_used }}{% endblocktranslate %} + {% else %} + Not used. + {% endif %} + {% endelement %} + {% element td align="right" %} + {% url 'mfa_edit_webauthn' pk=authenticator.pk as edit_url %} + {% element button href=edit_url icon="pencil" tags="mfa,authenticator,edit,tool" %} + {% translate "Edit" %} + {% endelement %} + {% url 'mfa_remove_webauthn' pk=authenticator.pk as remove_url %} + {% element button href=remove_url icon="trash-2" tags="mfa,authenticator,danger,delete,tool" %} + {% translate "Remove" %} + {% endelement %} + {% endelement %} + {% endelement %} + {% endfor %} + {% endelement %} + {% endelement %} + {% endif %} + {% url 'mfa_add_webauthn' as add_url %} + {% element button href=add_url icon="usb" %} + {% translate "Add" %} + {% endelement %} +{% endblock %}