[BUGFIX] Fix file permission methods in BackendUserAuthentication 37/23037/6
authorHelmut Hummel <helmut.hummel@typo3.org>
Wed, 7 Aug 2013 16:33:37 +0000 (18:33 +0200)
committerHelmut Hummel <helmut.hummel@typo3.org>
Thu, 15 Aug 2013 09:32:25 +0000 (11:32 +0200)
Take default TSConfig file permissions into account:

File permissions configured in User TSConfig,
are not taken into account, because the properties
from the getTSConfig method are returned in a
'properties' key of an array but the top level
array is used. Use getTSConfigProp instead.

Fix the bit wise check for old file permissions:

XOR the value is wrong because it gives the wrong
result if more bits are set. Use AND operation
to properly check if a specific bit is not set.

Check if the user is admin in getFilePermissionsForStorage()

Add tests to confirm the desired behaviour.

Releases: 6.0, 6.1, 6.2
Resolves: #51004
Change-Id: Ia5d6fa1cb47a74306fe5465a0e70c2f2aea2a4b8
Reviewed-on: https://review.typo3.org/23037
Reviewed-by: Anja Leichsenring
Tested-by: Anja Leichsenring
Reviewed-by: Stefan Neufeind
Reviewed-by: Helmut Hummel
Tested-by: Helmut Hummel
typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php
typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php

index ef804e0..e3e0374 100644 (file)
@@ -1561,6 +1561,7 @@ class BackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\AbstractU
         *
         * addFolder = 1
         * readFolder = 1
+        * copyFolder = 1
         * moveFolder = 1
         * writeFolder = 1
         * renameFolder = 1
@@ -1583,36 +1584,35 @@ class BackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\AbstractU
        public function getFilePermissions() {
                if (!isset($this->filePermissions)) {
                        $defaultOptions = array(
+                               // File permissions
                                'addFile' => TRUE,
-                               // new option
                                'readFile' => TRUE,
-                               // new option, generic check of the user rights
                                'editFile' => TRUE,
-                               // new option
                                'writeFile' => TRUE,
-                               // new option, generic check of the user rights
                                'uploadFile' => TRUE,
                                'copyFile' => TRUE,
                                'moveFile' => TRUE,
                                'renameFile' => TRUE,
                                'unzipFile' => TRUE,
                                'removeFile' => TRUE,
+                               // Folder permissions
                                'addFolder' => TRUE,
                                'readFolder' => TRUE,
-                               // new option,, generic check of the user rights
+                               'copyFolder' => TRUE,
                                'moveFolder' => TRUE,
                                'renameFolder' => TRUE,
                                'writeFolder' => TRUE,
-                               // new option, generic check of the user rights
                                'removeFolder' => TRUE,
                                'removeSubfolders' => TRUE
                        );
                        if (!$this->isAdmin()) {
-                               $this->filePermissions = $this->getTSConfig('permissions.file.default');
-                               if (empty($this->filePermissions)) {
+                               $defaultPermissionsTsConfig = $this->getTSConfigProp('permissions.file.default');
+                               if (!empty($defaultPermissionsTsConfig)) {
+                                       $defaultOptions = $defaultPermissionsTsConfig;
+                               } else {
                                        $oldFileOperationPermissions = $this->getFileoperationPermissions();
                                        // Lower permissions if the old file operation permissions are not set
-                                       if ($oldFileOperationPermissions ^ 1) {
+                                       if (!($oldFileOperationPermissions & 1)) {
                                                $defaultOptions['addFile'] = FALSE;
                                                $defaultOptions['uploadFile'] = FALSE;
                                                $defaultOptions['copyFile'] = FALSE;
@@ -1622,20 +1622,20 @@ class BackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\AbstractU
                                                $defaultOptions['editFile'] = FALSE;
                                                $defaultOptions['writeFile'] = FALSE;
                                        }
-                                       if ($oldFileOperationPermissions ^ 2) {
+                                       if (!($oldFileOperationPermissions & 2)) {
                                                $defaultOptions['unzipFile'] = FALSE;
                                        }
-                                       if ($oldFileOperationPermissions ^ 4) {
+                                       if (!($oldFileOperationPermissions & 4)) {
                                                $defaultOptions['addFolder'] = FALSE;
                                                $defaultOptions['moveFolder'] = FALSE;
                                                $defaultOptions['renameFolder'] = FALSE;
                                                $defaultOptions['removeFolder'] = FALSE;
                                                $defaultOptions['writeFolder'] = FALSE;
                                        }
-                                       if ($oldFileOperationPermissions ^ 8) {
+                                       if (!($oldFileOperationPermissions & 8)) {
                                                $defaultOptions['copyFolder'] = FALSE;
                                        }
-                                       if ($oldFileOperationPermissions ^ 16) {
+                                       if (!($oldFileOperationPermissions & 16)) {
                                                $defaultOptions['removeSubfolders'] = FALSE;
                                        }
                                }
@@ -1648,21 +1648,22 @@ class BackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\AbstractU
        /**
         * Gets the file permissions for a storage
         * by merging any storage-specific permissions for a
-        * storage with the default settings
+        * storage with the default settings.
+        * Admin users will always get the default settings.
         *
         * @api
         * @param \TYPO3\CMS\Core\Resource\ResourceStorage $storageObject
         * @return array
         */
        public function getFilePermissionsForStorage(\TYPO3\CMS\Core\Resource\ResourceStorage $storageObject) {
-               $defaultFilePermissions = $this->getFilePermissions();
-               $storagePermissionsArray = $this->getTSConfig('permissions.file.storage.' . $storageObject->getUid());
-               $storageFilePermissions = $storagePermissionsArray['properties'];
-               if (is_array($storageFilePermissions) && count($storageFilePermissions)) {
-                       return array_merge($defaultFilePermissions, $storageFilePermissions);
-               } else {
-                       return $defaultFilePermissions;
+               $finalUserPermissions = $this->getFilePermissions();
+               if (!$this->isAdmin()) {
+                       $storageFilePermissions = $this->getTSConfigProp('permissions.file.storage.' . $storageObject->getUid());
+                       if (!empty($storageFilePermissions)) {
+                               $finalUserPermissions = array_merge($finalUserPermissions, $storageFilePermissions);
+                       }
                }
+               return $finalUserPermissions;
        }
 
        /**
index 5fafd51..9b136d4 100644 (file)
@@ -212,6 +212,474 @@ class BackendUserAuthenticationTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                $this->assertSame($expectedConfiguration, $actualConfiguration);
        }
 
+       /**
+        * @return array
+        */
+       public function getFilePermissionsTakesUserDefaultAndStoragePermissionsIntoAccountIfUserIsNotAdminDataProvider() {
+               return array(
+                       'Only read permissions' => array(
+                               array(
+                                       'addFile' => 0,
+                                       'readFile' => 1,
+                                       'editFile' => 0,
+                                       'writeFile' => 0,
+                                       'uploadFile' => 0,
+                                       'copyFile' => 0,
+                                       'moveFile' => 0,
+                                       'renameFile' => 0,
+                                       'unzipFile' => 0,
+                                       'removeFile' => 0,
+                                       'addFolder' => 0,
+                                       'readFolder' => 1,
+                                       'copyFolder' => 0,
+                                       'moveFolder' => 0,
+                                       'renameFolder' => 0,
+                                       'writeFolder' => 0,
+                                       'removeFolder' => 0,
+                                       'removeSubfolders' => 0
+                               )
+                       ),
+                       'Uploading allowed' => array(
+                               array(
+                                       'addFile' => 1,
+                                       'readFile' => 1,
+                                       'editFile' => 1,
+                                       'writeFile' => 1,
+                                       'uploadFile' => 1,
+                                       'copyFile' => 1,
+                                       'moveFile' => 1,
+                                       'renameFile' => 1,
+                                       'unzipFile' => 0,
+                                       'removeFile' => 1,
+                                       'addFolder' => 0,
+                                       'readFolder' => 1,
+                                       'copyFolder' => 0,
+                                       'moveFolder' => 0,
+                                       'renameFolder' => 0,
+                                       'writeFolder' => 0,
+                                       'removeFolder' => 0,
+                                       'removeSubfolders' => 0
+                               )
+                       ),
+                       'One value is enough' => array(
+                               array(
+                                       'addFile' => 1,
+                               )
+                       ),
+               );
+       }
+
+       /**
+        * @param array $expectedPermissions
+        * @test
+        * @dataProvider getFilePermissionsTakesUserDefaultAndStoragePermissionsIntoAccountIfUserIsNotAdminDataProvider
+        */
+       public function getFilePermissionsTakesUserDefaultPermissionsFromTsConfigIntoAccountIfUserIsNotAdmin(array $expectedPermissions) {
+               $this->fixture = $this->getMock('TYPO3\\CMS\\Core\\Authentication\\BackendUserAuthentication', array('isAdmin', 'getFileoperationPermissions'));
+
+               $this->fixture
+                       ->expects($this->any())
+                       ->method('isAdmin')
+                       ->will($this->returnValue(FALSE));
+
+               $this->fixture
+                       ->expects($this->never())
+                       ->method('getFileoperationPermissions');
+
+               $this->fixture->userTS = array(
+                       'permissions.' => array(
+                               'file.' => array(
+                                       'default.' => $expectedPermissions
+                               ),
+                       )
+               );
+
+               $this->assertEquals($expectedPermissions, $this->fixture->getFilePermissions());
+       }
+
+       /**
+        * @return array
+        */
+       public function getFilePermissionsFromStorageDataProvider() {
+               $defaultPermissions = array(
+                       'addFile' => 1,
+                       'readFile' => 1,
+                       'editFile' => 1,
+                       'writeFile' => 1,
+                       'uploadFile' => 1,
+                       'copyFile' => 1,
+                       'moveFile' => 1,
+                       'renameFile' => 1,
+                       'unzipFile' => 1,
+                       'removeFile' => 1,
+                       'addFolder' => 1,
+                       'readFolder' => 1,
+                       'copyFolder' => 1,
+                       'moveFolder' => 1,
+                       'renameFolder' => 1,
+                       'writeFolder' => 1,
+                       'removeFolder' => 1,
+                       'removeSubfolders' => 1
+               );
+
+               return array(
+                       'Overwrites given storage permissions with default permissions' => array(
+                               $defaultPermissions,
+                               1,
+                               array(
+                                       'addFile' => 0,
+                                       'removeSubfolders' =>0
+                               ),
+                               array(
+                                       'addFile' => 0,
+                                       'readFile' => 1,
+                                       'editFile' => 1,
+                                       'writeFile' => 1,
+                                       'uploadFile' => 1,
+                                       'copyFile' => 1,
+                                       'moveFile' => 1,
+                                       'renameFile' => 1,
+                                       'unzipFile' => 1,
+                                       'removeFile' => 1,
+                                       'addFolder' => 1,
+                                       'readFolder' => 1,
+                                       'copyFolder' => 1,
+                                       'moveFolder' => 1,
+                                       'renameFolder' => 1,
+                                       'writeFolder' => 1,
+                                       'removeFolder' => 1,
+                                       'removeSubfolders' => 0
+                               )
+                       ),
+                       'Overwrites given storage 0 permissions with default permissions' => array(
+                               $defaultPermissions,
+                               0,
+                               array(
+                                       'addFile' => 0,
+                                       'removeSubfolders' =>0
+                               ),
+                               array(
+                                       'addFile' => 0,
+                                       'readFile' => 1,
+                                       'editFile' => 1,
+                                       'writeFile' => 1,
+                                       'uploadFile' => 1,
+                                       'copyFile' => 1,
+                                       'moveFile' => 1,
+                                       'renameFile' => 1,
+                                       'unzipFile' => 1,
+                                       'removeFile' => 1,
+                                       'addFolder' => 1,
+                                       'readFolder' => 1,
+                                       'copyFolder' => 1,
+                                       'moveFolder' => 1,
+                                       'renameFolder' => 1,
+                                       'writeFolder' => 1,
+                                       'removeFolder' => 1,
+                                       'removeSubfolders' => 0
+                               )
+                       ),
+                       'Returns default permissions if no storage permissions are found' => array(
+                               $defaultPermissions,
+                               1,
+                               array(),
+                               array(
+                                       'addFile' => 1,
+                                       'readFile' => 1,
+                                       'editFile' => 1,
+                                       'writeFile' => 1,
+                                       'uploadFile' => 1,
+                                       'copyFile' => 1,
+                                       'moveFile' => 1,
+                                       'renameFile' => 1,
+                                       'unzipFile' => 1,
+                                       'removeFile' => 1,
+                                       'addFolder' => 1,
+                                       'readFolder' => 1,
+                                       'copyFolder' => 1,
+                                       'moveFolder' => 1,
+                                       'renameFolder' => 1,
+                                       'writeFolder' => 1,
+                                       'removeFolder' => 1,
+                                       'removeSubfolders' => 1
+                               )
+                       ),
+               );
+       }
+
+       /**
+        * @param array $defaultPermissions
+        * @param integer $storageUid
+        * @param array $storagePermissions
+        * @param array $expectedPermissions
+        * @test
+        * @dataProvider getFilePermissionsFromStorageDataProvider
+        */
+       public function getFilePermissionsFromStorageOverwritesDefaultPermissions(array $defaultPermissions, $storageUid, array $storagePermissions, array $expectedPermissions) {
+               $this->fixture = $this->getMock('TYPO3\\CMS\\Core\\Authentication\\BackendUserAuthentication', array('isAdmin', 'getFilePermissions'));
+               $storageMock = $this->getMock('TYPO3\\CMS\\Core\\Resource\\ResourceStorage', array(), array(), '', FALSE);
+               $storageMock->expects($this->any())->method('getUid')->will($this->returnValue($storageUid));
+
+               $this->fixture
+                       ->expects($this->any())
+                       ->method('isAdmin')
+                       ->will($this->returnValue(FALSE));
+
+               $this->fixture
+                       ->expects($this->any())
+                       ->method('getFilePermissions')
+                       ->will($this->returnValue($defaultPermissions));
+
+               $this->fixture->userTS = array(
+                       'permissions.' => array(
+                               'file.' => array(
+                                       'storage.' => array(
+                                               $storageUid . '.' => $storagePermissions
+                                       ),
+                               ),
+                       )
+               );
+
+               $this->assertEquals($expectedPermissions, $this->fixture->getFilePermissionsForStorage($storageMock));
+       }
+
+       /**
+        * @param array $defaultPermissions
+        * @param $storageUid
+        * @param array $storagePermissions
+        * @test
+        * @dataProvider getFilePermissionsFromStorageDataProvider
+        */
+       public function getFilePermissionsFromStorageAlwaysReturnsDefaultPermissionsForAdmins(array $defaultPermissions, $storageUid, array $storagePermissions) {
+               $this->fixture = $this->getMock('TYPO3\\CMS\\Core\\Authentication\\BackendUserAuthentication', array('isAdmin', 'getFilePermissions'));
+               $storageMock = $this->getMock('TYPO3\\CMS\\Core\\Resource\\ResourceStorage', array(), array(), '', FALSE);
+               $storageMock->expects($this->any())->method('getUid')->will($this->returnValue($storageUid));
+
+               $this->fixture
+                       ->expects($this->any())
+                       ->method('isAdmin')
+                       ->will($this->returnValue(TRUE));
+
+               $this->fixture
+                       ->expects($this->any())
+                       ->method('getFilePermissions')
+                       ->will($this->returnValue($defaultPermissions));
+
+               $this->fixture->userTS = array(
+                       'permissions.' => array(
+                               'file.' => array(
+                                       'storage.' => array(
+                                               $storageUid . '.' => $storagePermissions
+                                       ),
+                               ),
+                       )
+               );
+
+               $this->assertEquals($defaultPermissions, $this->fixture->getFilePermissionsForStorage($storageMock));
+       }
+
+       /**
+        * @return array
+        */
+       public function getFilePermissionsTakesUserDefaultPermissionsFromRecordIntoAccountIfUserIsNotAdminDataProvider() {
+               return array(
+                       'No old permission' => array(
+                               0,
+                               array(
+                                       'addFile' => 0,
+                                       'readFile' => 1,
+                                       'editFile' => 0,
+                                       'writeFile' => 0,
+                                       'uploadFile' => 0,
+                                       'copyFile' => 0,
+                                       'moveFile' => 0,
+                                       'renameFile' => 0,
+                                       'unzipFile' => 0,
+                                       'removeFile' => 0,
+                                       'addFolder' => 0,
+                                       'readFolder' => 1,
+                                       'copyFolder' => 0,
+                                       'moveFolder' => 0,
+                                       'renameFolder' => 0,
+                                       'writeFolder' => 0,
+                                       'removeFolder' => 0,
+                                       'removeSubfolders' => 0
+                               )
+                       ),
+                       'Uploading allowed' => array(
+                               1,
+                               array(
+                                       'addFile' => 1,
+                                       'readFile' => 1,
+                                       'editFile' => 1,
+                                       'writeFile' => 1,
+                                       'uploadFile' => 1,
+                                       'copyFile' => 1,
+                                       'moveFile' => 1,
+                                       'renameFile' => 1,
+                                       'unzipFile' => 0,
+                                       'removeFile' => 1,
+                                       'addFolder' => 0,
+                                       'readFolder' => 1,
+                                       'copyFolder' => 0,
+                                       'moveFolder' => 0,
+                                       'renameFolder' => 0,
+                                       'writeFolder' => 0,
+                                       'removeFolder' => 0,
+                                       'removeSubfolders' => 0
+                               )
+                       ),
+                       'Unzip allowed' => array(
+                               2,
+                               array(
+                                       'addFile' => 0,
+                                       'readFile' => 1,
+                                       'editFile' => 0,
+                                       'writeFile' => 0,
+                                       'uploadFile' => 0,
+                                       'copyFile' => 0,
+                                       'moveFile' => 0,
+                                       'renameFile' => 0,
+                                       'unzipFile' => 1,
+                                       'removeFile' => 0,
+                                       'addFolder' => 0,
+                                       'readFolder' => 1,
+                                       'copyFolder' => 0,
+                                       'moveFolder' => 0,
+                                       'renameFolder' => 0,
+                                       'writeFolder' => 0,
+                                       'removeFolder' => 0,
+                                       'removeSubfolders' => 0
+                               )
+                       ),
+                       'Write folder allowed' => array(
+                               4,
+                               array(
+                                       'addFile' => 0,
+                                       'readFile' => 1,
+                                       'editFile' => 0,
+                                       'writeFile' => 0,
+                                       'uploadFile' => 0,
+                                       'copyFile' => 0,
+                                       'moveFile' => 0,
+                                       'renameFile' => 0,
+                                       'unzipFile' => 0,
+                                       'removeFile' => 0,
+                                       'addFolder' => 1,
+                                       'readFolder' => 1,
+                                       'copyFolder' => 0,
+                                       'moveFolder' => 1,
+                                       'renameFolder' => 1,
+                                       'writeFolder' => 1,
+                                       'removeFolder' => 1,
+                                       'removeSubfolders' => 0
+                               )
+                       ),
+                       'Copy folder allowed' => array(
+                               8,
+                               array(
+                                       'addFile' => 0,
+                                       'readFile' => 1,
+                                       'editFile' => 0,
+                                       'writeFile' => 0,
+                                       'uploadFile' => 0,
+                                       'copyFile' => 0,
+                                       'moveFile' => 0,
+                                       'renameFile' => 0,
+                                       'unzipFile' => 0,
+                                       'removeFile' => 0,
+                                       'addFolder' => 0,
+                                       'readFolder' => 1,
+                                       'copyFolder' => 1,
+                                       'moveFolder' => 0,
+                                       'renameFolder' => 0,
+                                       'writeFolder' => 0,
+                                       'removeFolder' => 0,
+                                       'removeSubfolders' => 0
+                               )
+                       ),
+                       'Copy folder and remove subfolders allowed' => array(
+                               24,
+                               array(
+                                       'addFile' => 0,
+                                       'readFile' => 1,
+                                       'editFile' => 0,
+                                       'writeFile' => 0,
+                                       'uploadFile' => 0,
+                                       'copyFile' => 0,
+                                       'moveFile' => 0,
+                                       'renameFile' => 0,
+                                       'unzipFile' => 0,
+                                       'removeFile' => 0,
+                                       'addFolder' => 0,
+                                       'readFolder' => 1,
+                                       'copyFolder' => 1,
+                                       'moveFolder' => 0,
+                                       'renameFolder' => 0,
+                                       'writeFolder' => 0,
+                                       'removeFolder' => 0,
+                                       'removeSubfolders' => 1
+                               )
+                       ),
+               );
+       }
+
+       /**
+        * @test
+        * @dataProvider getFilePermissionsTakesUserDefaultPermissionsFromRecordIntoAccountIfUserIsNotAdminDataProvider
+        */
+       public function getFilePermissionsTakesUserDefaultPermissionsFromRecordIntoAccountIfUserIsNotAdmin($oldPermissionValue, $expectedPermissions) {
+               $this->fixture = $this->getMock('TYPO3\\CMS\\Core\\Authentication\\BackendUserAuthentication', array('isAdmin', 'getFileoperationPermissions'));
+
+               $this->fixture
+                       ->expects($this->any())
+                       ->method('isAdmin')
+                       ->will($this->returnValue(FALSE));
+
+               $this->fixture
+                       ->expects($this->any())
+                       ->method('getFileoperationPermissions')
+                       ->will($this->returnValue($oldPermissionValue));
+
+               $this->fixture->userTS = array();
+               $this->assertEquals($expectedPermissions, $this->fixture->getFilePermissions());
+       }
+
+       /**
+        * @test
+        */
+       public function getFilePermissionsGrantsAllPermissionsToAdminUsers() {
+               $this->fixture = $this->getMock('TYPO3\\CMS\\Core\\Authentication\\BackendUserAuthentication', array('isAdmin', 'getFileoperationPermissions'));
+
+               $this->fixture
+                       ->expects($this->any())
+                       ->method('isAdmin')
+                       ->will($this->returnValue(TRUE));
+
+               $expectedPermissions = array(
+                       'addFile' => TRUE,
+                       'readFile' => TRUE,
+                       'editFile' => TRUE,
+                       'writeFile' => TRUE,
+                       'uploadFile' => TRUE,
+                       'copyFile' => TRUE,
+                       'moveFile' => TRUE,
+                       'renameFile' => TRUE,
+                       'unzipFile' => TRUE,
+                       'removeFile' => TRUE,
+                       'addFolder' => TRUE,
+                       'readFolder' => TRUE,
+                       'copyFolder' => TRUE,
+                       'moveFolder' => TRUE,
+                       'renameFolder' => TRUE,
+                       'writeFolder' => TRUE,
+                       'removeFolder' => TRUE,
+                       'removeSubfolders' => TRUE
+               );
+
+               $this->assertEquals($expectedPermissions, $this->fixture->getFilePermissions());
+       }
+
 }
 
 ?>
\ No newline at end of file