From 7292a986a0d58011c84111c1020e07a0e5543eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jun 2018 09:56:38 +0200 Subject: [PATCH 01/16] Add type for room shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This type represents shares with a Nextcloud Talk conversation. Signed-off-by: Daniel Calviño Sánchez --- core/js/share.js | 1 + lib/private/Share/Constants.php | 1 + 2 files changed, 2 insertions(+) diff --git a/core/js/share.js b/core/js/share.js index e4d9364b2d198..cef05eb6479f1 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -12,6 +12,7 @@ OC.Share = _.extend(OC.Share || {}, { SHARE_TYPE_CIRCLE:7, SHARE_TYPE_GUEST:8, SHARE_TYPE_REMOTE_GROUP:9, + SHARE_TYPE_ROOM:10, /** * Regular expression for splitting parts of remote share owners: diff --git a/lib/private/Share/Constants.php b/lib/private/Share/Constants.php index 4eb79734c0616..a79bbf677f7cf 100644 --- a/lib/private/Share/Constants.php +++ b/lib/private/Share/Constants.php @@ -38,6 +38,7 @@ class Constants { const SHARE_TYPE_CIRCLE = 7; const SHARE_TYPE_GUEST = 8; const SHARE_TYPE_REMOTE_GROUP = 9; + const SHARE_TYPE_ROOM = 10; const FORMAT_NONE = -1; const FORMAT_STATUSES = -2; From 857bb4536628814cdae3591c795bb1e21e9704b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jun 2018 09:58:07 +0200 Subject: [PATCH 02/16] Add comment with IDs of internal share types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Share/Constants.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/private/Share/Constants.php b/lib/private/Share/Constants.php index a79bbf677f7cf..72dc5cd43be52 100644 --- a/lib/private/Share/Constants.php +++ b/lib/private/Share/Constants.php @@ -31,6 +31,7 @@ class Constants { const SHARE_TYPE_USER = 0; const SHARE_TYPE_GROUP = 1; + // const SHARE_TYPE_USERGROUP = 2; // Internal type used by DefaultShareProvider const SHARE_TYPE_LINK = 3; const SHARE_TYPE_EMAIL = 4; const SHARE_TYPE_CONTACT = 5; // ToDo Check if it is still in use otherwise remove it @@ -39,6 +40,7 @@ class Constants { const SHARE_TYPE_GUEST = 8; const SHARE_TYPE_REMOTE_GROUP = 9; const SHARE_TYPE_ROOM = 10; + // const SHARE_TYPE_USERROOM = 11; // Internal type used by RoomShareProvider const FORMAT_NONE = -1; const FORMAT_STATUSES = -2; From 4ee839d69c83bcd71d194864eed47f4448a6ca85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jun 2018 10:10:58 +0200 Subject: [PATCH 03/16] Add provider for room shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The RoomShareProvider is provided by the Talk app, so it is necessary to check whether the app is available or not, and also whether the class itself exists or not (just in case an older version of the app that did not have support yet for room shares is being used). Signed-off-by: Daniel Calviño Sánchez --- lib/private/Share20/ProviderFactory.php | 34 +++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/lib/private/Share20/ProviderFactory.php b/lib/private/Share20/ProviderFactory.php index 0aacca409d148..6cb6c082df576 100644 --- a/lib/private/Share20/ProviderFactory.php +++ b/lib/private/Share20/ProviderFactory.php @@ -60,6 +60,8 @@ class ProviderFactory implements IProviderFactory { private $shareByCircleProvider = null; /** @var bool */ private $circlesAreNotAvailable = false; + /** @var \OCA\Spreed\Share\RoomShareProvider */ + private $roomShareProvider = null; /** * IProviderFactory constructor. @@ -221,6 +223,30 @@ protected function getShareByCircleProvider() { return $this->shareByCircleProvider; } + /** + * Create the room share provider + * + * @return RoomShareProvider + */ + protected function getRoomShareProvider() { + if ($this->roomShareProvider === null) { + /* + * Check if the app is enabled + */ + $appManager = $this->serverContainer->getAppManager(); + if (!$appManager->isEnabledForUser('spreed')) { + return null; + } + + try { + $this->roomShareProvider = $this->serverContainer->query('\OCA\Spreed\Share\RoomShareProvider'); + } catch (\OCP\AppFramework\QueryException $e) { + return null; + } + } + + return $this->roomShareProvider; + } /** * @inheritdoc @@ -235,6 +261,8 @@ public function getProvider($id) { $provider = $this->getShareByMailProvider(); } else if ($id === 'ocCircleShare') { $provider = $this->getShareByCircleProvider(); + } else if ($id === 'ocRoomShare') { + $provider = $this->getRoomShareProvider(); } if ($provider === null) { @@ -261,6 +289,8 @@ public function getProviderForType($shareType) { $provider = $this->getShareByMailProvider(); } else if ($shareType === \OCP\Share::SHARE_TYPE_CIRCLE) { $provider = $this->getShareByCircleProvider(); + } else if ($shareType === \OCP\Share::SHARE_TYPE_ROOM) { + $provider = $this->getRoomShareProvider(); } @@ -281,6 +311,10 @@ public function getAllProviders() { if ($shareByCircle !== null) { $shares[] = $shareByCircle; } + $roomShare = $this->getRoomShareProvider(); + if ($roomShare !== null) { + $shares[] = $roomShare; + } return $shares; } From d9458b303a422c968673dafc45367c0716037e1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jun 2018 10:40:02 +0200 Subject: [PATCH 04/16] Add support for room shares to the share manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Room shares are implemented in an external app (Nextcloud Talk), so in order to keep the share manager as isolated as possible from room share specifics all the validity checks are done in the provider of room shares. However, due to the code structure it is necessary to explicitly check for room shares in "generalCreateChecks" to prevent an exception from being thrown. Signed-off-by: Daniel Calviño Sánchez --- lib/private/Share20/Manager.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index d0316b44c1a4d..7cfa83dbb4a0f 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -239,6 +239,7 @@ protected function generalCreateChecks(\OCP\Share\IShare $share) { if ($circle === null) { throw new \InvalidArgumentException('SharedWith is not a valid circle'); } + } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_ROOM) { } else { // We can't handle other types yet throw new \InvalidArgumentException('unknown share type'); From 4ed7131e26fd2653d6c735d84ecacb7b242ce2f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jun 2018 13:22:26 +0200 Subject: [PATCH 05/16] Add support for room shares to ShareAPIController MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In some cases, the ShareAPIController requires explicit handling of each type of share (for example, to format a share for a DataResponse). Room shares are implemented in an external app (Nextcloud Talk), so in order to keep the controller as isolated as possible from room share specifics all that explicit handling is done in a helper class provided by the Talk app. In other cases it is just enough to call the share manager specifying a room share type; note that the share manager is guarded against share types for which there is no provider, so it is not necessary to explicitly check that before passing room shares to the share manager. Signed-off-by: Daniel Calviño Sánchez --- .../lib/Controller/ShareAPIController.php | 66 ++- apps/files_sharing/tests/ApiTest.php | 5 +- .../Controller/ShareAPIControllerTest.php | 491 +++++++++++++++++- 3 files changed, 547 insertions(+), 15 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index bda0047e66c94..15dda8928d44c 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -37,6 +37,7 @@ use OCP\AppFramework\OCS\OCSForbiddenException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; +use OCP\AppFramework\QueryException; use OCP\Constants; use OCP\Files\Folder; use OCP\Files\Node; @@ -46,6 +47,7 @@ use OCP\IL10N; use OCP\IUserManager; use OCP\IRequest; +use OCP\IServerContainer; use OCP\IURLGenerator; use OCP\Files\IRootFolder; use OCP\Lock\LockedException; @@ -84,6 +86,8 @@ class ShareAPIController extends OCSController { private $config; /** @var IAppManager */ private $appManager; + /** @var IServerContainer */ + private $serverContainer; /** * Share20OCS constructor. @@ -99,6 +103,7 @@ class ShareAPIController extends OCSController { * @param IL10N $l10n * @param IConfig $config * @param IAppManager $appManager + * @param IServerContainer $serverContainer */ public function __construct( string $appName, @@ -111,7 +116,8 @@ public function __construct( string $userId, IL10N $l10n, IConfig $config, - IAppManager $appManager + IAppManager $appManager, + IServerContainer $serverContainer ) { parent::__construct($appName, $request); @@ -125,6 +131,7 @@ public function __construct( $this->l = $l10n; $this->config = $config; $this->appManager = $appManager; + $this->serverContainer = $serverContainer; } /** @@ -231,6 +238,14 @@ protected function formatShare(\OCP\Share\IShare $share, Node $recipientNode = n $shareWithStart = ($hasCircleId? strrpos($share->getSharedWith(), '[') + 1: 0); $shareWithLength = ($hasCircleId? -1: strpos($share->getSharedWith(), ' ')); $result['share_with'] = substr($share->getSharedWith(), $shareWithStart, $shareWithLength); + } else if ($share->getShareType() === Share::SHARE_TYPE_ROOM) { + $result['share_with'] = $share->getSharedWith(); + $result['share_with_displayname'] = ''; + + try { + $result = array_merge($result, $this->getRoomShareHelper()->formatShare($share)); + } catch (QueryException $e) { + } } @@ -315,7 +330,8 @@ public function deleteShare(string $id): DataResponse { throw new OCSNotFoundException($this->l->t('Could not delete share')); } - if ($share->getShareType() === Share::SHARE_TYPE_GROUP && + if (($share->getShareType() === Share::SHARE_TYPE_GROUP || + $share->getShareType() === Share::SHARE_TYPE_ROOM) && $share->getShareOwner() !== $this->currentUser && $share->getSharedBy() !== $this->currentUser) { $this->shareManager->deleteFromSelf($share, $this->currentUser); @@ -515,6 +531,12 @@ public function createShare( } $share->setSharedWith($shareWith); $share->setPermissions($permissions); + } else if ($shareType === Share::SHARE_TYPE_ROOM) { + try { + $this->getRoomShareHelper()->createShare($share, $shareWith, $permissions, $expireDate); + } catch (QueryException $e) { + throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$path->getPath()])); + } } else { throw new OCSBadRequestException($this->l->t('Unknown share type')); } @@ -546,8 +568,9 @@ private function getSharedWithMe($node = null, bool $includeTags): DataResponse $userShares = $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_USER, $node, -1, 0); $groupShares = $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_GROUP, $node, -1, 0); $circleShares = $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_CIRCLE, $node, -1, 0); + $roomShares = $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_ROOM, $node, -1, 0); - $shares = array_merge($userShares, $groupShares, $circleShares); + $shares = array_merge($userShares, $groupShares, $circleShares, $roomShares); $shares = array_filter($shares, function (IShare $share) { return $share->getShareOwner() !== $this->currentUser; @@ -594,6 +617,7 @@ private function getSharesInDir(Node $folder): DataResponse { if ($this->shareManager->outgoingServer2ServerSharesAllowed()) { $shares = array_merge($shares, $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_REMOTE, $node, false, -1, 0)); } + $shares = array_merge($shares, $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_ROOM, $node, false, -1, 0)); } $formatted = []; @@ -679,8 +703,9 @@ public function getShares( } else { $circleShares = []; } + $roomShares = $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_ROOM, $path, $reshares, -1, 0); - $shares = array_merge($userShares, $groupShares, $linkShares, $mailShares, $circleShares); + $shares = array_merge($userShares, $groupShares, $linkShares, $mailShares, $circleShares, $roomShares); if ($this->shareManager->outgoingServer2ServerSharesAllowed()) { $federatedShares = $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_REMOTE, $path, $reshares, -1, 0); @@ -864,6 +889,7 @@ public function updateShare( /* Check if this is an incomming share */ $incomingShares = $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_USER, $share->getNode(), -1, 0); $incomingShares = array_merge($incomingShares, $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0)); + $incomingShares = array_merge($incomingShares, $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_ROOM, $share->getNode(), -1, 0)); /** @var \OCP\Share\IShare[] $incomingShares */ if (!empty($incomingShares)) { @@ -921,6 +947,14 @@ protected function canAccessShare(\OCP\Share\IShare $share, bool $checkGroups = return true; } + if ($share->getShareType() === Share::SHARE_TYPE_ROOM) { + try { + return $this->getRoomShareHelper()->canAccessShare($share, $this->currentUser); + } catch (QueryException $e) { + return false; + } + } + return false; } @@ -988,6 +1022,13 @@ private function getShareById(string $id): IShare { // Do nothing, just try the other share type } + try { + $share = $this->shareManager->getShareById('ocRoomShare:' . $id, $this->currentUser); + return $share; + } catch (ShareNotFound $e) { + // Do nothing, just try the other share type + } + if (!$this->shareManager->outgoingServer2ServerSharesAllowed()) { throw new ShareNotFound(); } @@ -1016,4 +1057,21 @@ public function cleanup() { $this->lockedNode->unlock(ILockingProvider::LOCK_SHARED); } } + + /** + * Returns the helper of ShareAPIController for room shares. + * + * If the Talk application is not enabled or the helper is not available + * a QueryException is thrown instead. + * + * @return \OCA\Spreed\Share\Helper\ShareAPIController + * @throws QueryException + */ + private function getRoomShareHelper() { + if (!$this->appManager->isEnabledForUser('spreed')) { + throw new QueryException(); + } + + return $this->serverContainer->query('\OCA\Spreed\Share\Helper\ShareAPIController'); + } } diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index a68ec7de1f079..0616daed62db6 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -41,6 +41,7 @@ use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; +use OCP\IServerContainer; /** * Class ApiTest @@ -109,6 +110,7 @@ private function createOCS($userId) { })); $config = $this->createMock(IConfig::class); $appManager = $this->createMock(IAppManager::class); + $serverContainer = $this->createMock(IServerContainer::class); return new ShareAPIController( self::APP_NAME, @@ -121,7 +123,8 @@ private function createOCS($userId) { $userId, $l, $config, - $appManager + $appManager, + $serverContainer ); } diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 4fd8162db3d4e..15c4071bc46ae 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -38,6 +38,7 @@ use OCP\Files\NotFoundException; use OCP\IGroupManager; use OCP\IUserManager; +use OCP\IServerContainer; use OCP\IRequest; use OCP\IURLGenerator; use OCP\IUser; @@ -92,6 +93,9 @@ class ShareAPIControllerTest extends TestCase { /** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */ private $appManager; + /** @var IServerContainer|\PHPUnit_Framework_MockObject_MockObject */ + private $serverContainer; + protected function setUp() { $this->shareManager = $this->createMock(IManager::class); $this->shareManager @@ -112,6 +116,7 @@ protected function setUp() { })); $this->config = $this->createMock(IConfig::class); $this->appManager = $this->createMock(IAppManager::class); + $this->serverContainer = $this->createMock(IServerContainer::class); $this->ocs = new ShareAPIController( $this->appName, @@ -124,7 +129,8 @@ protected function setUp() { $this->currentUser, $this->l, $this->config, - $this->appManager + $this->appManager, + $this->serverContainer ); } @@ -144,7 +150,8 @@ private function mockFormatShare() { $this->currentUser, $this->l, $this->config, - $this->appManager + $this->appManager, + $this->serverContainer ])->setMethods(['formatShare']) ->getMock(); } @@ -159,10 +166,10 @@ private function newShare() { */ public function testDeleteShareShareNotFound() { $this->shareManager - ->expects($this->exactly(2)) + ->expects($this->exactly(3)) ->method('getShareById') ->will($this->returnCallback(function($id) { - if ($id === 'ocinternal:42' || $id === 'ocFederatedSharing:42') { + if ($id === 'ocinternal:42' || $id === 'ocRoomShare:42' || $id === 'ocFederatedSharing:42') { throw new \OCP\Share\Exceptions\ShareNotFound(); } else { throw new \Exception(); @@ -461,7 +468,8 @@ public function testGetShare(\OCP\Share\IShare $share, array $result) { $this->currentUser, $this->l, $this->config, - $this->appManager + $this->appManager, + $this->serverContainer ])->setMethods(['canAccessShare']) ->getMock(); @@ -596,6 +604,65 @@ public function testCanAccessShare() { $this->assertFalse($this->invokePrivate($this->ocs, 'canAccessShare', [$share])); } + public function dataCanAccessRoomShare() { + $result = []; + + $share = $this->createMock(IShare::class); + $share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_ROOM); + $share->method('getSharedWith')->willReturn('recipientRoom'); + + $result[] = [ + false, $share, false, false + ]; + + $result[] = [ + false, $share, false, true + ]; + + $result[] = [ + true, $share, true, true + ]; + + $result[] = [ + false, $share, true, false + ]; + + return $result; + } + + /** + * @dataProvider dataCanAccessRoomShare + * + * @param bool $expects + * @param \OCP\Share\IShare $share + * @param bool helperAvailable + * @param bool canAccessShareByHelper + */ + public function testCanAccessRoomShare(bool $expected, \OCP\Share\IShare $share, bool $helperAvailable, bool $canAccessShareByHelper) { + if (!$helperAvailable) { + $this->appManager->method('isEnabledForUser') + ->with('spreed') + ->willReturn(false); + } else { + $this->appManager->method('isEnabledForUser') + ->with('spreed') + ->willReturn(true); + + $helper = $this->getMockBuilder('\OCA\Spreed\Share\Helper\ShareAPIController') + ->setMethods(array('canAccessShare')) + ->getMock(); + $helper->method('canAccessShare') + ->with($share, $this->currentUser) + ->willReturn($canAccessShareByHelper); + + $this->serverContainer->method('query') + ->with('\OCA\Spreed\Share\Helper\ShareAPIController') + ->willReturn($helper); + } + + $this->assertEquals($expected, $this->invokePrivate($this->ocs, 'canAccessShare', [$share])); + } + /** * @expectedException \OCP\AppFramework\OCS\OCSNotFoundException * @expectedExceptionMessage Please specify a file or folder path @@ -733,7 +800,8 @@ public function testCreateShareUser() { $this->currentUser, $this->l, $this->config, - $this->appManager + $this->appManager, + $this->serverContainer ])->setMethods(['formatShare']) ->getMock(); @@ -832,7 +900,8 @@ public function testCreateShareGroup() { $this->currentUser, $this->l, $this->config, - $this->appManager + $this->appManager, + $this->serverContainer ])->setMethods(['formatShare']) ->getMock(); @@ -1128,6 +1197,186 @@ public function testCreateShareInvalidExpireDate() { $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, 'a1b2d3'); } + public function testCreateShareRoom() { + $ocs = $this->mockFormatShare(); + + $share = $this->newShare(); + $this->shareManager->method('newShare')->willReturn($share); + + $userFolder = $this->getMockBuilder(Folder::class)->getMock(); + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('currentUser') + ->willReturn($userFolder); + + $path = $this->getMockBuilder(File::class)->getMock(); + $storage = $this->getMockBuilder(Storage::class)->getMock(); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); + $userFolder->expects($this->once()) + ->method('get') + ->with('valid-path') + ->willReturn($path); + + $path->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + + $this->appManager->method('isEnabledForUser') + ->with('spreed') + ->willReturn(true); + + $helper = $this->getMockBuilder('\OCA\Spreed\Share\Helper\ShareAPIController') + ->setMethods(array('createShare')) + ->getMock(); + $helper->method('createShare') + ->with( + $share, + 'recipientRoom', + \OCP\Constants::PERMISSION_ALL & + ~\OCP\Constants::PERMISSION_DELETE & + ~\OCP\Constants::PERMISSION_CREATE, + '' + )->will($this->returnCallback( + function ($share) { + $share->setSharedWith('recipientRoom'); + $share->setPermissions( + \OCP\Constants::PERMISSION_ALL & + ~\OCP\Constants::PERMISSION_DELETE & + ~\OCP\Constants::PERMISSION_CREATE + ); + } + )); + + $this->serverContainer->method('query') + ->with('\OCA\Spreed\Share\Helper\ShareAPIController') + ->willReturn($helper); + + $this->shareManager->method('createShare') + ->with($this->callback(function (\OCP\Share\IShare $share) use ($path) { + return $share->getNode() === $path && + $share->getPermissions() === ( + \OCP\Constants::PERMISSION_ALL & + ~\OCP\Constants::PERMISSION_DELETE & + ~\OCP\Constants::PERMISSION_CREATE + ) && + $share->getShareType() === \OCP\Share::SHARE_TYPE_ROOM && + $share->getSharedWith() === 'recipientRoom' && + $share->getSharedBy() === 'currentUser'; + })) + ->will($this->returnArgument(0)); + + $expected = new DataResponse([]); + $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_ROOM, 'recipientRoom'); + + $this->assertInstanceOf(get_class($expected), $result); + $this->assertEquals($expected->getData(), $result->getData()); + } + + /** + * @expectedException \OCP\AppFramework\OCS\OCSForbiddenException + * @expectedExceptionMessage Sharing valid-path failed because the back end does not support room shares + */ + public function testCreateShareRoomHelperNotAvailable() { + $ocs = $this->mockFormatShare(); + + $share = $this->newShare(); + $this->shareManager->method('newShare')->willReturn($share); + + $userFolder = $this->getMockBuilder(Folder::class)->getMock(); + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('currentUser') + ->willReturn($userFolder); + + $path = $this->getMockBuilder(File::class)->getMock(); + $storage = $this->getMockBuilder(Storage::class)->getMock(); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); + $path->method('getPath')->willReturn('valid-path'); + $userFolder->expects($this->once()) + ->method('get') + ->with('valid-path') + ->willReturn($path); + + $path->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + + $this->appManager->method('isEnabledForUser') + ->with('spreed') + ->willReturn(false); + + $this->shareManager->expects($this->never())->method('createShare'); + + $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_ROOM, 'recipientRoom'); + } + + /** + * @expectedException \OCP\AppFramework\OCS\OCSNotFoundException + * @expectedExceptionMessage Exception thrown by the helper + */ + public function testCreateShareRoomHelperThrowException() { + $ocs = $this->mockFormatShare(); + + $share = $this->newShare(); + $this->shareManager->method('newShare')->willReturn($share); + + $userFolder = $this->getMockBuilder(Folder::class)->getMock(); + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('currentUser') + ->willReturn($userFolder); + + $path = $this->getMockBuilder(File::class)->getMock(); + $storage = $this->getMockBuilder(Storage::class)->getMock(); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); + $userFolder->expects($this->once()) + ->method('get') + ->with('valid-path') + ->willReturn($path); + + $path->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + + $this->appManager->method('isEnabledForUser') + ->with('spreed') + ->willReturn(true); + + $helper = $this->getMockBuilder('\OCA\Spreed\Share\Helper\ShareAPIController') + ->setMethods(array('createShare')) + ->getMock(); + $helper->method('createShare') + ->with( + $share, + 'recipientRoom', + \OCP\Constants::PERMISSION_ALL & + ~\OCP\Constants::PERMISSION_DELETE & + ~\OCP\Constants::PERMISSION_CREATE, + '' + )->will($this->returnCallback( + function ($share) { + throw new OCSNotFoundException("Exception thrown by the helper"); + } + )); + + $this->serverContainer->method('query') + ->with('\OCA\Spreed\Share\Helper\ShareAPIController') + ->willReturn($helper); + + $this->shareManager->expects($this->never())->method('createShare'); + + $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_ROOM, 'recipientRoom'); + } + /** * Test for https://github.com/owncloud/core/issues/22587 * TODO: Remove once proper solution is in place @@ -1149,7 +1398,8 @@ public function testCreateReshareOfFederatedMountNoDeletePermissions() { $this->currentUser, $this->l, $this->config, - $this->appManager + $this->appManager, + $this->serverContainer ])->setMethods(['formatShare']) ->getMock(); @@ -1681,7 +1931,8 @@ public function testUpdateShareCannotIncreasePermissions() { ->method('getSharedWith') ->will($this->returnValueMap([ ['currentUser', \OCP\Share::SHARE_TYPE_USER, $share->getNode(), -1, 0, []], - ['currentUser', \OCP\Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0, [$incomingShare]] + ['currentUser', \OCP\Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0, [$incomingShare]], + ['currentUser', \OCP\Share::SHARE_TYPE_ROOM, $share->getNode(), -1, 0, []] ])); $this->shareManager->expects($this->never())->method('updateShare'); @@ -1726,7 +1977,8 @@ public function testUpdateShareCannotIncreasePermissionsLinkShare() { ->method('getSharedWith') ->will($this->returnValueMap([ ['currentUser', \OCP\Share::SHARE_TYPE_USER, $share->getNode(), -1, 0, [$incomingShare]], - ['currentUser', \OCP\Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0, []] + ['currentUser', \OCP\Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0, []], + ['currentUser', \OCP\Share::SHARE_TYPE_ROOM, $share->getNode(), -1, 0, []] ])); $this->shareManager->expects($this->never())->method('updateShare'); @@ -1740,6 +1992,69 @@ public function testUpdateShareCannotIncreasePermissionsLinkShare() { } } + public function testUpdateShareCannotIncreasePermissionsRoomShare() { + $ocs = $this->mockFormatShare(); + + $folder = $this->createMock(Folder::class); + + $share = \OC::$server->getShareManager()->newShare(); + $share + ->setId(42) + ->setSharedBy($this->currentUser) + ->setShareOwner('anotheruser') + ->setShareType(\OCP\Share::SHARE_TYPE_ROOM) + ->setSharedWith('group1') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder); + + // note: updateShare will modify the received instance but getSharedWith will reread from the database, + // so their values will be different + $incomingShare = \OC::$server->getShareManager()->newShare(); + $incomingShare + ->setId(42) + ->setSharedBy($this->currentUser) + ->setShareOwner('anotheruser') + ->setShareType(\OCP\Share::SHARE_TYPE_ROOM) + ->setSharedWith('group1') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder); + + $this->request + ->method('getParam') + ->will($this->returnValueMap([ + ['permissions', null, '31'], + ])); + + $this->shareManager + ->method('getShareById') + ->will($this->returnCallback( + function ($id) use ($share) { + if ($id !== 'ocRoomShare:42') { + throw new \OCP\Share\Exceptions\ShareNotFound(); + } + + return $share; + } + )); + + $this->shareManager->expects($this->any()) + ->method('getSharedWith') + ->will($this->returnValueMap([ + ['currentUser', \OCP\Share::SHARE_TYPE_USER, $share->getNode(), -1, 0, []], + ['currentUser', \OCP\Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0, []], + ['currentUser', \OCP\Share::SHARE_TYPE_ROOM, $share->getNode(), -1, 0, [$incomingShare]] + ])); + + $this->shareManager->expects($this->never())->method('updateShare'); + + try { + $ocs->updateShare(42, 31); + $this->fail(); + } catch (OCSNotFoundException $e) { + $this->assertEquals('Cannot increase permissions', $e->getMessage()); + } + } + public function testUpdateShareCanIncreasePermissionsIfOwner() { $ocs = $this->mockFormatShare(); @@ -2410,4 +2725,160 @@ public function testFormatShare(array $expects, \OCP\Share\IShare $share, array $this->assertTrue($exception); } } + + public function dataFormatRoomShare() { + $file = $this->getMockBuilder(File::class)->getMock(); + $parent = $this->getMockBuilder(Folder::class)->getMock(); + + $file->method('getMimeType')->willReturn('myMimeType'); + + $file->method('getPath')->willReturn('file'); + + $parent->method('getId')->willReturn(1); + $file->method('getId')->willReturn(3); + + $file->method('getParent')->willReturn($parent); + + $cache = $this->getMockBuilder('OCP\Files\Cache\ICache')->getMock(); + $cache->method('getNumericStorageId')->willReturn(100); + $storage = $this->getMockBuilder(Storage::class)->getMock(); + $storage->method('getId')->willReturn('storageId'); + $storage->method('getCache')->willReturn($cache); + + $file->method('getStorage')->willReturn($storage); + + $result = []; + + $share = \OC::$server->getShareManager()->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_ROOM) + ->setSharedWith('recipientRoom') + ->setSharedBy('initiator') + ->setShareOwner('owner') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($file) + ->setShareTime(new \DateTime('2000-01-01T00:01:02')) + ->setTarget('myTarget') + ->setNote('personal note') + ->setId(42); + + $result[] = [ + [ + 'id' => 42, + 'share_type' => \OCP\Share::SHARE_TYPE_ROOM, + 'uid_owner' => 'initiator', + 'displayname_owner' => 'initiator', + 'permissions' => 1, + 'stime' => 946684862, + 'parent' => null, + 'expiration' => null, + 'token' => null, + 'uid_file_owner' => 'owner', + 'displayname_file_owner' => 'owner', + 'note' => 'personal note', + 'path' => 'file', + 'item_type' => 'file', + 'storage_id' => 'storageId', + 'storage' => 100, + 'item_source' => 3, + 'file_source' => 3, + 'file_parent' => 1, + 'file_target' => 'myTarget', + 'share_with' => 'recipientRoom', + 'share_with_displayname' => '', + 'mail_send' => 0, + 'mimetype' => 'myMimeType', + ], $share, false, [] + ]; + + $share = \OC::$server->getShareManager()->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_ROOM) + ->setSharedWith('recipientRoom') + ->setSharedBy('initiator') + ->setShareOwner('owner') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($file) + ->setShareTime(new \DateTime('2000-01-01T00:01:02')) + ->setTarget('myTarget') + ->setNote('personal note') + ->setId(42); + + $result[] = [ + [ + 'id' => 42, + 'share_type' => \OCP\Share::SHARE_TYPE_ROOM, + 'uid_owner' => 'initiator', + 'displayname_owner' => 'initiator', + 'permissions' => 1, + 'stime' => 946684862, + 'parent' => null, + 'expiration' => null, + 'token' => null, + 'uid_file_owner' => 'owner', + 'displayname_file_owner' => 'owner', + 'note' => 'personal note', + 'path' => 'file', + 'item_type' => 'file', + 'storage_id' => 'storageId', + 'storage' => 100, + 'item_source' => 3, + 'file_source' => 3, + 'file_parent' => 1, + 'file_target' => 'myTarget', + 'share_with' => 'recipientRoom', + 'share_with_displayname' => 'recipientRoomName', + 'mail_send' => 0, + 'mimetype' => 'myMimeType', + ], $share, true, [ + 'share_with_displayname' => 'recipientRoomName' + ] + ]; + + return $result; + } + + /** + * @dataProvider dataFormatRoomShare + * + * @param array $expects + * @param \OCP\Share\IShare $share + * @param bool $helperAvailable + * @param array $formatShareByHelper + */ + public function testFormatRoomShare(array $expects, \OCP\Share\IShare $share, bool $helperAvailable, array $formatShareByHelper) { + $this->rootFolder->method('getUserFolder') + ->with($this->currentUser) + ->will($this->returnSelf()); + + $this->rootFolder->method('getById') + ->with($share->getNodeId()) + ->willReturn([$share->getNode()]); + + $this->rootFolder->method('getRelativePath') + ->with($share->getNode()->getPath()) + ->will($this->returnArgument(0)); + + if (!$helperAvailable) { + $this->appManager->method('isEnabledForUser') + ->with('spreed') + ->willReturn(false); + } else { + $this->appManager->method('isEnabledForUser') + ->with('spreed') + ->willReturn(true); + + $helper = $this->getMockBuilder('\OCA\Spreed\Share\Helper\ShareAPIController') + ->setMethods(array('formatShare')) + ->getMock(); + $helper->method('formatShare') + ->with($share) + ->willReturn($formatShareByHelper); + + $this->serverContainer->method('query') + ->with('\OCA\Spreed\Share\Helper\ShareAPIController') + ->willReturn($helper); + } + + $result = $this->invokePrivate($this->ocs, 'formatShare', [$share]); + $this->assertEquals($expects, $result); + } } From 382b27d03524d18885330847cc8dc261b041c027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 15 Jul 2018 21:51:14 +0200 Subject: [PATCH 06/16] Add support for room shares to DeletedShareAPIController MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In some cases, the DeletedShareAPIController requires explicit handling of each type of share (for example, to format a share for a DataResponse). Room shares are implemented in an external app (Nextcloud Talk), so in order to keep the controller as isolated as possible from room share specifics all that explicit handling is done in a helper class provided by the Talk app. In other cases it is just enough to call the share manager specifying a room share type; note that the share manager is guarded against share types for which there is no provider, so it is not necessary to explicitly check that before passing room shares to the share manager. Signed-off-by: Daniel Calviño Sánchez --- .../Controller/DeletedShareAPIController.php | 53 +++++++++++++++++-- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/apps/files_sharing/lib/Controller/DeletedShareAPIController.php b/apps/files_sharing/lib/Controller/DeletedShareAPIController.php index d95b434e48fee..7648c4e432dff 100644 --- a/apps/files_sharing/lib/Controller/DeletedShareAPIController.php +++ b/apps/files_sharing/lib/Controller/DeletedShareAPIController.php @@ -26,14 +26,17 @@ namespace OCA\Files_Sharing\Controller; +use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; +use OCP\AppFramework\QueryException; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\IGroupManager; use OCP\IRequest; +use OCP\IServerContainer; use OCP\IUserManager; use OCP\Share\Exceptions\GenericShareException; use OCP\Share\Exceptions\ShareNotFound; @@ -57,13 +60,21 @@ class DeletedShareAPIController extends OCSController { /** @var IRootFolder */ private $rootFolder; + /** @var IAppManager */ + private $appManager; + + /** @var IServerContainer */ + private $serverContainer; + public function __construct(string $appName, IRequest $request, ShareManager $shareManager, string $UserId, IUserManager $userManager, IGroupManager $groupManager, - IRootFolder $rootFolder) { + IRootFolder $rootFolder, + IAppManager $appManager, + IServerContainer $serverContainer) { parent::__construct($appName, $request); $this->shareManager = $shareManager; @@ -71,6 +82,8 @@ public function __construct(string $appName, $this->userManager = $userManager; $this->groupManager = $groupManager; $this->rootFolder = $rootFolder; + $this->appManager = $appManager; + $this->serverContainer = $serverContainer; } private function formatShare(IShare $share): array { @@ -120,9 +133,19 @@ private function formatShare(IShare $share): array { $result['expiration'] = $expiration->format('Y-m-d 00:00:00'); } - $group = $this->groupManager->get($share->getSharedWith()); - $result['share_with'] = $share->getSharedWith(); - $result['share_with_displayname'] = $group !== null ? $group->getDisplayName() : $share->getSharedWith(); + if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) { + $group = $this->groupManager->get($share->getSharedWith()); + $result['share_with'] = $share->getSharedWith(); + $result['share_with_displayname'] = $group !== null ? $group->getDisplayName() : $share->getSharedWith(); + } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_ROOM) { + $result['share_with'] = $share->getSharedWith(); + $result['share_with_displayname'] = ''; + + try { + $result = array_merge($result, $this->getRoomShareHelper()->formatShare($share)); + } catch (QueryException $e) { + } + } return $result; @@ -132,7 +155,10 @@ private function formatShare(IShare $share): array { * @NoAdminRequired */ public function index(): DataResponse { - $shares = $this->shareManager->getDeletedSharedWith($this->userId, \OCP\Share::SHARE_TYPE_GROUP, null, -1, 0); + $groupShares = $this->shareManager->getDeletedSharedWith($this->userId, \OCP\Share::SHARE_TYPE_GROUP, null, -1, 0); + $roomShares = $this->shareManager->getDeletedSharedWith($this->userId, \OCP\Share::SHARE_TYPE_ROOM, null, -1, 0); + + $shares = array_merge($groupShares, $roomShares); $shares = array_map(function (IShare $share) { return $this->formatShare($share); @@ -165,4 +191,21 @@ public function undelete(string $id): DataResponse { return new DataResponse([]); } + + /** + * Returns the helper of DeletedShareAPIController for room shares. + * + * If the Talk application is not enabled or the helper is not available + * a QueryException is thrown instead. + * + * @return \OCA\Spreed\Share\Helper\DeletedShareAPIController + * @throws QueryException + */ + private function getRoomShareHelper() { + if (!$this->appManager->isEnabledForUser('spreed')) { + throw new QueryException(); + } + + return $this->serverContainer->query('\OCA\Spreed\Share\Helper\DeletedShareAPIController'); + } } From de403f2f3da09015a07400d90703642a5eabcc65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jun 2018 13:35:17 +0200 Subject: [PATCH 07/16] Add integration test for creating room shares when Talk is not enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test just ensures that the controller will gracefully reject the creation instead of failing miserably; the integration tests when Talk is enabled are in the Talk repository. Signed-off-by: Daniel Calviño Sánchez --- .../features/bootstrap/Provisioning.php | 24 +++++++++++++++++++ build/integration/features/sharing-v1.feature | 12 ++++++++++ 2 files changed, 36 insertions(+) diff --git a/build/integration/features/bootstrap/Provisioning.php b/build/integration/features/bootstrap/Provisioning.php index 1a2c32ed8c685..c320be2be6c21 100644 --- a/build/integration/features/bootstrap/Provisioning.php +++ b/build/integration/features/bootstrap/Provisioning.php @@ -679,6 +679,30 @@ public function appIsEnabled($app) { Assert::assertEquals(200, $this->response->getStatusCode()); } + /** + * @Given /^app "([^"]*)" is not enabled$/ + * + * Checks that the app is disabled or not installed. + * + * @param string $app + */ + public function appIsNotEnabled($app) { + $fullUrl = $this->baseUrl . "v2.php/cloud/apps?filter=enabled"; + $client = new Client(); + $options = []; + if ($this->currentUser === 'admin') { + $options['auth'] = $this->adminUser; + } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; + + $this->response = $client->get($fullUrl, $options); + $respondedArray = $this->getArrayOfAppsResponded($this->response); + Assert::assertNotContains($app, $respondedArray); + Assert::assertEquals(200, $this->response->getStatusCode()); + } + /** * @Then /^user "([^"]*)" is disabled$/ * @param string $user diff --git a/build/integration/features/sharing-v1.feature b/build/integration/features/sharing-v1.feature index 5708b7115e4e3..dd5cc9fff4f8b 100644 --- a/build/integration/features/sharing-v1.feature +++ b/build/integration/features/sharing-v1.feature @@ -43,6 +43,18 @@ Feature: sharing Then the OCS status code should be "100" And the HTTP status code should be "200" + Scenario: Creating a new room share when Talk is not enabled + Given As an "admin" + And app "spreed" is not enabled + And user "user0" exists + And As an "user0" + When creating a share with + | path | welcome.txt | + | shareWith | a-room-token | + | shareType | 10 | + Then the OCS status code should be "403" + And the HTTP status code should be "401" + Scenario: Creating a new public share Given user "user0" exists And As an "user0" From 8084ba516f637e1e08e32712dde433c1d3474ef6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jun 2018 13:50:25 +0200 Subject: [PATCH 08/16] Add room shares to the MountProvider for shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MountProvider for shares creates mount points for the files shared with the user, which makes possible to use the received shared files and folders as regular files and folders. Signed-off-by: Daniel Calviño Sánchez --- apps/files_sharing/lib/MountProvider.php | 1 + .../files_sharing/tests/MountProviderTest.php | 24 ++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index fd4c537210f7c..cb02a2b5f230a 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -74,6 +74,7 @@ public function getMountsForUser(IUser $user, IStorageFactory $storageFactory) { $shares = $this->shareManager->getSharedWith($user->getUID(), \OCP\Share::SHARE_TYPE_USER, null, -1); $shares = array_merge($shares, $this->shareManager->getSharedWith($user->getUID(), \OCP\Share::SHARE_TYPE_GROUP, null, -1)); $shares = array_merge($shares, $this->shareManager->getSharedWith($user->getUID(), \OCP\Share::SHARE_TYPE_CIRCLE, null, -1)); + $shares = array_merge($shares, $this->shareManager->getSharedWith($user->getUID(), \OCP\Share::SHARE_TYPE_ROOM, null, -1)); // filter out excluded shares and group shares that includes self $shares = array_filter($shares, function (\OCP\Share\IShare $share) use ($user) { diff --git a/apps/files_sharing/tests/MountProviderTest.php b/apps/files_sharing/tests/MountProviderTest.php index b521e109cf932..7b533bf8106de 100644 --- a/apps/files_sharing/tests/MountProviderTest.php +++ b/apps/files_sharing/tests/MountProviderTest.php @@ -114,6 +114,12 @@ public function testExcludeShares() { $this->makeMockShare(4, 101, 'user2', '/share4', 31), $this->makeMockShare(5, 100, 'user1', '/share4', 31), ]; + $roomShares = [ + $this->makeMockShare(6, 102, 'user2', '/share6', 0), + $this->makeMockShare(7, 102, 'user1', '/share6', 31), + $this->makeMockShare(8, 102, 'user2', '/share6', 31), + $this->makeMockShare(9, 102, 'user2', '/share6', 31), + ]; // tests regarding circles are made in the app itself. $circleShares = []; $this->user->expects($this->any()) @@ -131,15 +137,20 @@ public function testExcludeShares() { ->method('getSharedWith') ->with('user1', \OCP\Share::SHARE_TYPE_CIRCLE, null, -1) ->will($this->returnValue($circleShares)); + $this->shareManager->expects($this->at(3)) + ->method('getSharedWith') + ->with('user1', \OCP\Share::SHARE_TYPE_ROOM, null, -1) + ->will($this->returnValue($roomShares)); $this->shareManager->expects($this->any()) ->method('newShare') ->will($this->returnCallback(function() use ($rootFolder, $userManager) { return new \OC\Share20\Share($rootFolder, $userManager); })); $mounts = $this->provider->getMountsForUser($this->user, $this->loader); - $this->assertCount(2, $mounts); + $this->assertCount(3, $mounts); $this->assertInstanceOf('OCA\Files_Sharing\SharedMount', $mounts[0]); $this->assertInstanceOf('OCA\Files_Sharing\SharedMount', $mounts[1]); + $this->assertInstanceOf('OCA\Files_Sharing\SharedMount', $mounts[2]); $mountedShare1 = $mounts[0]->getShare(); $this->assertEquals('2', $mountedShare1->getId()); $this->assertEquals('user2', $mountedShare1->getShareOwner()); @@ -152,6 +163,12 @@ public function testExcludeShares() { $this->assertEquals(101, $mountedShare2->getNodeId()); $this->assertEquals('/share4', $mountedShare2->getTarget()); $this->assertEquals(31, $mountedShare2->getPermissions()); + $mountedShare3 = $mounts[2]->getShare(); + $this->assertEquals('8', $mountedShare3->getId()); + $this->assertEquals('user2', $mountedShare3->getShareOwner()); + $this->assertEquals(102, $mountedShare3->getNodeId()); + $this->assertEquals('/share6', $mountedShare3->getTarget()); + $this->assertEquals(31, $mountedShare3->getPermissions()); } public function mergeSharesDataProvider() { @@ -316,6 +333,7 @@ public function testMergeShares($userShares, $groupShares, $expectedShares, $mov // tests regarding circles are made in the app itself. $circleShares = []; + $roomShares = []; $this->shareManager->expects($this->at(0)) ->method('getSharedWith') ->with('user1', \OCP\Share::SHARE_TYPE_USER) @@ -328,6 +346,10 @@ public function testMergeShares($userShares, $groupShares, $expectedShares, $mov ->method('getSharedWith') ->with('user1', \OCP\Share::SHARE_TYPE_CIRCLE, null, -1) ->will($this->returnValue($circleShares)); + $this->shareManager->expects($this->at(3)) + ->method('getSharedWith') + ->with('user1', \OCP\Share::SHARE_TYPE_ROOM, null, -1) + ->will($this->returnValue($roomShares)); $this->shareManager->expects($this->any()) ->method('newShare') ->will($this->returnCallback(function() use ($rootFolder, $userManager) { From 1ccc99ed8349a3318b799b485e80140643dfbe0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jun 2018 20:54:05 +0200 Subject: [PATCH 09/16] Add support for room shares to the share owner updater MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user can move her own shares into a received share. When that happens she is effectively handing over the ownership of the file, so the share needs to be updated to reflect the new owner. Signed-off-by: Daniel Calviño Sánchez --- apps/files_sharing/lib/Updater.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/files_sharing/lib/Updater.php b/apps/files_sharing/lib/Updater.php index 784fe8b72b19c..848ada62c7b40 100644 --- a/apps/files_sharing/lib/Updater.php +++ b/apps/files_sharing/lib/Updater.php @@ -59,6 +59,7 @@ static private function moveShareToShare($path) { $shares = $shareManager->getSharesBy($userFolder->getOwner()->getUID(), \OCP\Share::SHARE_TYPE_USER, $src, false, -1); $shares = array_merge($shares, $shareManager->getSharesBy($userFolder->getOwner()->getUID(), \OCP\Share::SHARE_TYPE_GROUP, $src, false, -1)); + $shares = array_merge($shares, $shareManager->getSharesBy($userFolder->getOwner()->getUID(), \OCP\Share::SHARE_TYPE_ROOM, $src, false, -1)); // If the path we move is not a share we don't care if (empty($shares)) { From 523fdb612c26281873d0dad6e193630de468b154 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sat, 30 Jun 2018 13:45:23 +0200 Subject: [PATCH 10/16] Add room shares to DAV and recent files "share-types" property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- apps/dav/lib/Connector/Sabre/SharesPlugin.php | 1 + .../unit/Connector/Sabre/SharesPluginTest.php | 1 + apps/files/lib/Controller/ApiController.php | 3 ++- apps/files_sharing/js/share.js | 2 ++ apps/files_sharing/tests/js/shareSpec.js | 20 +++++++++++++++++++ 5 files changed, 26 insertions(+), 1 deletion(-) diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index 825b9821a2a9f..990cc4a808fdf 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -124,6 +124,7 @@ private function getShareTypes(\OCP\Files\Node $node) { \OCP\Share::SHARE_TYPE_LINK, \OCP\Share::SHARE_TYPE_REMOTE, \OCP\Share::SHARE_TYPE_EMAIL, + \OCP\Share::SHARE_TYPE_ROOM, ]; foreach ($requestedShareTypes as $requestedShareType) { // one of each type is enough to find out about the types diff --git a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php index 63b84eb5c6ca2..d4dd3b49f3403 100644 --- a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php @@ -298,6 +298,7 @@ function sharesGetPropertiesDataProvider() { [[\OCP\Share::SHARE_TYPE_GROUP]], [[\OCP\Share::SHARE_TYPE_LINK]], [[\OCP\Share::SHARE_TYPE_REMOTE]], + [[\OCP\Share::SHARE_TYPE_ROOM]], [[\OCP\Share::SHARE_TYPE_USER, \OCP\Share::SHARE_TYPE_GROUP]], [[\OCP\Share::SHARE_TYPE_USER, \OCP\Share::SHARE_TYPE_GROUP, \OCP\Share::SHARE_TYPE_LINK]], [[\OCP\Share::SHARE_TYPE_USER, \OCP\Share::SHARE_TYPE_LINK]], diff --git a/apps/files/lib/Controller/ApiController.php b/apps/files/lib/Controller/ApiController.php index d71b998ffb16e..9443b9776ae3d 100644 --- a/apps/files/lib/Controller/ApiController.php +++ b/apps/files/lib/Controller/ApiController.php @@ -214,7 +214,8 @@ private function getShareTypes(Node $node) { \OCP\Share::SHARE_TYPE_GROUP, \OCP\Share::SHARE_TYPE_LINK, \OCP\Share::SHARE_TYPE_REMOTE, - \OCP\Share::SHARE_TYPE_EMAIL + \OCP\Share::SHARE_TYPE_EMAIL, + \OCP\Share::SHARE_TYPE_ROOM ]; foreach ($requestedShareTypes as $requestedShareType) { // one of each type is enough to find out about the types diff --git a/apps/files_sharing/js/share.js b/apps/files_sharing/js/share.js index a925920f3bc8e..0f8dc58a85e93 100644 --- a/apps/files_sharing/js/share.js +++ b/apps/files_sharing/js/share.js @@ -144,6 +144,8 @@ hasShares = true; } else if (shareType === OC.Share.SHARE_TYPE_CIRCLE) { hasShares = true; + } else if (shareType === OC.Share.SHARE_TYPE_ROOM) { + hasShares = true; } }); OCA.Sharing.Util._updateFileActionIcon($tr, hasShares, hasLink); diff --git a/apps/files_sharing/tests/js/shareSpec.js b/apps/files_sharing/tests/js/shareSpec.js index 91060f6a73596..554a3a82b3490 100644 --- a/apps/files_sharing/tests/js/shareSpec.js +++ b/apps/files_sharing/tests/js/shareSpec.js @@ -108,6 +108,26 @@ describe('OCA.Sharing.Util tests', function() { expect($action.find('.icon').hasClass('icon-public')).toEqual(false); expect(OC.basename(getImageUrl($tr.find('.filename .thumbnail')))).toEqual('folder-shared.svg'); }); + it('shows simple share text with share icon when shared to a room', function() { + var $action, $tr; + fileList.setFiles([{ + id: 1, + type: 'dir', + name: 'One', + path: '/subdir', + mimetype: 'text/plain', + size: 12, + permissions: OC.PERMISSION_ALL, + etag: 'abc', + shareTypes: [OC.Share.SHARE_TYPE_ROOM] + }]); + $tr = fileList.$el.find('tbody tr:first'); + $action = $tr.find('.action-share'); + expect($action.find('>span').text().trim()).toEqual('Shared'); + expect($action.find('.icon').hasClass('icon-shared')).toEqual(true); + expect($action.find('.icon').hasClass('icon-public')).toEqual(false); + expect(OC.basename(getImageUrl($tr.find('.filename .thumbnail')))).toEqual('folder-shared.svg'); + }); it('shows simple share text with public icon when shared with link', function() { var $action, $tr; OC.Share.statuses = {1: {link: true, path: '/subdir'}}; From 30d8e3ee05b38208883ec7b9e57e81a9b8508c9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sat, 30 Jun 2018 13:56:23 +0200 Subject: [PATCH 11/16] Transfer room shares too with the "files:transfer-ownership" command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- apps/files/lib/Command/TransferOwnership.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files/lib/Command/TransferOwnership.php b/apps/files/lib/Command/TransferOwnership.php index 6113f6df54c0c..f417898f2172d 100644 --- a/apps/files/lib/Command/TransferOwnership.php +++ b/apps/files/lib/Command/TransferOwnership.php @@ -217,7 +217,7 @@ private function collectUsersShares(OutputInterface $output) { $output->writeln("Collecting all share information for files and folder of $this->sourceUser ..."); $progress = new ProgressBar($output, count($this->shares)); - foreach([\OCP\Share::SHARE_TYPE_GROUP, \OCP\Share::SHARE_TYPE_USER, \OCP\Share::SHARE_TYPE_LINK, \OCP\Share::SHARE_TYPE_REMOTE] as $shareType) { + foreach([\OCP\Share::SHARE_TYPE_GROUP, \OCP\Share::SHARE_TYPE_USER, \OCP\Share::SHARE_TYPE_LINK, \OCP\Share::SHARE_TYPE_REMOTE, \OCP\Share::SHARE_TYPE_ROOM] as $shareType) { $offset = 0; while (true) { $sharePage = $this->shareManager->getSharesBy($this->sourceUser, $shareType, null, true, 50, $offset); From 0fab46c817f8a94905937c2e805d38b2f09d4d6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sat, 30 Jun 2018 14:00:24 +0200 Subject: [PATCH 12/16] Log sharing and unsharing with a room in the auditing app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- apps/admin_audit/lib/Actions/Sharing.php | 25 ++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/apps/admin_audit/lib/Actions/Sharing.php b/apps/admin_audit/lib/Actions/Sharing.php index b66c9f50eb844..0c4601eef38ce 100644 --- a/apps/admin_audit/lib/Actions/Sharing.php +++ b/apps/admin_audit/lib/Actions/Sharing.php @@ -78,6 +78,19 @@ public function shared(array $params) { 'id', ] ); + } elseif($params['shareType'] === Share::SHARE_TYPE_ROOM) { + $this->log( + 'The %s "%s" with ID "%s" has been shared to the room "%s" with permissions "%s" (Share ID: %s)', + $params, + [ + 'itemType', + 'itemTarget', + 'itemSource', + 'shareWith', + 'permissions', + 'id', + ] + ); } } @@ -122,6 +135,18 @@ public function unshare(array $params) { 'id', ] ); + } elseif($params['shareType'] === Share::SHARE_TYPE_ROOM) { + $this->log( + 'The %s "%s" with ID "%s" has been unshared from the room "%s" (Share ID: %s)', + $params, + [ + 'itemType', + 'fileTarget', + 'itemSource', + 'shareWith', + 'id', + ] + ); } } From 2d062daa11bdbbb1ac652e34869eaa0a1e550878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 1 Jul 2018 17:50:18 +0200 Subject: [PATCH 13/16] Add custom handling for room shares to the list of sharees MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Like done for other types of shares, room shares are now explicitly described as such in the UI. The avatar used is the image provided in the "shareWithAvatar" property of the share. If none is given then the avatar is the first letter of the display name of the room share with a coloured background seeded from the room token. If the display name of the room is empty then no letter is shown in the avatar; no special handling is done in that case. Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogshareelistview.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 2ed46f46768f4..2627d5fa66256 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -243,6 +243,8 @@ } else if (shareType === OC.Share.SHARE_TYPE_EMAIL) { shareWithDisplayName = shareWithDisplayName + " (" + t('core', 'email') + ')'; } else if (shareType === OC.Share.SHARE_TYPE_CIRCLE) { + } else if (shareType === OC.Share.SHARE_TYPE_ROOM) { + shareWithDisplayName = shareWithDisplayName + " (" + t('core', 'conversation') + ')'; } if (shareType === OC.Share.SHARE_TYPE_GROUP) { @@ -291,7 +293,7 @@ shareWithTitle: shareWithTitle, shareType: shareType, shareId: this.model.get('shares')[shareIndex].id, - modSeed: shareType !== OC.Share.SHARE_TYPE_USER && (shareType !== OC.Share.SHARE_TYPE_CIRCLE || shareWithAvatar), + modSeed: shareWithAvatar || (shareType !== OC.Share.SHARE_TYPE_USER && shareType !== OC.Share.SHARE_TYPE_CIRCLE && shareType !== OC.Share.SHARE_TYPE_ROOM), isRemoteShare: shareType === OC.Share.SHARE_TYPE_REMOTE, isRemoteGroupShare: shareType === OC.Share.SHARE_TYPE_REMOTE_GROUP, isNoteAvailable: shareType !== OC.Share.SHARE_TYPE_REMOTE && shareType !== OC.Share.SHARE_TYPE_REMOTE_GROUP, From e1561f0e8fa0437e485820cadb8c225e8d10d3df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 1 Jul 2018 17:50:57 +0200 Subject: [PATCH 14/16] Add custom handling for room shares to the resharer information MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Like done with group shares, received room shares are described as such in the UI. Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogresharerinfoview.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/core/js/sharedialogresharerinfoview.js b/core/js/sharedialogresharerinfoview.js index fadd0a41f7b0a..06e18fef3f2f6 100644 --- a/core/js/sharedialogresharerinfoview.js +++ b/core/js/sharedialogresharerinfoview.js @@ -99,6 +99,17 @@ undefined, {escape: false} ); + } else if (this.model.getReshareType() === OC.Share.SHARE_TYPE_ROOM) { + sharedByText = t( + 'core', + 'Shared with you and the conversation {conversation} by {owner}', + { + conversation: this.model.getReshareWithDisplayName(), + owner: ownerDisplayName + }, + undefined, + {escape: false} + ); } else { sharedByText = t( 'core', From e2e6f23b6722eaf9c0aa2bf94626c6b1674aff0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 24 Jul 2018 11:57:52 +0200 Subject: [PATCH 15/16] Suppress Phan warnings about calling undeclared class methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DeletedShareAPIController and ShareAPIController helpers for room shares are defined in Talk, so the classes do not exist when Talk is not installed. Due to this when the object returned by "getRoomShareHelper" is used Phan complains that the class is not declared. This is not a problem, though, because when the class is not available "getRoomShareHelper" throws an exception, which is then caught where that method was called. Therefore now those warnings from Phan are suppressed (it would be better to use "@phan-suppress-next-line" instead, but it is not yet available in our Phan version). Signed-off-by: Daniel Calviño Sánchez --- .../lib/Controller/DeletedShareAPIController.php | 3 +++ apps/files_sharing/lib/Controller/ShareAPIController.php | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/apps/files_sharing/lib/Controller/DeletedShareAPIController.php b/apps/files_sharing/lib/Controller/DeletedShareAPIController.php index 7648c4e432dff..6c7242ef6133e 100644 --- a/apps/files_sharing/lib/Controller/DeletedShareAPIController.php +++ b/apps/files_sharing/lib/Controller/DeletedShareAPIController.php @@ -86,6 +86,9 @@ public function __construct(string $appName, $this->serverContainer = $serverContainer; } + /** + * @suppress PhanUndeclaredClassMethod + */ private function formatShare(IShare $share): array { $result = [ diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 15dda8928d44c..461c0e47320b3 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -141,6 +141,8 @@ public function __construct( * @param Node|null $recipientNode * @return array * @throws NotFoundException In case the node can't be resolved. + * + * @suppress PhanUndeclaredClassMethod */ protected function formatShare(\OCP\Share\IShare $share, Node $recipientNode = null): array { $sharedBy = $this->userManager->get($share->getSharedBy()); @@ -914,6 +916,9 @@ public function updateShare( return new DataResponse($this->formatShare($share)); } + /** + * @suppress PhanUndeclaredClassMethod + */ protected function canAccessShare(\OCP\Share\IShare $share, bool $checkGroups = true): bool { // A file with permissions 0 can't be accessed by us. So Don't show it if ($share->getPermissions() === 0) { From 4b7fa4ac2e2562025672a389567ee659adff01ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 1 Aug 2018 19:04:32 +0200 Subject: [PATCH 16/16] Add support for tokens in room shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tokens will be used to give access to a share to guests in public rooms. Although the token itself is created in the provider of room shares and no changes are needed for that, due to the code structure it is necessary to explicitly call the provider from the manager when getting a room share by token. Signed-off-by: Daniel Calviño Sánchez --- lib/private/Share20/Manager.php | 9 ++++++ tests/lib/Share20/ManagerTest.php | 50 +++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 7cfa83dbb4a0f..037ea53048a37 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1248,6 +1248,15 @@ public function getShareByToken($token) { } } + if ($share === null && $this->shareProviderExists(\OCP\Share::SHARE_TYPE_ROOM)) { + try { + $provider = $this->factory->getProviderForType(\OCP\Share::SHARE_TYPE_ROOM); + $share = $provider->getShareByToken($token); + } catch (ProviderException $e) { + } catch (ShareNotFound $e) { + } + } + if ($share === null) { throw new ShareNotFound($this->l->t('The requested share does not exist anymore')); } diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 7106e22b264d7..1125cae95658b 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -2165,6 +2165,56 @@ public function testGetShareByToken() { $this->assertSame($share, $ret); } + public function testGetShareByTokenRoom() { + $this->config + ->expects($this->once()) + ->method('getAppValue') + ->with('core', 'shareapi_allow_links', 'yes') + ->willReturn('no'); + + $factory = $this->createMock(IProviderFactory::class); + + $manager = new Manager( + $this->logger, + $this->config, + $this->secureRandom, + $this->hasher, + $this->mountManager, + $this->groupManager, + $this->l, + $this->l10nFactory, + $factory, + $this->userManager, + $this->rootFolder, + $this->eventDispatcher, + $this->mailer, + $this->urlGenerator, + $this->defaults + ); + + $share = $this->createMock(IShare::class); + + $roomShareProvider = $this->createMock(IShareProvider::class); + + $factory->expects($this->any()) + ->method('getProviderForType') + ->will($this->returnCallback(function($shareType) use ($roomShareProvider) { + if ($shareType !== \OCP\Share::SHARE_TYPE_ROOM) { + throw new Exception\ProviderException(); + } + + return $roomShareProvider; + })); + + $roomShareProvider->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->willReturn($share); + + $ret = $manager->getShareByToken('token'); + $this->assertSame($share, $ret); + } + public function testGetShareByTokenWithException() { $this->config ->expects($this->once())