From 58ac445e3b7347872c8059413ef0beb53d9a16f6 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Tue, 16 Apr 2024 00:06:30 +0200 Subject: [PATCH] Traverse to theme resources from the navigation root again. Only when this gives an Unauthorized, try it on the portal as a fall back. This fixes other use cases of traversing to absolute urls in a theme. Fixes https://github.com/plone/plone.app.theming/issues/236 --- news/236.bugfix | 4 + src/plone/app/theming/tests/browser.py | 6 + src/plone/app/theming/tests/configure.zcml | 14 ++ src/plone/app/theming/tests/test_utils.py | 147 +++++++++++++++++++++ src/plone/app/theming/utils.py | 18 ++- 5 files changed, 185 insertions(+), 4 deletions(-) create mode 100644 news/236.bugfix create mode 100644 src/plone/app/theming/tests/browser.py diff --git a/news/236.bugfix b/news/236.bugfix new file mode 100644 index 00000000..a82788d2 --- /dev/null +++ b/news/236.bugfix @@ -0,0 +1,4 @@ +Traverse to theme resources from the navigation root again. +Only when this gives an Unauthorized, try it on the portal as a fall back. +This fixes other use cases of traversing to absolute urls in a theme. +[maurits] diff --git a/src/plone/app/theming/tests/browser.py b/src/plone/app/theming/tests/browser.py new file mode 100644 index 00000000..36058cae --- /dev/null +++ b/src/plone/app/theming/tests/browser.py @@ -0,0 +1,6 @@ +from Products.Five import BrowserView + + +class Title(BrowserView): + def __call__(self): + return self.context.Title() diff --git a/src/plone/app/theming/tests/configure.zcml b/src/plone/app/theming/tests/configure.zcml index 54c940f3..e42d8d6d 100644 --- a/src/plone/app/theming/tests/configure.zcml +++ b/src/plone/app/theming/tests/configure.zcml @@ -28,4 +28,18 @@ permission="zope.Public" /> + + + + diff --git a/src/plone/app/theming/tests/test_utils.py b/src/plone/app/theming/tests/test_utils.py index 7baa55bf..e3c4330c 100644 --- a/src/plone/app/theming/tests/test_utils.py +++ b/src/plone/app/theming/tests/test_utils.py @@ -1,11 +1,18 @@ +from plone.app.testing import setRoles from plone.app.testing import SITE_OWNER_NAME from plone.app.testing import SITE_OWNER_PASSWORD +from plone.app.testing import TEST_USER_ID from plone.app.theming.testing import THEMING_FUNCTIONAL_TESTING from plone.app.theming.testing import THEMING_INTEGRATION_TESTING from plone.app.theming.utils import applyTheme from plone.app.theming.utils import extractThemeInfo from plone.app.theming.utils import getTheme +from plone.app.theming.utils import InternalResolver +from plone.base.interfaces import INavigationRoot from plone.testing.zope import Browser +from Products.CMFCore.utils import getToolByName +from zExceptions import Unauthorized +from zope.interface import alsoProvides import os.path import tempfile @@ -33,6 +40,26 @@ PACKAGE_THEME = os.path.join(HERE, PACKAGE_THEME_FILENAME) +class InternalResolverAsString(InternalResolver): + """InternalResolver with some simplicifications. + + InternalResolver has this main method: + + def resolve(self, system_url, public_id, context): + + At the end it calls: + + return self.resolve_string(result, context) + + This turns a string into some internal lxml document, and I don't know how + to turn that back into a string for easier testing. So override that + method to simply return the original string. + """ + + def resolve_string(self, result, context): + return result + + class TestIntegration(unittest.TestCase): layer = THEMING_INTEGRATION_TESTING @@ -329,6 +356,126 @@ def test_createThemeFromTemplate_ja_unicode_title(self): self.fail(msg="Unicode Encode Error") +class TestInternalResolverNavigationRoot(unittest.TestCase): + """Test how the InternalResolver handles navigation roots.""" + + layer = THEMING_INTEGRATION_TESTING + + def setUp(self): + self.portal = self.layer["portal"] + self.request = self.layer["request"] + + def resolve(self, system_url): + """Resolve the system_url. + + The standard resolve method ignores the public_id and the context, + so I don't want to pass it in all tests. + """ + resolver = InternalResolverAsString() + return resolver.resolve(system_url, public_id=None, context=None) + + def setup_public(self): + # Create a public navigation root containing a public page. + setRoles(self.portal, TEST_USER_ID, ("Manager",)) + self.portal.invokeFactory("Folder", "public", title="Public Folder") + folder = self.portal.public + alsoProvides(folder, INavigationRoot) + folder.invokeFactory("Document", "page", title="Public page in public folder") + wftool = getToolByName(self.portal, "portal_workflow") + wftool.doActionFor(folder, action="publish") + wftool.doActionFor(folder.page, action="publish") + + # If we want a page in the site root: + # self.portal.invokeFactory("Document", "page", title="Public page") + # wftool.doActionFor(self.portal.page, action="publish") + setRoles(self.portal, TEST_USER_ID, ("Member",)) + return folder + + def setup_private(self): + # Create a private navigation root containing a public page. + setRoles(self.portal, TEST_USER_ID, ("Manager",)) + self.portal.invokeFactory("Folder", "private", title="Private Folder") + folder = self.portal.private + alsoProvides(folder, INavigationRoot) + folder.invokeFactory("Document", "page", title="Public page in private folder") + wftool = getToolByName(self.portal, "portal_workflow") + wftool.doActionFor(folder.page, action="publish") + setRoles(self.portal, TEST_USER_ID, ("Member",)) + return folder + + def test_internal_resolver_site_root(self): + self.request.traverse("/plone") + # absolute + self.assertEqual("Plone site", self.resolve("/@@test-title")) + self.assertIn( + "A CSS file", + self.resolve("/++theme++plone.app.theming.tests/resource.css"), + ) + # relative + self.assertEqual("Plone site", self.resolve("@@test-title")) + self.assertIn( + "A CSS file", + self.resolve("++theme++plone.app.theming.tests/resource.css"), + ) + + def test_internal_resolver_navigation_root_public(self): + self.setup_public() + self.request.traverse("/plone/public") + # absolute + self.assertEqual("Public Folder", self.resolve("/@@test-title")) + self.assertIn( + "A CSS file", + self.resolve("/++theme++plone.app.theming.tests/resource.css"), + ) + # relative + self.assertEqual("Public Folder", self.resolve("@@test-title")) + self.assertIn( + "A CSS file", + self.resolve("++theme++plone.app.theming.tests/resource.css"), + ) + + def test_internal_resolver_navigation_root_public_page(self): + self.setup_public() + self.request.traverse("/plone/public/page") + # absolute + self.assertEqual("Public Folder", self.resolve("/@@test-title")) + self.assertIn( + "A CSS file", + self.resolve("/++theme++plone.app.theming.tests/resource.css"), + ) + # relative + self.assertEqual("Public page in public folder", self.resolve("@@test-title")) + self.assertIn( + "A CSS file", + self.resolve("++theme++plone.app.theming.tests/resource.css"), + ) + + def test_internal_resolver_navigation_root_private(self): + self.setup_private() + # A traverse to "/plone/private" fails, because we are anonymous and + # cannot access this private navigation root: + with self.assertRaises(Unauthorized): + self.request.traverse("/plone/private") + self.request.traverse("/plone/private/page") + # An absolute browser view would fail, because we are not authorized + # to access this view on the private navigation root. But we fall back + # to accessing it on the site root. + self.assertEqual("Plone site", self.resolve("/@@test-title")) + # A publicly available version of the same browser view works fine though: + self.assertEqual("Private Folder", self.resolve("/@@test-public-title")) + # absolute resource + self.assertIn( + "A CSS file", + self.resolve("/++theme++plone.app.theming.tests/resource.css"), + ) + # relative + self.assertEqual("Public page in private folder", self.resolve("@@test-title")) + self.assertIn( + "A CSS file", + self.resolve("++theme++plone.app.theming.tests/resource.css"), + ) + + class TestUnit(unittest.TestCase): def _open_zipfile(self, filename): """Helper that opens a zip file in our test directory""" diff --git a/src/plone/app/theming/utils.py b/src/plone/app/theming/utils.py index dcba1a4e..58041df4 100644 --- a/src/plone/app/theming/utils.py +++ b/src/plone/app/theming/utils.py @@ -1,3 +1,4 @@ +from Acquisition import aq_base from configparser import ConfigParser from diazo.compiler import compile_theme from diazo.compiler import quote_param @@ -161,10 +162,10 @@ def resolve(self, system_url, public_id, context): context = findContext(request) portalState = queryMultiAdapter((context, request), name="plone_portal_state") - portal = portalState.portal() + root = portalState.navigation_root() - if not system_url.startswith("/"): # only for relative urls - root = portalState.navigation_root() + is_absolute_url = system_url.startswith("/") + if not is_absolute_url: root_path = root.getPhysicalPath() context_path = context.getPhysicalPath()[len(root_path) :] if len(context_path) == 0: @@ -172,7 +173,16 @@ def resolve(self, system_url, public_id, context): else: system_url = "/{:s}/{:s}".format("/".join(context_path), system_url) - response = subrequest(system_url, root=portal) + response = subrequest(system_url, root=root) + if is_absolute_url and response.status == 401: + # If we tried on the navigation root we can retry on the portal: + # the navigation root may be private. This is especially needed + # when requesting theme resources: otherwise accessing a public + # page within a private navigation root would show unstyled. + # See https://github.com/plone/plone.app.theming/issues/142 + portal = portalState.portal() + if aq_base(portal) is not aq_base(root): + response = subrequest(system_url, root=portal) if response.status != 200: LOGGER.error(f"Couldn't resolve {system_url:s}") return None