From f4f482cd4074898f5344a3a078e27800bbd060fd Mon Sep 17 00:00:00 2001 From: Étienne Loks Date: Thu, 26 Oct 2023 17:03:41 +0200 Subject: ✨ refactoring import permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ishtar_common/fixtures/initial_data-fr.json | 11 +- ishtar_common/forms_common.py | 33 +- .../migrations/0231_default_mandatory_keys.py | 38 -- ...31_default_mandatory_keys_import_permissions.py | 116 +++++ ishtar_common/models_common.py | 3 +- ishtar_common/models_imports.py | 67 ++- ishtar_common/templates/ishtar/import_list.html | 4 +- ishtar_common/templates/ishtar/import_table.html | 6 +- ishtar_common/tests.py | 466 +++++++++++++++++---- ishtar_common/tests/error-file.csv | 3 + ishtar_common/urls.py | 28 +- ishtar_common/views.py | 147 +++++-- 12 files changed, 730 insertions(+), 192 deletions(-) delete mode 100644 ishtar_common/migrations/0231_default_mandatory_keys.py create mode 100644 ishtar_common/migrations/0231_default_mandatory_keys_import_permissions.py create mode 100644 ishtar_common/tests/error-file.csv (limited to 'ishtar_common') diff --git a/ishtar_common/fixtures/initial_data-fr.json b/ishtar_common/fixtures/initial_data-fr.json index f3c539d3c..980ed3d1a 100644 --- a/ishtar_common/fixtures/initial_data-fr.json +++ b/ishtar_common/fixtures/initial_data-fr.json @@ -301,7 +301,16 @@ "Auteurs : lecture" ], [ - "Import : ajout/modification/suppression" + "Imports : lecture" + ], + [ + "Imports : modification" + ], + [ + "Imports : suppression" + ], + [ + "Imports : ajout" ] ] } diff --git a/ishtar_common/forms_common.py b/ishtar_common/forms_common.py index dd3a83971..0d739fbfb 100644 --- a/ishtar_common/forms_common.py +++ b/ishtar_common/forms_common.py @@ -246,19 +246,31 @@ class BaseImportForm(IshtarForm, forms.ModelForm): super().__init__(*args, **kwargs) self.fields["imported_file"].required = True self._filter_group(user) - self._filter_importer_type() + self._filter_importer_type(user) if "imported_images" in self.fields: self.fields["imported_images"].validators = [file_size_validator] self.fields["imported_file"].validators = [file_size_validator] self._post_init() - def _filter_importer_type(self): - self.fields["importer_type"].choices = [("", "--")] + [ - (imp.pk, imp.name) - for imp in models.ImporterType.objects.filter( + def _filter_importer_type_query(self, q, user): + if user.is_superuser or user.ishtaruser.has_right("add_import"): + return q + if not user.ishtaruser.has_right("add_own_import"): + self.fields["importer_type"].choices = [("", "--")] + return + q = q.filter(users__pk=user.ishtaruser.pk) + return q + + def _filter_importer_type(self, user): + q = models.ImporterType.objects.filter( available=True, is_import=True, type=self.importer_type - ) + ) + q = self._filter_importer_type_query(q, user) + if not q: + return + self.fields["importer_type"].choices = [("", "--")] + [ + (imp.pk, imp.name) for imp in q.all() ] def _filter_group(self, user): @@ -511,10 +523,13 @@ class NewImportGroupForm(NewImportForm): "imported_images": FormHeader(_("Documents/Images")), } - def _filter_importer_type(self): + def _filter_importer_type(self, user): + q = models.ImporterGroup.objects.filter(available=True) + q = self._filter_importer_type_query(q, user) + if not q: + return self.fields["importer_type"].choices = [("", "--")] + [ - (imp.pk, imp.name) - for imp in models.ImporterGroup.objects.filter(available=True) + (imp.pk, imp.name) for imp in q.all() ] def _filter_group(self, user): diff --git a/ishtar_common/migrations/0231_default_mandatory_keys.py b/ishtar_common/migrations/0231_default_mandatory_keys.py deleted file mode 100644 index cd245322a..000000000 --- a/ishtar_common/migrations/0231_default_mandatory_keys.py +++ /dev/null @@ -1,38 +0,0 @@ -# Generated by Django 2.2.24 on 2023-09-18 17:05 - -from django.db import migrations - -EXCLUDE_LIST = [ - "-", - "auto_external_id", - "spatial_reference_system", - "public_domain", -] - -FULL_COPY_LIST = [ - "scientist__attached_to", -] - - -def migrate(apps, __): - ImporterDefault = apps.get_model('ishtar_common', 'ImporterDefault') - for default in ImporterDefault.objects.all(): - if default.target not in EXCLUDE_LIST: - req = default.target - if req not in FULL_COPY_LIST: - req = req.split("__")[0] - if req.endswith("_type") or req.endswith("_types"): - continue - default.required_fields = req - default.save() - - -class Migration(migrations.Migration): - - dependencies = [ - ('ishtar_common', '0230_auto_20231024_1045'), - ] - - operations = [ - migrations.RunPython(migrate), - ] diff --git a/ishtar_common/migrations/0231_default_mandatory_keys_import_permissions.py b/ishtar_common/migrations/0231_default_mandatory_keys_import_permissions.py new file mode 100644 index 000000000..120711f09 --- /dev/null +++ b/ishtar_common/migrations/0231_default_mandatory_keys_import_permissions.py @@ -0,0 +1,116 @@ +# Generated by Django 2.2.24 on 2023-09-18 17:05 + +from django.db import migrations + +COLOR_WARNING = "\033[93m" +COLOR_ENDC = "\033[0m" + +EXCLUDE_LIST = [ + "-", + "auto_external_id", + "spatial_reference_system", + "public_domain", +] + +FULL_COPY_LIST = [ + "scientist__attached_to", +] + +GROUPS = [ + [ + "Imports : lecture", + "view_import", + "ishtar_common", + "import" + ], + [ + "Imports rattachés : lecture", + "view_own_import", + "ishtar_common", + "import" + ], + [ + "Imports : modification", + "change_import", + "ishtar_common", + "import" + ], + [ + "Imports rattachés : modification", + "change_own_import", + "ishtar_common", + "import" + ], + [ + "Imports : suppression", + "delete_import", + "ishtar_common", + "import" + ], + [ + "Imports rattachés : suppression", + "delete_own_import", + "ishtar_common", + "import" + ], + [ + "Imports : ajout", + "add_import", + "ishtar_common", + "import" + ], + [ + "Imports rattachés : ajout", + "add_own_import", + "ishtar_common", + "import" + ], +] + + +def migrate(apps, __): + ImporterDefault = apps.get_model('ishtar_common', 'ImporterDefault') + for default in ImporterDefault.objects.all(): + if default.target not in EXCLUDE_LIST: + req = default.target + if req not in FULL_COPY_LIST: + req = req.split("__")[0] + if req.endswith("_type") or req.endswith("_types"): + continue + default.required_fields = req + default.save() + + print("") + ProfileType = apps.get_model('ishtar_common', 'ProfileType') + q = ProfileType.objects.filter(txt_idx="administrator") + administrator = None + if not q.count(): + print(COLOR_WARNING + "** No administrator profile found. **" + COLOR_ENDC) + else: + administrator = q.all()[0] + + Permission = apps.get_model("auth", "Permission") + Group = apps.get_model("auth", "Group") + ContentType = apps.get_model("contenttypes", "ContentType") + for name, codename, app, model in GROUPS: + ct, __ = ContentType.objects.get_or_create(app_label=app, model=model) + perm, __ = Permission.objects.get_or_create( + codename=codename, defaults={"name": name, "content_type": ct} + ) + group, __ = Group.objects.get_or_create(name=name) + group.permissions.add(perm) + if administrator: + administrator.groups.add(group) + + print(COLOR_WARNING + "** Verify import permissions in profiles **" + COLOR_ENDC) + + +class Migration(migrations.Migration): + + dependencies = [ + ('ishtar_common', '0230_auto_20231024_1045'), + ] + + operations = [ + migrations.RunPython(migrate), + ] diff --git a/ishtar_common/models_common.py b/ishtar_common/models_common.py index 43677bca4..dcb9d030d 100644 --- a/ishtar_common/models_common.py +++ b/ishtar_common/models_common.py @@ -1111,9 +1111,10 @@ class Imported(models.Model): if not user.ishtaruser: return [] q = getattr(self, key) + lst = [] if user.is_superuser or user.ishtaruser.has_right("view_import"): lst = list(q.all()) - else: + elif user.ishtaruser.has_right("view_own_import"): lst = q.filter(Q(user=user.ishtaruser) | Q(importer_type__users__pk=user.ishtaruser.pk)) new_lst = [] for imprt in lst: diff --git a/ishtar_common/models_imports.py b/ishtar_common/models_imports.py index 77dad558e..2967c18aa 100644 --- a/ishtar_common/models_imports.py +++ b/ishtar_common/models_imports.py @@ -225,6 +225,10 @@ class ImporterType(models.Model): def __str__(self): return self.name + @classmethod + def is_own(cls, ishtar_user): + return bool(cls.objects.filter(users__pk=ishtar_user.pk).count()) + @property def type_label(self): if self.type in IMPORT_TYPES_DICT: @@ -445,6 +449,10 @@ class ImporterGroup(models.Model): def __str__(self): return self.name + @classmethod + def is_own(cls, ishtar_user): + return bool(cls.objects.filter(users__pk=ishtar_user.pk).count()) + @property def importer_types_label(self) -> str: return " ; ".join([imp.importer_type.name for imp in self.importer_types.all()]) @@ -1413,18 +1421,35 @@ class BaseImport(models.Model, OwnPerms, SheetItem): abstract = True @classmethod - def query_can_access(cls, user): + def get_permissions_for_actions(cls, user, session): + if not hasattr(user, "ishtaruser") or not user.ishtaruser: + return False, False, False, False + can_edit_all, can_delete_all, can_edit_own, can_delete_own = False, False, False, False + if user.is_superuser: + can_edit_all = True + can_delete_all = True + if user.ishtaruser.has_right("change_import", session=session): + can_edit_all = True + elif user.ishtaruser.has_right("change_own_import", session=session): + can_edit_own = True + if user.ishtaruser.has_right("delete_import", session=session): + can_delete_all = True + elif user.ishtaruser.has_right("delete_own_import", session=session): + can_delete_own = True + return can_edit_all, can_delete_all, can_edit_own, can_delete_own + + @classmethod + def query_can_access(cls, user, perm="view_import"): """ Filter the query to check access permissions :param user: User instance :return: import query """ q = cls.objects - if user.is_superuser: + if user.is_superuser or (hasattr(user, "ishtaruser") and user.ishtaruser and + user.ishtaruser.has_right(perm)): return q - IshtarUser = apps.get_model("ishtar_common", "IshtarUser") - ishtar_user = IshtarUser.objects.get(pk=user.pk) - q = q.filter(Q(user=ishtar_user) | Q(importer_type__users__pk=ishtar_user.pk)) + q = q.filter(Q(importer_type__users__pk=user.ishtaruser.pk)) return q @classmethod @@ -1557,25 +1582,28 @@ class ImportGroup(BaseImport): return "" return IMPORT_GROUP_STATE_DCT[str(self.state)] - def get_actions(self): + def get_actions(self, can_edit=False, can_delete=False): """ Get available action relevant with the current status """ actions = [] - if self.state == "C": + if not can_edit and not can_delete: + return actions + if can_edit and self.state == "C": actions.append(("A", _("Analyse"))) - if self.state == "A": + if can_edit and self.state == "A": actions.append(("A", _("Re-analyse"))) if not any(-1 for imp in self.import_list() if not imp.pre_import_form_is_valid): actions.append(("I", _("Launch import"))) - if self.state in ("F", "FE"): + if can_edit and self.state in ("F", "FE"): actions.append(("A", _("Re-analyse"))) actions.append(("I", _("Re-import"))) actions.append(("AC", _("Archive"))) - if self.state == "AC": + if can_edit and self.state == "AC": state = "FE" if any(1 for imp in self.imports.all() if imp.error_file) else "F" actions.append((state, _("Unarchive"))) - actions.append(("D", _("Delete"))) + if can_delete: + actions.append(("D", _("Delete"))) return actions def initialize(self, user=None, session_key=None): @@ -2203,16 +2231,18 @@ class Import(BaseImport): idx_line ) in self.imported_line_numbers.split(",") - def get_actions(self): + def get_actions(self, can_edit=False, can_delete=False): """ Get available action relevant with the current status """ IshtarSiteProfile = apps.get_model("ishtar_common", "IshtarSiteProfile") profile = IshtarSiteProfile.get_current_profile() actions = [] - if self.state == "C": + if not can_edit and not can_delete: + return actions + if can_edit and self.state == "C": actions.append(("A", _("Analyse"))) - if self.state in ("A", "PI"): + if can_edit and self.state in ("A", "PI"): actions.append(("A", _("Re-analyse"))) if self.pre_import_form_is_valid: actions.append(("I", _("Launch import"))) @@ -2222,7 +2252,7 @@ class Import(BaseImport): actions.append(("CH", _("Re-check for changes"))) else: actions.append(("CH", _("Check for changes"))) - if self.state in ("F", "FE"): + if can_edit and self.state in ("F", "FE"): actions.append(("A", _("Re-analyse"))) actions.append(("I", _("Re-import"))) if profile.experimental_feature: @@ -2232,12 +2262,13 @@ class Import(BaseImport): else: actions.append(("CH", _("Check for changes"))) actions.append(("AC", _("Archive"))) - if self.state == "AC": + if can_edit and self.state == "AC": state = "FE" if self.error_file else "F" actions.append((state, _("Unarchive"))) - if self.state in ("C", "A"): + if can_delete and self.state in ("C", "A"): actions.append(("ED", _("Edit"))) - actions.append(("D", _("Delete"))) + if can_delete: + actions.append(("D", _("Delete"))) return actions @property diff --git a/ishtar_common/templates/ishtar/import_list.html b/ishtar_common/templates/ishtar/import_list.html index 35821c5bf..ec5e714de 100644 --- a/ishtar_common/templates/ishtar/import_list.html +++ b/ishtar_common/templates/ishtar/import_list.html @@ -16,7 +16,7 @@ {% endblock %} {% block content %} -
+{% if can_create_import %}
{% if has_import_table %} {% trans 'import (table)' %} {% endif %} @@ -26,7 +26,7 @@ {% if has_import_group %} {% trans 'import (group)' %} {% endif %} -
+
{% endif %}
{% include "ishtar/import_table.html" %}
diff --git a/ishtar_common/templates/ishtar/import_table.html b/ishtar_common/templates/ishtar/import_table.html index 08c949654..56e73c6f0 100644 --- a/ishtar_common/templates/ishtar/import_table.html +++ b/ishtar_common/templates/ishtar/import_table.html @@ -78,14 +78,18 @@ {{import.status}} + {% if import.action_list %} + {% else %} + – + {% endif %}