From 49d8b1ccd8178d29465aa6a0640e518381114158 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Thu, 7 May 2020 14:35:37 +0200
Subject: [PATCH] [BUGFIX] Revert PageReadPermission check for TreeController

In order to allow non-admins to fetch nodes which have no "pid=0"
the change to only fetch pages with access, the change to
check on a DB query basis is reverted.

Additionally a functional tests is extended to cover the problematic case.

Resolves: #91348
Related: #90880
Releases: master, 9.5
Change-Id: I3f737c92c8164c572f7e58335d92a82a4a5aa4dc
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/64431
Tested-by: Tymoteusz Motylewski <t.motylewski@gmail.com>
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Tymoteusz Motylewski <t.motylewski@gmail.com>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Benni Mack <benni@typo3.org>
---
 .../Controller/Page/TreeController.php        |  8 +++----
 .../Page/Fixtures/PagesWithBEPermissions.yaml | 15 ++++++++++---
 .../Controller/Page/TreeControllerTest.php    | 22 +++++++++++++++++++
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/typo3/sysext/backend/Classes/Controller/Page/TreeController.php b/typo3/sysext/backend/Classes/Controller/Page/TreeController.php
index 4eb082647f05..5dacf7a8d98c 100644
--- a/typo3/sysext/backend/Classes/Controller/Page/TreeController.php
+++ b/typo3/sysext/backend/Classes/Controller/Page/TreeController.php
@@ -23,9 +23,7 @@ use TYPO3\CMS\Backend\Configuration\BackendUserConfiguration;
 use TYPO3\CMS\Backend\Tree\Repository\PageTreeRepository;
 use TYPO3\CMS\Backend\Utility\BackendUtility;
 use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
-use TYPO3\CMS\Core\Context\Context;
 use TYPO3\CMS\Core\Database\Query\Restriction\DocumentTypeExclusionRestriction;
-use TYPO3\CMS\Core\Database\Query\Restriction\PagePermissionRestriction;
 use TYPO3\CMS\Core\Exception\Page\RootLineException;
 use TYPO3\CMS\Core\Exception\SiteNotFoundException;
 use TYPO3\CMS\Core\Http\JsonResponse;
@@ -373,7 +371,6 @@ class TreeController
         if (!empty($excludedDocumentTypes)) {
             $additionalQueryRestrictions[] = GeneralUtility::makeInstance(DocumentTypeExclusionRestriction::class, $excludedDocumentTypes);
         }
-        $additionalQueryRestrictions[] = GeneralUtility::makeInstance(PagePermissionRestriction::class, GeneralUtility::makeInstance(Context::class)->getAspect('backend.user'), Permission::PAGE_SHOW);
 
         $repository = GeneralUtility::makeInstance(
             PageTreeRepository::class,
@@ -419,7 +416,10 @@ class TreeController
                 }
             }
 
-            $entryPoint = $repository->getTree($entryPoint);
+            $entryPoint = $repository->getTree($entryPoint, function ($page) use ($backendUser) {
+                // Check each page if the user has permission to access it
+                return $backendUser->doesUserHaveAccess($page, Permission::PAGE_SHOW);
+            });
             if (!is_array($entryPoint)) {
                 unset($entryPoints[$k]);
             }
diff --git a/typo3/sysext/backend/Tests/Functional/Controller/Page/Fixtures/PagesWithBEPermissions.yaml b/typo3/sysext/backend/Tests/Functional/Controller/Page/Fixtures/PagesWithBEPermissions.yaml
index a5eb0a3c589e..a05ef07ca426 100644
--- a/typo3/sysext/backend/Tests/Functional/Controller/Page/Fixtures/PagesWithBEPermissions.yaml
+++ b/typo3/sysext/backend/Tests/Functional/Controller/Page/Fixtures/PagesWithBEPermissions.yaml
@@ -52,7 +52,7 @@ entities:
     - self: {id: 2, title: 'Franco-Canadian', code: 'fr'}
     - self: {id: 3, title: 'Spanish', code: 'es'}
   beGroup:
-    - self: {id: 9, title: 'editors', db_mountpoints: '1000,2000', tables_select: 'pages,tt_content', tables_modify: 'pages,tt_content', page_types_select: '1,4,7,254', groupMods: 'web_layout,web_list'}
+    - self: {id: 9, title: 'editors', db_mountpoints: '1000,2000,8110', tables_select: 'pages,tt_content', tables_modify: 'pages,tt_content', page_types_select: '1,4,7,254', groupMods: 'web_layout,web_list'}
   page:
     - self: {id: *idAcmeRootPage, title: 'ACME Inc', type: *pageShortcut, shortcut: 'first', root: true, slug: '/'}
       children:
@@ -116,7 +116,7 @@ entities:
               - self: {id: 1, username: 'john@doe.local', groups: '10'}
               - self: {id: 2, username: 'manager@other-inc.local', groups: '20'}
               - self: {id: 3, username: 'big-boss@acme-inc.local', groups: '10,20'}
-    - self: {id: 2000, title: 'ACME Blog', type: *pageShortcut, shortcut: 'first', root: true, slug: '/', perms_userid: 1, perms_groupid: 1, description: "not accessible"}
+    - self: {id: 2000, title: 'ACME Blog', type: *pageShortcut, shortcut: 'first', root: true, slug: '/', perms_userid: 1, perms_groupid: 1, description: 'not accessible'}
       children:
         - self: {id: 2100, title: 'Authors', slug: '/authors'}
           children:
@@ -128,10 +128,19 @@ entities:
                 - self: {id: 2121, title: 'About', slug: '/about-jane'}
         - self: {id: 2700, title: 'Announcements & News', type: *pageMount, mount: 7100, slug: '/news'}
         - self: {id: 2930, title: 'ACME Inc', type: *pageShortcut, shortcut: 1000, slug: '/acme'}
-    - self: {id: 7000, title: 'Common Collection', type: *pageFolder, root: true, slug: '/common'}
+    - self: {id: 7000, title: 'Common Collection', type: *pageFolder, root: true, slug: '/common', description: 'not visible as not mounted'}
       children:
         - self: {id: 7100, title: 'Announcements & News', slug: '/common/news'}
           children:
             - self: {id: 7110, title: 'Markets', slug: '/common/markets'}
             - self: {id: 7120, title: 'Products', slug: '/common/products'}
             - self: {id: 7130, title: 'Partners', slug: '/common/partners'}
+    - self: {id: 8000, title: 'Divisions', type: *pageFolder, root: true, slug: '/divisions', perms_userid: 1, perms_groupid: 1, description: 'not accessible'}
+      children:
+        - self: {id: 8100, title: 'EMEA', slug: '/divisions/emea', perms_userid: 1, perms_groupid: 1}
+          children:
+            - self: {id: 8110, title: 'Europe', slug: '/divisions/emea/europe', description: 'mounted, thus visible'}
+              children:
+                - self: {id: 811000, title: 'France', slug: '/divisions/emea/europe/france'}
+            - self: {id: 8120, title: 'Asia', slug: '/divisions/asia', description: 'not visible as not mounted'}
+            - self: {id: 8130, title: 'South America', slug: '/divisions/south-america'}
diff --git a/typo3/sysext/backend/Tests/Functional/Controller/Page/TreeControllerTest.php b/typo3/sysext/backend/Tests/Functional/Controller/Page/TreeControllerTest.php
index 17043525328a..e066862f370d 100644
--- a/typo3/sysext/backend/Tests/Functional/Controller/Page/TreeControllerTest.php
+++ b/typo3/sysext/backend/Tests/Functional/Controller/Page/TreeControllerTest.php
@@ -215,6 +215,17 @@ class TreeControllerTest extends FunctionalTestCase
                     ],
                 ],
             ],
+            [
+                'uid' => 8110,
+                'title' => 'Europe',
+                '_children' => [
+                    [
+                        'uid' => 811000,
+                        'title' => 'France',
+                        '_children' => [],
+                    ],
+                ],
+            ],
         ];
         self::assertEquals($expected, $actual);
     }
@@ -370,6 +381,17 @@ class TreeControllerTest extends FunctionalTestCase
                     ],
                 ],
             ],
+            [
+                'uid' => 8110,
+                'title' => 'Europe',
+                '_children' => [
+                    [
+                        'uid' => 811000,
+                        'title' => 'France',
+                        '_children' => [],
+                    ],
+                ],
+            ],
         ];
         self::assertEquals($expected, $actual);
     }
-- 
GitLab