From 884a2cc97e6f46e5edaf16a6eb99fc8bb42ec856 Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Thu, 28 Jul 2022 15:18:19 +0200
Subject: [PATCH 2/8] auth_saml: rename attributes using model (#67025)

---
 src/authentic2_auth_saml/adapters.py | 19 ++++++-------------
 src/authentic2_auth_saml/models.py   |  1 +
 tests/test_auth_saml.py              | 14 +++++++++-----
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/src/authentic2_auth_saml/adapters.py b/src/authentic2_auth_saml/adapters.py
index e5c6b130..cff0c40d 100644
--- a/src/authentic2_auth_saml/adapters.py
+++ b/src/authentic2_auth_saml/adapters.py
@@ -127,13 +127,9 @@ class AuthenticAdapter(DefaultAdapter):
          If an attribute is not mandatory any error is just logged, if the attribute is
          mandatory, login will fail.
         """
-
         saml_attributes = saml_attributes.copy()
         attribute_mapping = get_setting(idp, 'A2_ATTRIBUTE_MAPPING', [])
 
-        if not attribute_mapping:
-            return
-
         if not isinstance(attribute_mapping, list):
             raise MappingError(_('invalid A2_ATTRIBUTE_MAPPING'))
 
@@ -141,6 +137,8 @@ class AuthenticAdapter(DefaultAdapter):
             user.save()
 
     def apply_attribute_mapping(self, user, idp, saml_attributes, attribute_mapping):
+        self.rename_attributes(idp, saml_attributes)
+
         user_modified = False
         for mapping in attribute_mapping:
             if not isinstance(mapping, dict):
@@ -166,15 +164,10 @@ class AuthenticAdapter(DefaultAdapter):
                 logger.warning('auth_saml: mapping action failed: %s', e)
         return user_modified
 
-    def action_rename(self, user, idp, saml_attributes, mapping):
-        from_name = mapping.get('from')
-        if not from_name or not isinstance(from_name, str):
-            raise MappingError(_('missing from in rename'))
-        to_name = mapping.get('to')
-        if not to_name or not isinstance(to_name, str):
-            raise MappingError(_('missing to in rename'))
-        if from_name in saml_attributes:
-            saml_attributes[to_name] = saml_attributes[from_name]
+    def rename_attributes(self, idp, saml_attributes):
+        for action in idp['authenticator'].rename_attribute_actions.all():
+            if action.from_name in saml_attributes:
+                saml_attributes[action.to_name] = saml_attributes[action.from_name]
 
     def action_set_attribute(self, user, idp, saml_attributes, mapping):
         attribute = mapping.get('attribute')
diff --git a/src/authentic2_auth_saml/models.py b/src/authentic2_auth_saml/models.py
index 404b2bb3..1d9f4e01 100644
--- a/src/authentic2_auth_saml/models.py
+++ b/src/authentic2_auth_saml/models.py
@@ -191,6 +191,7 @@ class SAMLAuthenticator(BaseAuthenticator):
             if not settings[setting]:
                 del settings[setting]
 
+        settings['authenticator'] = self
         return settings
 
     @property
diff --git a/tests/test_auth_saml.py b/tests/test_auth_saml.py
index 99ae057b..e6925b4d 100644
--- a/tests/test_auth_saml.py
+++ b/tests/test_auth_saml.py
@@ -28,7 +28,7 @@ from authentic2.apps.authenticators.models import LoginPasswordAuthenticator
 from authentic2.custom_user.models import DeletedUser
 from authentic2.models import Attribute
 from authentic2_auth_saml.adapters import AuthenticAdapter, MappingError
-from authentic2_auth_saml.models import SAMLAuthenticator
+from authentic2_auth_saml.models import RenameAttributeAction, SAMLAuthenticator
 
 from .utils import login
 
@@ -84,7 +84,6 @@ def idp(db):
                 'saml_attribute': 'mail',
                 'mandatory': True,
             },
-            {'action': 'rename', 'from': 'http://fucking/attribute/givenName', 'to': 'first_name'},
             {
                 'attribute': 'title',
                 'saml_attribute': 'title',
@@ -95,6 +94,11 @@ def idp(db):
             },
         ],
     )
+    RenameAttributeAction.objects.create(
+        authenticator=authenticator,
+        from_name='http://nice/attribute/givenName',
+        to_name='first_name',
+    )
     return authenticator.settings
 
 
@@ -111,7 +115,7 @@ def saml_attributes():
         'name_id_format': lasso.SAML2_NAME_IDENTIFIER_FORMAT_PERSISTENT,
         'mail': ['john.doe@example.com'],
         'title': ['Mr.'],
-        'http://fucking/attribute/givenName': ['John'],
+        'http://nice/attribute/givenName': ['John'],
     }
 
 
@@ -144,7 +148,7 @@ def test_apply_attribute_mapping_missing_attribute_logged(
     caplog, adapter, idp, saml_attributes, title_attribute, user
 ):
     caplog.set_level('WARNING')
-    saml_attributes['http://fucking/attribute/givenName'] = []
+    saml_attributes['http://nice/attribute/givenName'] = []
     adapter.apply_attribute_mapping(user, idp, saml_attributes, idp['A2_ATTRIBUTE_MAPPING'])
     assert re.match('.*no value.*first_name', caplog.records[-1].message)
 
@@ -152,7 +156,7 @@ def test_apply_attribute_mapping_missing_attribute_logged(
 def test_apply_attribute_mapping_missing_attribute_exception(
     adapter, idp, saml_attributes, title_attribute, user, rf
 ):
-    saml_attributes['http://fucking/attribute/givenName'] = []
+    saml_attributes['http://nice/attribute/givenName'] = []
     idp['A2_ATTRIBUTE_MAPPING'][-1]['mandatory'] = True
     with pytest.raises(MappingError, match='no value'):
         adapter.apply_attribute_mapping(user, idp, saml_attributes, idp['A2_ATTRIBUTE_MAPPING'])
-- 
2.30.2