From e512d99e7ff105a62487bd2e1b96fc837272c402 Mon Sep 17 00:00:00 2001
From: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Thu, 9 Apr 2020 16:15:20 +0200
Subject: [PATCH 1/2] data_transfer: validate uniqueness of roles (#41342)

---
 src/authentic2/data_transfer.py | 24 ++++++++++++++++++++++++
 src/authentic2/manager/views.py | 12 ++++++++----
 tests/test_data_transfer.py     | 17 +++++++++++++++++
 3 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/src/authentic2/data_transfer.py b/src/authentic2/data_transfer.py
index ee5789ff..e0ae0c76 100644
--- a/src/authentic2/data_transfer.py
+++ b/src/authentic2/data_transfer.py
@@ -14,17 +14,41 @@
 # 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/>.
 
+from django.core.exceptions import ValidationError, NON_FIELD_ERRORS
 from django.contrib.contenttypes.models import ContentType
+from django.utils.translation import ugettext_lazy as _
 
 from django_rbac.models import Operation
 from django_rbac.utils import (
     get_ou_model, get_role_model, get_role_parenting_model, get_permission_model)
+
+from authentic2.decorators import errorcollector
 from authentic2.a2_rbac.models import RoleAttribute
 
 
 def update_model(obj, d):
     for attr, value in d.items():
         setattr(obj, attr, value)
+    errors = {}
+    with errorcollector(errors):
+        if hasattr(obj, 'validate'):
+            obj.validate()
+
+    with errorcollector(errors):
+        if hasattr(obj, 'validate_unique'):
+            obj.validate_unique()
+    if errors:
+        errorlist = []
+        for key, messages in list(errors.items()):
+            if key == NON_FIELD_ERRORS:
+                errorlist.extend(messages)
+            else:
+                value = getattr(obj, key)
+                for error in messages:
+                    for message in error if isinstance(error, ValidationError) else [error]:
+                        errorlist.append(_(u'%s %s="%s": %s') % (obj.__class__._meta.verbose_name, key, value, message))
+                del errors[key]
+        raise DataImportError(ValidationError(errorlist))
     obj.save()
 
 
diff --git a/src/authentic2/manager/views.py b/src/authentic2/manager/views.py
index af447e51..f468011c 100644
--- a/src/authentic2/manager/views.py
+++ b/src/authentic2/manager/views.py
@@ -17,7 +17,7 @@
 import json
 import inspect
 
-from django.core.exceptions import PermissionDenied
+from django.core.exceptions import PermissionDenied, ValidationError
 from django.db import transaction
 from django.views.generic.base import ContextMixin
 from django.views.generic import (FormView, UpdateView, CreateView, DeleteView, TemplateView,
@@ -708,8 +708,8 @@ class SiteImportView(MediaMixin, FormView):
 
     def form_valid(self, form):
         try:
-            json_site = json.loads(force_text(
-                    self.request.FILES['site_json'].read()))
+            json_site = json.loads(
+                force_text(self.request.FILES['site_json'].read()))
         except ValueError:
             form.add_error('site_json', _('File is not in the expected JSON format.'))
             return self.form_invalid(form)
@@ -718,7 +718,11 @@ class SiteImportView(MediaMixin, FormView):
             with transaction.atomic():
                 import_site(json_site, ImportContext())
         except DataImportError as e:
-            form.add_error('site_json', six.text_type(e))
+            if e.args and isinstance(e.args[0], ValidationError):
+                e = e.args[0]
+            else:
+                e = six.text_type(e)
+            form.add_error('site_json', e)
             return self.form_invalid(form)
 
         return super(SiteImportView, self).form_valid(form)
diff --git a/tests/test_data_transfer.py b/tests/test_data_transfer.py
index afcae42a..30e1138b 100644
--- a/tests/test_data_transfer.py
+++ b/tests/test_data_transfer.py
@@ -540,3 +540,20 @@ def test_export_site(db):
     d = export_site(ExportContext(export_ous=False))
     assert 'ous' not in d
 
+
+def test_role_validate_unique(db):
+    ou = OU.objects.create(name='ou', slug='ou')
+    Role.objects.create(name='role1', slug='role1', ou=ou)
+    Role.objects.create(name='role2', slug='role2', ou=ou)
+
+    data = {
+        'roles': [
+            {
+                'name': 'role1',
+                'slug': 'role2',
+                'ou': {'slug': 'ou'},
+            }
+        ]
+    }
+    with pytest.raises(DataImportError, match=r'role name="role1": Name already used'):
+        import_site(data)
-- 
2.24.0