From 02d1523107b6fdf0cb20d86a4cb823e772f7af30 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne 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 | 80 +++++++++++++++++++++------- src/authentic2/manager/views.py | 12 ++--- src/authentic2/utils/lazy.py | 26 +++++++++ tests/test_data_transfer.py | 33 +++++++++--- tests/test_import_export_site_cmd.py | 9 ++-- 5 files changed, 124 insertions(+), 36 deletions(-) create mode 100644 src/authentic2/utils/lazy.py diff --git a/src/authentic2/data_transfer.py b/src/authentic2/data_transfer.py index ee5789ff..00509f0e 100644 --- a/src/authentic2/data_transfer.py +++ b/src/authentic2/data_transfer.py @@ -14,17 +14,50 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +from functools import wraps + +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.utils.text import format_lazy 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 +from authentic2.utils.lazy import lazy_join 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) + + def error_list(messages): + for message in messages: + if isinstance(message, ValidationError): + yield message.message + else: + yield message + for message in error_list(messages): + errorlist.append(format_lazy(u'{}="{}": {}', obj.__class__._meta.get_field(key).verbose_name, value, message)) + raise ValidationError(errorlist) obj.save() @@ -128,10 +161,6 @@ class ImportContext(object): self.role_attributes_update = role_attributes_update -class DataImportError(Exception): - pass - - class RoleDeserializer(object): def __init__(self, d, import_context): self._import_context = import_context @@ -151,13 +180,24 @@ class RoleDeserializer(object): else: self._role_d[key] = value + def wraps_validationerror(func): + @wraps(func) + def f(self, *args, **kwargs): + try: + return func(self, *args, **kwargs) + except ValidationError as e: + raise ValidationError(_('Role "%s": %s') % ( + self._role_d.get('name', self._role_d.get('slug')), + lazy_join(', ', [v.message for v in e.error_list]))) + return f + + @wraps_validationerror def deserialize(self): ou_d = self._role_d['ou'] has_ou = bool(ou_d) ou = None if not has_ou else search_ou(ou_d) if has_ou and not ou: - raise DataImportError( - "Can't import role because missing Organizational Unit: %s" % ou_d) + raise ValidationError(_("Can't import role because missing Organizational Unit: %s") % ou_d) kwargs = self._role_d.copy() kwargs.pop('ou', None) @@ -172,8 +212,7 @@ class RoleDeserializer(object): update_model(self._obj, kwargs) else: # Create role if 'uuid' in kwargs and not kwargs['uuid']: - raise DataImportError("Cannot import role '%s' with empty uuid" - % kwargs.get('name')) + raise ValidationError(_("Cannot import role '%s' with empty uuid") % kwargs.get('name')) self._obj = get_role_model().objects.create(**kwargs) status = 'created' @@ -184,6 +223,7 @@ class RoleDeserializer(object): self._obj.get_admin_role() return self._obj, status + @wraps_validationerror def attributes(self): """ Update attributes (delete everything then create) """ @@ -199,6 +239,7 @@ class RoleDeserializer(object): return created, deleted + @wraps_validationerror def parentings(self): """ Update parentings (delete everything then create) """ @@ -212,12 +253,13 @@ class RoleDeserializer(object): for parent_d in self._parents: parent = search_role(parent_d) if not parent: - raise DataImportError("Could not find role: %s" % parent_d) + raise ValidationError(_("Could not find parent role: %s") % parent_d) created.append(Parenting.objects.create( child=self._obj, direct=True, parent=parent)) return created, deleted + @wraps_validationerror def permissions(self): """ Update permissions (delete everything then create) """ @@ -300,15 +342,19 @@ def import_site(json_d, import_context=None): result = ImportResult() if not isinstance(json_d, dict): - raise DataImportError('Export file is invalid: not a dictionnary') + raise ValidationError(_('Import file is invalid: not a dictionnary')) if import_context.import_ous: for ou_d in json_d.get('ous', []): result.update_ous(*import_ou(ou_d)) if import_context.import_roles: - roles_ds = [RoleDeserializer(role_d, import_context) for role_d in json_d.get('roles', []) - if not role_d['slug'].startswith('_')] + roles_ds = [] + for role_d in json_d.get('roles', []): + # ignore internal roles + if role_d['slug'].startswith('_'): + continue + roles_ds.append(RoleDeserializer(role_d, import_context)) for ds in roles_ds: result.update_roles(*ds.deserialize()) @@ -326,14 +372,12 @@ def import_site(json_d, import_context=None): result.update_permissions(*ds.permissions()) if import_context.ou_delete_orphans: - raise DataImportError( - "Unsupported context value for ou_delete_orphans : %s" % ( - import_context.ou_delete_orphans)) + raise ValidationError(_("Unsupported context value for ou_delete_orphans : %s") % ( + import_context.ou_delete_orphans)) if import_context.role_delete_orphans: # FIXME : delete each role that is in DB but not in the export - raise DataImportError( - "Unsupported context value for role_delete_orphans : %s" % ( - import_context.role_delete_orphans)) + raise ValidationError(_("Unsupported context value for role_delete_orphans : %s") % ( + import_context.role_delete_orphans)) return result diff --git a/src/authentic2/manager/views.py b/src/authentic2/manager/views.py index af447e51..cea0c6a5 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, @@ -40,7 +40,7 @@ from gadjo.templatetags.gadjo import xstatic from django_rbac.utils import get_ou_model -from authentic2.data_transfer import export_site, import_site, DataImportError, ImportContext +from authentic2.data_transfer import export_site, import_site, ImportContext from authentic2.forms.profile import modelform_factory from authentic2.utils import redirect, batch_queryset from authentic2.decorators import json as json_view @@ -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) @@ -717,8 +717,8 @@ class SiteImportView(MediaMixin, FormView): try: with transaction.atomic(): import_site(json_site, ImportContext()) - except DataImportError as e: - form.add_error('site_json', six.text_type(e)) + except ValidationError as e: + form.add_error('site_json', e) return self.form_invalid(form) return super(SiteImportView, self).form_valid(form) diff --git a/src/authentic2/utils/lazy.py b/src/authentic2/utils/lazy.py new file mode 100644 index 00000000..0e5fea9d --- /dev/null +++ b/src/authentic2/utils/lazy.py @@ -0,0 +1,26 @@ +# authentic2 - versatile identity manager +# Copyright (C) 2010-2020 Entr'ouvert +# +# This program is free software: you can redistribute it and/or modify it +# under the terms of the GNU Affero General Public License as published +# by the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +from django.utils.text import format_lazy + + +def lazy_join(join, args): + if not args: + return '' + + fstring = '{}' + ''.join([join + '{}'] * (len(args) - 1)) + return format_lazy(fstring, *args) + diff --git a/tests/test_data_transfer.py b/tests/test_data_transfer.py index afcae42a..07a0816c 100644 --- a/tests/test_data_transfer.py +++ b/tests/test_data_transfer.py @@ -14,14 +14,16 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -from django_rbac.utils import get_role_model, get_ou_model import pytest +from django.core.exceptions import ValidationError + +from django_rbac.utils import get_role_model, get_ou_model + from authentic2.a2_rbac.models import RoleParenting from authentic2.data_transfer import ( export_site, ExportContext, - DataImportError, export_roles, import_site, export_ous, @@ -159,7 +161,7 @@ def test_role_deserializer_missing_ou(db): 'uuid': get_hex_uuid(), 'name': 'some role', 'description': 'role description', 'slug': 'some-role', 'ou': {'slug': 'some-ou'}, 'service': None}, ImportContext()) - with pytest.raises(DataImportError): + with pytest.raises(ValidationError): rd.deserialize() @@ -255,10 +257,10 @@ def test_role_deserializer_parenting_non_existing_parent(db): 'uuid': get_hex_uuid(), 'ou': None, 'service': None} rd = RoleDeserializer(child_role_dict, ImportContext()) rd.deserialize() - with pytest.raises(DataImportError) as excinfo: + with pytest.raises(ValidationError) as excinfo: rd.parentings() - assert "Could not find role" in str(excinfo.value) + assert "Could not find parent role" in str(excinfo.value) def test_role_deserializer_permissions(db): @@ -473,7 +475,7 @@ def test_import_role_handle_manager_role_parenting(db): def test_import_roles_role_delete_orphans(db): roles = [{ 'name': 'some role', 'description': 'some role description', 'slug': '_some-role'}] - with pytest.raises(DataImportError): + with pytest.raises(ValidationError): import_site({'roles': roles}, ImportContext(role_delete_orphans=True)) @@ -512,7 +514,7 @@ def test_import_context_flags(db): import_site(d, ImportContext(import_roles=False, import_ous=False)) assert Role.objects.exclude(slug__startswith='_').count() == 0 assert OU.objects.exclude(slug='default').count() == 0 - with pytest.raises(DataImportError) as e: + with pytest.raises(ValidationError) as e: import_site(d, ImportContext(import_roles=True, import_ous=False)) assert 'missing Organizational' in e.value.args[0] assert Role.objects.exclude(slug__startswith='_').count() == 0 @@ -540,3 +542,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(ValidationError, match=r'Role "role1": name="role1": Name already used'): + import_site(data) diff --git a/tests/test_import_export_site_cmd.py b/tests/test_import_export_site_cmd.py index ebae030b..8c10c07c 100644 --- a/tests/test_import_export_site_cmd.py +++ b/tests/test_import_export_site_cmd.py @@ -17,6 +17,8 @@ import random import json +from django.core.exceptions import ValidationError + from django.utils import six from django.utils.six.moves import builtins as __builtin__ from django.core import management @@ -122,8 +124,6 @@ def test_import_site_transaction_rollback_on_dry_run(db, monkeypatch, capsys, js def test_import_site_cmd_unhandled_context_option(db, monkeypatch, capsys, json_fixture): - from authentic2.data_transfer import DataImportError - content = { 'roles': [ { @@ -138,7 +138,7 @@ def test_import_site_cmd_unhandled_context_option(db, monkeypatch, capsys, json_ Role.objects.create(uuid='dqfewrvesvews2532', slug='role-slug', name='role-name') - with pytest.raises(DataImportError): + with pytest.raises(ValidationError): management.call_command( 'import_site', '-o', 'role-delete-orphans', json_fixture(content)) @@ -205,8 +205,7 @@ def test_import_site_update_roles(db, json_fixture): def test_import_site_empty_uuids(db, monkeypatch, json_fixture): - from authentic2.data_transfer import DataImportError - with pytest.raises(DataImportError): + with pytest.raises(ValidationError): management.call_command('import_site', json_fixture({ 'roles': [ { -- 2.24.0