Merge pull request #3781 from tractorcow/pulls/3.1/fix-file-canedit

Fix file and uploadfield permissions
This commit is contained in:
Damian Mooyman 2015-01-12 17:22:11 +13:00
commit c49f16442b
5 changed files with 136 additions and 40 deletions

View File

@ -305,7 +305,7 @@ class File extends DataObject {
$result = $this->extendedCan('canEdit', $member);
if($result !== null) return $result;
return true;
return Permission::checkMember($member, 'CMS_ACCESS_AssetAdmin');
}
/**

View File

@ -1431,6 +1431,7 @@ class UploadField_ItemHandler extends RequestHandler {
// Check item permissions
$item = $this->getItem();
if(!$item) return $this->httpError(404);
if($item instanceof Folder) return $this->httpError(403);
if(!$item->canDelete()) return $this->httpError(403);
// Delete the file from the filesystem. The file will be removed
@ -1452,6 +1453,7 @@ class UploadField_ItemHandler extends RequestHandler {
// Check item permissions
$item = $this->getItem();
if(!$item) return $this->httpError(404);
if($item instanceof Folder) return $this->httpError(403);
if(!$item->canEdit()) return $this->httpError(403);
Requirements::css(FRAMEWORK_DIR . '/css/UploadField.css');
@ -1495,6 +1497,8 @@ class UploadField_ItemHandler extends RequestHandler {
// Check item permissions
$item = $this->getItem();
if(!$item) return $this->httpError(404);
if($item instanceof Folder) return $this->httpError(403);
if(!$item->canEdit()) return $this->httpError(403);
$form->saveInto($item);
$item->write();

View File

@ -381,6 +381,30 @@ class FileTest extends SapphireTest {
$file2->write();
$this->assertEquals($member1->ID, $file2->OwnerID, 'Owner not overwritten on existing files');
}
public function testCanEdit() {
$file = $this->objFromFixture('File', 'gif');
// Test anonymous permissions
Session::set('loggedInAs', null);
$this->assertFalse($file->canEdit(), "Anonymous users can't edit files");
// Test permissionless user
$this->objFromFixture('Member', 'frontend')->logIn();
$this->assertFalse($file->canEdit(), "Permissionless users can't edit files");
// Test cms non-asset user
$this->objFromFixture('Member', 'cms')->logIn();
$this->assertFalse($file->canEdit(), "Basic CMS users can't edit files");
// Test asset-admin user
$this->objFromFixture('Member', 'assetadmin')->logIn();
$this->assertTrue($file->canEdit(), "Asset admin users can edit files");
// Test admin
$this->objFromFixture('Member', 'admin')->logIn();
$this->assertTrue($file->canEdit(), "Admins can edit files");
}
/////////////////////////////////////////////////////////////////////////////////////////////////////////////

View File

@ -1,30 +1,59 @@
Folder:
subfolder:
Name: FileTest-subfolder
folder1:
Name: FileTest-folder1
folder2:
Name: FileTest-folder2
folder1-subfolder1:
Name: FileTest-folder1-subfolder1
ParentID: =>Folder.folder1
subfolder:
Name: FileTest-subfolder
folder1:
Name: FileTest-folder1
folder2:
Name: FileTest-folder2
folder1-subfolder1:
Name: FileTest-folder1-subfolder1
ParentID: =>Folder.folder1
File:
asdf:
Filename: assets/FileTest.txt
gif:
Filename: assets/FileTest.gif
pdf:
Filename: assets/FileTest.pdf
setfromname:
Name: FileTest.png
ParentID: 0
subfolderfile:
Filename: assets/FileTest-subfolder/FileTestSubfolder.txt
ParentID: =>Folder.subfolder
subfolderfile-setfromname:
Name: FileTestSubfolder2.txt
ParentID: =>Folder.subfolder
file1-folder1:
Filename: assets/FileTest-folder1/File1.txt
Name: File1.txt
ParentID: =>Folder.folder1
asdf:
Filename: assets/FileTest.txt
gif:
Filename: assets/FileTest.gif
pdf:
Filename: assets/FileTest.pdf
setfromname:
Name: FileTest.png
ParentID: 0
subfolderfile:
Filename: assets/FileTest-subfolder/FileTestSubfolder.txt
ParentID: =>Folder.subfolder
subfolderfile-setfromname:
Name: FileTestSubfolder2.txt
ParentID: =>Folder.subfolder
file1-folder1:
Filename: assets/FileTest-folder1/File1.txt
Name: File1.txt
ParentID: =>Folder.folder1
Permission:
admin:
Code: ADMIN
cmsmain:
Code: CMS_ACCESS_LeftAndMain
assetadmin:
Code: CMS_ACCESS_AssetAdmin
Group:
admins:
Title: Administrators
Permissions: =>Permission.admin
cmsusers:
Title: 'CMS Users'
Permissions: =>Permission.cmsmain
assetusers:
Title: 'Asset Users'
Permissions: =>Permission.cmsmain, =>Permission.assetadmin
Member:
frontend:
Email: frontend@example.com
cms:
Email: cms@silverstripe.com
Groups: =>Group.cmsusers
admin:
Email: admin@silverstripe.com
Groups: =>Group.admins
assetadmin:
Email: assetadmin@silverstripe.com
Groups: =>Group.assetusers

View File

@ -404,7 +404,12 @@ class UploadFieldTest extends FunctionalTest {
$this->assertFileNotExists($file4->FullPath, 'File is also removed from filesystem');
// Test record-based permissions
$response = $this->mockFileDelete('ManyManyFiles/', $fileNoDelete->ID);
$response = $this->mockFileDelete('ManyManyFiles', $fileNoDelete->ID);
$this->assertEquals(403, $response->getStatusCode());
// Test that folders can't be deleted
$folder = $this->objFromFixture('Folder', 'folder1-subfolder1');
$response = $this->mockFileDelete('ManyManyFiles', $folder->ID);
$this->assertEquals(403, $response->getStatusCode());
}
@ -441,14 +446,13 @@ class UploadFieldTest extends FunctionalTest {
$record = $this->objFromFixture('UploadFieldTest_Record', 'record1');
$file4 = $this->objFromFixture('File', 'file4');
$file5 = $this->objFromFixture('File', 'file5');
$fileNoEdit = $this->objFromFixture('File', 'file-noedit');
$baseUrl = 'UploadFieldTest_Controller/Form/field/ManyManyFiles/item/' . $file4->ID;
$folder = $this->objFromFixture('Folder', 'folder1-subfolder1');
$response = $this->get($baseUrl . '/edit');
$response = $this->mockFileEditForm('ManyManyFiles', $file4->ID);
$this->assertFalse($response->isError());
$response = $this->post($baseUrl . '/EditForm', array('Title' => 'File 4 modified'));
$response = $this->mockFileEdit('ManyManyFiles', $file4->ID, array('Title' => 'File 4 modified'));
$this->assertFalse($response->isError());
$record = DataObject::get_by_id($record->class, $record->ID, false);
@ -456,10 +460,17 @@ class UploadFieldTest extends FunctionalTest {
$this->assertEquals('File 4 modified', $file4->Title);
// Test record-based permissions
$response = $this->post(
'UploadFieldTest_Controller/Form/field/ManyManyFiles/item/' . $fileNoEdit->ID . '/edit',
array()
);
$response = $this->mockFileEditForm('ManyManyFiles', $fileNoEdit->ID);
$this->assertEquals(403, $response->getStatusCode());
$response = $this->mockFileEdit('ManyManyFiles', $fileNoEdit->ID, array());
$this->assertEquals(403, $response->getStatusCode());
// Test folder permissions
$response = $this->mockFileEditForm('ManyManyFiles', $folder->ID);
$this->assertEquals(403, $response->getStatusCode());
$response = $this->mockFileEdit('ManyManyFiles', $folder->ID, array());
$this->assertEquals(403, $response->getStatusCode());
}
@ -791,7 +802,35 @@ class UploadFieldTest extends FunctionalTest {
"UploadFieldTest_Controller/Form/field/{$fileField}/fileexists?filename=".urlencode($fileName)
);
}
/**
* Gets the edit form for the given file
*
* @param string $fileField Name of the field
* @param integer $fileID ID of the file to delete
* @return SS_HTTPResponse form response
*/
protected function mockFileEditForm($fileField, $fileID) {
return $this->get(
"UploadFieldTest_Controller/Form/field/{$fileField}/item/{$fileID}/edit"
);
}
/**
* Mocks edit submissions to a file
*
* @param string $fileField Name of the field
* @param integer $fileID ID of the file to delete
* @param array $fields Fields to update
* @return SS_HTTPResponse form response
*/
protected function mockFileEdit($fileField, $fileID, $fields = array()) {
return $this->post(
"UploadFieldTest_Controller/Form/field/{$fileField}/item/{$fileID}/EditForm",
$fields
);
}
/**
* Simulates a physical file deletion
*
@ -801,7 +840,7 @@ class UploadFieldTest extends FunctionalTest {
*/
protected function mockFileDelete($fileField, $fileID) {
return $this->post(
"UploadFieldTest_Controller/Form/field/HasOneFile/item/{$fileID}/delete",
"UploadFieldTest_Controller/Form/field/{$fileField}/item/{$fileID}/delete",
array()
);
}