From 7849630cef494096df1290f6d3b9d3eeed6ce5e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 10 Jul 2018 13:01:31 +0200 Subject: [PATCH] Add support for sending the password by Talk to ShareAPIController MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../lib/Controller/ShareAPIController.php | 33 +++++- apps/files_sharing/tests/ApiTest.php | 17 +-- .../Controller/ShareAPIControllerTest.php | 104 ++++++++++++++---- 3 files changed, 121 insertions(+), 33 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 33782d21b5..bda0047e66 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -30,6 +30,7 @@ declare(strict_types=1); namespace OCA\Files_Sharing\Controller; use OCA\Files\Helper; +use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSException; @@ -81,6 +82,8 @@ class ShareAPIController extends OCSController { private $lockedNode; /** @var IConfig */ private $config; + /** @var IAppManager */ + private $appManager; /** * Share20OCS constructor. @@ -95,6 +98,7 @@ class ShareAPIController extends OCSController { * @param string $userId * @param IL10N $l10n * @param IConfig $config + * @param IAppManager $appManager */ public function __construct( string $appName, @@ -106,7 +110,8 @@ class ShareAPIController extends OCSController { IURLGenerator $urlGenerator, string $userId, IL10N $l10n, - IConfig $config + IConfig $config, + IAppManager $appManager ) { parent::__construct($appName, $request); @@ -119,6 +124,7 @@ class ShareAPIController extends OCSController { $this->currentUser = $userId; $this->l = $l10n; $this->config = $config; + $this->appManager = $appManager; } /** @@ -206,6 +212,7 @@ class ShareAPIController extends OCSController { } else if ($share->getShareType() === Share::SHARE_TYPE_EMAIL) { $result['share_with'] = $share->getSharedWith(); $result['password'] = $share->getPassword(); + $result['send_password_by_talk'] = $share->getSendPasswordByTalk(); $result['share_with_displayname'] = $this->getDisplayNameFromAddressBook($share->getSharedWith(), 'EMAIL'); $result['token'] = $share->getToken(); } else if ($share->getShareType() === Share::SHARE_TYPE_CIRCLE) { @@ -328,6 +335,7 @@ class ShareAPIController extends OCSController { * @param string $shareWith * @param string $publicUpload * @param string $password + * @param bool $sendPasswordByTalk * @param string $expireDate * * @return DataResponse @@ -345,6 +353,7 @@ class ShareAPIController extends OCSController { string $shareWith = null, string $publicUpload = 'false', string $password = '', + string $sendPasswordByTalk = null, string $expireDate = '' ): DataResponse { $share = $this->shareManager->newShare(); @@ -485,6 +494,14 @@ class ShareAPIController extends OCSController { $share->setPermissions($permissions); } $share->setSharedWith($shareWith); + + if ($sendPasswordByTalk === 'true') { + if (!$this->appManager->isEnabledForUser('spreed')) { + throw new OCSForbiddenException($this->l->t('Sharing %s sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled', [$path->getPath()])); + } + + $share->setSendPasswordByTalk(true); + } } else if ($shareType === Share::SHARE_TYPE_CIRCLE) { if (!\OC::$server->getAppManager()->isEnabledForUser('circles') || !class_exists('\OCA\Circles\ShareByCircleProvider')) { throw new OCSNotFoundException($this->l->t('You cannot share to a Circle if the app is not enabled')); @@ -697,6 +714,7 @@ class ShareAPIController extends OCSController { * @param string $id * @param int $permissions * @param string $password + * @param string $sendPasswordByTalk * @param string $publicUpload * @param string $expireDate * @param string $note @@ -711,6 +729,7 @@ class ShareAPIController extends OCSController { string $id, int $permissions = null, string $password = null, + string $sendPasswordByTalk = null, string $publicUpload = null, string $expireDate = null, string $note = null @@ -727,7 +746,7 @@ class ShareAPIController extends OCSController { throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist')); } - if ($permissions === null && $password === null && $publicUpload === null && $expireDate === null && $note === null) { + if ($permissions === null && $password === null && $sendPasswordByTalk === null && $publicUpload === null && $expireDate === null && $note === null) { throw new OCSBadRequestException($this->l->t('Wrong or no update parameter given')); } @@ -816,6 +835,16 @@ class ShareAPIController extends OCSController { } else if ($password !== null) { $share->setPassword($password); } + + if ($sendPasswordByTalk === 'true') { + if (!$this->appManager->isEnabledForUser('spreed')) { + throw new OCSForbiddenException($this->l->t('Sharing sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled')); + } + + $share->setSendPasswordByTalk(true); + } else { + $share->setSendPasswordByTalk(false); + } } if ($expireDate === '') { diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index bf4cca5389..a68ec7de1f 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -33,6 +33,7 @@ namespace OCA\Files_Sharing\Tests; use OC\Files\Cache\Scanner; use OCA\Files_Sharing\Controller\ShareAPIController; +use OCP\App\IAppManager; use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSForbiddenException; @@ -107,6 +108,7 @@ class ApiTest extends TestCase { return vsprintf($text, $parameters); })); $config = $this->createMock(IConfig::class); + $appManager = $this->createMock(IAppManager::class); return new ShareAPIController( self::APP_NAME, @@ -118,7 +120,8 @@ class ApiTest extends TestCase { \OC::$server->getURLGenerator(), $userId, $l, - $config + $config, + $appManager ); } @@ -960,7 +963,7 @@ class ApiTest extends TestCase { // update public upload $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); - $ocs->updateShare($share1->getId(), null, null, 'true'); + $ocs->updateShare($share1->getId(), null, null, null, 'true'); $ocs->cleanup(); $share1 = $this->shareManager->getShareById($share1->getFullId()); @@ -1003,7 +1006,7 @@ class ApiTest extends TestCase { // update expire date to a valid value $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); - $ocs->updateShare($share1->getId(), null, null, null, $dateWithinRange->format('Y-m-d')); + $ocs->updateShare($share1->getId(), null, null, null, null, $dateWithinRange->format('Y-m-d')); $ocs->cleanup(); $share1 = $this->shareManager->getShareById($share1->getFullId()); @@ -1234,7 +1237,7 @@ class ApiTest extends TestCase { $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); try { - $result = $ocs->createShare($this->folder, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', $date); + $result = $ocs->createShare($this->folder, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, $date); $this->assertTrue($valid); } catch (OCSNotFoundException $e) { $this->assertFalse($valid); @@ -1270,7 +1273,7 @@ class ApiTest extends TestCase { $date->add(new \DateInterval('P5D')); $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); - $result = $ocs->createShare($this->filename, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', $date->format('Y-m-d')); + $result = $ocs->createShare($this->filename, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, $date->format('Y-m-d')); $ocs->cleanup(); $data = $result->getData(); @@ -1304,7 +1307,7 @@ class ApiTest extends TestCase { $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); try { - $ocs->createShare($this->filename, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', $date->format('Y-m-d')); + $ocs->createShare($this->filename, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, $date->format('Y-m-d')); $this->fail(); } catch (OCSException $e) { $this->assertEquals(404, $e->getCode()); @@ -1325,7 +1328,7 @@ class ApiTest extends TestCase { $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); try { - $ocs->createShare($this->filename, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', $date->format('Y-m-d')); + $ocs->createShare($this->filename, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, $date->format('Y-m-d')); $this->fail(); } catch(OCSException $e) { $this->assertEquals(404, $e->getCode()); diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 30041c3a27..4fd8162db3 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -26,6 +26,7 @@ */ namespace OCA\Files_Sharing\Tests\Controller; +use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\Files\File; @@ -88,6 +89,9 @@ class ShareAPIControllerTest extends TestCase { /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ private $config; + /** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */ + private $appManager; + protected function setUp() { $this->shareManager = $this->createMock(IManager::class); $this->shareManager @@ -107,6 +111,7 @@ class ShareAPIControllerTest extends TestCase { return vsprintf($text, $parameters); })); $this->config = $this->createMock(IConfig::class); + $this->appManager = $this->createMock(IAppManager::class); $this->ocs = new ShareAPIController( $this->appName, @@ -118,7 +123,8 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ); } @@ -137,7 +143,8 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['formatShare']) ->getMock(); } @@ -453,7 +460,8 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['canAccessShare']) ->getMock(); @@ -724,7 +732,8 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['formatShare']) ->getMock(); @@ -822,7 +831,8 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['formatShare']) ->getMock(); @@ -1008,7 +1018,7 @@ class ShareAPIControllerTest extends TestCase { )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'true', '', ''); + $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'true', '', null, ''); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1042,7 +1052,7 @@ class ShareAPIControllerTest extends TestCase { )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', 'password', ''); + $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', 'password', null, ''); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1089,7 +1099,7 @@ class ShareAPIControllerTest extends TestCase { )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', '2000-01-01'); + $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, '2000-01-01'); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1115,7 +1125,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('shareApiAllowLinks')->willReturn(true); $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); - $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', 'a1b2d3'); + $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, 'a1b2d3'); } /** @@ -1138,7 +1148,8 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['formatShare']) ->getMock(); @@ -1264,7 +1275,7 @@ class ShareAPIControllerTest extends TestCase { ->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, '', 'false', ''); + $result = $ocs->updateShare(42, null, '', null, 'false', ''); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1299,7 +1310,7 @@ class ShareAPIControllerTest extends TestCase { ->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, 'password', 'true', '2000-01-01'); + $result = $ocs->updateShare(42, null, 'password', null, 'true', '2000-01-01'); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1333,7 +1344,7 @@ class ShareAPIControllerTest extends TestCase { )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, $permissions, $password, $publicUpload, $expireDate); + $result = $ocs->updateShare(42, $permissions, $password, null, $publicUpload, $expireDate); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1357,7 +1368,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); - $ocs->updateShare(42, null, 'password', 'true', '2000-01-a'); + $ocs->updateShare(42, null, 'password', null, 'true', '2000-01-a'); } public function publicUploadParamsProvider() { @@ -1395,7 +1406,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(false); - $ocs->updateShare(42, $permissions, $password, $publicUpload, $expireDate); + $ocs->updateShare(42, $permissions, $password, null, $publicUpload, $expireDate); } /** @@ -1416,7 +1427,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); - $ocs->updateShare(42, null, 'password', 'true', ''); + $ocs->updateShare(42, null, 'password', null, 'true', ''); } public function testUpdateLinkSharePasswordDoesNotChangeOther() { @@ -1450,7 +1461,7 @@ class ShareAPIControllerTest extends TestCase { )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, 'newpassword', null, null); + $result = $ocs->updateShare(42, null, 'newpassword', null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1487,7 +1498,7 @@ class ShareAPIControllerTest extends TestCase { )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, null, null, '2010-12-23'); + $result = $ocs->updateShare(42, null, null, null, null, '2010-12-23'); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1524,7 +1535,7 @@ class ShareAPIControllerTest extends TestCase { ->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, null, 'true', null); + $result = $ocs->updateShare(42, null, null, null, 'true', null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1560,7 +1571,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getSharedWith')->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, 7, null, null, null); + $result = $ocs->updateShare(42, 7, null, null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1596,7 +1607,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getSharedWith')->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, 31, null, null, null); + $result = $ocs->updateShare(42, 31, null, null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1625,7 +1636,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getSharedWith')->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, 31, null, null, null); + $result = $ocs->updateShare(42, 31, null, null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1722,7 +1733,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); try { - $ocs->updateShare(42, null, null, 'true'); + $ocs->updateShare(42, null, null, null, 'true'); $this->fail(); } catch (OCSNotFoundException $e) { $this->assertEquals('Cannot increase permissions', $e->getMessage()); @@ -2276,7 +2287,52 @@ class ShareAPIControllerTest extends TestCase { 'share_with_displayname' => 'mail display name', 'mail_send' => 0, 'mimetype' => 'myFolderMimeType', - 'password' => 'password' + 'password' => 'password', + 'send_password_by_talk' => false + ], $share, [], false + ]; + + $share = \OC::$server->getShareManager()->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setSharedBy('initiator') + ->setSharedWith('user@server.com') + ->setShareOwner('owner') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder) + ->setShareTime(new \DateTime('2000-01-01T00:01:02')) + ->setTarget('myTarget') + ->setId(42) + ->setPassword('password') + ->setSendPasswordByTalk(true); + + $result[] = [ + [ + 'id' => 42, + 'share_type' => \OCP\Share::SHARE_TYPE_EMAIL, + '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' => '', + 'path' => 'folder', + 'item_type' => 'folder', + 'storage_id' => 'storageId', + 'storage' => 100, + 'item_source' => 2, + 'file_source' => 2, + 'file_parent' => 1, + 'file_target' => 'myTarget', + 'share_with' => 'user@server.com', + 'share_with_displayname' => 'mail display name', + 'mail_send' => 0, + 'mimetype' => 'myFolderMimeType', + 'password' => 'password', + 'send_password_by_talk' => true ], $share, [], false ];