auth_saml: remove metadata file path field (#70491)

src/authentic2_auth_saml/migrations/0013_metadata_file_to_db.py

@@ -0,0 +1,46 @@
+# Generated by Django 2.2.26 on 2022-10-26 10:01
+
+import logging
+import os
+
+from django.db import migrations, transaction
+
+logger = logging.getLogger('authentic2.auth_saml')
+
+
+def metadata_file_to_db(apps, schema_editor):
+    SAMLAuthenticator = apps.get_model('authentic2_auth_saml', 'SAMLAuthenticator')
+
+    def move_data(authenticator):
+        path = authenticator.metadata_path
+
+        if not os.path.exists(path):
+            return
+
+        try:
+            with open(path) as fd:
+                metadata = fd.read()
+        except OSError:
+            logger.warning('auth_saml: open()/read() call failed for %s on %s', authenticator, path)
+            return
+
+        authenticator.metadata = metadata
+        authenticator.save()
+
+    for authenticator in SAMLAuthenticator.objects.exclude(metadata_path=''):
+        try:
+            with transaction.atomic():
+                move_data(authenticator)
+        except Exception:
+            logger.exception('auth_saml: could not move metadata file to db for %s', authenticator)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('authentic2_auth_saml', '0012_move_add_role_action'),
+    ]
+
+    operations = [
+        migrations.RunPython(metadata_file_to_db, reverse_code=migrations.RunPython.noop),
+    ]

src/authentic2_auth_saml/migrations/0014_remove_samlauthenticator_metadata_path.py

@@ -0,0 +1,17 @@
+# Generated by Django 2.2.26 on 2022-10-26 10:09
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('authentic2_auth_saml', '0013_metadata_file_to_db'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='samlauthenticator',
+            name='metadata_path',
+        ),
+    ]

src/authentic2_auth_saml/models.py

@@ -52,12 +52,6 @@ class SAMLAuthenticator(BaseAuthenticator):
     metadata_cache_time = models.PositiveSmallIntegerField(_('Metadata cache time'), default=3600)
     metadata_http_timeout = models.PositiveSmallIntegerField(_('Metadata HTTP timeout'), default=10)
 
-    metadata_path = models.CharField(
-        _('Metadata file path'),
-        max_length=300,
-        help_text=_('Absolute path to the IdP metadata file.'),
-        blank=True,
-    )
     metadata = models.TextField(_('Metadata (XML)'), blank=True, validators=[validate_metadata])
 
     provision = models.BooleanField(_('Create user if their username does not already exists'), default=True)
@@ -162,7 +156,7 @@ 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', 'provision']
 
     class Meta:
         verbose_name = _('SAML')
@@ -173,7 +167,7 @@ class SAMLAuthenticator(BaseAuthenticator):
 
         settings['AUTHN_CLASSREF'] = [x.strip() for x in settings['AUTHN_CLASSREF'].split(',') if x.strip()]
 
-        for setting in ('METADATA', 'METADATA_PATH', 'METADATA_URL'):
+        for setting in ('METADATA', 'METADATA_URL'):
             if not settings[setting]:
                 del settings[setting]
 
@@ -206,7 +200,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_url):
             raise ValidationError(_('One of the metadata fields must be filled.'))
 
     def autorun(self, request, block_id):

tests/auth_saml/test_login.py

@@ -14,7 +14,6 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import os
 from unittest import mock
 
 import pytest
@@ -57,9 +56,7 @@ def test_providers_on_login_page(db, app, settings):
 
 
 def test_login_with_conditionnal_authenticators(db, app, settings, caplog):
-    authenticator = SAMLAuthenticator.objects.create(
-        enabled=True, metadata_path=os.path.join(os.path.dirname(__file__), 'metadata.xml'), slug='idp1'
-    )
+    authenticator = SAMLAuthenticator.objects.create(enabled=True, metadata='xxx', slug='idp1')
 
     response = app.get('/login/')
     assert 'login-saml-idp1' in response
@@ -69,9 +66,7 @@ def test_login_with_conditionnal_authenticators(db, app, settings, caplog):
     response = app.get('/login/')
     assert 'login-saml-idp1' not in response
 
-    authenticator2 = SAMLAuthenticator.objects.create(
-        enabled=True, metadata_path=os.path.join(os.path.dirname(__file__), 'metadata.xml'), slug='idp2'
-    )
+    authenticator2 = SAMLAuthenticator.objects.create(enabled=True, metadata='xxx', slug='idp2')
     response = app.get('/login/')
     assert 'login-saml-idp1' not in response
     assert 'login-saml-idp2' in response
@@ -86,13 +81,13 @@ def test_login_with_conditionnal_authenticators(db, app, settings, caplog):
 def test_login_condition_dnsbl(db, app, settings, caplog):
     SAMLAuthenticator.objects.create(
         enabled=True,
-        metadata_path=os.path.join(os.path.dirname(__file__), 'metadata.xml'),
+        metadata='xxx',
         slug='idp1',
         show_condition='remote_addr in dnsbl(\'dnswl.example.com\')',
     )
     SAMLAuthenticator.objects.create(
         enabled=True,
-        metadata_path=os.path.join(os.path.dirname(__file__), 'metadata.xml'),
+        metadata='xxx',
         slug='idp2',
         show_condition='remote_addr not in dnsbl(\'dnswl.example.com\')',
     )
@@ -105,9 +100,7 @@ def test_login_condition_dnsbl(db, app, settings, caplog):
 def test_login_autorun(db, app, settings, patched_adapter):
     response = app.get('/login/')
 
-    authenticator = SAMLAuthenticator.objects.create(
-        enabled=True, metadata_path=os.path.join(os.path.dirname(__file__), 'metadata.xml'), slug='idp1'
-    )
+    authenticator = SAMLAuthenticator.objects.create(enabled=True, metadata='xxx', slug='idp1')
     # hide password block
     LoginPasswordAuthenticator.objects.update_or_create(
         slug='password-authenticator', defaults={'enabled': False}

tests/auth_saml/test_migrations.py

@@ -400,3 +400,29 @@ def test_saml_authenticator_data_migration_rename_attributes(migration, settings
     assert set_attribute1.user_field == 'first_name'
     assert set_attribute2.saml_attribute == 'title'
     assert set_attribute2.user_field == 'title'
+
+
+def test_saml_authenticator_data_migration_metadata_file_to_db(migration, settings):
+    migrate_from = [('authentic2_auth_saml', '0012_move_add_role_action')]
+    migrate_to = [('authentic2_auth_saml', '0014_remove_samlauthenticator_metadata_path')]
+
+    old_apps = migration.before(migrate_from)
+    SAMLAuthenticator = old_apps.get_model('authentic2_auth_saml', 'SAMLAuthenticator')
+
+    SAMLAuthenticator.objects.create(
+        slug='idp1', metadata_path=os.path.join(os.path.dirname(__file__), 'metadata.xml')
+    )
+
+    SAMLAuthenticator.objects.create(slug='idp2', metadata='xxx')
+    SAMLAuthenticator.objects.create(slug='idp3', metadata_url='https://example.com')
+    SAMLAuthenticator.objects.create(slug='idp4', metadata_path='/unknown/')
+
+    new_apps = migration.apply(migrate_to)
+    SAMLAuthenticator = new_apps.get_model('authentic2_auth_saml', 'SAMLAuthenticator')
+
+    authenticator = SAMLAuthenticator.objects.get(slug='idp1')
+    assert authenticator.metadata.startswith('<?xml version="1.0"?>')
+    assert authenticator.metadata.endswith('</EntityDescriptor>\n')
+
+    assert SAMLAuthenticator.objects.filter(slug='idp2', metadata='xxx').count() == 1
+    assert SAMLAuthenticator.objects.filter(slug='idp3', metadata_url='https://example.com').count() == 1

tests/auth_saml/test_misc.py

@@ -49,17 +49,15 @@ def test_saml_authenticator_settings(db):
     )
 
     assert 'METADATA' in authenticator.settings
-    assert 'METADATA_PATH' not in authenticator.settings
     assert 'METADATA_URL' not in authenticator.settings
     assert authenticator.settings['AUTHN_CLASSREF'] == ['a', 'b']
 
     authenticator.metadata = ''
-    authenticator.metadata_path = '/some/path/metadata.xml'
+    authenticator.metadata_url = 'https://example.com/metadata.xml'
     authenticator.save()
 
-    assert 'METADATA_PATH' in authenticator.settings
+    assert 'METADATA_URL' in authenticator.settings
     assert 'METADATA' not in authenticator.settings
-    assert 'METADATA_URL' not in authenticator.settings
 
     authenticator.authn_classref = ''
     authenticator.save()

tests/test_manager_authenticators.py

@@ -466,9 +466,9 @@ 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_url'] = 'https://example.com/metadata.xml'
     resp = resp.form.submit().follow()
-    assert 'Metadata file path: /var/lib/authentic2/metadata.xml' in resp.text
+    assert 'Metadata URL: https://example.com/metadata.xml' in resp.text
 
     resp = resp.click('Enable').follow()
     assert 'Authenticator has been enabled.' in resp.text
@@ -494,7 +494,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()