[TASK] Doctrine: Migrate BackendUserAuthentication::getPagePermsClause 46/48446/2
authorMorton Jonuschat <m.jonuschat@mojocode.de>
Sat, 4 Jun 2016 09:10:40 +0000 (11:10 +0200)
committerFrank Naegler <frank.naegler@typo3.org>
Mon, 6 Jun 2016 13:02:38 +0000 (15:02 +0200)
Migrate the getPagePermsClause() method to Doctrine and add Tests.

Change-Id: Ib8ac3dd557ecd011f8a37c9f4a7f86a861ded4a9
Resolves: #75556
Releases: master
Reviewed-on: https://review.typo3.org/48446
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
Reviewed-by: Frank Naegler <frank.naegler@typo3.org>
Tested-by: Frank Naegler <frank.naegler@typo3.org>
typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php
typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php

index 909f293..daf5bac 100644 (file)
@@ -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';
         }
index e664c4e..e5ecd27 100644 (file)
@@ -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));
+    }
 }