misc: render authenticators names in their own templates (#53264) #5

Merged
smihai merged 2 commits from wip/53264-move-authenticator-name-in-own-template into main 2023-03-28 13:26:42 +02:00
16 changed files with 149 additions and 73 deletions

View File

@ -1,13 +1,15 @@
{% load i18n static gadjo %}
{% block login-block-title %}
{% endblock %}
{% block login %}
<div>
{% block form %}
{% if authenticator.button_description %}
<p>{{ authenticator.button_description }}</p>
{% endif %}
{% block form %}
{% if authenticator.button_description %}
<p>{{ authenticator.button_description }}</p>
{% endif %}
<div>
<form method="post" id="login-password-form" class="pk-mark-optional-fields">
{% csrf_token %}
{{ form|with_template }}
@ -21,22 +23,21 @@
{% endblock %}
</form>
{{ form.media }}
</div>
{% endblock %}
{% block actions %}
{% if can_reset_password or registration_authorized %}
<div class="login-actions">
<ul>
{% if can_reset_password %}
<li><p>→ {% trans "Forgot password?" %} <a href="{% url 'password_reset' %}{% if request.GET.next %}?next={{ request.GET.next|urlencode }}{% endif %}">{% trans "Reset it!" %}</a></p></li>
{% endif %}
{% if registration_authorized and not hide_login_registration_link %}
<li><p>→ {% trans "Not a member?" %} <a href="{{ registration_url }}">{% trans "Register!" %}</a></p></li>
{% endif %}
</ul>
</div>
{% endif %}
{% endblock %}
{% endblock %}
{% block actions %}
{% if can_reset_password or registration_authorized %}
<div class="login-actions">
<ul>
{% if can_reset_password %}
<li><p>→ {% trans "Forgot password?" %} <a href="{% url 'password_reset' %}{% if request.GET.next %}?next={{ request.GET.next|urlencode }}{% endif %}">{% trans "Reset it!" %}</a></p></li>
{% endif %}
{% if registration_authorized and not hide_login_registration_link %}
<li><p>→ {% trans "Not a member?" %} <a href="{{ registration_url }}">{% trans "Register!" %}</a></p></li>
{% endif %}
</ul>
</div>
{% endif %}
{% endblock %}
</div>
{% endblock %}

View File

@ -1,11 +1,19 @@
{% load i18n gadjo %}
{% block registration %}
<form enctype="multipart/form-data" method="post" class="pk-mark-optional-fields">
{% csrf_token %}
{{ form|with_template }}
<div class="buttons">
<button class="submit-button">{% trans 'Submit' %}</button>
</div>
</form>
{% block registration-block-title %}
<h2>{% trans "Registration" %}</h2>
{% endblock %}
{% block registration %}
<div>
<p>
<form enctype="multipart/form-data" method="post" class="pk-mark-optional-fields">
{% csrf_token %}
{{ form|with_template }}
<div class="buttons">
<button class="submit-button">{% trans 'Submit' %}</button>
</div>
</form>
</p>
</div>
{% endblock %}

View File

@ -7,16 +7,11 @@
{% block content %}
<h2>{{ view.title }}</h2>
{% include "authentic2/service_info_fragment.html" %}
{% for id, block in frontends.items %}
<div class="registration_frontend">
<h2>{{ block.name }}</h2>
<p>
{{ block.content|safe }}
</p>
{{ block.content|safe }}
</div>
{% endfor %}

View File

@ -136,7 +136,7 @@ class FcAuthenticator(BaseAuthenticator):
return views.profile(request, *args, **kwargs)
def registration(self, request, *args, **kwargs):
return self.login(request, *args, **kwargs)
return views.registration(request, *args, **kwargs)
class FcAccount(models.Model):

View File

@ -1,17 +1,24 @@
{% load static %}
{% load i18n %}
{% block login %}
{% include "authentic2_auth_fc/explanation.html" %}
<div id="fc-button-wrapper">
<div id="fc-button">
<a href="{{ login_url }}"
class="button connexion">
<span class="sr-only">{% trans 'Log in with FranceConnect' %}</span>
</a>
{% block title %}
{% block login-block-title %}
{% endblock %}
{% endblock %}
{% block content %}
<div>
{% include "authentic2_auth_fc/explanation.html" %}
<div id="fc-button-wrapper">
<div id="fc-button">
<a href="{{ login_url }}"
class="button connexion">
<span class="sr-only">{% trans 'Log in with FranceConnect' %}</span>
</a>
</div>
</div>
<div id="fc-explanation-link">
Review

Clairement il y a eu un loupé ici, la div fc-explanation-link est déjà présente (et inchangée) quelques lignes plus bas dans ce même gabarit.

Clairement il y a eu un loupé ici, la div `fc-explanation-link` est déjà présente (et inchangée) quelques lignes plus bas dans ce même gabarit.
Review

En effet, je viens de corriger et pousser la branche.

En effet, je viens de corriger et pousser la branche.
<a href="{{ about_url }}" target="_blank" rel="noopener" title="{% trans "opens in new window" %}">{% trans "What is FranceConnect?" %}</a>
</div>
</div>
<div id="fc-explanation-link">
<a href="{{ about_url }}" target="_blank" rel="noopener" title="{% trans "What is FranceConnect?" %} {% trans "(opens in new window)" %}">{% trans "What is FranceConnect?" %}</a>
</div>
{% endblock %}

View File

@ -0,0 +1,7 @@
{% extends "authentic2_auth_fc/login.html" %}
{% block title %}
{% block registration-block-title %}
<h2>FranceConnect</h2>
{% endblock %}
{% endblock %}

View File

@ -77,10 +77,15 @@ def login(request, *args, **kwargs):
)
context['login_url'] = utils_misc.make_url('fc-login-or-link', keep_params=True, request=request)
context['block-extra-css-class'] = 'fc-login'
template = 'authentic2_auth_fc/login.html'
template = kwargs.get('template', 'authentic2_auth_fc/login.html')
Review

Est-ce que tu peux m’expliquer à quel moment on a besoin d’avoir ça stp ? J’ai loupé dans le reste du patch le moment qui justifiait d’avoir ce nom de template qui peut varier, avec le fallback sur la valeur qui était en dur dans le code jusque là.

Est-ce que tu peux m’expliquer à quel moment on a besoin d’avoir ça stp ? J’ai loupé dans le reste du patch le moment qui justifiait d’avoir ce nom de template qui peut varier, avec le fallback sur la valeur qui était en dur dans le code jusque là.
Review

L'idée ici est de rajouter un template pour la page d'inscription, qui permettrait de définir le titre du bloc FC.

L'idée ici est de rajouter un template pour la page d'inscription, qui permettrait de définir le titre du bloc FC.
return TemplateResponse(request, template, context)
def registration(request, *args, **kwargs):
kwargs['template'] = 'authentic2_auth_fc/registration.html'
return login(request, *args, **kwargs)
def profile(request, *args, **kwargs):
# We prevent unlinking if the user has no usable password and can't change it
# because we assume that the password is the unique other mean of authentication

View File

@ -1,11 +1,12 @@
{% block login %}
<div>
Review

Ici pourquoi juste on ajoute une div ? Pas compris ce que la modification de ce fichier vient faire là, mais je loupe peut-

Ici pourquoi juste on ajoute une div ? Pas compris ce que la modification de ce fichier vient faire là, mais je loupe peut-
Review

C'est la suggestion (juste) de @tjund pour uniformiser la structure DOM de chaque bloc d'authentification.

C'est la suggestion (juste) de @tjund pour uniformiser la structure DOM de chaque bloc d'authentification.
{% if provider.button_description %}
<p>{{ provider.button_description }}</p>
{% endif %}
{% if provider.button_description %}
<p>{{ provider.button_description }}</p>
{% endif %}
<p id="oidc-p-{% firstof provider.slug provider.name|slugify %}">
<a id="oidc-a-{% firstof provider.slug provider.name|slugify %}"
href="{{ login_url }}" class="pk-button">{{ provider.button_label }}</a>
</p>
<p id="oidc-p-{% firstof provider.slug provider.name|slugify %}">
<a id="oidc-a-{% firstof provider.slug provider.name|slugify %}"
href="{{ login_url }}" class="pk-button">{{ provider.button_label }}</a>
</p>
</div>
{% endblock %}

View File

@ -1,21 +1,28 @@
{% load i18n %}
{% block before-login %}
Review

Ok mais ici dans le ticket PBT lié on ne traite pas toutes les occurrences de surcharge du bloc before-content il me semble. Je vais coller la liste dans la PR PBT liée.

Ok mais ici dans le ticket PBT lié on ne traite pas toutes les occurrences de surcharge du bloc `before-content` il me semble. Je vais coller la liste dans la PR PBT liée.
{% if authenticator.button_description %}
<p>{{ authenticator.button_description }}</p>
{% endif %}
{% block login-block-title %}
{% endblock %}
{% block login %}
<form method="post">
<div class="buttons">
<button class="submit-button" name="{{ submit_name }}">{{ authenticator.button_label }}</button>
{% if cancel %}
<button class="cancel-button" name="cancel" formnovalidate>{% trans 'Cancel' %}</button>
{% block content %}
<div>
{% block before-login %}
{% if authenticator.button_description %}
<p>{{ authenticator.button_description }}</p>
{% endif %}
</div>
</form>
{% endblock %}
{% endblock %}
{% block after-login %}
{% block login %}
<form method="post">
<div class="buttons">
<button class="submit-button" name="{{ submit_name }}">{{ authenticator.button_label }}</button>
{% if cancel %}
<button class="cancel-button" name="cancel" formnovalidate>{% trans 'Cancel' %}</button>
{% endif %}
</div>
</form>
{% endblock %}
{% block after-login %}
{% endblock %}
</div>
{% endblock %}

View File

@ -35,6 +35,7 @@ from authentic2.manager.utils import get_ou_count
from authentic2.models import Attribute, Service
from authentic2.utils import hooks as a2_hooks
from authentic2.utils.evaluate import BaseExpressionValidator
from authentic2_auth_fc.models import FcAuthenticator
from authentic2_auth_oidc.utils import get_provider_by_issuer
from authentic2_idp_oidc.models import OIDCClient
@ -556,3 +557,14 @@ def scoped_db(django_db_setup, django_db_blocker):
transaction.set_rollback(True)
return scoped_db
@pytest.fixture
def fc(db):
FcAuthenticator.objects.create(
enabled=True,
client_id='xxx',
client_secret='yyy',
platform='test',
scopes=['profile', 'email'],
)

View File

@ -73,7 +73,7 @@ A2_VALIDATE_EMAIL_DOMAIN = False
A2_HOOKS_PROPAGATE_EXCEPTIONS = True
TEMPLATES[0]['DIRS'].append('tests/templates') # pylint: disable=undefined-variable
TEMPLATES[0]['DIRS'].insert(0, 'tests/templates') # pylint: disable=undefined-variable
TEMPLATES[0]['OPTIONS']['debug'] = True # pylint: disable=undefined-variable
SITE_BASE_URL = 'https://testserver'

View File

@ -0,0 +1,3 @@
{% extends "authentic2/login_password_form.html" %}
{% block login-block-title %}<h2>Log in with Password</h2>{% endblock %}

View File

@ -0,0 +1,4 @@
{% extends "authentic2/login_password_registration_form.html" %}
{% block registration-block-title %}
<h2>Register with Password</h2>
{% endblock %}

View File

@ -0,0 +1,5 @@
{% extends "authentic2_auth_fc/login.html" %}
{% block login-block-title %}
<h2>Log in with FC</h2>
{% endblock %}

View File

@ -0,0 +1,5 @@
{% extends "authentic2_auth_fc/registration.html" %}
{% block registration-block-title %}
<h2>Register with FC</h2>
{% endblock %}

View File

@ -16,6 +16,7 @@
import pytest
from authentic2.utils import misc as utils_misc
from authentic2.utils.template import Template, TemplateError
pytestmark = pytest.mark.django_db
@ -108,3 +109,18 @@ def test_render_template_missing_variable():
with pytest.raises(TemplateError) as raised:
template.render(context=context)
assert 'missing template variable' in raised
def test_registration_with_custom_titles(app, fc):
url = utils_misc.make_url('registration_register')
response = app.get(url)
assert '<h2>Registration</h2>' not in response.text
assert '<h2>Register with Password</h2>' in response.text
assert '<h2>Register with FC</h2>' in response.text
def test_login_with_custom_titles(app, fc):
url = utils_misc.make_url('auth_login')
response = app.get(url)
assert '<h2>Log in with Password</h2>' in response.text
assert '<h2>Log in with FC</h2>' in response.text