Fix and simplify permission management with profiles (refs #4046)

5 files changed, 91 insertions, 71 deletions

diff --git a/ishtar_common/ishtar_menu.py b/ishtar_common/ishtar_menu.py
index f925936e9..1b39ea3ca 100644
--- a/ishtar_common/ishtar_menu.py
+++ b/ishtar_common/ishtar_menu.py
@@ -33,13 +33,13 @@ MENU_SECTIONS = [
             'account', _(u"Account"),
             childs=[MenuItem('account_management', _(u"Addition/modification"),
                              model=models.IshtarUser,
-                             access_controls=['add_ishtaruser', ]),
+                             access_controls=['administrator', ]),
                     MenuItem('account_deletion', _(u"Deletion"),
                              model=models.IshtarUser,
-                             access_controls=['add_ishtaruser', ]), ]),
+                             access_controls=['administrator', ]), ]),
         MenuItem('admin-globalvar', _(u"Global variables"),
                  model=models.GlobalVar,
-                 access_controls=['add_globalvar', ]),
+                 access_controls=['administrator', ]),
     ])
     ),
     (10, SectionItem('administration', _(u"Directory"),
@@ -62,11 +62,11 @@ MENU_SECTIONS = [
                 MenuItem(
                     'person-merge', _(u"Automatic merge"),
                     model=models.Person,
-                    access_controls=['merge_person']),
+                    access_controls=['administrator']),
                 MenuItem(
                     'person-manual-merge', _(u"Manual merge"),
                     model=models.Person,
-                    access_controls=['merge_person']),
+                    access_controls=['administrator']),
                 MenuItem(
                     'person_deletion', _(u"Deletion"),
                     model=models.Person,
@@ -93,11 +93,11 @@ MENU_SECTIONS = [
                 MenuItem(
                     'organization-merge', _(u"Automatic merge"),
                     model=models.Organization,
-                    access_controls=['merge_organization']),
+                    access_controls=['administrator']),
                 MenuItem(
                     'orga-manual-merge', _(u"Manual merge"),
                     model=models.Organization,
-                    access_controls=['merge_organization']),
+                    access_controls=['administrator']),
                 MenuItem(
                     'organization_deletion', _(u"Deletion"),
                     model=models.Organization,
diff --git a/ishtar_common/menu_base.py b/ishtar_common/menu_base.py
index d18964c40..d88c5c7c9 100644
--- a/ishtar_common/menu_base.py
+++ b/ishtar_common/menu_base.py
@@ -25,9 +25,7 @@ class SectionItem:
                  css=''):
         self.idx = idx
         self._label = label
-        if not childs:
-            childs = []
-        self.childs = childs
+        self.childs = childs or []
         self.available = False
         self.items = {}
         self.profile_restriction = profile_restriction
@@ -76,12 +74,12 @@ class SectionItem:
 
 
 class MenuItem:
-    def __init__(self, idx, label, model=None, access_controls=[],
+    def __init__(self, idx, label, model=None, access_controls=None,
                  profile_restriction=None, css=''):
         self.idx = idx
         self.label = label
         self.model = model
-        self.access_controls = access_controls
+        self.access_controls = access_controls or []
         self.available = False
         self.profile_restriction = profile_restriction
         self.css = css
@@ -102,19 +100,10 @@ class MenuItem:
             return True
         if not hasattr(user, 'ishtaruser'):
             return False
-        # manage by specific idx - person type
-        if user.ishtaruser.has_right(self.idx, session=session):
-            return True
-        prefix = (self.model._meta.app_label + '.') if self.model else ''
         for access_control in self.access_controls:
-            # check by person type
-            if user.ishtaruser.has_right(access_control, session=session):
-                return True
-            access_control = prefix + access_control
-            # check by specific access control
-            if user.ishtaruser.has_perm(access_control, self.model,
-                                        session=session) or \
-               access_control in user.get_group_permissions():
+            # check by profile
+            if user.ishtaruser.person.has_right(access_control,
+                                                session=session):
                 return True
         return False
 
@@ -123,15 +112,11 @@ class MenuItem:
             return False
         if not self.access_controls:
             return True
-        prefix = (self.model._meta.app_label + '.') if self.model else ''
+        if not hasattr(user, 'ishtaruser'):
+            return False
         for access_control in self.access_controls:
-            access_control = prefix + access_control
-            if user.has_perm(access_control, self.model, obj=obj):
-                #              session=session):
-                return True
-        # manage by person type
-        if hasattr(user, 'ishtaruser'):
-            if user.ishtaruser.has_right(self.idx, session=session):
+            if user.ishtaruser.person.has_right(
+                    access_control, obj=obj, session=session):
                 return True
         return False
 
diff --git a/ishtar_common/menus.py b/ishtar_common/menus.py
index 7cc7b751e..99348fde1 100644
--- a/ishtar_common/menus.py
+++ b/ishtar_common/menus.py
@@ -27,6 +27,8 @@ from django.conf import settings
 from django.core.cache import cache
 from django.core.urlresolvers import reverse
 
+from django.contrib.auth.models import User
+
 _extra_menus = []
 # collect menu from INSTALLED_APPS
 for app in settings.INSTALLED_APPS:
@@ -73,6 +75,9 @@ class Menu:
         self.items_by_idx = {}
 
     def reinit_menu_for_all_user(self):
+        """
+        Force cache deletion and reinitialization of menu for all
+        """
         lst_cache_key = u"{}-{}".format(
             settings.PROJECT_SLUG, 'menu_updated_list',
         )
@@ -83,6 +88,9 @@ class Menu:
             self.init(idx, force=True)
 
     def set_menu_updated_key(self, cache_key, user_id):
+        """
+        Stock updated information in cache
+        """
         lst_cache_key = u"{}-{}".format(
             settings.PROJECT_SLUG, 'menu_updated_list',
         )
@@ -98,8 +106,16 @@ class Menu:
         self.initialized = time
 
     def init(self, user_id=0, force=False):
+        """
+        Get or init menu for a user
+        """
         if not user_id and self.user:
             user_id = self.user.pk
+        else:
+            try:
+                self.user = User.objects.get(pk=user_id)
+            except User.DoesNotExist:
+                return
         cache_key = u"{}-{}-{}".format(
             settings.PROJECT_SLUG, 'menu_updated',
             user_id
@@ -111,18 +127,34 @@ class Menu:
         self.set_menu_updated_key(cache_key, user_id)
         self.items = {}
         self.items_by_idx = {}
-        for idx, main_menu in enumerate(self.childs):
+        childs = self.childs[:]
+        for idx, main_menu in enumerate(reversed(self.childs)):
+            if not main_menu.can_be_available(self.user, self.session):
+                childs.pop(len(self.childs) - idx - 1)
+                continue
             self.items_by_idx[main_menu.idx] = main_menu
-            for child in main_menu.childs:
+            sub_childs = main_menu.childs[:]
+            for s_idx, child in enumerate(reversed(main_menu.childs)):
+                if not child.can_be_available(self.user, self.session):
+                    sub_childs.pop(len(main_menu.childs) - s_idx - 1)
+                    continue
                 self.items_by_idx[child.idx] = child
                 if hasattr(child, 'childs'):
-                    for subchild in child.childs:
+                    sub_sub_childs = child.childs[:]
+                    for ss_idx, subchild in enumerate(reversed(child.childs)):
+                        if not subchild.can_be_available(
+                                self.user, self.session):
+                            sub_sub_childs.pop(len(child.childs) - ss_idx - 1)
+                            continue
                         self.items_by_idx[subchild.idx] = subchild
+                    child.childs = sub_sub_childs
+            main_menu.childs = sub_childs
             selected = main_menu.set_items(
                 self.user, self.items,
                 self.current_action, session=self.session)
             if selected:
                 self.selected_idx = idx
+        self.childs = childs
         self.initialized = True
 
     def get_current_selection(self, current_path):
diff --git a/ishtar_common/models.py b/ishtar_common/models.py
index 24b16c873..d76d2e219 100644
--- a/ishtar_common/models.py
+++ b/ishtar_common/models.py
@@ -118,11 +118,8 @@ def check_model_access_control(request, model, available_perms=None):
     for perm, lbl in model._meta.permissions:
         if perm not in available_perms:
             continue
-        cperm = model._meta.app_label + '.' + perm
-        if request.user.has_perm(cperm) \
-                or cperm in request.user.get_all_permissions() \
-                or request.user.ishtaruser.has_right(
-                    perm, session=request.session):
+        if request.user.ishtaruser.person.has_right(
+                perm, session=request.session):
             allowed = True
             if "_own_" not in perm:
                 own = False
@@ -2545,7 +2542,7 @@ class Person(Address, Merge, OwnPerms, ValueGetter):
     def is_natural(self):
         return not self.attached_to
 
-    def has_right(self, right_name, session=None):
+    def has_right(self, right_name, session=None, obj=None):
         if '.' in right_name:
             right_name = right_name.split('.')[-1]
         res, cache_key = "", ""
@@ -2559,13 +2556,10 @@ class Person(Address, Merge, OwnPerms, ValueGetter):
             key_list = cache.get(cache_key_list, [])
             key_list.append(cache_key)
             cache.set(cache_key_list, key_list, settings.CACHE_TIMEOUT)
-
         if type(right_name) in (list, tuple):
-            res = bool(
-                self.profiles.filter(
+            res = bool(self.profiles.filter(
                     current=True,
-                    profile_type__txt_idx__in=right_name
-                ).count()) or \
+                    profile_type__txt_idx__in=right_name).count()) or \
                 bool(self.profiles.filter(
                     current=True,
                     profile_type__groups__permissions__codename__in=right_name
@@ -2577,10 +2571,9 @@ class Person(Address, Merge, OwnPerms, ValueGetter):
                      codename__in=right_name).count())
         else:
             res = bool(
-                self.profiles.filter(
-                    current=True,
-                    profile_type__txt_idx__in=right_name
-                ).count()) or \
+                    self.profiles.filter(
+                        current=True,
+                        profile_type__txt_idx=right_name).count()) or \
                 bool(self.profiles.filter(
                     current=True,
                     profile_type__groups__permissions__codename=right_name
@@ -2736,13 +2729,19 @@ class IshtarUser(FullSearch):
         if not q.count():
             return
         ishtaruser = q.all()[0]
-        admin, created = PersonType.objects.get_or_create(
-            txt_idx='administrator')
         person = ishtaruser.person
+        admin, created = ProfileType.objects.get_or_create(
+            txt_idx='administrator')
         if user.is_superuser:
-            person.person_types.add(admin)
-        elif admin in person.person_types.all():
-            person.person_types.remove(admin)
+            UserProfile.objects.get_or_create(
+                profile_type=admin, person=person,
+                defaults={'current': True})
+        else:
+            q = UserProfile.objects.filter(
+                profile_type=admin, person=person)
+            if q.count():
+                for up in q.all():
+                    up.delete()
 
     @classmethod
     def create_from_user(cls, user):
@@ -2759,22 +2758,12 @@ class IshtarUser(FullSearch):
     def has_right(self, right_name, session=None):
         return self.person.has_right(right_name, session=session)
 
+    def has_perm(self, perm, model=None, session=None, obj=None):
+        return self.person.has_right(perm, session=session, obj=None)
+
     def full_label(self):
         return self.person.full_label()
 
-    def has_perm(self, perm, model=None, session=None, obj=None):
-        if not session:
-            return self.user_ptr.has_perm(perm, model)
-        cache_key = 'usersession-{}-{}-{}-{}'.format(
-            session.session_key, perm, model.__name__ if model else 'no',
-            obj.pk if obj else 'no')
-        res = cache.get(cache_key)
-        if res in (True, False):
-            return res
-        res = self.user_ptr.has_perm(perm, model)
-        cache.set(cache_key, res, settings.CACHE_SMALLTIMEOUT)
-        return res
-
 
 class Basket(models.Model):
     """
diff --git a/ishtar_common/tests.py b/ishtar_common/tests.py
index 7bce12a4d..4ee47d723 100644
--- a/ishtar_common/tests.py
+++ b/ishtar_common/tests.py
@@ -416,12 +416,21 @@ class AccessControlTest(TestCase):
         user.save()
         ishtar_user = models.IshtarUser.objects.get(
             user_ptr__username='myusername')
-        self.assertIn(admin, ishtar_user.person.person_types.all())
+        self.assertEqual(
+              models.UserProfile.objects.filter(
+                  person__ishtaruser=ishtar_user,
+                  profile_type__txt_idx='administrator'
+              ).count(), 1
+        )
         user = ishtar_user.user_ptr
         user.is_superuser = False
         user.save()
-        ishtar_user = models.IshtarUser.objects.get(
-            user_ptr__username='myusername')
+        self.assertEqual(
+            models.UserProfile.objects.filter(
+                person__ishtaruser=ishtar_user,
+                profile_type__txt_idx='administrator'
+            ).count(), 0
+        )
         self.assertNotIn(admin, ishtar_user.person.person_types.all())
 
 
@@ -1153,6 +1162,11 @@ class ImportTest(TestCase):
 
 
 class IshtarSiteProfileTest(TestCase):
+    fixtures = [settings.ROOT_PATH +
+                '../fixtures/initial_data-auth-fr.json',
+                settings.ROOT_PATH +
+                '../ishtar_common/fixtures/initial_data-fr.json',]
+
     def testRelevance(self):
         cache.set('default-ishtarsiteprofile-is-current-profile', None,
                   settings.CACHE_TIMEOUT)