diff --git a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php index 909f293071c16778c91eb5649a9a0c5ab7c3e954..daf5bacaedeba333c8ebdd66000384cf5c6bdcc1 100644 --- a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php +++ b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php @@ -16,6 +16,7 @@ namespace TYPO3\CMS\Core\Authentication; use TYPO3\CMS\Backend\Utility\BackendUtility; use TYPO3\CMS\Core\Database\ConnectionPool; +use TYPO3\CMS\Core\Database\Query\Expression\ExpressionBuilder; use TYPO3\CMS\Core\Database\Query\QueryHelper; use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction; use TYPO3\CMS\Core\Database\Query\Restriction\HiddenRestriction; @@ -459,27 +460,58 @@ class BackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\AbstractU if ($this->isAdmin()) { return ' 1=1'; } - $perms = (int)$perms; // Make sure it's integer. - $str = ' (' . '(pages.perms_everybody & ' . $perms . ' = ' . $perms . ')' . ' OR (pages.perms_userid = ' - . $this->user['uid'] . ' AND pages.perms_user & ' . $perms . ' = ' . $perms . ')'; + $perms = (int)$perms; + $expressionBuilder = GeneralUtility::makeInstance(ConnectionPool::class) + ->getQueryBuilderForTable('pages') + ->expr(); + // User + $constraint = $expressionBuilder->orX( + $expressionBuilder->comparison( + $expressionBuilder->bitAnd('pages.perms_everybody', $perms), + ExpressionBuilder::EQ, + $perms + ), + $expressionBuilder->andX( + $expressionBuilder->eq('pages.perms_userid', (int)$this->user['uid']), + $expressionBuilder->comparison( + $expressionBuilder->bitAnd('pages.perms_user', $perms), + ExpressionBuilder::EQ, + $perms + ) + ) + ); + + // Group (if any is set) if ($this->groupList) { - // Group (if any is set) - $str .= ' OR (pages.perms_groupid in (' . $this->groupList . ') AND pages.perms_group & ' - . $perms . ' = ' . $perms . ')'; + $constraint->add( + $expressionBuilder->andX( + $expressionBuilder->in( + 'pages.perms_groupid', + GeneralUtility::intExplode(',', $this->groupList) + ), + $expressionBuilder->comparison( + $expressionBuilder->bitAnd('pages.perms_group', $perms), + ExpressionBuilder::EQ, + $perms + ) + ) + ); } - $str .= ')'; + + $constraint = ' (' . (string)$constraint . ')'; + // **************** // getPagePermsClause-HOOK // **************** if (is_array($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_userauthgroup.php']['getPagePermsClause'])) { foreach ($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_userauthgroup.php']['getPagePermsClause'] as $_funcRef) { - $_params = array('currentClause' => $str, 'perms' => $perms); + $_params = array('currentClause' => $constraint, 'perms' => $perms); $str = GeneralUtility::callUserFunction($_funcRef, $_params, $this); } } - return $str; + return $constraint; } else { return ' 1=0'; } diff --git a/typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php b/typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php index e664c4e7148d243a2f4f84626d47653f4ba3279e..e5ecd2701c8eb2ba9d0321497c732a9b8688b82d 100644 --- a/typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php +++ b/typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php @@ -20,6 +20,9 @@ use TYPO3\CMS\Core\Authentication\BackendUserAuthentication; use TYPO3\CMS\Core\Database\Connection; use TYPO3\CMS\Core\Database\ConnectionPool; use TYPO3\CMS\Core\Database\DatabaseConnection; +use TYPO3\CMS\Core\Database\Query\Expression\ExpressionBuilder; +use TYPO3\CMS\Core\Database\Query\QueryBuilder; +use TYPO3\CMS\Core\Tests\Unit\Database\Mocks\MockPlatform; use TYPO3\CMS\Core\Type\Bitmask\JsConfirmation; use TYPO3\CMS\Core\Utility\GeneralUtility; @@ -716,4 +719,93 @@ class BackendUserAuthenticationTest extends \TYPO3\CMS\Core\Tests\UnitTestCase $this->assertTrue($subject->jsConfirmation(JsConfirmation::TYPE_CHANGE)); } + + /** + * Data provider to test page permissions constraints + * returns an array of test conditions: + * - permission bit(s) as integer + * - admin flag + * - groups for user + * - expected SQL fragment + * + * @return array + */ + public function getPagePermissionsClauseWithValidUserDataProvider(): array + { + return [ + 'for admin' => [ + 1, + true, + '', + ' 1=1' + ], + 'for admin with groups' => [ + 11, + true, + '1,2', + ' 1=1' + ], + 'for user' => [ + 2, + false, + '', + ' ((`pages`.`perms_everybody` & 2 = 2) OR' . + ' ((`pages`.`perms_userid` = 123) AND (`pages`.`perms_user` & 2 = 2)))' + ], + 'for user with groups' => [ + 8, + false, + '1,2', + ' ((`pages`.`perms_everybody` & 8 = 8) OR' . + ' ((`pages`.`perms_userid` = 123) AND (`pages`.`perms_user` & 8 = 8))' . + ' OR ((`pages`.`perms_groupid` IN (1, 2)) AND (`pages`.`perms_group` & 8 = 8)))' + ], + ]; + } + + /** + * @test + * @dataProvider getPagePermissionsClauseWithValidUserDataProvider + * @param int $perms + * @param bool $admin + * @param string $groups + * @param string $expected + */ + public function getPagePermissionsClauseWithValidUser(int $perms, bool $admin, string $groups, string $expected) + { + // We only need to setup the mocking for the non-admin cases + // If this setup is done for admin cases the FIFO behavior + // of GeneralUtility::addInstance will influence other tests + // as the ConnectionPool is never used! + if (!$admin) { + /** @var Connection|ObjectProphecy $connectionProphet */ + $connectionProphet = $this->prophesize(Connection::class); + $connectionProphet->getDatabasePlatform()->willReturn(new MockPlatform()); + $connectionProphet->quoteIdentifier(Argument::cetera())->will(function ($args) { + return '`' . str_replace('.', '`.`', $args[0]) . '`'; + }); + + /** @var QueryBuilder|ObjectProphecy $queryBuilderProphet */ + $queryBuilderProphet = $this->prophesize(QueryBuilder::class); + $queryBuilderProphet->expr()->willReturn( + GeneralUtility::makeInstance(ExpressionBuilder::class, $connectionProphet->reveal()) + ); + + /** @var ConnectionPool|ObjectProphecy $databaseProphet */ + $databaseProphet = $this->prophesize(ConnectionPool::class); + $databaseProphet->getQueryBuilderForTable('pages')->willReturn($queryBuilderProphet->reveal()); + GeneralUtility::addInstance(ConnectionPool::class, $databaseProphet->reveal()); + } + + /** @var BackendUserAuthentication|\PHPUnit_Framework_MockObject_MockObject $subject */ + $subject = $this->getMock(BackendUserAuthentication::class, ['isAdmin']); + $subject->expects($this->any()) + ->method('isAdmin') + ->will($this->returnValue($admin)); + + $subject->user = ['uid' => 123]; + $subject->groupList = $groups; + + $this->assertEquals($expected, $subject->getPagePermsClause($perms)); + } }