From ab641f729ddcc9fc771c26005973553d280ad7a3 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Wed, 17 Aug 2022 18:16:08 +0200 Subject: [PATCH] auth_saml: prefer metadata file upload over metadata path (#67451) --- src/authentic2/apps/authenticators/models.py | 18 ++++++++++---- src/authentic2_auth_saml/forms.py | 5 ++++ .../0009_samlauthenticator_metadata_file.py | 22 +++++++++++++++++ src/authentic2_auth_saml/models.py | 21 ++++++++++++++-- tests/test_manager_authenticators.py | 24 +++++++++++++++---- 5 files changed, 79 insertions(+), 11 deletions(-) create mode 100644 src/authentic2_auth_saml/migrations/0009_samlauthenticator_metadata_file.py diff --git a/src/authentic2/apps/authenticators/models.py b/src/authentic2/apps/authenticators/models.py index 61c77f9e8..8994506ae 100644 --- a/src/authentic2/apps/authenticators/models.py +++ b/src/authentic2/apps/authenticators/models.py @@ -16,12 +16,16 @@ import datetime import logging +import os import uuid from django.core.exceptions import ValidationError from django.db import models +from django.db.models.fields.files import FieldFile from django.shortcuts import render, reverse from django.utils.formats import date_format +from django.utils.html import format_html +from django.utils.safestring import mark_safe from django.utils.text import capfirst from django.utils.translation import pgettext_lazy from django.utils.translation import ugettext_lazy as _ @@ -115,11 +119,15 @@ class BaseAuthenticator(models.Model): if isinstance(value, datetime.datetime): value = date_format(value, 'DATETIME_FORMAT') - - yield _('%(field)s: %(value)s') % { - 'field': capfirst(self._meta.get_field(field).verbose_name), - 'value': value, - } + elif isinstance(value, FieldFile): + filename = os.path.basename(value.path) + value = mark_safe(f'{filename}') + + yield format_html( + _('{field}: {value}'), + field=capfirst(self._meta.get_field(field).verbose_name), + value=value, + ) def shown(self, ctx=()): if not self.show_condition: diff --git a/src/authentic2_auth_saml/forms.py b/src/authentic2_auth_saml/forms.py index c0af5890a..8c3d3a6b9 100644 --- a/src/authentic2_auth_saml/forms.py +++ b/src/authentic2_auth_saml/forms.py @@ -45,6 +45,11 @@ class SAMLAuthenticatorForm(forms.ModelForm): 'attribute_mapping', ) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if not self.initial.get('metadata_path'): + del self.fields['metadata_path'] + class SAMLAuthenticatorAdvancedForm(forms.ModelForm): class Meta: diff --git a/src/authentic2_auth_saml/migrations/0009_samlauthenticator_metadata_file.py b/src/authentic2_auth_saml/migrations/0009_samlauthenticator_metadata_file.py new file mode 100644 index 000000000..1fa61ed11 --- /dev/null +++ b/src/authentic2_auth_saml/migrations/0009_samlauthenticator_metadata_file.py @@ -0,0 +1,22 @@ +# Generated by Django 2.2.26 on 2022-09-19 13:46 + +from django.db import migrations, models + +import authentic2_auth_saml.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('authentic2_auth_saml', '0008_auto_20220913_1105'), + ] + + operations = [ + migrations.AddField( + model_name='samlauthenticator', + name='metadata_file', + field=models.FileField( + blank=True, null=True, upload_to=authentic2_auth_saml.models.metadata_directory_path + ), + ), + ] diff --git a/src/authentic2_auth_saml/models.py b/src/authentic2_auth_saml/models.py index 7c35d4920..5dd92e69f 100644 --- a/src/authentic2_auth_saml/models.py +++ b/src/authentic2_auth_saml/models.py @@ -14,6 +14,8 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +import uuid + from django.conf import settings from django.contrib.postgres.fields import JSONField from django.core.exceptions import ValidationError @@ -26,11 +28,16 @@ from authentic2.manager.utils import label_from_role from authentic2.utils.misc import redirect_to_login +def metadata_directory_path(instance, filename): + return 'saml_authenticator_%s/%s/metadata.xml' % (instance.pk, uuid.uuid4()) + + class SAMLAuthenticator(BaseAuthenticator): metadata_url = models.URLField(_('Metadata URL'), max_length=300, blank=True) metadata_cache_time = models.PositiveSmallIntegerField(_('Metadata cache time'), default=3600) metadata_http_timeout = models.PositiveSmallIntegerField(_('Metadata HTTP timeout'), default=10) + metadata_file = models.FileField(null=True, blank=True, upload_to=metadata_directory_path) metadata_path = models.CharField( _('Metadata file path'), max_length=300, @@ -141,7 +148,14 @@ class SAMLAuthenticator(BaseAuthenticator): type = 'saml' how = ['saml'] manager_view_template_name = 'authentic2_auth_saml/authenticator_detail.html' - description_fields = ['show_condition', 'metadata_url', 'metadata_path', 'metadata', 'provision'] + description_fields = [ + 'show_condition', + 'metadata_url', + 'metadata_file', + 'metadata_path', + 'metadata', + 'provision', + ] class Meta: verbose_name = _('SAML') @@ -156,6 +170,9 @@ class SAMLAuthenticator(BaseAuthenticator): if not settings[setting]: del settings[setting] + if self.metadata_file: + settings['METADATA_PATH'] = self.metadata_file.path + settings['LOOKUP_BY_ATTRIBUTES'] = [lookup.as_dict() for lookup in self.attribute_lookups.all()] settings['authenticator'] = self @@ -171,7 +188,7 @@ class SAMLAuthenticator(BaseAuthenticator): ] def clean(self): - if not (self.metadata or self.metadata_path or self.metadata_url): + if not (self.metadata or self.metadata_path or self.metadata_url or self.metadata_file): raise ValidationError(_('One of the metadata fields must be filled.')) def autorun(self, request, block_id): diff --git a/tests/test_manager_authenticators.py b/tests/test_manager_authenticators.py index cca74fc44..25d5b7566 100644 --- a/tests/test_manager_authenticators.py +++ b/tests/test_manager_authenticators.py @@ -17,6 +17,7 @@ import pytest from django import VERSION as DJ_VERSION from django.utils.html import escape +from webtest import Upload from authentic2.a2_rbac.utils import get_default_ou from authentic2.apps.authenticators.models import BaseAuthenticator, LoginPasswordAuthenticator @@ -285,7 +286,7 @@ def test_authenticators_saml(app, superuser, ou1, ou2): authenticator = SAMLAuthenticator.objects.filter(slug='test').get() resp = app.get(authenticator.get_absolute_url()) assert 'Create user if their username does not already exists: True' in resp.text - assert 'Metadata file path' not in resp.text + assert 'Metadata file' not in resp.text assert 'Enable' not in resp.text assert 'configuration is not complete' in resp.text @@ -297,9 +298,11 @@ def test_authenticators_saml(app, superuser, ou1, ou2): resp = resp.form.submit() assert 'One of the metadata fields must be filled.' in resp.text - resp.form['metadata_path'] = '/var/lib/authentic2/metadata.xml' + resp.form['metadata_file'] = Upload('metadata.xml', b'some-xml', 'text/xml') resp = resp.form.submit().follow() - assert 'Metadata file path: /var/lib/authentic2/metadata.xml' in resp.text + + authenticator.refresh_from_db() + assert 'Metadata file: metadata.xml' % authenticator.metadata_file.url in resp.text resp = resp.click('Enable').follow() assert 'Authenticator has been enabled.' in resp.text @@ -325,7 +328,6 @@ def test_authenticators_saml_hide_metadata_url_advanced_fields(app, superuser, o assert 'Metadata cache time' not in resp.text assert 'Metadata HTTP timeout' not in resp.text - resp.form['metadata_path'] = '' resp.form['metadata_url'] = 'https://example.com/metadata.xml' resp = resp.form.submit().follow() @@ -347,6 +349,20 @@ def test_authenticators_saml_missing_signing_key(app, superuser, settings): assert 'Signing key is missing' not in resp.text +def test_authenticators_saml_hide_metadata_path(app, superuser): + authenticator = SAMLAuthenticator.objects.create(slug='idp1') + + resp = login(app, superuser) + resp = app.get('/manage/authenticators/%s/edit/' % authenticator.pk) + assert 'metadata_path' not in resp.form.fields + + authenticator.metadata_path = '/var/lib/authentic2/metadata.xml' + authenticator.save() + + resp = app.get('/manage/authenticators/%s/edit/' % authenticator.pk) + assert 'metadata_path' in resp.form.fields + + def test_authenticators_saml_attribute_lookup(app, superuser): authenticator = SAMLAuthenticator.objects.create(metadata='meta1.xml', slug='idp1') resp = login(app, superuser, path=authenticator.get_absolute_url()) -- 2.30.2