From 2e49bace5416a734d71b80e120a1aa2f7e3b738b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9rome=20Perrin?= <jerome@nexedi.com>
Date: Wed, 24 Apr 2024 14:43:58 +0900
Subject: [PATCH] Fix authentication error viewing ZMI resource with virtual
 hosting (#1204)

* ZMI: re-implement the logic to prepend authentication path

The previous approach from #1196 was not correct when using virtual host,
because AUTHENTICATION_PATH is not usable in virtual host contexts.

This uses a different approach, by making the js and css path
subscribers take care of generating the URLs with the authentication
path prepended using request API aware of virtual hosting.

* ZMI: use /++resource++logo/ for public resources

/++resource++zmi/logo/ and /++resource++logo/ is the same folder on disk,
the former is protected by Manage portal and the later is public.
Protected resources were problematic, because we want to serve them from
the root, but the manager user might not have permission on the root.

* ZMI: include zmi.localstorage.api.js using js subscriber

Using this approach we serve a resource relative to the path where user
is authenticated.

* copyright.dtml: update bootstrap URL

This was referencing an old URL.

Co-authored-by: Maurits van Rees <maurits@vanrees.org>
---
 CHANGES.rst                                   |   3 +-
 src/App/dtml/copyright.dtml                   |  16 +--
 src/App/dtml/manage.dtml                      |  14 +--
 src/App/dtml/manage_page_header.dtml          |  20 ++--
 src/App/dtml/menu.dtml                        |   1 -
 .../resources/logo/favicon/browserconfig.xml  |   8 +-
 .../resources/logo/favicon/site.webmanifest   |   4 +-
 src/zmi/styles/subscriber.py                  |  54 ++++++++--
 src/zmi/styles/tests.py                       | 101 ++++++++++++++++--
 9 files changed, 173 insertions(+), 48 deletions(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index 4bc40ca282..edac84affb 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -30,7 +30,8 @@ https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst
   Fixes `#1077 <https://github.com/zopefoundation/Zope/issues/1077>`_.
 
 - Fix authentication error viewing ZMI with a user defined outside of zope root.
-  Fixes `#1195 <https://github.com/zopefoundation/Zope/issues/1195>`_.
+  Fixes `#1195 <https://github.com/zopefoundation/Zope/issues/1195>`_ and
+  `#1203 <https://github.com/zopefoundation/Zope/issues/1195>`_.
 
 - Work around ``Products.CMFCore`` and ``Products.CMFPlone`` design bug
   (registering non callable constructors).
diff --git a/src/App/dtml/copyright.dtml b/src/App/dtml/copyright.dtml
index 7b1d544f08..6843c58d45 100644
--- a/src/App/dtml/copyright.dtml
+++ b/src/App/dtml/copyright.dtml
@@ -6,16 +6,16 @@
   <meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no" />
 
   <title>Zope Public License (ZPL) Version 2.1</title>
-  <link rel="stylesheet" type="text/css" href="&dtml-BASEPATH1;/++resource++zmi/bootstrap-4.1.1/bootstrap.min.css" />
+  <link rel="stylesheet" type="text/css" href="&dtml-BASEPATH1;/++resource++zmi/bootstrap-4.6.0/bootstrap.min.css" />
   <link rel="stylesheet" type="text/css" href="&dtml-BASEPATH1;/++resource++zmi/zmi_base.css" />
 
-  <link rel="shortcut icon" type="image/x-icon" href="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/favicon.ico" />
-  <link rel="apple-touch-icon" sizes="180x180" href="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/apple-touch-icon.png" />
-  <link rel="icon" type="image/png" sizes="32x32" href="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/favicon-32x32.png" />
-  <link rel="icon" type="image/png" sizes="16x16" href="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/favicon-16x16.png" />
-  <link rel="manifest" href="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/site.webmanifest" />
-  <link rel="mask-icon" href="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/safari-pinned-tab.svg" color="#5bbad5" />
-  <meta name="msapplication-config" content="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/browserconfig.xml"/>
+  <link rel="shortcut icon" type="image/x-icon" href="&dtml-BASEPATH1;/++resource++logo/favicon/favicon.ico" />
+  <link rel="apple-touch-icon" sizes="180x180" href="&dtml-BASEPATH1;/++resource++logo/favicon/apple-touch-icon.png" />
+  <link rel="icon" type="image/png" sizes="32x32" href="&dtml-BASEPATH1;/++resource++logo/favicon/favicon-32x32.png" />
+  <link rel="icon" type="image/png" sizes="16x16" href="&dtml-BASEPATH1;/++resource++logo/favicon/favicon-16x16.png" />
+  <link rel="manifest" href="&dtml-BASEPATH1;/++resource++logo/favicon/site.webmanifest" />
+  <link rel="mask-icon" href="&dtml-BASEPATH1;/++resource++logo/favicon/safari-pinned-tab.svg" color="#5bbad5" />
+  <meta name="msapplication-config" content="&dtml-BASEPATH1;/++resource++logo/favicon/browserconfig.xml"/>
   <meta name="msapplication-TileColor" content="#2d89ef" />
   <meta name="theme-color" content="#ffffff" />
 </head>
diff --git a/src/App/dtml/manage.dtml b/src/App/dtml/manage.dtml
index 89800b76bc..1d6f237f2c 100644
--- a/src/App/dtml/manage.dtml
+++ b/src/App/dtml/manage.dtml
@@ -2,13 +2,13 @@
 <head>
 	<title>Zope on &dtml-BASE0;</title>
 
-	<link rel="shortcut icon" type="image/x-icon" href="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/favicon.ico" />
-	<link rel="apple-touch-icon" sizes="180x180" href="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/apple-touch-icon.png" />
-	<link rel="icon" type="image/png" sizes="32x32" href="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/favicon-32x32.png" />
-	<link rel="icon" type="image/png" sizes="16x16" href="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/favicon-16x16.png" />
-	<link rel="manifest" href="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/site.webmanifest" />
-	<link rel="mask-icon" href="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/safari-pinned-tab.svg" color="#5bbad5" />
-	<meta name="msapplication-config" content="&dtml-BASEPATH1;/++resource++zmi/logo/favicon/browserconfig.xml"/>
+	<link rel="shortcut icon" type="image/x-icon" href="&dtml-BASEPATH1;/++resource++logo/favicon/favicon.ico" />
+	<link rel="apple-touch-icon" sizes="180x180" href="&dtml-BASEPATH1;/++resource++logo/favicon/apple-touch-icon.png" />
+	<link rel="icon" type="image/png" sizes="32x32" href="&dtml-BASEPATH1;/++resource++logo/favicon/favicon-32x32.png" />
+	<link rel="icon" type="image/png" sizes="16x16" href="&dtml-BASEPATH1;/++resource++logo/favicon/favicon-16x16.png" />
+	<link rel="manifest" href="&dtml-BASEPATH1;/++resource++logo/favicon/site.webmanifest" />
+	<link rel="mask-icon" href="&dtml-BASEPATH1;/++resource++logo/favicon/safari-pinned-tab.svg" color="#5bbad5" />
+	<meta name="msapplication-config" content="&dtml-BASEPATH1;/++resource++logo/favicon/browserconfig.xml"/>
 	<meta name="msapplication-TileColor" content="#2d89ef" />
 	<meta name="theme-color" content="#ffffff" />
 
diff --git a/src/App/dtml/manage_page_header.dtml b/src/App/dtml/manage_page_header.dtml
index e8b8659e10..a70451a9c3 100644
--- a/src/App/dtml/manage_page_header.dtml
+++ b/src/App/dtml/manage_page_header.dtml
@@ -10,27 +10,25 @@
 </dtml-let>
 
 <title><dtml-if title_or_id><dtml-var title_or_id><dtml-else>Zope</dtml-if></title>
-<dtml-let basepath="'/'.join([''] + [p for p in (REQUEST['BASEPATH1'], REQUEST.get('AUTHENTICATION_PATH')) if p])">
 
 <dtml-in css_urls>
-	<link rel="stylesheet" type="text/css" href="&dtml-basepath;&dtml-sequence-item;" />
+	<link rel="stylesheet" type="text/css" href="&dtml-sequence-item;" />
 </dtml-in>
 <dtml-in js_urls>
-	<script src="&dtml-basepath;&dtml-sequence-item;"></script>
+	<script src="&dtml-sequence-item;"></script>
 </dtml-in>
 
-<link rel="shortcut icon" type="image/x-icon" href="&dtml-basepath;/++resource++zmi/logo/favicon/favicon.ico" />
-<link rel="apple-touch-icon" sizes="180x180" href="&dtml-basepath;/++resource++zmi/logo/favicon/apple-touch-icon.png" />
-<link rel="icon" type="image/png" sizes="32x32" href="&dtml-basepath;/++resource++zmi/logo/favicon/favicon-32x32.png" />
-<link rel="icon" type="image/png" sizes="16x16" href="&dtml-basepath;/++resource++zmi/logo/favicon/favicon-16x16.png" />
-<link rel="manifest" href="&dtml-basepath;/++resource++zmi/logo/favicon/site.webmanifest" />
-<link rel="mask-icon" href="&dtml-basepath;/++resource++zmi/logo/favicon/safari-pinned-tab.svg" color="#5bbad5" />
-<meta name="msapplication-config" content="&dtml-basepath;/++resource++zmi/logo/favicon/browserconfig.xml"/>
+<link rel="shortcut icon" type="image/x-icon" href="&dtml-BASEPATH1;/++resource++logo/favicon/favicon.ico" />
+<link rel="apple-touch-icon" sizes="180x180" href="&dtml-BASEPATH1;/++resource++logo/favicon/apple-touch-icon.png" />
+<link rel="icon" type="image/png" sizes="32x32" href="&dtml-BASEPATH1;/++resource++logo/favicon/favicon-32x32.png" />
+<link rel="icon" type="image/png" sizes="16x16" href="&dtml-BASEPATH1;/++resource++logo/favicon/favicon-16x16.png" />
+<link rel="manifest" href="&dtml-BASEPATH1;/++resource++logo/favicon/site.webmanifest" />
+<link rel="mask-icon" href="&dtml-BASEPATH1;/++resource++logo/favicon/safari-pinned-tab.svg" color="#5bbad5" />
+<meta name="msapplication-config" content="&dtml-BASEPATH1;/++resource++logo/favicon/browserconfig.xml"/>
 <meta name="msapplication-TileColor" content="#2d89ef" />
 <meta name="theme-color" content="#ffffff" />
 
 </head>
-</dtml-let>
 <!-- REFACT what is a better way to get the last part of the current URL? -->
 <body id="nodeid-<dtml-var "getId()">" class="zmi zmi-<dtml-var "this().meta_type.replace(' ', '-').replace('(', '').replace(')', '')"> zmi-<dtml-var "URL0[_.len(URL1)+1:]">">
 </dtml-unless>
diff --git a/src/App/dtml/menu.dtml b/src/App/dtml/menu.dtml
index a23800fd28..548c1c5185 100644
--- a/src/App/dtml/menu.dtml
+++ b/src/App/dtml/menu.dtml
@@ -64,7 +64,6 @@
 
 </main>
 
-<script src="&dtml-BASEPATH1;/++resource++zmi/zmi.localstorage.api.js"></script>
 <script>
 	function menu_resize(init) {
 		var key = "ZMI.manage_menu.width";
diff --git a/src/zmi/styles/resources/logo/favicon/browserconfig.xml b/src/zmi/styles/resources/logo/favicon/browserconfig.xml
index 94b6a441b9..971d80bc6f 100644
--- a/src/zmi/styles/resources/logo/favicon/browserconfig.xml
+++ b/src/zmi/styles/resources/logo/favicon/browserconfig.xml
@@ -2,10 +2,10 @@
 <browserconfig>
     <msapplication>
         <tile>
-            <square70x70logo src="/++resource++zmi/logo/favicon/mstile-70x70.png"/>
-            <square150x150logo src="/++resource++zmi/logo/favicon/mstile-150x150.png"/>
-            <wide310x150logo src="/++resource++zmi/logo/favicon/mstile-310x150.png"/>
-            <square310x310logo src="/++resource++zmi/logo/favicon/mstile-310x310.png"/>
+            <square70x70logo src="/++resource++logo/favicon/mstile-70x70.png"/>
+            <square150x150logo src="/++resource++logo/favicon/mstile-150x150.png"/>
+            <wide310x150logo src="/++resource++logo/favicon/mstile-310x150.png"/>
+            <square310x310logo src="/++resource++logo/favicon/mstile-310x310.png"/>
             <TileColor>#00aad4</TileColor>
         </tile>
     </msapplication>
diff --git a/src/zmi/styles/resources/logo/favicon/site.webmanifest b/src/zmi/styles/resources/logo/favicon/site.webmanifest
index 9f465e9bdb..a94ec47774 100644
--- a/src/zmi/styles/resources/logo/favicon/site.webmanifest
+++ b/src/zmi/styles/resources/logo/favicon/site.webmanifest
@@ -3,12 +3,12 @@
     "short_name": "",
     "icons": [
         {
-            "src": "/++resource++zmi/logo/favicon/android-chrome-192x192.png",
+            "src": "/++resource++logo/favicon/android-chrome-192x192.png",
             "sizes": "192x192",
             "type": "image/png"
         },
         {
-            "src": "/++resource++zmi/logo/favicon/android-chrome-256x256.png",
+            "src": "/++resource++logo/favicon/android-chrome-256x256.png",
             "sizes": "256x256",
             "type": "image/png"
         }
diff --git a/src/zmi/styles/subscriber.py b/src/zmi/styles/subscriber.py
index 3320a3c6de..872776efcf 100644
--- a/src/zmi/styles/subscriber.py
+++ b/src/zmi/styles/subscriber.py
@@ -1,14 +1,50 @@
+import itertools
+
 import zope.component
 import zope.interface
+from AccessControl.SecurityManagement import getSecurityManager
+from Acquisition import aq_parent
+
+
+def prepend_authentication_path(context, path):
+    """Prepend the path of the user folder.
+
+    Because ++resource++zmi is protected, we generate URLs relative to the
+    user folder of the logged-in user, so that the user can access the
+    resources.
+    """
+    request = getattr(context, 'REQUEST', None)
+    if request is None:
+        return path
+    user_folder = aq_parent(getSecurityManager().getUser())
+    if user_folder is None:
+        return path
+    # prepend the authentication path, unless it is already part of the
+    # virtual host root.
+    authentication_path = []
+    ufpp = aq_parent(user_folder).getPhysicalPath()
+    vrpp = request.get("VirtualRootPhysicalPath") or ()
+    for ufp, vrp in itertools.zip_longest(ufpp, vrpp):
+        if ufp == vrp:
+            continue
+        authentication_path.append(ufp)
+
+    parts = [
+        p for p in itertools.chain(authentication_path, path.split("/")) if p]
+
+    return request.physicalPathToURL(parts, relative=True)
 
 
 @zope.component.adapter(zope.interface.Interface)
 def css_paths(context):
     """Return paths to CSS files needed for the Zope 4 ZMI."""
     return (
-        '/++resource++zmi/bootstrap-4.6.0/bootstrap.min.css',
-        '/++resource++zmi/fontawesome-free-5.15.2/css/all.css',
-        '/++resource++zmi/zmi_base.css',
+        prepend_authentication_path(context, path)
+        for path in (
+            '/++resource++zmi/bootstrap-4.6.0/bootstrap.min.css',
+            '/++resource++zmi/fontawesome-free-5.15.2/css/all.css',
+            '/++resource++zmi/zmi_base.css',
+        )
     )
 
 
@@ -16,8 +52,12 @@ def css_paths(context):
 def js_paths(context):
     """Return paths to JS files needed for the Zope 4 ZMI."""
     return (
-        '/++resource++zmi/jquery-3.5.1.min.js',
-        '/++resource++zmi/bootstrap-4.6.0/bootstrap.bundle.min.js',
-        '/++resource++zmi/ace.ajax.org/ace.js',
-        '/++resource++zmi/zmi_base.js',
+        prepend_authentication_path(context, path)
+        for path in (
+            '/++resource++zmi/jquery-3.5.1.min.js',
+            '/++resource++zmi/bootstrap-4.6.0/bootstrap.bundle.min.js',
+            '/++resource++zmi/ace.ajax.org/ace.js',
+            '/++resource++zmi/zmi_base.js',
+            '/++resource++zmi/zmi.localstorage.api.js',
+        )
     )
diff --git a/src/zmi/styles/tests.py b/src/zmi/styles/tests.py
index 36c7b65bec..c0c52d6aa1 100644
--- a/src/zmi/styles/tests.py
+++ b/src/zmi/styles/tests.py
@@ -1,4 +1,7 @@
+"""Testing .subscriber.*"""
+
 import Testing.ZopeTestCase
+from Products.SiteAccess.VirtualHostMonster import VirtualHostMonster
 from Testing.ZopeTestCase.placeless import temporaryPlacelessSetUp
 from Testing.ZopeTestCase.placeless import zcml
 from zope.security.management import endInteraction
@@ -19,9 +22,16 @@ def setupZCML():
 
 
 class SubscriberTests(Testing.ZopeTestCase.FunctionalTestCase):
-    """Testing .subscriber.*"""
+    """Test subscribers URL generation with a user from a user folder
+    not at the root.
+    """
+    request_path = f"/{Testing.ZopeTestCase.folder_name}/manage_main"
+    resources_base_path = f"/{Testing.ZopeTestCase.folder_name}"
 
-    base_path = f'/{Testing.ZopeTestCase.folder_name}'
+    def afterSetUp(self):
+        if "virtual_hosting" not in self.app:
+            vhm = VirtualHostMonster()
+            vhm.addToContainer(self.app)
 
     def call_manage_main(self):
         """Call /folder/manage_main and return the HTML text."""
@@ -30,9 +40,7 @@ def _call_manage_main(self):
             # temporaryPlacelessSetUp insists in creating an interaction
             # which the WSGI publisher does not expect.
             endInteraction()
-            response = self.publish(
-                f'{self.base_path}/manage_main',
-                basic=basic_auth)
+            response = self.publish(self.request_path, basic=basic_auth)
             return str(response)
         return temporaryPlacelessSetUp(
             _call_manage_main, required_zcml=setupZCML)(self)
@@ -42,11 +50,90 @@ def test_subscriber__css_paths__1(self):
         from .subscriber import css_paths
         body = self.call_manage_main()
         for path in css_paths(None):
-            self.assertIn(f'href="{self.base_path}{path}"', body)
+            self.assertIn(f'href="{self.resources_base_path}{path}"', body)
 
     def test_subscriber__js_paths__1(self):
         """The paths it returns are rendered in the ZMI."""
         from .subscriber import js_paths
         body = self.call_manage_main()
         for path in js_paths(None):
-            self.assertIn(f'src="{self.base_path}{path}"', body)
+            self.assertIn(f'src="{self.resources_base_path}{path}"', body)
+
+
+class SubscriberTestsUserFromRootUserFolderViewingRootFolder(SubscriberTests):
+    """Tests subscribers URL generation with a user from the root acl_users,
+    viewing the root of ZMI.
+    """
+
+    request_path = "/manage_main"
+    resources_base_path = ""
+
+    def _setupFolder(self):
+        self.folder = self.app
+
+    def _setupUserFolder(self):
+        # we use the user folder from self.app
+        pass
+
+
+class SubscriberTestsUserFromRootUserFolderViewingFolder(SubscriberTests):
+    """Tests subscribers URL generation with a user from the root acl_users,
+    viewing a folder not at the root of ZMI. In such case, the URLs are not
+    relative to that folder, the resources are served from the root.
+    """
+
+    request_path = f"/{Testing.ZopeTestCase.folder_name}/manage_main"
+    resources_base_path = ""
+
+    def _setupUser(self):
+        uf = self.app.acl_users
+        uf.userFolderAddUser(
+            Testing.ZopeTestCase.user_name,
+            Testing.ZopeTestCase.user_password,
+            ["Manager"],
+            [],
+        )
+
+    def setRoles(self, roles, name=...):
+        # we set roles in _setupUser
+        pass
+
+    def login(self):
+        pass
+
+
+class SubscriberUrlWithVirtualHostingTests(SubscriberTests):
+    """Tests subscribers URL generation using virtual host."""
+
+    request_path = (
+        "/VirtualHostBase/https/example.org:443/VirtualHostRoot/"
+        f"{Testing.ZopeTestCase.folder_name}/manage_main"
+    )
+    resources_base_path = f"/{Testing.ZopeTestCase.folder_name}"
+
+
+class SubscriberUrlWithVirtualHostingAndUserFolderInVirtualHostTests(
+        SubscriberTests):
+    """Tests subscribers URL generation using virtual host, when
+    the authentication path is part of the virtual host base.
+    """
+
+    request_path = (
+        "/VirtualHostBase/https/example.org:443/"
+        f"{Testing.ZopeTestCase.folder_name}/VirtualHostRoot/manage_main"
+    )
+    resources_base_path = ""
+
+
+class SubscriberUrlWithVirtualHostingAndVHTests(SubscriberTests):
+    """Tests subscribers URL generation using virtual host, when
+    the authentication path is part of the virtual host base and
+    using a "_vh_" path element.
+    """
+
+    request_path = (
+        "/VirtualHostBase/https/example.org:443"
+        f"/{Testing.ZopeTestCase.folder_name}/"
+        "VirtualHostRoot/_vh_zz/manage_main"
+    )
+    resources_base_path = "/zz"