From 8124d6214d2e81f1232f6323cb9123930deaec2e Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Wed, 21 Sep 2022 11:09:25 +0200 Subject: [PATCH 03/10] auth_saml: genericize related object code (#53442) --- src/authentic2/apps/authenticators/models.py | 2 + .../journal_event_types.py | 36 ++++++++------- ..._samlattributelookup_setattributeaction.py | 4 ++ src/authentic2_auth_saml/models.py | 44 ++++++++++++++++--- .../authenticator_detail.html | 22 ++++------ .../related_object_list.html | 6 +-- src/authentic2_auth_saml/urls.py | 6 +-- src/authentic2_auth_saml/views.py | 34 ++++++-------- tests/test_manager_authenticators.py | 6 +-- tests/test_manager_journal.py | 12 ++--- 10 files changed, 99 insertions(+), 73 deletions(-) diff --git a/src/authentic2/apps/authenticators/models.py b/src/authentic2/apps/authenticators/models.py index 6f732e96d..6bedc7554 100644 --- a/src/authentic2/apps/authenticators/models.py +++ b/src/authentic2/apps/authenticators/models.py @@ -74,6 +74,8 @@ class BaseAuthenticator(models.Model): authenticators = AuthenticatorManager() type = '' + related_models = [] + related_object_form_class = None manager_view_template_name = 'authentic2/authenticators/authenticator_detail.html' unique = False protected = False diff --git a/src/authentic2_auth_saml/journal_event_types.py b/src/authentic2_auth_saml/journal_event_types.py index bef5295bd..689560e29 100644 --- a/src/authentic2_auth_saml/journal_event_types.py +++ b/src/authentic2_auth_saml/journal_event_types.py @@ -16,27 +16,27 @@ from django.utils.translation import gettext_lazy as _ -from authentic2.apps.journal.models import EventTypeDefinition +from authentic2.apps.authenticators.journal_event_types import AuthenticatorEvents +from authentic2.apps.authenticators.models import BaseAuthenticator from authentic2.apps.journal.utils import form_to_old_new -from .models import SAMLAuthenticator - -class SAMLAuthenticatorEvents(EventTypeDefinition): +class AuthenticatorRelatedObjectEvents(AuthenticatorEvents): @classmethod def record(cls, *, user, session, related_object, data=None): data = data or {} data.update({'related_object': related_object.get_journal_text()}) - super().record(user=user, session=session, references=[related_object.authenticator], data=data) + super().record(user=user, session=session, authenticator=related_object.authenticator, data=data) -class SAMLAuthenticatorRelatedObjectCreation(SAMLAuthenticatorEvents): - name = 'authenticator.saml.related_object.creation' - label = _('SAML authenticator related object creation') +class AuthenticatorRelatedObjectCreation(AuthenticatorRelatedObjectEvents): + name = 'authenticator.related_object.creation' + label = _('Authenticator related object creation') @classmethod def get_message(cls, event, context): - (authenticator,) = event.get_typed_references(SAMLAuthenticator) + (authenticator,) = event.get_typed_references(BaseAuthenticator) + authenticator = authenticator or event.get_data('authenticator_name') related_object = event.get_data('related_object') if context != authenticator: return _('creation of object "{related_object}" in authenticator "{authenticator}"').format( @@ -46,9 +46,9 @@ class SAMLAuthenticatorRelatedObjectCreation(SAMLAuthenticatorEvents): return _('creation of object "%s"') % related_object -class SAMLAuthenticatorRelatedObjectEdit(SAMLAuthenticatorEvents): - name = 'authenticator.saml.related_object.edit' - label = _('SAML authenticator related object edit') +class AuthenticatorRelatedObjectEdit(AuthenticatorRelatedObjectEvents): + name = 'authenticator.related_object.edit' + label = _('Authenticator related object edit') @classmethod def record(cls, *, user, session, form): @@ -61,7 +61,8 @@ class SAMLAuthenticatorRelatedObjectEdit(SAMLAuthenticatorEvents): @classmethod def get_message(cls, event, context): - (authenticator,) = event.get_typed_references(SAMLAuthenticator) + (authenticator,) = event.get_typed_references(BaseAuthenticator) + authenticator = authenticator or event.get_data('authenticator_name') related_object = event.get_data('related_object') new = event.get_data('new') or {} edited_attributes = ', '.join(new) or '' @@ -79,13 +80,14 @@ class SAMLAuthenticatorRelatedObjectEdit(SAMLAuthenticatorEvents): ) -class SAMLAuthenticatorRelatedObjectDeletion(SAMLAuthenticatorEvents): - name = 'authenticator.saml.related_object.deletion' - label = _('SAML authenticator related object deletion') +class AuthenticatorRelatedObjectDeletion(AuthenticatorRelatedObjectEvents): + name = 'authenticator.related_object.deletion' + label = _('Authenticator related object deletion') @classmethod def get_message(cls, event, context): - (authenticator,) = event.get_typed_references(SAMLAuthenticator) + (authenticator,) = event.get_typed_references(BaseAuthenticator) + authenticator = authenticator or event.get_data('authenticator_name') related_object = event.get_data('related_object') if context != authenticator: return _('deletion of object "{related_object}" in authenticator "{authenticator}"').format( diff --git a/src/authentic2_auth_saml/migrations/0005_addroleaction_renameattributeaction_samlattributelookup_setattributeaction.py b/src/authentic2_auth_saml/migrations/0005_addroleaction_renameattributeaction_samlattributelookup_setattributeaction.py index f9d4a4c5f..19cb6425f 100644 --- a/src/authentic2_auth_saml/migrations/0005_addroleaction_renameattributeaction_samlattributelookup_setattributeaction.py +++ b/src/authentic2_auth_saml/migrations/0005_addroleaction_renameattributeaction_samlattributelookup_setattributeaction.py @@ -39,6 +39,7 @@ class Migration(migrations.Migration): ], options={ 'verbose_name': 'Set an attribute', + 'verbose_name_plural': 'Set attributes', 'default_related_name': 'set_attribute_actions', }, ), @@ -63,6 +64,7 @@ class Migration(migrations.Migration): ], options={ 'verbose_name': 'Attribute lookup', + 'verbose_name_plural': 'Lookup by attributes', 'default_related_name': 'attribute_lookups', }, ), @@ -86,6 +88,7 @@ class Migration(migrations.Migration): ], options={ 'verbose_name': 'Rename an attribute', + 'verbose_name_plural': 'Rename attributes', 'default_related_name': 'rename_attribute_actions', }, ), @@ -126,6 +129,7 @@ class Migration(migrations.Migration): ], options={ 'verbose_name': 'Add a role', + 'verbose_name_plural': 'Add roles', 'default_related_name': 'add_role_actions', }, ), diff --git a/src/authentic2_auth_saml/models.py b/src/authentic2_auth_saml/models.py index 1566ed5e6..592b2cc3a 100644 --- a/src/authentic2_auth_saml/models.py +++ b/src/authentic2_auth_saml/models.py @@ -170,6 +170,20 @@ class SAMLAuthenticator(BaseAuthenticator): (_('Advanced'), SAMLAuthenticatorAdvancedForm), ] + @property + def related_object_form_class(self): + from .forms import SAMLRelatedObjectForm + + return SAMLRelatedObjectForm + + @property + def related_models(self): + return { + SAMLAttributeLookup: self.attribute_lookups.all(), + SetAttributeAction: self.set_attribute_actions.all(), + AddRoleAction: self.add_role_actions.all(), + } + def clean(self): if not (self.metadata or self.metadata_path or self.metadata_url): raise ValidationError(_('One of the metadata fields must be filled.')) @@ -199,7 +213,7 @@ class SAMLAuthenticator(BaseAuthenticator): return views.profile(request, *args, **kwargs) -class SAMLRelatedObjectBase(models.Model): +class AuthenticatorRelatedObjectBase(models.Model): authenticator = models.ForeignKey(SAMLAuthenticator, on_delete=models.CASCADE) class Meta: @@ -208,13 +222,16 @@ class SAMLRelatedObjectBase(models.Model): def get_journal_text(self): return '%s (%s)' % (self._meta.verbose_name, self.pk) - def get_user_field_display(self): - from authentic2.forms.widgets import SelectAttributeWidget + @property + def model_name(self): + return self._meta.model_name - return SelectAttributeWidget.get_options().get(self.user_field, self.user_field) + @property + def verbose_name_plural(self): + return self._meta.verbose_name_plural -class SAMLAttributeLookup(SAMLRelatedObjectBase): +class SAMLAttributeLookup(AuthenticatorRelatedObjectBase): user_field = models.CharField(_('User field'), max_length=256) saml_attribute = models.CharField(_('SAML attribute'), max_length=1024) ignore_case = models.BooleanField(_('Ignore case'), default=False) @@ -222,6 +239,7 @@ class SAMLAttributeLookup(SAMLRelatedObjectBase): class Meta: default_related_name = 'attribute_lookups' verbose_name = _('Attribute lookup') + verbose_name_plural = _('Lookup by attributes') def __str__(self): label = _('"%(saml_attribute)s" (from "%(user_field)s")') % { @@ -239,8 +257,13 @@ class SAMLAttributeLookup(SAMLRelatedObjectBase): 'ignore-case': self.ignore_case, } + def get_user_field_display(self): + from authentic2.forms.widgets import SelectAttributeWidget + + return SelectAttributeWidget.get_options().get(self.user_field, self.user_field) + -class SetAttributeAction(SAMLRelatedObjectBase): +class SetAttributeAction(AuthenticatorRelatedObjectBase): user_field = models.CharField(_('User field'), max_length=256) saml_attribute = models.CharField(_('SAML attribute name'), max_length=1024) mandatory = models.BooleanField(_('Mandatory'), default=False, help_text=_('Deny login if action fails.')) @@ -248,6 +271,7 @@ class SetAttributeAction(SAMLRelatedObjectBase): class Meta: default_related_name = 'set_attribute_actions' verbose_name = _('Set an attribute') + verbose_name_plural = _('Set attributes') def __str__(self): label = _('"%(attribute)s" from "%(saml_attribute)s"') % { @@ -258,8 +282,13 @@ class SetAttributeAction(SAMLRelatedObjectBase): label = '%s (%s)' % (label, _('mandatory')) return label + def get_user_field_display(self): + from authentic2.forms.widgets import SelectAttributeWidget + + return SelectAttributeWidget.get_options().get(self.user_field, self.user_field) + -class AddRoleAction(SAMLRelatedObjectBase): +class AddRoleAction(AuthenticatorRelatedObjectBase): role = models.ForeignKey(Role, verbose_name=_('Role'), on_delete=models.CASCADE) condition = models.CharField(_('Condition (unused)'), editable=False, max_length=256, blank=True) mandatory = models.BooleanField(_('Mandatory (unused)'), editable=False, default=False) @@ -267,6 +296,7 @@ class AddRoleAction(SAMLRelatedObjectBase): class Meta: default_related_name = 'add_role_actions' verbose_name = _('Add a role') + verbose_name_plural = _('Add roles') def __str__(self): return label_from_role(self.role) diff --git a/src/authentic2_auth_saml/templates/authentic2_auth_saml/authenticator_detail.html b/src/authentic2_auth_saml/templates/authentic2_auth_saml/authenticator_detail.html index 9a03b60ba..797224758 100644 --- a/src/authentic2_auth_saml/templates/authentic2_auth_saml/authenticator_detail.html +++ b/src/authentic2_auth_saml/templates/authentic2_auth_saml/authenticator_detail.html @@ -12,21 +12,15 @@ {% endblock %} {% block extra-tab-buttons %} - - - + {% for model in object.related_models %} + + {% endfor %} {% endblock %} {% block extra-tab-list %} - - - - - + {% for model, objects in object.related_models.items %} + + {% endfor %} {% endblock %} diff --git a/src/authentic2_auth_saml/templates/authentic2_auth_saml/related_object_list.html b/src/authentic2_auth_saml/templates/authentic2_auth_saml/related_object_list.html index 7186030c9..b9ce1d9fc 100644 --- a/src/authentic2_auth_saml/templates/authentic2_auth_saml/related_object_list.html +++ b/src/authentic2_auth_saml/templates/authentic2_auth_saml/related_object_list.html @@ -3,9 +3,9 @@ diff --git a/src/authentic2_auth_saml/urls.py b/src/authentic2_auth_saml/urls.py index f2473f404..c9269f6c9 100644 --- a/src/authentic2_auth_saml/urls.py +++ b/src/authentic2_auth_saml/urls.py @@ -32,17 +32,17 @@ urlpatterns += required( path( 'authenticators///add/', views.add_related_object, - name='a2-manager-saml-add-related-object', + name='a2-manager-authenticators-add-related-object', ), path( 'authenticators////edit/', views.edit_related_object, - name='a2-manager-saml-edit-related-object', + name='a2-manager-authenticators-edit-related-object', ), path( 'authenticators////delete/', views.delete_related_object, - name='a2-manager-saml-delete-related-object', + name='a2-manager-authenticators-delete-related-object', ), ], ) diff --git a/src/authentic2_auth_saml/views.py b/src/authentic2_auth_saml/views.py index ba8958a94..7b0362cd5 100644 --- a/src/authentic2_auth_saml/views.py +++ b/src/authentic2_auth_saml/views.py @@ -7,12 +7,10 @@ from django.urls import reverse from django.views.generic import CreateView, DeleteView, UpdateView from mellon.utils import get_idp +from authentic2.apps.authenticators.models import BaseAuthenticator from authentic2.manager.views import MediaMixin, TitleMixin from authentic2.utils.misc import redirect_to_login -from .forms import SAMLRelatedObjectForm -from .models import AddRoleAction, SAMLAttributeLookup, SAMLAuthenticator, SetAttributeAction - def login(request, authenticator, *args, **kwargs): context = kwargs.pop('context', {}).copy() @@ -45,21 +43,21 @@ def profile(request, *args, **kwargs): return render_to_string('authentic2_auth_saml/profile.html', context, request=request) -class SAMLAuthenticatorMixin(MediaMixin, TitleMixin): - allowed_models = (SAMLAttributeLookup, SetAttributeAction, AddRoleAction) - +class AuthenticatorRelatedObjectMixin(MediaMixin, TitleMixin): def dispatch(self, request, *args, **kwargs): - self.authenticator = get_object_or_404(SAMLAuthenticator, pk=kwargs.get('authenticator_pk')) + self.authenticator = get_object_or_404( + BaseAuthenticator.authenticators.all(), pk=kwargs.get('authenticator_pk') + ) model_name = kwargs.get('model_name') - if model_name not in (x._meta.model_name for x in self.allowed_models): + if model_name not in (x._meta.model_name for x in self.authenticator.related_models): raise Http404() - self.model = apps.get_model('authentic2_auth_saml', model_name) + self.model = apps.get_model(self.authenticator._meta.app_label, model_name) return super().dispatch(request, *args, **kwargs) def get_form_class(self): - return modelform_factory(self.model, SAMLRelatedObjectForm) + return modelform_factory(self.model, self.authenticator.related_object_form_class) def get_form_kwargs(self): kwargs = super().get_form_kwargs() @@ -79,40 +77,36 @@ class SAMLAuthenticatorMixin(MediaMixin, TitleMixin): return self.model._meta.verbose_name -class RelatedObjectAddView(SAMLAuthenticatorMixin, CreateView): +class RelatedObjectAddView(AuthenticatorRelatedObjectMixin, CreateView): template_name = 'authentic2/manager/form.html' def form_valid(self, form): resp = super().form_valid(form) - self.request.journal.record( - 'authenticator.saml.related_object.creation', related_object=form.instance - ) + self.request.journal.record('authenticator.related_object.creation', related_object=form.instance) return resp add_related_object = RelatedObjectAddView.as_view() -class RelatedObjectEditView(SAMLAuthenticatorMixin, UpdateView): +class RelatedObjectEditView(AuthenticatorRelatedObjectMixin, UpdateView): template_name = 'authentic2/manager/form.html' def form_valid(self, form): resp = super().form_valid(form) - self.request.journal.record('authenticator.saml.related_object.edit', form=form) + self.request.journal.record('authenticator.related_object.edit', form=form) return resp edit_related_object = RelatedObjectEditView.as_view() -class RelatedObjectDeleteView(SAMLAuthenticatorMixin, DeleteView): +class RelatedObjectDeleteView(AuthenticatorRelatedObjectMixin, DeleteView): template_name = 'authentic2/authenticators/authenticator_delete_form.html' title = '' def delete(self, *args, **kwargs): - self.request.journal.record( - 'authenticator.saml.related_object.deletion', related_object=self.get_object() - ) + self.request.journal.record('authenticator.related_object.deletion', related_object=self.get_object()) return super().delete(*args, **kwargs) diff --git a/tests/test_manager_authenticators.py b/tests/test_manager_authenticators.py index f5b4c81af..decb86fde 100644 --- a/tests/test_manager_authenticators.py +++ b/tests/test_manager_authenticators.py @@ -355,7 +355,7 @@ def test_authenticators_saml_attribute_lookup(app, superuser): resp.form['user_field'].select(text='Email address (email)') resp.form['saml_attribute'] = 'mail' resp = resp.form.submit() - assert_event('authenticator.saml.related_object.creation', user=superuser, session=app.session) + assert_event('authenticator.related_object.creation', user=superuser, session=app.session) assert '#open:samlattributelookup' in resp.location resp = resp.follow() @@ -365,7 +365,7 @@ def test_authenticators_saml_attribute_lookup(app, superuser): resp.form['ignore_case'] = True resp = resp.form.submit().follow() assert escape('"mail" (from "Email address (email)"), case insensitive') in resp.text - assert_event('authenticator.saml.related_object.edit', user=superuser, session=app.session) + assert_event('authenticator.related_object.edit', user=superuser, session=app.session) Attribute.objects.create(kind='string', name='test', label='Test') resp = resp.click('mail') @@ -376,7 +376,7 @@ def test_authenticators_saml_attribute_lookup(app, superuser): resp = resp.click('Remove', href='samlattributelookup') resp = resp.form.submit().follow() assert 'mail' not in resp.text - assert_event('authenticator.saml.related_object.deletion', user=superuser, session=app.session) + assert_event('authenticator.related_object.deletion', user=superuser, session=app.session) def test_authenticators_saml_set_attribute(app, superuser): diff --git a/tests/test_manager_journal.py b/tests/test_manager_journal.py index f36defd2b..05d75ae8a 100644 --- a/tests/test_manager_journal.py +++ b/tests/test_manager_journal.py @@ -308,7 +308,7 @@ def events(db, freezer): make('authenticator.disable', user=agent, session=session2, authenticator=authenticator) make('authenticator.deletion', user=agent, session=session2, authenticator=authenticator) make( - 'authenticator.saml.related_object.creation', + 'authenticator.related_object.creation', user=agent, session=session2, related_object=set_attribute_action, @@ -318,9 +318,9 @@ def events(db, freezer): action_edit_form.initial = {'from_name': 'old'} action_edit_form.changed_data = ['from_name'] action_edit_form.cleaned_data = {'from_name': 'new'} - make('authenticator.saml.related_object.edit', user=agent, session=session2, form=action_edit_form) + make('authenticator.related_object.edit', user=agent, session=session2, form=action_edit_form) make( - 'authenticator.saml.related_object.deletion', + 'authenticator.related_object.deletion', user=agent, session=session2, related_object=set_attribute_action, @@ -730,21 +730,21 @@ def test_global_journal(app, superuser, events): 'message': 'creation of object "Set an attribute (%s)" in authenticator "SAML"' % set_attribute_action.pk, 'timestamp': 'Jan. 3, 2020, 8 a.m.', - 'type': 'authenticator.saml.related_object.creation', + 'type': 'authenticator.related_object.creation', 'user': 'agent', }, { 'message': 'edit of object "Set an attribute (%s)" in authenticator "SAML" (from_name)' % set_attribute_action.pk, 'timestamp': 'Jan. 3, 2020, 9 a.m.', - 'type': 'authenticator.saml.related_object.edit', + 'type': 'authenticator.related_object.edit', 'user': 'agent', }, { 'message': 'deletion of object "Set an attribute (%s)" in authenticator "SAML"' % set_attribute_action.pk, 'timestamp': 'Jan. 3, 2020, 10 a.m.', - 'type': 'authenticator.saml.related_object.deletion', + 'type': 'authenticator.related_object.deletion', 'user': 'agent', }, { -- 2.30.2