From 958c64903cac11ded7b391e65ec9d7f2953361fa Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Wed, 13 Apr 2022 14:06:23 +0200 Subject: [PATCH] authenticators: set order through dragndrop (#65479) --- src/authentic2/apps/authenticators/forms.py | 11 +++- .../apps/authenticators/manager_urls.py | 5 ++ .../authenticators/migrations/0001_initial.py | 4 +- src/authentic2/apps/authenticators/models.py | 4 +- .../authenticators/authenticators.html | 1 + .../authenticators_order_form.html | 46 ++++++++++++++++ src/authentic2/apps/authenticators/views.py | 39 ++++++++++--- .../static/authentic2/manager/css/style.scss | 8 +++ tests/test_manager_authenticators.py | 55 ++++++++++++++++++- 9 files changed, 158 insertions(+), 15 deletions(-) create mode 100644 src/authentic2/apps/authenticators/templates/authentic2/authenticators/authenticators_order_form.html diff --git a/src/authentic2/apps/authenticators/forms.py b/src/authentic2/apps/authenticators/forms.py index e010f9b5..11f0ae28 100644 --- a/src/authentic2/apps/authenticators/forms.py +++ b/src/authentic2/apps/authenticators/forms.py @@ -16,6 +16,7 @@ from django import forms from django.core.exceptions import ValidationError +from django.db.models import Max from django.utils.translation import ugettext as _ from authentic2.forms.mixins import SlugMixin @@ -35,6 +36,10 @@ class AuthenticatorFormMixin: return condition +class AuthenticatorsOrderForm(forms.Form): + order = forms.CharField(widget=forms.HiddenInput) + + class AuthenticatorAddForm(SlugMixin, forms.ModelForm): field_order = ('authenticator', 'name', 'ou') @@ -55,8 +60,12 @@ class AuthenticatorAddForm(SlugMixin, forms.ModelForm): ] def save(self): + max_order = BaseAuthenticator.objects.aggregate(max=Max('order'))['max'] or 0 + Authenticator = self.authenticators[self.cleaned_data['authenticator']] - self.instance = Authenticator(name=self.cleaned_data['name'], ou=self.cleaned_data['ou']) + self.instance = Authenticator( + name=self.cleaned_data['name'], ou=self.cleaned_data['ou'], order=max_order + 1 + ) return super().save() diff --git a/src/authentic2/apps/authenticators/manager_urls.py b/src/authentic2/apps/authenticators/manager_urls.py index c25c9bf3..664bf05c 100644 --- a/src/authentic2/apps/authenticators/manager_urls.py +++ b/src/authentic2/apps/authenticators/manager_urls.py @@ -76,5 +76,10 @@ urlpatterns = required( views.journal, name='a2-manager-authenticator-journal', ), + path( + 'authenticators/order/', + views.order, + name='a2-manager-authenticators-order', + ), ], ) diff --git a/src/authentic2/apps/authenticators/migrations/0001_initial.py b/src/authentic2/apps/authenticators/migrations/0001_initial.py index fb234311..3be40e3a 100644 --- a/src/authentic2/apps/authenticators/migrations/0001_initial.py +++ b/src/authentic2/apps/authenticators/migrations/0001_initial.py @@ -26,7 +26,7 @@ class Migration(migrations.Migration): ('uuid', models.CharField(default=uuid.uuid4, editable=False, max_length=255, unique=True)), ('name', models.CharField(max_length=128, blank=True, verbose_name='Name')), ('slug', models.SlugField(unique=True)), - ('order', models.IntegerField(default=0, verbose_name='Order')), + ('order', models.IntegerField(default=0, verbose_name='Order', editable=False)), ('enabled', models.BooleanField(default=False, editable=False)), ( 'show_condition', @@ -56,7 +56,7 @@ class Migration(migrations.Migration): ), ], options={ - 'ordering': ('-enabled', 'name', 'slug', 'ou'), + 'ordering': ('-enabled', 'order', 'name', 'slug', 'ou'), }, ), ] diff --git a/src/authentic2/apps/authenticators/models.py b/src/authentic2/apps/authenticators/models.py index ecc8cc19..9195dfc9 100644 --- a/src/authentic2/apps/authenticators/models.py +++ b/src/authentic2/apps/authenticators/models.py @@ -44,7 +44,7 @@ class BaseAuthenticator(models.Model): blank=True, on_delete=models.CASCADE, ) - order = models.IntegerField(_('Order'), default=0) + order = models.IntegerField(_('Order'), default=0, editable=False) enabled = models.BooleanField(default=False, editable=False) show_condition = models.CharField( _('Show condition'), @@ -69,7 +69,7 @@ class BaseAuthenticator(models.Model): description_fields = ['show_condition'] class Meta: - ordering = ('-enabled', 'name', 'slug', 'ou') + ordering = ('-enabled', 'order', 'name', 'slug', 'ou') def __str__(self): if self.name: diff --git a/src/authentic2/apps/authenticators/templates/authentic2/authenticators/authenticators.html b/src/authentic2/apps/authenticators/templates/authentic2/authenticators/authenticators.html index 3202e86c..c9e81ae4 100644 --- a/src/authentic2/apps/authenticators/templates/authentic2/authenticators/authenticators.html +++ b/src/authentic2/apps/authenticators/templates/authentic2/authenticators/authenticators.html @@ -4,6 +4,7 @@ {% block appbar %} {{ block.super }} + {% trans "Edit order" %} {% trans "Add new authenticator" %} {% endblock %} diff --git a/src/authentic2/apps/authenticators/templates/authentic2/authenticators/authenticators_order_form.html b/src/authentic2/apps/authenticators/templates/authentic2/authenticators/authenticators_order_form.html new file mode 100644 index 00000000..3f0e4285 --- /dev/null +++ b/src/authentic2/apps/authenticators/templates/authentic2/authenticators/authenticators_order_form.html @@ -0,0 +1,46 @@ +{% extends "authentic2/authenticators/authenticator_common.html" %} +{% load i18n gadjo %} + +{% block breadcrumb %} + {{ block.super }} + +{% endblock %} + +{% block content %} +
+ + + + {% csrf_token %} + {{ form|with_template }} +
+ + {% trans 'Cancel' %} +
+ + +
+{% endblock %} diff --git a/src/authentic2/apps/authenticators/views.py b/src/authentic2/apps/authenticators/views.py index 55fa6d94..23f7040f 100644 --- a/src/authentic2/apps/authenticators/views.py +++ b/src/authentic2/apps/authenticators/views.py @@ -21,7 +21,7 @@ from django.shortcuts import get_object_or_404 from django.urls import reverse, reverse_lazy from django.utils.functional import cached_property from django.utils.translation import ugettext as _ -from django.views.generic import CreateView, DeleteView, DetailView, UpdateView +from django.views.generic import CreateView, DeleteView, DetailView, FormView, UpdateView from django.views.generic.list import ListView from authentic2.apps.authenticators import forms @@ -32,13 +32,14 @@ from authentic2.manager.views import MediaMixin, TitleMixin class AuthenticatorsMixin(MediaMixin, TitleMixin): + model = BaseAuthenticator + def get_queryset(self): return self.model.authenticators.all() class AuthenticatorsView(AuthenticatorsMixin, ListView): template_name = 'authentic2/authenticators/authenticators.html' - model = BaseAuthenticator title = _('Authenticators') @@ -64,7 +65,6 @@ add = AuthenticatorAddView.as_view() class AuthenticatorDetailView(AuthenticatorsMixin, DetailView): template_name = 'authentic2/authenticators/authenticator_detail.html' - model = BaseAuthenticator @property def title(self): @@ -77,7 +77,6 @@ detail = AuthenticatorDetailView.as_view() class AuthenticatorEditView(AuthenticatorsMixin, UpdateView): template_name = 'authentic2/authenticators/authenticator_edit_form.html' title = _('Edit authenticator') - model = BaseAuthenticator def get_form_class(self): return self.object.manager_form_class @@ -93,7 +92,6 @@ edit = AuthenticatorEditView.as_view() class AuthenticatorDeleteView(AuthenticatorsMixin, DeleteView): template_name = 'authentic2/authenticators/authenticator_delete_form.html' title = _('Delete authenticator') - model = BaseAuthenticator success_url = reverse_lazy('a2-manager-authenticators') def dispatch(self, *args, **kwargs): @@ -110,8 +108,6 @@ delete = AuthenticatorDeleteView.as_view() class AuthenticatorToggleView(AuthenticatorsMixin, DetailView): - model = BaseAuthenticator - def dispatch(self, *args, **kwargs): self.authenticator = self.get_object() if self.authenticator.protected or not self.authenticator.has_valid_configuration(): @@ -153,3 +149,32 @@ class AuthenticatorJournal(JournalViewWithContext, BaseJournalView): journal = AuthenticatorJournal.as_view() + + +class AuthenticatorsOrderView(AuthenticatorsMixin, FormView): + template_name = 'authentic2/authenticators/authenticators_order_form.html' + title = _('Configure display order') + form_class = forms.AuthenticatorsOrderForm + success_url = reverse_lazy('a2-manager-authenticators') + + def form_valid(self, form): + order_by_pk = {pk: i for i, pk in enumerate(form.cleaned_data['order'].split(','))} + + authenticators = list(self.get_queryset()) + for authenticator in authenticators: + authenticator.order = order_by_pk[str(authenticator.pk)] + + BaseAuthenticator.objects.bulk_update(authenticators, ['order']) + return super().form_valid(form) + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context['authenticators'] = self.get_queryset() + return context + + def get_queryset(self): + qs = super().get_queryset() + return qs.filter(enabled=True) + + +order = AuthenticatorsOrderView.as_view() diff --git a/src/authentic2/manager/static/authentic2/manager/css/style.scss b/src/authentic2/manager/static/authentic2/manager/css/style.scss index 13edc9af..11922787 100644 --- a/src/authentic2/manager/static/authentic2/manager/css/style.scss +++ b/src/authentic2/manager/static/authentic2/manager/css/style.scss @@ -297,3 +297,11 @@ ul#id_scopes li { -webkit-column-width: 20em; column-width: 20em; } + +span.handle { + cursor: move; + display: inline-block; + padding: 0.5ex; + text-align: center; + width: 1em; +} diff --git a/tests/test_manager_authenticators.py b/tests/test_manager_authenticators.py index 57eccf8d..de03fde1 100644 --- a/tests/test_manager_authenticators.py +++ b/tests/test_manager_authenticators.py @@ -17,7 +17,7 @@ import pytest from django import VERSION as DJ_VERSION -from authentic2.apps.authenticators.models import LoginPasswordAuthenticator +from authentic2.apps.authenticators.models import BaseAuthenticator, LoginPasswordAuthenticator from authentic2_auth_fc.models import FcAuthenticator from authentic2_auth_oidc.models import OIDCProvider from authentic2_auth_saml.models import SAMLAuthenticator @@ -54,7 +54,6 @@ def test_authenticators_password(app, superuser): resp = resp.click('Edit') assert list(resp.form.fields) == [ 'csrfmiddlewaretoken', - 'order', 'show_condition', 'remember_me', 'include_ou_selector', @@ -209,7 +208,6 @@ def test_authenticators_fc(app, superuser): resp = resp.click('Edit') assert list(resp.form.fields) == [ 'csrfmiddlewaretoken', - 'order', 'show_condition', 'platform', 'client_id', @@ -275,3 +273,54 @@ def test_authenticators_saml(app, superuser, ou1, ou2): resp = resp.click('Enable').follow() assert 'Authenticator has been enabled.' in resp.text + + +def test_authenticators_order(app, superuser): + resp = login(app, superuser, path='/manage/authenticators/') + + saml_authenticator = SAMLAuthenticator.objects.create(name='Test', slug='test', enabled=True, order=42) + SAMLAuthenticator.objects.create(name='Test disabled', slug='test-disabled', enabled=False) + fc_authenticator = FcAuthenticator.objects.create(slug='fc-authenticator', enabled=True, order=-1) + password_authenticator = LoginPasswordAuthenticator.objects.get() + + assert fc_authenticator.order == -1 + assert password_authenticator.order == 0 + assert saml_authenticator.order == 42 + + resp = resp.click('Edit order') + assert resp.text.index('FranceConnect') < resp.text.index('Password') < resp.text.index('SAML - Test') + assert 'SAML - Test disabled' not in resp.text + + resp.form['order'] = '%s,%s,%s' % (saml_authenticator.pk, password_authenticator.pk, fc_authenticator.pk) + resp.form.submit() + + fc_authenticator.refresh_from_db() + password_authenticator.refresh_from_db() + saml_authenticator.refresh_from_db() + assert fc_authenticator.order == 2 + assert password_authenticator.order == 1 + assert saml_authenticator.order == 0 + + +def test_authenticators_add_last(app, superuser): + resp = login(app, superuser, path='/manage/authenticators/') + + BaseAuthenticator.objects.all().delete() + + resp = resp.click('Add new authenticator') + resp.form['name'] = 'Test' + resp.form['authenticator'] = 'saml' + resp.form.submit() + + authenticator = SAMLAuthenticator.objects.get() + assert authenticator.order == 1 + + authenticator.order = 42 + authenticator.save() + resp = app.get('/manage/authenticators/add/') + resp.form['name'] = 'Test 2' + resp.form['authenticator'] = 'saml' + resp.form.submit() + + authenticator = SAMLAuthenticator.objects.filter(slug='test-2').get() + assert authenticator.order == 43 -- 2.30.2