Merge pull request #3834 from yakky/feature/merge_1558

Merge #1558

CHANGELOG.txt

@@ -325,6 +325,7 @@ Please see Install/2.4 release notes *before* attempting to upgrade to version 2
 - Fixed issues relating to the logout function
 
 ==== 3.1 (unreleased) ===
-- Removed django-mptt in favor of django-treebeard
-- Removed compatibility with Django 1.4 / 1.5
+- Remove django-mptt in favor of django-treebeard
+- Remove compatibility with Django 1.4 / 1.5
 - General code cleanup
+- Simplify loading of view restrictions in the menu

cms/admin/change_list.py

@@ -3,7 +3,7 @@
 from cms.models import Title, Page, EmptyTitle
 from cms.utils import get_language_list
 from cms.utils.conf import get_cms_setting
-from cms.utils.permissions import get_user_sites_queryset
+from cms.utils.permissions import get_user_sites_queryset, load_view_restrictions
 from django.contrib.admin.views.main import ChangeList, ALL_VAR, IS_POPUP_VAR, \
     ORDER_TYPE_VAR, ORDER_VAR, SEARCH_VAR
 from django.contrib.sites.models import Site
@@ -111,6 +111,8 @@ def set_items(self, request):
                 pages = pages.filter(pk__in=perm_edit_ids)
 
         root_pages = []
+        # Cache view restrictions for the is_restricted template tag
+        load_view_restrictions(request, pages)
         pages = list(pages)
         all_pages = pages[:] # That is, basically, a copy.
         # Unfortunately we cannot use the MPTT builtin code for pre-caching

cms/menu.py

@@ -1,27 +1,19 @@
 # -*- coding: utf-8 -*-
-from collections import defaultdict
-
-from django.contrib.sites.models import Site
 from django.utils.translation import get_language
 
 from cms.apphook_pool import apphook_pool
-from cms.models.permissionmodels import ACCESS_DESCENDANTS
-from cms.models.permissionmodels import ACCESS_PAGE_AND_DESCENDANTS
-from cms.models.permissionmodels import ACCESS_CHILDREN
-from cms.models.permissionmodels import ACCESS_PAGE_AND_CHILDREN
-from cms.models.permissionmodels import ACCESS_PAGE
-from cms.models.permissionmodels import PagePermission, GlobalPagePermission
+from cms.utils.permissions import load_view_restrictions, has_global_page_permission
 from cms.utils import get_language_from_request
 from cms.utils.conf import get_cms_setting
+from cms.utils.helpers import current_site
 from cms.utils.i18n import get_fallback_languages, hide_untranslated
 from cms.utils.page_resolver import get_page_queryset
 from cms.utils.moderator import get_title_queryset, use_draft
-from cms.utils.helpers import current_site
 from menus.base import Menu, NavigationNode, Modifier
 from menus.menu_pool import menu_pool
 
 
-def get_visible_pages(request, pages, site=None):
+def get_visible_page_objects(request, pages, site=None):
     """
      This code is basically a many-pages-at-once version of
      Page.has_view_permission.
@@ -30,125 +22,73 @@ def get_visible_pages(request, pages, site=None):
      that needs a permission page visibility calculation
     """
     public_for = get_cms_setting('PUBLIC_FOR')
-    is_setting_public_all = public_for == 'all'
-    is_setting_public_staff = public_for == 'staff'
+    can_see_unrestricted = public_for == 'all' or (
+        public_for == 'staff' and request.user.is_staff)
     is_auth_user = request.user.is_authenticated()
-    visible_page_ids = []
-    restricted_pages = defaultdict(list)
-    page_permissions = PagePermission.objects.filter(can_view=True).select_related(
-            'page').prefetch_related('group__user_set')
-
-    for perm in page_permissions:
-        # collect the pages that are affected by permissions
-        if site and perm.page.site_id != site.pk:
-            continue
-        if perm is not None and perm not in restricted_pages[perm.page_id]:
-            # affective restricted pages gathering
-            # add the page with the perm itself
-
-            if perm.grant_on in [ACCESS_PAGE, ACCESS_PAGE_AND_CHILDREN, ACCESS_PAGE_AND_DESCENDANTS]:
-                restricted_pages[perm.page_id].append(perm)
-                restricted_pages[perm.page.publisher_public_id].append(perm)
-                # add children
-            if perm.grant_on in [ACCESS_CHILDREN, ACCESS_PAGE_AND_CHILDREN] and perm.page.numchild:
-                child_ids = perm.page.get_children().values_list('id', 'publisher_public_id')
-                for id, public_id in child_ids:
-                    restricted_pages[id].append(perm)
-                    restricted_pages[public_id].append(perm)
-            # add descendants
-            elif perm.grant_on in [ACCESS_DESCENDANTS, ACCESS_PAGE_AND_DESCENDANTS] and perm.page.numchild:
-                child_ids = perm.page.get_descendants().values_list('id', 'publisher_public_id')
-                for id, public_id in child_ids:
-                    restricted_pages[id].append(perm)
-                    restricted_pages[public_id].append(perm)
-
-    # anonymous
-    # no restriction applied at all
-    if (not is_auth_user and
-        is_setting_public_all and
-        not restricted_pages):
-        return [page.pk for page in pages]
-
-    if site is None:
-        site = current_site(request)
 
-    # authenticated user and global permission
-    if is_auth_user:
-        global_view_perms = GlobalPagePermission.objects.user_has_view_permission(
-            request.user, site.pk).exists()
-
-        #no page perms edge case - all visible
-        if ((is_setting_public_all or (
-                is_setting_public_staff and request.user.is_staff)) and
-            not restricted_pages and
-            not global_view_perms):
-            return [page.pk for page in pages]
-        #no page perms edge case - none visible
-        elif (is_setting_public_staff and
-            not request.user.is_staff and
-            not restricted_pages and
-            not global_view_perms):
-            return []
+    restricted_pages = load_view_restrictions(request, pages)
+    if not restricted_pages:
+        if can_see_unrestricted:
+            return pages
+        elif not is_auth_user:
+            return []  # Unauth user can't acquire global or user perm to see pages
 
+    if get_cms_setting('PERMISSION') and not site:
+        site = current_site(request)  # avoid one extra query when possible
+    if has_global_page_permission(request, site, can_view=True):
+        return pages
 
     def has_global_perm():
         if has_global_perm.cache < 0:
             has_global_perm.cache = 1 if request.user.has_perm('cms.view_page') else 0
         return bool(has_global_perm.cache)
-
     has_global_perm.cache = -1
 
-    def has_permission_membership(page):
+    def has_permission_membership(page_id):
         """
         PagePermission user group membership tests
         """
         user_pk = request.user.pk
-        page_pk = page.pk
-        for perm in restricted_pages[page_pk]:
+        for perm in restricted_pages[page_id]:
             if perm.user_id == user_pk:
                 return True
             if not perm.group_id:
                 continue
-            user_set = getattr(perm.group, 'user_set')
-            # Optimization equivalent to
-            # if user_pk in user_set.values_list('pk', flat=True)
-            if any(user_pk == user.pk for user in user_set.all()):
+            if has_permission_membership.user_groups is None:
+                has_permission_membership.user_groups = request.user.groups.all().values_list(
+                    'pk', flat=True)
+            if perm.group_id in has_permission_membership.user_groups:
                 return True
         return False
+    has_permission_membership.user_groups = None
 
+    visible_pages = []
     for page in pages:
         to_add = False
-        # default to false, showing a restricted page is bad
-        # explicitly check all the conditions
-        # of settings and permissions
-        is_restricted = page.pk in restricted_pages
+        page_id = page.pk
+        is_restricted = page_id in restricted_pages
         # restricted_pages contains as key any page.pk that is
         # affected by a permission grant_on
-        if is_auth_user:
-            # a global permission was given to the request's user
-            if global_view_perms:
-                to_add = True
+        if not is_restricted and can_see_unrestricted:
+            to_add = True
+        elif is_auth_user:
             # setting based handling of unrestricted pages
-            elif not is_restricted and (
-                    is_setting_public_all or (
-                        is_setting_public_staff and request.user.is_staff)
-            ):
-                # authenticated user, no restriction and public for all
-                # or
-                # authenticated staff user, no restriction and public for staff
-                to_add = True
             # check group and user memberships to restricted pages
-            elif is_restricted and has_permission_membership(page):
+            if is_restricted and has_permission_membership(page_id):
                 to_add = True
             elif has_global_perm():
                 to_add = True
-        # anonymous user, no restriction
-        elif not is_restricted and is_setting_public_all:
-            to_add = True
-            # store it
+
         if to_add:
-            visible_page_ids.append(page.pk)
-    return visible_page_ids
+            visible_pages.append(page)
+
+    return visible_pages
+
+
+def get_visible_pages(request, pages, site=None):
+    """Returns the IDs of all visible pages"""
+    pages = get_visible_page_objects(request, pages, site)
+    return [page.pk for page in pages]
 
 
 def page_to_node(page, home, cut):
@@ -163,16 +103,16 @@ def page_to_node(page, home, cut):
     # Theses are simple to port over, since they are not calculated.
     # Other attributes will be added conditionnally later.
     attr = {'soft_root': page.soft_root,
-        'auth_required': page.login_required,
-        'reverse_id': page.reverse_id, }
+            'auth_required': page.login_required,
+            'reverse_id': page.reverse_id, }
 
     parent_id = page.parent_id
     # Should we cut the Node from its parents?
     if home and page.parent_id == home.pk and cut:
         parent_id = None
 
     # possible fix for a possible problem
-    #if parent_id and not page.parent.get_calculated_status():
+    # if parent_id and not page.parent.get_calculated_status():
     #    parent_id = None # ????
 
     if page.limit_visibility_in_menu is None:
@@ -193,7 +133,7 @@ def page_to_node(page, home, cut):
     # but otherwise, just request the title normally
     if not hasattr(page, 'title_cache') or lang in page.title_cache:
         app_name = page.get_application_urls(fallback=False)
-        if app_name: # it means it is an apphook
+        if app_name:  # it means it is an apphook
             app = apphook_pool.get_apphook(app_name)
             for menu in app.menus:
                 extenders.append(menu.__name__)
@@ -219,7 +159,7 @@ def page_to_node(page, home, cut):
 class CMSMenu(Menu):
     def get_nodes(self, request):
         page_queryset = get_page_queryset(request)
-        site = Site.objects.get_current()
+        site = current_site(request)
         lang = get_language_from_request(request)
 
         filters = {
@@ -263,7 +203,7 @@ def get_nodes(self, request):
             langs.extend(get_fallback_languages(lang))
 
         titles = list(get_title_queryset(request).filter(page__in=ids, language__in=langs))
-        for title in titles: # add the title and slugs and some meta data
+        for title in titles:  # add the title and slugs and some meta data
             page = ids[title.page_id]
             page.title_cache[title.language] = title
 
@@ -289,10 +229,11 @@ def modify(self, request, nodes, namespace, root_id, post_cut, breadcrumb):
             extenders = node.attr.get("navigation_extenders", None)
             if extenders:
                 for ext in extenders:
-                    if not ext in exts:
+                    if ext not in exts:
                         exts.append(ext)
                     for extnode in nodes:
-                        if extnode.namespace == ext and not extnode.parent_id:# if home has nav extenders but home is not visible
+                        # if home has nav extenders but home is not visible
+                        if extnode.namespace == ext and not extnode.parent_id:
                             if node.attr.get("is_home", False) and not node.visible:
                                 extnode.parent_id = None
                                 extnode.parent_namespace = None

cms/models/managers.py

@@ -198,7 +198,7 @@ def user_has_permission(self, user, site_id, perm):
         permission exists.
         """
         # if the user has add rights to this site explicitly
-        this_site = Q(**{perm: True, 'sites__in':[site_id]})
+        this_site = Q(**{perm: True, 'sites__in': [site_id]})
         # if the user can add to all sites
         all_sites = Q(**{perm: True, 'sites__isnull': True})
         return self.with_user(user).filter(this_site | all_sites)
@@ -392,9 +392,8 @@ def get_restricted_id_list(self, site):
             MASK_CHILDREN, MASK_DESCENDANTS, MASK_PAGE)
 
         global_permissions = GlobalPagePermission.objects.all()
-        if global_permissions.filter(**{
-            'can_view': True, 'sites__in': [site]
-        }).exists():
+        if global_permissions.filter(Q(sites__in=[site]) | Q(sites__isnull=True)
+                ).filter(can_view=True).exists():
             # user or his group are allowed to do `attr` action
             # !IMPORTANT: page permissions must not override global permissions
             from cms.models import Page
@@ -408,7 +407,7 @@ def get_restricted_id_list(self, site):
         page_id_allow_list = []
         for permission in qs:
             if permission.grant_on & MASK_PAGE:
-                page_id_allow_list.append(permission.page.id)
+                page_id_allow_list.append(permission.page_id)
             if permission.grant_on & MASK_CHILDREN:
                 page_id_allow_list.extend(permission.page.get_children().values_list('id', flat=True))
             elif permission.grant_on & MASK_DESCENDANTS:
@@ -449,7 +448,7 @@ def __get_id_list(self, user, site, attr):
             if getattr(permission, attr):
                 # can add is special - we are actually adding page under current page
                 if permission.grant_on & MASK_PAGE or attr is "can_add":
-                    page_id_allow_list.append(permission.page.id)
+                    page_id_allow_list.append(permission.page_id)
                 if permission.grant_on & MASK_CHILDREN and not attr is "can_add":
                     page_id_allow_list.extend(permission.page.get_children().values_list('id', flat=True))
                 elif permission.grant_on & MASK_DESCENDANTS: