More permission tweaks

backend/models.py

@@ -62,24 +62,6 @@ class PersonVisitorPermissions(object):
             if p.advocates.filter(pk=self.visitor.pk).exists(): return True
         return False
 
-    @cached_property
-    def _is_current_am(self):
-        """
-        Return True if the visitor is the am of any active process not in
-        FD/DAM hands
-        """
-        if self.visitor is None: return False
-        try:
-            am = self.visitor.am
-        except AM.DoesNotExist:
-            return False
-
-        for p in self.processes:
-            if not p.is_active: continue
-            if p.progress in self.fddam_states: continue
-            if p.manager == am: return True
-        return False
-
     @cached_property
     def _can_edit_bio(self):
         """
@@ -89,7 +71,7 @@ class PersonVisitorPermissions(object):
         if self.visitor.is_admin: return True
         if self.person.pending: return False
         if self.visitor.pk == self.person.pk: return True
-        return self._is_current_advocate or self._is_current_am
+        return self.visitor.is_active_am
 
     @cached_property
     def _can_update_keycheck(self):
@@ -100,7 +82,7 @@ class PersonVisitorPermissions(object):
         if self.visitor.is_admin: return True
         if self.person.pending: return False
         if self.visitor.pk == self.person.pk: return True
-        return self._is_current_advocate or self._is_current_am
+        return self.visitor.is_active_am or self._is_current_advocate
 
     @cached_property
     def _has_ldap_record(self):
@@ -127,7 +109,7 @@ class PersonVisitorPermissions(object):
 
         # Only the person themselves, or an am, can potentially edit LDAP
         # fields
-        if self.person.pk != self.visitor.pk and not self._is_current_am: return False
+        if self.person.pk != self.visitor.pk and not self.visitor.is_active_am: return False
 
         # Check if there is some process in a state for which nobody should
         # interfere
@@ -520,6 +502,13 @@ class Person(PermissionsMixin, models.Model):
     def is_am(self):
         return "am" in self.perms
 
+    @property
+    def is_active_am(self):
+        try:
+            return self.am.is_am
+        except AM.DoesNotExist:
+            return False
+
     @property
     def is_admin(self):
         return "admin" in self.perms

backend/tests/test_perms.py

@@ -276,7 +276,7 @@ class TestVisitApplicant(ProcessFixtureMixin, TestVisitPersonMixin, TestCase):
         self.assertApplicantPermsInitialProcess(expected)
 
         self.processes.app.advocates.add(self.persons.adv)
-        expected.set_perms("adv", "update_keycheck edit_bio view_person_audit_log view_mbox")
+        expected.set_perms("adv", "update_keycheck view_person_audit_log view_mbox")
         expected.patch_advs("adv", "-dc_ga")
         self.assertApplicantPermsHasAdvocate(expected)
 
@@ -310,7 +310,7 @@ class TestVisitApplicant(ProcessFixtureMixin, TestVisitPersonMixin, TestCase):
         self.assertApplicantPermsInitialProcess(expected)
 
         self.processes.app.advocates.add(self.persons.adv)
-        expected.set_perms("adv", "update_keycheck edit_bio view_person_audit_log view_mbox")
+        expected.set_perms("adv", "update_keycheck view_person_audit_log view_mbox")
         expected.patch_advs("adv", "-dc_ga")
         self.assertApplicantPermsHasAdvocate(expected)
 
@@ -374,7 +374,7 @@ class TestVisitApplicant(ProcessFixtureMixin, TestVisitPersonMixin, TestCase):
         self.assertApplicantPermsInitialProcess(expected)
 
         self.processes.app.advocates.add(self.persons.adv)
-        expected.set_perms("adv", "update_keycheck edit_bio view_person_audit_log view_mbox")
+        expected.set_perms("adv", "update_keycheck view_person_audit_log view_mbox")
         expected.patch_advs("adv", "-dm_ga")
         self.assertApplicantPermsHasAdvocate(expected)
 
@@ -406,7 +406,7 @@ class TestVisitApplicant(ProcessFixtureMixin, TestVisitPersonMixin, TestCase):
         self.assertApplicantPermsInitialProcess(expected)
 
         self.processes.app.advocates.add(self.persons.adv)
-        expected.set_perms("adv", "update_keycheck edit_bio view_person_audit_log view_mbox")
+        expected.set_perms("adv", "update_keycheck view_person_audit_log view_mbox")
         expected.patch_advs("adv", "-dm")
         self.assertApplicantPermsHasAdvocate(expected)
 
@@ -439,7 +439,7 @@ class TestVisitApplicant(ProcessFixtureMixin, TestVisitPersonMixin, TestCase):
         self.assertApplicantPermsInitialProcess(expected)
 
         self.processes.app.advocates.add(self.persons.adv)
-        expected.set_perms("adv", "update_keycheck edit_bio view_person_audit_log view_mbox")
+        expected.set_perms("adv", "update_keycheck view_person_audit_log view_mbox")
         expected.patch_advs("adv", "-dd_nu -dd_u")
         self.assertApplicantPermsHasAdvocate(expected)
 
@@ -471,7 +471,7 @@ class TestVisitApplicant(ProcessFixtureMixin, TestVisitPersonMixin, TestCase):
         self.assertApplicantPermsInitialProcess(expected)
 
         self.processes.app.advocates.add(self.persons.adv)
-        expected.set_perms("adv", "update_keycheck edit_bio view_person_audit_log view_mbox")
+        expected.set_perms("adv", "update_keycheck view_person_audit_log view_mbox")
         expected.patch_advs("adv", "-dd_nu -dd_u")
         self.assertApplicantPermsHasAdvocate(expected)
 
@@ -503,7 +503,7 @@ class TestVisitApplicant(ProcessFixtureMixin, TestVisitPersonMixin, TestCase):
         self.assertApplicantPermsInitialProcess(expected)
 
         self.processes.app.advocates.add(self.persons.adv)
-        expected.set_perms("adv", "update_keycheck edit_bio view_person_audit_log view_mbox")
+        expected.set_perms("adv", "update_keycheck view_person_audit_log view_mbox")
         expected.patch_advs("adv", "-dd_nu -dd_u")
         self.assertApplicantPermsHasAdvocate(expected)
 
@@ -535,7 +535,7 @@ class TestVisitApplicant(ProcessFixtureMixin, TestVisitPersonMixin, TestCase):
         self.assertApplicantPermsInitialProcess(expected)
 
         self.processes.app.advocates.add(self.persons.adv)
-        expected.set_perms("adv", "update_keycheck edit_bio view_person_audit_log view_mbox")
+        expected.set_perms("adv", "update_keycheck view_person_audit_log view_mbox")
         expected.patch_advs("adv", "-dd_nu -dd_u")
         self.assertApplicantPermsHasAdvocate(expected)
 
@@ -568,7 +568,7 @@ class TestVisitApplicant(ProcessFixtureMixin, TestVisitPersonMixin, TestCase):
         self.assertApplicantPermsInitialProcess(expected)
 
         self.processes.app.advocates.add(self.persons.adv)
-        expected.set_perms("adv", "update_keycheck edit_bio view_person_audit_log view_mbox")
+        expected.set_perms("adv", "update_keycheck view_person_audit_log view_mbox")
         expected.patch_advs("adv", "-dd_nu -dd_u")
         self.assertApplicantPermsHasAdvocate(expected)
 
@@ -600,7 +600,7 @@ class TestVisitApplicant(ProcessFixtureMixin, TestVisitPersonMixin, TestCase):
         self.assertApplicantPermsInitialProcess(expected)
 
         self.processes.app.advocates.add(self.persons.adv)
-        expected.set_perms("adv", "update_keycheck edit_bio view_person_audit_log view_mbox")
+        expected.set_perms("adv", "update_keycheck view_person_audit_log view_mbox")
         expected.patch_advs("adv", "-dd_nu -dd_u")
         self.assertApplicantPermsHasAdvocate(expected)
 

fprs/tests/test_views.py

@@ -98,10 +98,8 @@ class TestPersonFingerprints(PersonFixtureMixin, TestCase):
     @classmethod
     def setUpClass(cls):
         super(TestPersonFingerprints, cls).setUpClass()
-        cls.persons.create("app", status=const.STATUS_DC, alioth=True, fd_comment="FD_COMMENTS")
-        cls.persons.create("adv", status=const.STATUS_DD_NU)
         cls.persons.create("am", status=const.STATUS_DD_NU)
-        #cls.processes.create("app", person=cls.persons.app, applying_for=const.STATUS_DD_NU, progress=const.PROGRESS_AM, manager=cls.persons.am, advocates=[cls.persons.adv])
+        cls.ams.create("am", person=cls.persons.am)
 
     @classmethod
     def __add_extra_tests__(cls):
@@ -118,13 +116,16 @@ class TestPersonFingerprints(PersonFixtureMixin, TestCase):
             cls._add_method(cls._test_get_forbidden, person, person)
             cls._add_method(cls._test_post_forbidden, person, person)
 
-        # Only applicant, advocate, am, fd and dam can see and edit the keys of an applicant
-        for person in ("app", "am", "fd", "dam"):
-            cls._add_method(cls._test_get_success, person, "app")
-            cls._add_method(cls._test_post_success, person, "app")
-        for person in ("pending", "dc", "dc_ga", "dm", "dm_ga", "adv", "dd_nu", "dd_u"):
-            cls._add_method(cls._test_get_forbidden, person, "app")
-            cls._add_method(cls._test_post_forbidden, person, "app")
+        # active ams, fd and dam can see and edit the keys of anyone who is not in LDAP
+        for visitor in ("am", "fd", "dam"):
+            for visited in ("dc", "dm"):
+                cls._add_method(cls._test_get_success, visitor, visited)
+                cls._add_method(cls._test_post_success, visitor, visited)
+        for visitor in ("pending", "dc", "dc_ga", "dm", "dm_ga", "dd_nu", "dd_u"):
+            for visited in ("dc", "dm"):
+                if visitor == visited: continue
+                cls._add_method(cls._test_get_forbidden, visitor, visited)
+                cls._add_method(cls._test_post_forbidden, visitor, visited)
 
     def _test_get_success(self, visitor, visited):
         client = self.make_test_client(visitor)