From d7aa9c8c4c03069afd3554e6408f1a703c66d721 Mon Sep 17 00:00:00 2001 From: Jonas Obrist Date: Fri, 5 Aug 2011 15:45:17 +0200 Subject: [PATCH] Drastically improved coverage of cms.menu.get_visible_pages and fixed some bugs in the process --- cms/menu.py | 58 +++-------- cms/tests/menu.py | 224 ++++++++++++++++++++++++++++++++++++++++++- cms/utils/plugins.py | 2 +- 3 files changed, 238 insertions(+), 46 deletions(-) diff --git a/cms/menu.py b/cms/menu.py index ab468082fb0..d0337c559e6 100644 --- a/cms/menu.py +++ b/cms/menu.py @@ -1,7 +1,5 @@ # -*- coding: utf-8 -*- from cms.apphook_pool import apphook_pool -from cms.cache.permissions import get_permission_cache, set_permission_cache -from cms.models.managers import PagePermissionsPermissionManager from cms.models.moderatormodels import (ACCESS_DESCENDANTS, ACCESS_PAGE_AND_DESCENDANTS, ACCESS_CHILDREN, ACCESS_PAGE_AND_CHILDREN) from cms.models.permissionmodels import PagePermission, GlobalPagePermission @@ -18,7 +16,7 @@ from menus.menu_pool import menu_pool -def get_visible_pages(request, pages): +def get_visible_pages(request, pages, site=None): # This code is basically a many-pages-at-once version of # Page.has_view_permission, check there to see wtf is going on here. if request.user.is_staff and settings.CMS_PUBLIC_FOR in ('staff', 'all'): @@ -37,64 +35,38 @@ def get_visible_pages(request, pages): page_permissions = PagePermission.objects.filter(pages_perms_q).select_related('page') restriced_pages = [page_permission.page.pk for page_permission in page_permissions] - site = current_site(request) + if site is None: + site = current_site(request) if request.user.is_authenticated(): #return self.filter(Q(user=user) | Q(group__user=user)) global_page_perm_q = Q( Q(user=request.user) | Q(group__user=request.user) - ) & Q(can_view=True, sites__in=[site.pk]) + ) & Q(can_view=True) & Q(Q(sites__in=[site.pk]) | Q(sites__isnull=True)) global_view_perms = GlobalPagePermission.objects.filter(global_page_perm_q).exists() - - def get_generic_permissions(): - if request.user.is_superuser or not settings.CMS_PERMISSION: - # got superuser, or permissions aren't enabled? just return grant - # all mark - return [page.pk for page in pages] - # read from cache if posssible - cached = get_permission_cache(request.user, 'can_view') - if cached is not None: - return cached - return [] - - generic_permissions = lazy(get_generic_permissions, list)() - has_global_perm = lazy(request.user.has_perm, bool)('cms.view_page') + 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 + for page in pages: - # TODO: needs to be refactored to not contain continue statements is_restricted = page.pk in restriced_pages if request.user.is_authenticated(): # a global permission was given to the request's user if global_view_perms: page_ids.append(page.pk) - continue - # authenticated user, no restriction and public for all fallback - if (not is_restricted and not global_view_perms and - not settings.CMS_PUBLIC_FOR == 'all'): - continue # authenticated user, no restriction and public for all - if (not is_restricted and not global_view_perms and - settings.CMS_PUBLIC_FOR == 'all'): + elif settings.CMS_PUBLIC_FOR == 'all': page_ids.append(page.pk) - continue - else: - #anonymous user - if is_restricted or not settings.CMS_PUBLIC_FOR == 'all': - # anyonymous user, page has restriction and global access is permitted - continue - else: - # anonymous user, no restriction saved in database + elif has_global_perm(): page_ids.append(page.pk) - continue - if has_global_perm: - page_ids.append(page.pk) - continue - - if page.pk in generic_permissions: + elif not is_restricted and settings.CMS_PUBLIC_FOR == 'all': + # anonymous user, no restriction saved in database page_ids.append(page.pk) - continue return page_ids def page_to_node(page, home, cut): @@ -187,7 +159,7 @@ def get_nodes(self, request): actual_pages = [] # cache view perms - visible_pages = get_visible_pages(request, pages) + visible_pages = get_visible_pages(request, pages, site) for page in pages: # Pages are ordered by tree_id, therefore the first page is the root diff --git a/cms/tests/menu.py b/cms/tests/menu.py index 69e2537eb3d..5ed4fa9bb85 100644 --- a/cms/tests/menu.py +++ b/cms/tests/menu.py @@ -1,13 +1,18 @@ # -*- coding: utf-8 -*- from __future__ import with_statement from cms.api import create_page -from cms.menu import CMSMenu +from cms.menu import CMSMenu, get_visible_pages from cms.models import Page +from cms.models.permissionmodels import GlobalPagePermission, PagePermission from cms.test_utils.fixtures.menus import (MenusFixture, SubMenusFixture, SoftrootFixture) from cms.test_utils.testcases import SettingsOverrideTestCase +from cms.test_utils.util.context_managers import SettingsOverride from cms.test_utils.util.mock import AttributeObject from django.conf import settings +from django.contrib.auth.models import AnonymousUser, User, Permission +from django.contrib.contenttypes.models import ContentType +from django.contrib.sites.models import Site from django.template import Template from menus.base import NavigationNode from menus.menu_pool import menu_pool, _build_nodes_inner_for_one_menu @@ -17,7 +22,6 @@ class BaseMenuTest(SettingsOverrideTestCase): settings_overrides = { 'CMS_MODERATOR': False, - 'DEBUG': True } def _get_nodes(self, path='/'): @@ -812,3 +816,219 @@ def test_not_in_navigation_num_queries(self): """ tpl = Template("{% load menu_tags %}{% show_menu_below_id 'a' 0 100 100 100 %}") tpl.render(context) + + +class ViewPermissionMenuTests(SettingsOverrideTestCase): + settings_overrides = { + 'CMS_MODERATOR': False, + 'CMS_PERMISSION': True, + 'CMS_PUBLIC_FOR': 'all', + } + + def get_request(self, user=None): + attrs = { + 'user': user or AnonymousUser(), + 'REQUEST': {}, + 'session': {}, + } + return type('Request', (object,), attrs) + + def test_public_for_all_staff(self): + request = self.get_request() + request.user.is_staff = True + page = Page() + page.pk = 1 + pages = [page] + result = get_visible_pages(request, pages) + self.assertEqual(result, [1]) + + def test_public_for_all_staff_assert_num_queries(self): + request = self.get_request() + request.user.is_staff = True + page = Page() + page.pk = 1 + pages = [page] + with self.assertNumQueries(0): + get_visible_pages(request, pages) + + def test_public_for_all(self): + user = User.objects.create_user('user', 'user@domain.com', 'user') + request = self.get_request(user) + page = Page() + page.pk = 1 + page.level = 0 + page.tree_id = 1 + pages = [page] + result = get_visible_pages(request, pages) + self.assertEqual(result, [1]) + + def test_public_for_all_num_queries(self): + user = User.objects.create_user('user', 'user@domain.com', 'user') + request = self.get_request(user) + site = Site() + site.pk = 1 + page = Page() + page.pk = 1 + page.level = 0 + page.tree_id = 1 + pages = [page] + with self.assertNumQueries(2): + """ + The two queries are: + PagePermission query for affected pages + GlobalpagePermission query for user + """ + get_visible_pages(request, pages, site) + + def test_unauthed(self): + request = self.get_request() + page = Page() + page.pk = 1 + page.level = 0 + page.tree_id = 1 + pages = [page] + result = get_visible_pages(request, pages) + self.assertEqual(result, [1]) + + def test_unauthed_num_queries(self): + request = self.get_request() + site = Site() + site.pk = 1 + page = Page() + page.pk = 1 + page.level = 0 + page.tree_id = 1 + pages = [page] + with self.assertNumQueries(1): + """ + The query is: + PagePermission query for affected pages + + global is not executed because it's lazy + """ + get_visible_pages(request, pages, site) + + def test_authed_basic_perm(self): + with SettingsOverride(CMS_PUBLIC_FOR='staff'): + user = User.objects.create_user('user', 'user@domain.com', 'user') + user.user_permissions.add(Permission.objects.get(codename='view_page')) + request = self.get_request(user) + page = Page() + page.pk = 1 + page.level = 0 + page.tree_id = 1 + pages = [page] + result = get_visible_pages(request, pages) + self.assertEqual(result, [1]) + + def test_authed_basic_perm_num_queries(self): + site = Site() + site.pk = 1 + with SettingsOverride(CMS_PUBLIC_FOR='staff'): + user = User.objects.create_user('user', 'user@domain.com', 'user') + user.user_permissions.add(Permission.objects.get(codename='view_page')) + request = self.get_request(user) + page = Page() + page.pk = 1 + page.level = 0 + page.tree_id = 1 + pages = [page] + with self.assertNumQueries(4): + """ + The two queries are: + PagePermission query for affected pages + GlobalpagePermission query for user + Generic django permission lookup + content type lookup by permission lookup + """ + get_visible_pages(request, pages, site) + + def test_authed_no_access(self): + with SettingsOverride(CMS_PUBLIC_FOR='staff'): + user = User.objects.create_user('user', 'user@domain.com', 'user') + request = self.get_request(user) + page = Page() + page.pk = 1 + page.level = 0 + page.tree_id = 1 + pages = [page] + result = get_visible_pages(request, pages) + self.assertEqual(result, []) + + def test_authed_no_access_num_queries(self): + site = Site() + site.pk = 1 + with SettingsOverride(CMS_PUBLIC_FOR='staff'): + user = User.objects.create_user('user', 'user@domain.com', 'user') + request = self.get_request(user) + page = Page() + page.pk = 1 + page.level = 0 + page.tree_id = 1 + pages = [page] + with self.assertNumQueries(4): + """ + The two queries are: + PagePermission query for affected pages + GlobalpagePermission query for user + Generic django permission lookup + content type lookup by permission lookup + """ + get_visible_pages(request, pages, site) + + def test_unauthed_no_access(self): + with SettingsOverride(CMS_PUBLIC_FOR='staff'): + request = self.get_request() + page = Page() + page.pk = 1 + page.level = 0 + page.tree_id = 1 + pages = [page] + result = get_visible_pages(request, pages) + self.assertEqual(result, []) + + def test_unauthed_no_access_num_queries(self): + site = Site() + site.pk = 1 + with SettingsOverride(CMS_PUBLIC_FOR='staff'): + request = self.get_request() + page = Page() + page.pk = 1 + page.level = 0 + page.tree_id = 1 + pages = [page] + with self.assertNumQueries(1): + get_visible_pages(request, pages, site) + + def test_global_permission(self): + user = User.objects.create_user('user', 'user@domain.com', 'user') + GlobalPagePermission.objects.create(can_view=True, user=user) + request = self.get_request(user) + page = Page() + page.pk = 1 + page.level = 0 + page.tree_id = 1 + pages = [page] + result = get_visible_pages(request, pages) + self.assertEqual(result, [1]) + + def test_global_permission_num_queries(self): + site = Site() + site.pk = 1 + user = User.objects.create_user('user', 'user@domain.com', 'user') + GlobalPagePermission.objects.create(can_view=True, user=user) + request = self.get_request(user) + site = Site() + site.pk = 1 + page = Page() + page.pk = 1 + page.level = 0 + page.tree_id = 1 + pages = [page] + with self.assertNumQueries(2): + """ + The two queries are: + PagePermission query for affected pages + GlobalpagePermission query for user + """ + get_visible_pages(request, pages, site) diff --git a/cms/utils/plugins.py b/cms/utils/plugins.py index d02ecabae44..a5645714775 100644 --- a/cms/utils/plugins.py +++ b/cms/utils/plugins.py @@ -131,4 +131,4 @@ def current_site(request): except Site.DoesNotExist: return None else: - return Site.objects.get_current() \ No newline at end of file + return Site.objects.get_current()