mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #21038 from owncloud/share-computesharepermissions-notstore 

Fix (re)share permission checks in a few code paths
This commit is contained in:
Thomas Müller 2015-12-09 10:04:56 +01:00
parent 6ba22f0243 d0cca6c3ad
commit bc744ff6de
10 changed files with 76 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
apps/dav/tests/unit/connector/sabre/file.php
View File

@ -48,6 +48,14 @@ class File extends \Test\TestCase {
parent::tearDown(); parent::tearDown();
} }
private function getMockStorage() {
$storage = $this->getMock('\OCP\Files\Storage');
$storage->expects($this->any())
->method('getId')
->will($this->returnValue('home::someuser'));
return $storage;
}
/** /**
* @param string $string * @param string $string
*/ */
@ -149,7 +157,7 @@ class File extends \Test\TestCase {
->method('getRelativePath') ->method('getRelativePath')
->will($this->returnArgument(0)); ->will($this->returnArgument(0));
$info = new \OC\Files\FileInfo('/test.txt', null, null, array( $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
), null); ), null);
@ -209,7 +217,7 @@ class File extends \Test\TestCase {
$_SERVER['HTTP_OC_CHUNKED'] = true; $_SERVER['HTTP_OC_CHUNKED'] = true;
$info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-0', null, null, [ $info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-0', $this->getMockStorage(), null, [
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
], null); ], null);
$file = new \OCA\DAV\Connector\Sabre\File($view, $info); $file = new \OCA\DAV\Connector\Sabre\File($view, $info);
@ -219,7 +227,7 @@ class File extends \Test\TestCase {
$this->assertNull($file->put('test data one')); $this->assertNull($file->put('test data one'));
$file->releaseLock(ILockingProvider::LOCK_SHARED); $file->releaseLock(ILockingProvider::LOCK_SHARED);
$info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-1', null, null, [ $info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-1', $this->getMockStorage(), null, [
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
], null); ], null);
$file = new \OCA\DAV\Connector\Sabre\File($view, $info); $file = new \OCA\DAV\Connector\Sabre\File($view, $info);
@ -261,7 +269,7 @@ class File extends \Test\TestCase {
$info = new \OC\Files\FileInfo( $info = new \OC\Files\FileInfo(
$viewRoot . '/' . ltrim($path, '/'), $viewRoot . '/' . ltrim($path, '/'),
null, $this->getMockStorage(),
null, null,
['permissions' => \OCP\Constants::PERMISSION_ALL], ['permissions' => \OCP\Constants::PERMISSION_ALL],
null null
@ -450,7 +458,7 @@ class File extends \Test\TestCase {
$_SERVER['CONTENT_LENGTH'] = 123456; $_SERVER['CONTENT_LENGTH'] = 123456;
$_SERVER['REQUEST_METHOD'] = 'PUT'; $_SERVER['REQUEST_METHOD'] = 'PUT';
$info = new \OC\Files\FileInfo('/test.txt', null, null, array( $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
), null); ), null);
@ -483,7 +491,7 @@ class File extends \Test\TestCase {
// simulate situation where the target file is locked // simulate situation where the target file is locked
$view->lockFile('/test.txt', ILockingProvider::LOCK_EXCLUSIVE); $view->lockFile('/test.txt', ILockingProvider::LOCK_EXCLUSIVE);
$info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt', null, null, array( $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt', $this->getMockStorage(), null, array(
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
), null); ), null);
@ -518,7 +526,7 @@ class File extends \Test\TestCase {
$_SERVER['HTTP_OC_CHUNKED'] = true; $_SERVER['HTTP_OC_CHUNKED'] = true;
$info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-0', null, null, [ $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-0', $this->getMockStorage(), null, [
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
], null); ], null);
$file = new \OCA\DAV\Connector\Sabre\File($view, $info); $file = new \OCA\DAV\Connector\Sabre\File($view, $info);
@ -526,7 +534,7 @@ class File extends \Test\TestCase {
$this->assertNull($file->put('test data one')); $this->assertNull($file->put('test data one'));
$file->releaseLock(ILockingProvider::LOCK_SHARED); $file->releaseLock(ILockingProvider::LOCK_SHARED);
$info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-1', null, null, [ $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-1', $this->getMockStorage(), null, [
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
], null); ], null);
$file = new \OCA\DAV\Connector\Sabre\File($view, $info); $file = new \OCA\DAV\Connector\Sabre\File($view, $info);
@ -555,7 +563,7 @@ class File extends \Test\TestCase {
->method('getRelativePath') ->method('getRelativePath')
->will($this->returnArgument(0)); ->will($this->returnArgument(0));
$info = new \OC\Files\FileInfo('/*', null, null, array( $info = new \OC\Files\FileInfo('/*', $this->getMockStorage(), null, array(
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
), null); ), null);
$file = new \OCA\DAV\Connector\Sabre\File($view, $info); $file = new \OCA\DAV\Connector\Sabre\File($view, $info);
@ -591,7 +599,7 @@ class File extends \Test\TestCase {
->method('getRelativePath') ->method('getRelativePath')
->will($this->returnArgument(0)); ->will($this->returnArgument(0));
$info = new \OC\Files\FileInfo('/*', null, null, array( $info = new \OC\Files\FileInfo('/*', $this->getMockStorage(), null, array(
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
), null); ), null);
$file = new \OCA\DAV\Connector\Sabre\File($view, $info); $file = new \OCA\DAV\Connector\Sabre\File($view, $info);
@ -618,7 +626,7 @@ class File extends \Test\TestCase {
$_SERVER['CONTENT_LENGTH'] = 12345; $_SERVER['CONTENT_LENGTH'] = 12345;
$_SERVER['REQUEST_METHOD'] = 'PUT'; $_SERVER['REQUEST_METHOD'] = 'PUT';
$info = new \OC\Files\FileInfo('/test.txt', null, null, array( $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
), null); ), null);
@ -654,7 +662,7 @@ class File extends \Test\TestCase {
->method('unlink') ->method('unlink')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$info = new \OC\Files\FileInfo('/test.txt', null, null, array( $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
), null); ), null);
@ -672,7 +680,7 @@ class File extends \Test\TestCase {
$view = $this->getMock('\OC\Files\View', $view = $this->getMock('\OC\Files\View',
array()); array());
$info = new \OC\Files\FileInfo('/test.txt', null, null, array( $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
'permissions' => 0 'permissions' => 0
), null); ), null);
@ -695,7 +703,7 @@ class File extends \Test\TestCase {
->method('unlink') ->method('unlink')
->will($this->returnValue(false)); ->will($this->returnValue(false));
$info = new \OC\Files\FileInfo('/test.txt', null, null, array( $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
), null); ), null);
@ -718,7 +726,7 @@ class File extends \Test\TestCase {
->method('unlink') ->method('unlink')
->willThrowException(new ForbiddenException('', true)); ->willThrowException(new ForbiddenException('', true));
$info = new \OC\Files\FileInfo('/test.txt', null, null, array( $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
), null); ), null);
@ -753,7 +761,7 @@ class File extends \Test\TestCase {
$path = 'test-locking.txt'; $path = 'test-locking.txt';
$info = new \OC\Files\FileInfo( $info = new \OC\Files\FileInfo(
'/' . $this->user . '/files/' . $path, '/' . $this->user . '/files/' . $path,
null, $this->getMockStorage(),
null, null,
['permissions' => \OCP\Constants::PERMISSION_ALL], ['permissions' => \OCP\Constants::PERMISSION_ALL],
null null
@ -865,7 +873,7 @@ class File extends \Test\TestCase {
->method('fopen') ->method('fopen')
->will($this->returnValue(false)); ->will($this->returnValue(false));
$info = new \OC\Files\FileInfo('/test.txt', null, null, array( $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
), null); ), null);
@ -883,7 +891,7 @@ class File extends \Test\TestCase {
->method('fopen') ->method('fopen')
->willThrowException(new ForbiddenException('', true)); ->willThrowException(new ForbiddenException('', true));
$info = new \OC\Files\FileInfo('/test.txt', null, null, array( $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
'permissions' => \OCP\Constants::PERMISSION_ALL 'permissions' => \OCP\Constants::PERMISSION_ALL
), null); ), null);

9
apps/files/tests/controller/apicontrollertest.php
View File

@ -92,6 +92,7 @@ class ApiControllerTest extends TestCase {
[ [
'mtime' => 55, 'mtime' => 55,
'mimetype' => 'application/pdf', 'mimetype' => 'application/pdf',
'permissions' => 31,
'size' => 1234, 'size' => 1234,
'etag' => 'MyEtag', 'etag' => 'MyEtag',
], ],
@ -111,7 +112,7 @@ class ApiControllerTest extends TestCase {
'parentId' => null, 'parentId' => null,
'mtime' => 55000, 'mtime' => 55000,
'name' => 'root.txt', 'name' => 'root.txt',
'permissions' => null, 'permissions' => 31,
'mimetype' => 'application/pdf', 'mimetype' => 'application/pdf',
'size' => 1234, 'size' => 1234,
'type' => 'file', 'type' => 'file',
@ -139,6 +140,7 @@ class ApiControllerTest extends TestCase {
[ [
'mtime' => 55, 'mtime' => 55,
'mimetype' => 'application/pdf', 'mimetype' => 'application/pdf',
'permissions' => 31,
'size' => 1234, 'size' => 1234,
'etag' => 'MyEtag', 'etag' => 'MyEtag',
], ],
@ -155,6 +157,7 @@ class ApiControllerTest extends TestCase {
[ [
'mtime' => 999, 'mtime' => 999,
'mimetype' => 'application/binary', 'mimetype' => 'application/binary',
'permissions' => 31,
'size' => 9876, 'size' => 9876,
'etag' => 'SubEtag', 'etag' => 'SubEtag',
], ],
@ -174,7 +177,7 @@ class ApiControllerTest extends TestCase {
'parentId' => null, 'parentId' => null,
'mtime' => 55000, 'mtime' => 55000,
'name' => 'root.txt', 'name' => 'root.txt',
'permissions' => null, 'permissions' => 31,
'mimetype' => 'application/pdf', 'mimetype' => 'application/pdf',
'size' => 1234, 'size' => 1234,
'type' => 'file', 'type' => 'file',
@ -191,7 +194,7 @@ class ApiControllerTest extends TestCase {
'parentId' => null, 'parentId' => null,
'mtime' => 999000, 'mtime' => 999000,
'name' => 'root.txt', 'name' => 'root.txt',
'permissions' => null, 'permissions' => 31,
'mimetype' => 'application/binary', 'mimetype' => 'application/binary',
'size' => 9876, 'size' => 9876,
'type' => 'file', 'type' => 'file',

8
apps/files_sharing/lib/external/storage.php vendored
View File

@ -265,4 +265,12 @@ class Storage extends DAV implements ISharedStorage {
list(, $remote) = explode('://', $this->remote, 2); list(, $remote) = explode('://', $this->remote, 2);
return $this->remoteUser . '@' . $remote; return $this->remoteUser . '@' . $remote;
} }
public function isSharable($path) {
if (\OCP\Util::isSharingDisabledForUser() || !\OC\Share\Share::isResharingAllowed()) {
return false;
}
return ($this->getPermissions($path) & \OCP\Constants::PERMISSION_SHARE);
}
} }

2
apps/files_sharing/lib/sharedstorage.php
View File

@ -257,7 +257,7 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage {
} }
public function isSharable($path) { public function isSharable($path) {
if (\OCP\Util::isSharingDisabledForUser()) { if (\OCP\Util::isSharingDisabledForUser() || !\OC\Share\Share::isResharingAllowed()) {
return false; return false;
} }
return ($this->getPermissions($path) & \OCP\Constants::PERMISSION_SHARE); return ($this->getPermissions($path) & \OCP\Constants::PERMISSION_SHARE);

8
lib/private/files/fileinfo.php
View File

@ -100,6 +100,8 @@ class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess {
return $this->getType(); return $this->getType();
} else if ($offset === 'etag') { } else if ($offset === 'etag') {
return $this->getEtag(); return $this->getEtag();
} elseif ($offset === 'permissions') {
return $this->getPermissions();
} elseif (isset($this->data[$offset])) { } elseif (isset($this->data[$offset])) {
return $this->data[$offset]; return $this->data[$offset];
} else { } else {
@ -193,7 +195,11 @@ class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess {
* @return int * @return int
*/ */
public function getPermissions() { public function getPermissions() {
return $this->data['permissions']; $perms = $this->data['permissions'];
if (\OCP\Util::isSharingDisabledForUser() || ($this->isShared() && !\OC\Share\Share::isResharingAllowed())) {
$perms = $perms & ~\OCP\Constants::PERMISSION_SHARE;
}
return $perms;
} }
/** /**

4
lib/private/files/storage/common.php
View File

@ -141,10 +141,6 @@ abstract class Common implements Storage {
} }
public function isSharable($path) { public function isSharable($path) {
if (\OCP\Util::isSharingDisabledForUser()) {
return false;
}
return $this->isReadable($path); return $this->isReadable($path);
} }

2
lib/private/share/share.php
View File

@ -635,7 +635,7 @@ class Share extends Constants {
throw new \Exception($message_t); throw new \Exception($message_t);
} }
// verify that the user has share permission // verify that the user has share permission
if (!\OC\Files\Filesystem::isSharable($path)) { if (!\OC\Files\Filesystem::isSharable($path) || \OCP\Util::isSharingDisabledForUser()) {
$message = 'You are not allowed to share %s'; $message = 'You are not allowed to share %s';
$message_t = $l->t('You are not allowed to share %s', [$path]); $message_t = $l->t('You are not allowed to share %s', [$path]);
\OCP\Util::writeLog('OCP\Share', sprintf($message, $path), \OCP\Util::DEBUG); \OCP\Util::writeLog('OCP\Share', sprintf($message, $path), \OCP\Util::DEBUG);

10
tests/lib/files/node/file.php
View File

@ -21,8 +21,16 @@ class File extends \Test\TestCase {
$this->user = new \OC\User\User('', new \Test\Util\User\Dummy); $this->user = new \OC\User\User('', new \Test\Util\User\Dummy);
} }
protected function getMockStorage() {
$storage = $this->getMock('\OCP\Files\Storage');
$storage->expects($this->any())
->method('getId')
->will($this->returnValue('home::someuser'));
return $storage;
}
protected function getFileInfo($data) { protected function getFileInfo($data) {
return new FileInfo('', null, '', $data, null); return new FileInfo('', $this->getMockStorage(), '', $data, null);
} }
public function testDelete() { public function testDelete() {

10
tests/lib/files/node/folder.php
View File

@ -31,8 +31,16 @@ class Folder extends \Test\TestCase {
$this->user = new \OC\User\User('', new \Test\Util\User\Dummy); $this->user = new \OC\User\User('', new \Test\Util\User\Dummy);
} }
protected function getMockStorage() {
$storage = $this->getMock('\OCP\Files\Storage');
$storage->expects($this->any())
->method('getId')
->will($this->returnValue('home::someuser'));
return $storage;
}
protected function getFileInfo($data) { protected function getFileInfo($data) {
return new FileInfo('', null, '', $data, null); return new FileInfo('', $this->getMockStorage(), '', $data, null);
} }
public function testDelete() { public function testDelete() {

10
tests/lib/files/node/node.php
View File

@ -18,8 +18,16 @@ class Node extends \Test\TestCase {
$this->user = new \OC\User\User('', new \Test\Util\User\Dummy); $this->user = new \OC\User\User('', new \Test\Util\User\Dummy);
} }
protected function getMockStorage() {
$storage = $this->getMock('\OCP\Files\Storage');
$storage->expects($this->any())
->method('getId')
->will($this->returnValue('home::someuser'));
return $storage;
}
protected function getFileInfo($data) { protected function getFileInfo($data) {
return new FileInfo('', null, '', $data, null); return new FileInfo('', $this->getMockStorage(), '', $data, null);
} }
public function testStat() { public function testStat() {