✨ permissions refactoring: generate permissions, adapt permissions checks

4 files changed, 83 insertions, 16 deletions

diff --git a/archaeological_operations/models.py b/archaeological_operations/models.py
index a074adc9a..2fafa56ed 100644
--- a/archaeological_operations/models.py
+++ b/archaeological_operations/models.py
@@ -902,7 +902,8 @@ class ArchaeologicalSite(
 
     @classmethod
     def get_owns(
-        cls, user, menu_filtr=None, limit=None, values=None, get_short_menu_class=None
+        cls, user, menu_filtr=None, limit=None, values=None, get_short_menu_class=None,
+        no_auth_check=False, query=False
     ):
         replace_query = None
         if menu_filtr and "operation" in menu_filtr:
@@ -914,7 +915,11 @@ class ArchaeologicalSite(
             limit=limit,
             values=values,
             get_short_menu_class=get_short_menu_class,
+            no_auth_check=no_auth_check,
+            query=query
         )
+        if query:
+            return owns
         return cls._return_get_owns(owns, values, get_short_menu_class)
 
     def _generate_cached_label(self):
@@ -1782,7 +1787,8 @@ class Operation(
 
     @classmethod
     def get_owns(
-        cls, user, menu_filtr=None, limit=None, values=None, get_short_menu_class=None
+        cls, user, menu_filtr=None, limit=None, values=None, get_short_menu_class=None,
+        no_auth_check=False, query=False
     ):
         replace_query = None
         if menu_filtr and "file" in menu_filtr:
@@ -1794,7 +1800,11 @@ class Operation(
             limit=limit,
             values=values,
             get_short_menu_class=get_short_menu_class,
+            no_auth_check=no_auth_check,
+            query=query
         )
+        if query:
+            return owns
         return cls._return_get_owns(owns, values, get_short_menu_class)
 
     def __str__(self):
diff --git a/archaeological_operations/tests.py b/archaeological_operations/tests.py
index b0b9c9dae..2c5946e87 100644
--- a/archaeological_operations/tests.py
+++ b/archaeological_operations/tests.py
@@ -3480,18 +3480,26 @@ class OperationPermissionTest(TestCase, OperationInitTest):
     def setUp(self):
         IshtarSiteProfile.objects.get_or_create(slug="default", active=True)
         self.username, self.password, self.user = create_superuser()
+
         self.alt_username, self.alt_password, self.alt_user = create_user()
-        self.alt_user.user_permissions.add(
-            Permission.objects.get(codename="view_own_operation")
+        profile_type = ProfileType.objects.create(
+            label="xxCollaborateur",
+            txt_idx="xxcollaborator",
         )
-        self.alt_user.user_permissions.add(
-            Permission.objects.get(codename="change_own_operation")
+        UserProfile.objects.create(
+            profile_type=profile_type,
+            person=self.alt_user.ishtaruser.person,
+            current=True,
         )
+        gp = Group.objects.create(name="xxOpérations rattachées : voir et modification")
+        gp.permissions.add(Permission.objects.get(codename="view_own_operation"))
+        gp.permissions.add(Permission.objects.get(codename="change_own_operation"))
+        profile_type.groups.add(gp)
+
         # nosec: hard coded password for test purposes
         self.alt_username2, self.alt_password2, self.alt_user2 = create_user(  # nosec
             username="luke", password="iamyourfather"
         )
-        profile_type = ProfileType.objects.get(txt_idx="collaborator")
         profile = UserProfile.objects.create(
             profile_type=profile_type,
             person=self.alt_user2.ishtaruser.person,
@@ -3503,11 +3511,51 @@ class OperationPermissionTest(TestCase, OperationInitTest):
         profile.areas.add(area)
 
         self.orgas = self.create_orgas(self.user)
-        self.operations = self.create_operation(self.user, self.orgas[0])
-        self.operations += self.create_operation(self.alt_user, self.orgas[0])
+        self.create_operation(self.user, self.orgas[0])
+        self.operations = self.create_operation(self.alt_user, self.orgas[0])
         self.operations[1].towns.add(town)
         self.item = self.operations[0]
 
+    def test_permission_generation(self):
+        alt_user = IshtarUser.objects.get(pk=self.alt_user.pk)
+        self.assertFalse(
+            alt_user.user_ptr.has_perm(
+                "archaeological_operations.change_own_operation",
+                self.operations[1]
+            )
+        )
+        self.assertFalse(
+            alt_user.user_ptr.has_perm(
+                "archaeological_operations.view_own_operation",
+                self.operations[1]
+            )
+        )
+        alt_user.generate_permission()
+        self.assertTrue(
+            alt_user.user_ptr.has_perm(
+                "archaeological_operations.view_own_operation",
+                self.operations[1]
+            )
+        )
+        self.assertTrue(
+            alt_user.user_ptr.has_perm(
+                "archaeological_operations.change_own_operation",
+                self.operations[1]
+            )
+        )
+        # general permission is assigned
+        self.assertTrue(
+            alt_user.user_ptr.has_perm(
+                "archaeological_operations.change_own_operation",
+            )
+        )
+        self.assertFalse(
+            alt_user.user_ptr.has_perm(
+                "archaeological_operations.change_own_operation",
+                self.operations[0]
+            )
+        )
+
     def test_own_search(self):
         # no result when no authentification
         c = Client()
@@ -3515,6 +3563,8 @@ class OperationPermissionTest(TestCase, OperationInitTest):
         self.assertTrue(not json.loads(response.content.decode()))
 
         # possession
+        alt_user = IshtarUser.objects.get(pk=self.alt_user.pk)
+        alt_user.generate_permission()
         c = Client()
         c.login(username=self.alt_username, password=self.alt_password)
         response = c.get(reverse("get-operation"), {"year": "2010"})
@@ -3531,6 +3581,8 @@ class OperationPermissionTest(TestCase, OperationInitTest):
         self.assertEqual(json.loads(response.content.decode())["recordsTotal"], 1)
 
         # area filter
+        alt_user2 = IshtarUser.objects.get(pk=self.alt_user2.pk)
+        alt_user2.generate_permission()
         c = Client()
         c.login(username=self.alt_username2, password=self.alt_password2)
         response = c.get(reverse("get-operation"), {"year": "2010"})
@@ -3557,6 +3609,8 @@ class OperationPermissionTest(TestCase, OperationInitTest):
         self.assertRedirects(response, "/")
 
         # possession
+        alt_user = IshtarUser.objects.get(pk=self.alt_user.pk)
+        alt_user.generate_permission()
         c = Client()
         c.login(username=self.alt_username, password=self.alt_password)
         response = c.get(reverse("operation_modify", args=[operation_pk2]), follow=True)
@@ -3574,6 +3628,8 @@ class OperationPermissionTest(TestCase, OperationInitTest):
         )
 
         # area filter
+        alt_user2 = IshtarUser.objects.get(pk=self.alt_user2.pk)
+        alt_user2.generate_permission()
         c = Client()
         c.login(username=self.alt_username2, password=self.alt_password2)
         response = c.get(reverse("operation_modify", args=[operation_pk2]), follow=True)
diff --git a/archaeological_operations/views.py b/archaeological_operations/views.py
index 7b6a56597..270411bfd 100644
--- a/archaeological_operations/views.py
+++ b/archaeological_operations/views.py
@@ -540,7 +540,10 @@ def get_relation_modify(model, model_relation, formset_class, url_name,
         except model.DoesNotExist:
             raise Http404()
         if "_own_" in current_right:
-            if not item.is_own(request.user):
+            if not request.user.has_perm(current_right, item):
+                raise PermissionDenied()
+        elif current_right:
+            if not request.user.has_perm(current_right):
                 raise PermissionDenied()
         relations = model_relation.objects.filter(left_record_id=pk).all()
         form_kwargs = {"left_record": item}
diff --git a/archaeological_operations/wizards.py b/archaeological_operations/wizards.py
index ac8aaf40d..b510aa4cc 100644
--- a/archaeological_operations/wizards.py
+++ b/archaeological_operations/wizards.py
@@ -21,7 +21,6 @@ import logging
 
 from django.conf import settings
 from django.core.exceptions import ObjectDoesNotExist
-from django.http import Http404
 from django.shortcuts import render
 from django.urls import reverse
 from ishtar_common.utils import ugettext_lazy as _
@@ -34,7 +33,6 @@ from ishtar_common.models import get_current_profile
 from ishtar_common.wizards import (
     Wizard,
     ClosingWizard,
-    DeletionWizard,
     SearchWizard,
     MultipleDeletionWizard,
 )
@@ -240,7 +238,7 @@ class OperationDeletionWizard(MultipleDeletionWizard):
 
 
 class OperationAdministrativeActWizard(OperationWizard):
-    edit = False
+    modification = False
     wizard_done_window = reverse_lazy("show-administrativeact")
     current_obj_slug = "administrativeactop"
     ref_object_key = "operation"
@@ -303,7 +301,7 @@ class OperationAdministrativeActWizard(OperationWizard):
         dct["history_modifier"] = self.request.user
         if "pk" in dct:
             dct.pop("pk")
-        if self.edit:
+        if self.modification:
             admact = self.get_current_object()
             for k in dct:
                 if hasattr(admact, k):
@@ -358,7 +356,7 @@ class OperationAdministrativeActWizard(OperationWizard):
         return res
 
     def get_form_initial(self, step, data=None):
-        if not self.edit:
+        if not self.modification:
             return {}
         initial = super().get_form_initial(step)
         return initial
@@ -366,7 +364,7 @@ class OperationAdministrativeActWizard(OperationWizard):
 
 class OperationEditAdministrativeActWizard(OperationAdministrativeActWizard):
     model = models.AdministrativeAct
-    edit = True
+    modification = True
 
     def get_associated_item(self, dct):
         return self.get_current_object().operation