From c5b4c0205c13f01278b68c0d67d2be340f8531d2 Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Mon, 22 Aug 2022 10:14:38 +0200
Subject: [PATCH] auth_saml: catch any exception in data migration (#68273)

---
 .../migrations/0006_migrate_jsonfields.py     | 24 +++++++++++-
 tests/test_auth_saml.py                       | 37 +++++++++++++++++++
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/src/authentic2_auth_saml/migrations/0006_migrate_jsonfields.py b/src/authentic2_auth_saml/migrations/0006_migrate_jsonfields.py
index c01ea992..a001ced6 100644
--- a/src/authentic2_auth_saml/migrations/0006_migrate_jsonfields.py
+++ b/src/authentic2_auth_saml/migrations/0006_migrate_jsonfields.py
@@ -1,7 +1,12 @@
 # Generated by Django 2.2.26 on 2022-07-27 15:04
 
+import json
+import logging
+
 from django.core.exceptions import MultipleObjectsReturned
-from django.db import migrations
+from django.db import migrations, transaction
+
+logger = logging.getLogger('authentic2.auth_saml')
 
 
 def get_key(obj, name, max_length=None, default=''):
@@ -81,7 +86,7 @@ def migrate_jsonfields(apps, schema_editor):
     Role = apps.get_model('a2_rbac', 'Role')
     OU = apps.get_model('a2_rbac', 'OrganizationalUnit')
 
-    for authenticator in SAMLAuthenticator.objects.all():
+    def create_related_objects(authenticator):
         for obj in authenticator.lookup_by_attributes:
             saml_attribute = get_key(obj, 'saml_attribute', 1024)
             user_field = get_key(obj, 'user_field', 256)
@@ -124,6 +129,21 @@ def migrate_jsonfields(apps, schema_editor):
                         mandatory=get_key(obj, 'mandatory', default=False),
                     )
 
+    for authenticator in SAMLAuthenticator.objects.all():
+        try:
+            with transaction.atomic():
+                create_related_objects(authenticator)
+        except Exception:
+            logger.exception('could not create related objects for authenticator %s', authenticator)
+            logger.warning(
+                'attribute mapping for %s: %s', authenticator, json.dumps(authenticator.a2_attribute_mapping)
+            )
+            logger.warning(
+                'lookup by attributes for %s: %s',
+                authenticator,
+                json.dumps(authenticator.lookup_by_attributes),
+            )
+
 
 class Migration(migrations.Migration):
 
diff --git a/tests/test_auth_saml.py b/tests/test_auth_saml.py
index d91bc6f4..56f39404 100644
--- a/tests/test_auth_saml.py
+++ b/tests/test_auth_saml.py
@@ -642,3 +642,40 @@ def test_saml_authenticator_data_migration_json_fields(migration, settings):
     assert add_role.role.pk == role.pk
     assert add_role.condition == "roles == 'A'"
     assert add_role.mandatory is False
+
+
+def test_saml_authenticator_data_migration_json_fields_log_errors(migration, settings, caplog):
+    migrate_from = [
+        (
+            'authentic2_auth_saml',
+            '0005_addroleaction_renameattributeaction_samlattributelookup_setattributeaction',
+        ),
+        ('a2_rbac', '0029_use_unique_constraints'),
+    ]
+    migrate_to = [
+        ('authentic2_auth_saml', '0006_migrate_jsonfields'),
+        ('a2_rbac', '0029_use_unique_constraints'),
+    ]
+
+    old_apps = migration.before(migrate_from)
+    SAMLAuthenticator = old_apps.get_model('authentic2_auth_saml', 'SAMLAuthenticator')
+
+    SAMLAuthenticator.objects.create(
+        metadata='meta1.xml',
+        slug='idp1',
+        lookup_by_attributes=[{'saml_attribute': 'email', 'user_field': 'email'}],
+        a2_attribute_mapping=['bad'],
+    )
+
+    new_apps = migration.apply(migrate_to)
+    SAMLAuthenticator = new_apps.get_model('authentic2_auth_saml', 'SAMLAuthenticator')
+
+    authenticator = SAMLAuthenticator.objects.get()
+    assert not authenticator.attribute_lookups.exists()
+
+    assert caplog.messages == [
+        'could not create related objects for authenticator SAMLAuthenticator object (%s)' % authenticator.pk,
+        'attribute mapping for SAMLAuthenticator object (%s): ["bad"]' % authenticator.pk,
+        'lookup by attributes for SAMLAuthenticator object (%s): [{"user_field": "email", "saml_attribute": "email"}]'
+        % authenticator.pk,
+    ]
-- 
2.30.2