Skip to content

Commit

Permalink
[BUGFIX] Include Page Read Permissions to PageTreeController
Browse files Browse the repository at this point in the history
In order to reduce the amount of data fetched, the PageTree loaded
for editors will only fetch pages that the editor actually sees, reducing
the actual amount of pages to fetch.

If a pagetree has 80.000 pages, but the editor can only see 50, the
tree only loads these 50 pages.

Resolves: #90880
Releases: master, 9.5
Change-Id: I57985484ace07fbfb919573351c210b291697ae3
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/63983
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Susanne Moog <look@susi.dev>
Reviewed-by: Susanne Moog <look@susi.dev>
  • Loading branch information
bmack authored and susannemoog committed Apr 7, 2020
1 parent 548a10a commit 685e9a2
Show file tree
Hide file tree
Showing 3 changed files with 264 additions and 17 deletions.
23 changes: 6 additions & 17 deletions typo3/sysext/backend/Classes/Controller/Page/TreeController.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
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;
Expand Down Expand Up @@ -357,14 +359,14 @@ protected function pagesToFlatArray(array $page, int $entryPoint, int $depth = 0
protected function getAllEntryPointPageTrees(): array
{
$backendUser = $this->getBackendUser();

$userTsConfig = $this->getBackendUser()->getTSConfig();
$userTsConfig = $backendUser->getTSConfig();
$excludedDocumentTypes = GeneralUtility::intExplode(',', $userTsConfig['options.']['pageTree.']['excludeDoktypes'] ?? '', true);

$additionalQueryRestrictions = [];
if (!empty($excludedDocumentTypes)) {
$additionalQueryRestrictions[] = $this->retrieveDocumentTypeExclusionRestriction($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,
Expand Down Expand Up @@ -411,7 +413,7 @@ protected function getAllEntryPointPageTrees(): array
}

$entryPoint = $repository->getTree($entryPoint, function ($page) use ($backendUser) {
// check each page if the user has permission to access it
// Check each page if the user has permission to access it
return $backendUser->doesUserHaveAccess($page, Permission::PAGE_SHOW);
});
if (!is_array($entryPoint)) {
Expand All @@ -422,19 +424,6 @@ protected function getAllEntryPointPageTrees(): array
return $entryPoints;
}

/**
* @param int[] $excludedDocumentTypes
* @return DocumentTypeExclusionRestriction|null
*/
protected function retrieveDocumentTypeExclusionRestriction(array $excludedDocumentTypes): ?DocumentTypeExclusionRestriction
{
if (empty($excludedDocumentTypes)) {
return null;
}

return GeneralUtility::makeInstance(DocumentTypeExclusionRestriction::class, $excludedDocumentTypes);
}

/**
* Returns the first configured domain name for a page
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
<?php
declare(strict_types = 1);
namespace TYPO3\CMS\Core\Database\Query\Restriction;

/*
* This file is part of the TYPO3 CMS project.
*
* It is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License, either version 2
* of the License, or any later version.
*
* For the full copyright and license information, please read the
* LICENSE.txt file that was distributed with this source code.
*
* The TYPO3 project - inspiring people to share!
*/

use TYPO3\CMS\Core\Context\UserAspect;
use TYPO3\CMS\Core\Database\Query\Expression\CompositeExpression;
use TYPO3\CMS\Core\Database\Query\Expression\ExpressionBuilder;

/**
* Restriction to make queries respect backend user rights for pages.
*
* Adds a WHERE-clause for the pages-table where user permissions according to input argument, $permissions, is validated.
* $permissions is the "mask" used to select - see Permission Bitset.
* E.g. if $perms is 1 then you'll get all pages that a user can actually see!
* 2^0 = show (1)
* 2^1 = edit (2)
* 2^2 = delete (4)
* 2^3 = new (8)
* If the user is 'admin' no validation is used.
*
* If the user is not set at all (->user is not an array), then "AND 1=0" is returned (will cause no selection results at all)
*
* The 95% use of this function is "->getPagePermsClause(1)" which will
* return WHERE clauses for *selecting* pages in backend listings - in other words this will check read permissions.
*/
class PagePermissionRestriction implements QueryRestrictionInterface
{
/**
* @var int
*/
protected $permissions;

/**
* @var UserAspect
*/
protected $userAspect;

public function __construct(UserAspect $userAspect, int $permissions)
{
$this->permissions = $permissions;
$this->userAspect = $userAspect;
}

/**
* Main method to build expressions for given tables
*
* @param array $queriedTables Array of tables, where array key is table alias and value is a table name
* @param ExpressionBuilder $expressionBuilder Expression builder instance to add restrictions with
* @return CompositeExpression The result of query builder expression(s)
*/
public function buildExpression(array $queriedTables, ExpressionBuilder $expressionBuilder): CompositeExpression
{
$constraints = [];

foreach ($queriedTables as $tableAlias => $tableName) {
if ($tableName !== 'pages') {
continue;
}

$constraint = $this->buildUserConstraints($expressionBuilder, $tableAlias);
if ($constraint) {
$constraints[] = $expressionBuilder->andX($constraint);
}
}

return $expressionBuilder->andX(...$constraints);
}

/**
* @param ExpressionBuilder $expressionBuilder
* @param string $tableAlias
* @return string|CompositeExpression|null
* @throws \TYPO3\CMS\Core\Context\Exception\AspectPropertyNotFoundException
*/
protected function buildUserConstraints(ExpressionBuilder $expressionBuilder, string $tableAlias)
{
if (!$this->userAspect->isLoggedIn()) {
return $expressionBuilder->comparison(1, ExpressionBuilder::EQ, 0);
}
if ($this->userAspect->isAdmin()) {
return null;
}
// User permissions
$constraint = $expressionBuilder->orX(
$expressionBuilder->comparison(
$expressionBuilder->bitAnd($tableAlias . '.perms_everybody', $this->permissions),
ExpressionBuilder::EQ,
$this->permissions
),
$expressionBuilder->andX(
$expressionBuilder->eq($tableAlias . '.perms_userid', $this->userAspect->get('id')),
$expressionBuilder->comparison(
$expressionBuilder->bitAnd($tableAlias . '.perms_user', $this->permissions),
ExpressionBuilder::EQ,
$this->permissions
)
)
);

// User groups (if any are set)
$groupIds = array_map('intval', $this->userAspect->getGroupIds());
if (!empty($groupIds)) {
$constraint->add(
$expressionBuilder->andX(
$expressionBuilder->in(
$tableAlias . '.perms_groupid',
$groupIds
),
$expressionBuilder->comparison(
$expressionBuilder->bitAnd($tableAlias . '.perms_group', $this->permissions),
ExpressionBuilder::EQ,
$this->permissions
)
)
);
}
return $constraint;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
<?php
declare(strict_types = 1);
namespace TYPO3\CMS\Core\Tests\Unit\Database\Query\Restriction;

/*
* This file is part of the TYPO3 CMS project.
*
* It is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License, either version 2
* of the License, or any later version.
*
* For the full copyright and license information, please read the
* LICENSE.txt file that was distributed with this source code.
*
* The TYPO3 project - inspiring people to share!
*/

use TYPO3\CMS\Core\Context\UserAspect;
use TYPO3\CMS\Core\Database\Query\Restriction\PagePermissionRestriction;
use TYPO3\CMS\Core\Type\Bitmask\Permission;

class PagePermissionRestrictionTest extends AbstractRestrictionTestCase
{

/**
* Builds a shell for the user aspect object which returns the checked values in the Restriction.
*
* @param bool $isLoggedIn
* @param bool $isAdmin
* @param int $userId
* @param array $groupIds
* @return UserAspect
*/
protected function getPreparedUserAspect(bool $isLoggedIn, bool $isAdmin, int $userId, array $groupIds): UserAspect
{
return new class($isLoggedIn, $isAdmin, $userId, $groupIds) extends UserAspect {
private $isAdmin;
private $isLoggedIn;
private $userId;
private $groupIds;
public function __construct(bool $isLoggedIn, bool $isAdmin, int $userId, array $groupIds)
{
$this->isLoggedIn = $isLoggedIn;
$this->isAdmin = $isAdmin;
$this->userId = $userId;
$this->groupIds = $groupIds;
}
public function isAdmin(): bool
{
return $this->isAdmin;
}
public function isLoggedIn(): bool
{
return $this->isLoggedIn;
}
public function get($name)
{
if ($name === 'id') {
return $this->userId;
}
return parent::get($name);
}
public function getGroupIds(): array
{
return $this->groupIds;
}
};
}

/**
* @test
*/
public function buildRestrictionsOnlyWorksOnPagesTable()
{
$aspect = $this->getPreparedUserAspect(true, false, 2, [13]);
$subject = new PagePermissionRestriction($aspect, Permission::PAGE_SHOW);
$expression = $subject->buildExpression(['pages' => 'pages'], $this->expressionBuilder);
self::assertNotEmpty((string)$expression);
$expression = $subject->buildExpression(['anotherTable' => 'anotherTable'], $this->expressionBuilder);
self::assertEmpty((string)$expression);
}

/**
* @test
*/
public function buildRestrictionsReturnsAZeroReturnSetWhenNotLoggedIn()
{
$aspect = $this->getPreparedUserAspect(false, false, 2, [13]);
$subject = new PagePermissionRestriction($aspect, Permission::PAGE_SHOW);
$expression = $subject->buildExpression(['pages' => 'pages'], $this->expressionBuilder);
self::assertSame('1 = 0', (string)$expression);
}

/**
* @test
*/
public function buildRestrictionsIsSkippedForAdmins()
{
$aspect = $this->getPreparedUserAspect(true, true, 2, [13]);
$subject = new PagePermissionRestriction($aspect, Permission::PAGE_SHOW);
$expression = $subject->buildExpression(['pages' => 'pages'], $this->expressionBuilder);
self::assertEmpty((string)$expression);
}

/**
* @test
*/
public function buildRestrictionsContainsNonAdminUserIdAsOwnerAndGroupIdsAsOwnerGroup()
{
$aspect = $this->getPreparedUserAspect(true, false, 2, [13, 14, 15, 16]);
$subject = new PagePermissionRestriction($aspect, Permission::PAGE_SHOW);
$expression = $subject->buildExpression(['pages' => 'pages'], $this->expressionBuilder);
self::assertEquals('("pages"."perms_everybody" & 1 = 1) OR (("pages"."perms_userid" = 2) AND ("pages"."perms_user" & 1 = 1)) OR (("pages"."perms_groupid" IN (13, 14, 15, 16)) AND ("pages"."perms_group" & 1 = 1))', (string)$expression);
}

/**
* @test
*/
public function buildRestrictionsChecksForDeletionPermission()
{
$aspect = $this->getPreparedUserAspect(true, false, 42, [13, 14, 15, 16]);
$subject = new PagePermissionRestriction($aspect, Permission::PAGE_DELETE);
$expression = $subject->buildExpression(['pages' => 'pages'], $this->expressionBuilder);
self::assertEquals('("pages"."perms_everybody" & 4 = 4) OR (("pages"."perms_userid" = 42) AND ("pages"."perms_user" & 4 = 4)) OR (("pages"."perms_groupid" IN (13, 14, 15, 16)) AND ("pages"."perms_group" & 4 = 4))', (string)$expression);
}
}

0 comments on commit 685e9a2

Please sign in to comment.