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/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index bd1331a090..6ebffed2b5 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -61,6 +61,7 @@ use OCP\Files\NotFoundException; use OCP\Files\IRootFolder; use OCP\Share\Exceptions\ShareNotFound; use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\EventDispatcher\GenericEvent; use OCP\Share\IManager as ShareManager; /** @@ -143,6 +144,34 @@ class ShareController extends AuthPublicShareController { $this->shareManager = $shareManager; } + /** + * @PublicPage + * @NoCSRFRequired + * + * Show the authentication page + * The form has to submit to the authenticate method route + */ + public function showAuthenticate(): TemplateResponse { + $templateParameters = ['share' => $this->share]; + + $event = new GenericEvent(null, $templateParameters); + $this->eventDispatcher->dispatch('OCA\Files_Sharing::loadAdditionalScripts::publicShareAuth', $event); + + return new TemplateResponse('core', 'publicshareauth', $templateParameters, 'guest'); + } + + /** + * The template to show when authentication failed + */ + protected function showAuthFailed(): TemplateResponse { + $templateParameters = ['share' => $this->share, 'wrongpw' => true]; + + $event = new GenericEvent(null, $templateParameters); + $this->eventDispatcher->dispatch('OCA\Files_Sharing::loadAdditionalScripts::publicShareAuth', $event); + + return new TemplateResponse('core', 'publicshareauth', $templateParameters, 'guest'); + } + protected function verifyPassword(string $password): bool { return $this->shareManager->checkPassword($this->share, $password); } 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 ]; diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 73e962e329..6cc27957bb 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -338,7 +338,8 @@ class ShareByMailProvider implements IShareProvider { $share->getShareOwner(), $share->getPermissions(), $share->getToken(), - $share->getPassword() + $share->getPassword(), + $share->getSendPasswordByTalk() ); try { @@ -453,7 +454,7 @@ class ShareByMailProvider implements IShareProvider { $initiator = $share->getSharedBy(); $shareWith = $share->getSharedWith(); - if ($password === '' || $this->settingsManager->sendPasswordByMail() === false) { + if ($password === '' || $this->settingsManager->sendPasswordByMail() === false || $share->getSendPasswordByTalk()) { return false; } @@ -660,9 +661,11 @@ class ShareByMailProvider implements IShareProvider { * @param string $uidOwner * @param int $permissions * @param string $token + * @param string $password + * @param bool $sendPasswordByTalk * @return int */ - protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password) { + protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password, $sendPasswordByTalk) { $qb = $this->dbConnection->getQueryBuilder(); $qb->insert('share') ->setValue('share_type', $qb->createNamedParameter(\OCP\Share::SHARE_TYPE_EMAIL)) @@ -675,6 +678,7 @@ class ShareByMailProvider implements IShareProvider { ->setValue('permissions', $qb->createNamedParameter($permissions)) ->setValue('token', $qb->createNamedParameter($token)) ->setValue('password', $qb->createNamedParameter($password)) + ->setValue('password_by_talk', $qb->createNamedParameter($sendPasswordByTalk, IQueryBuilder::PARAM_BOOL)) ->setValue('stime', $qb->createNamedParameter(time())); /* @@ -703,7 +707,8 @@ class ShareByMailProvider implements IShareProvider { // a real password was given $validPassword = $plainTextPassword !== null && $plainTextPassword !== ''; - if($validPassword && $originalShare->getPassword() !== $share->getPassword()) { + if($validPassword && ($originalShare->getPassword() !== $share->getPassword() || + ($originalShare->getSendPasswordByTalk() && !$share->getSendPasswordByTalk()))) { $this->sendPassword($share, $plainTextPassword); } /* @@ -716,6 +721,7 @@ class ShareByMailProvider implements IShareProvider { ->set('uid_owner', $qb->createNamedParameter($share->getShareOwner())) ->set('uid_initiator', $qb->createNamedParameter($share->getSharedBy())) ->set('password', $qb->createNamedParameter($share->getPassword())) + ->set('password_by_talk', $qb->createNamedParameter($share->getSendPasswordByTalk(), IQueryBuilder::PARAM_BOOL)) ->set('expiration', $qb->createNamedParameter($share->getExpirationDate(), IQueryBuilder::PARAM_DATE)) ->set('note', $qb->createNamedParameter($share->getNote())) ->execute(); @@ -972,6 +978,7 @@ class ShareByMailProvider implements IShareProvider { $share->setShareTime($shareTime); $share->setSharedWith($data['share_with']); $share->setPassword($data['password']); + $share->setSendPasswordByTalk($data['password_by_talk']); if ($data['uid_initiator'] !== null) { $share->setShareOwner($data['uid_owner']); diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index f0d99e6026..4942dac702 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -38,6 +38,7 @@ use OCP\IUser; use OCP\IUserManager; use OCP\Mail\IEMailTemplate; use OCP\Mail\IMailer; +use OCP\Mail\IMessage; use OCP\Security\IHasher; use OCP\Security\ISecureRandom; use OCP\Share\IManager; @@ -204,6 +205,107 @@ class ShareByMailProviderTest extends TestCase { ); } + public function testCreateSendPasswordByMailWithoutEnforcedPasswordProtection() { + $share = $this->getMockBuilder(IShare::class)->getMock(); + $share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); + $share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false); + $share->expects($this->any())->method('getSharedBy')->willReturn('owner'); + + $node = $this->getMockBuilder(File::class)->getMock(); + $node->expects($this->any())->method('getName')->willReturn('filename'); + + $instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity']); + + $instance->expects($this->once())->method('getSharedWith')->willReturn([]); + $instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42); + $instance->expects($this->once())->method('createShareActivity')->with($share); + $instance->expects($this->once())->method('getRawShare')->with(42)->willReturn('rawShare'); + $instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject'); + $share->expects($this->any())->method('getNode')->willReturn($node); + + // The autogenerated password should not be mailed. + $this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(false); + $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); + $instance->expects($this->never())->method('autoGeneratePassword'); + + $this->mailer->expects($this->never())->method('send'); + + $this->assertSame('shareObject', + $instance->create($share) + ); + } + + public function testCreateSendPasswordByMailWithEnforcedPasswordProtection() { + $share = $this->getMockBuilder(IShare::class)->getMock(); + $share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); + $share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false); + $share->expects($this->any())->method('getSharedBy')->willReturn('owner'); + + $node = $this->getMockBuilder(File::class)->getMock(); + $node->expects($this->any())->method('getName')->willReturn('filename'); + + $instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity']); + + $instance->expects($this->once())->method('getSharedWith')->willReturn([]); + $instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42); + $instance->expects($this->once())->method('createShareActivity')->with($share); + $instance->expects($this->once())->method('getRawShare')->with(42)->willReturn('rawShare'); + $instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject'); + $share->expects($this->any())->method('getNode')->willReturn($node); + + // The autogenerated password should be mailed to the receiver of the share. + $this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(true); + $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); + $instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('password'); + + $message = $this->createMock(IMessage::class); + $message->expects($this->once())->method('setTo')->with(['receiver@example.com']); + $this->mailer->expects($this->once())->method('createMessage')->willReturn($message); + $this->mailer->expects($this->once())->method('send'); + + $this->assertSame('shareObject', + $instance->create($share) + ); + } + + public function testCreateSendPasswordByTalkWithEnforcedPasswordProtection() { + $share = $this->getMockBuilder(IShare::class)->getMock(); + $share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); + $share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(true); + $share->expects($this->any())->method('getSharedBy')->willReturn('owner'); + + $node = $this->getMockBuilder(File::class)->getMock(); + $node->expects($this->any())->method('getName')->willReturn('filename'); + + $instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity']); + + $instance->expects($this->once())->method('getSharedWith')->willReturn([]); + $instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42); + $instance->expects($this->once())->method('createShareActivity')->with($share); + $instance->expects($this->once())->method('getRawShare')->with(42)->willReturn('rawShare'); + $instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject'); + $share->expects($this->any())->method('getNode')->willReturn($node); + + // The autogenerated password should be mailed to the owner of the share. + $this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(true); + $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); + $instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('password'); + + $message = $this->createMock(IMessage::class); + $message->expects($this->once())->method('setTo')->with(['owner@example.com' => 'Owner display name']); + $this->mailer->expects($this->once())->method('createMessage')->willReturn($message); + $this->mailer->expects($this->once())->method('send'); + + $user = $this->createMock(IUser::class); + $this->userManager->expects($this->once())->method('get')->with('owner')->willReturn($user); + $user->expects($this->once())->method('getDisplayName')->willReturn('Owner display name'); + $user->expects($this->once())->method('getEMailAddress')->willReturn('owner@example.com'); + + $this->assertSame('shareObject', + $instance->create($share) + ); + } + /** * @expectedException \Exception */ @@ -296,6 +398,7 @@ class ShareByMailProviderTest extends TestCase { $permissions = 1; $token = 'token'; $password = 'password'; + $sendPasswordByTalk = true; $instance = $this->getInstance(); @@ -310,7 +413,8 @@ class ShareByMailProviderTest extends TestCase { $uidOwner, $permissions, $token, - $password + $password, + $sendPasswordByTalk ] ); @@ -330,6 +434,7 @@ class ShareByMailProviderTest extends TestCase { $this->assertSame($permissions, (int)$result[0]['permissions']); $this->assertSame($token, $result[0]['token']); $this->assertSame($password, $result[0]['password']); + $this->assertSame($sendPasswordByTalk, (bool)$result[0]['password_by_talk']); } @@ -377,6 +482,60 @@ class ShareByMailProviderTest extends TestCase { $this->assertSame($note, $result[0]['note']); } + public function dataUpdateSendPassword() { + return [ + ['password', 'hashed', 'hashed new', false, false, true], + ['', 'hashed', 'hashed new', false, false, false], + [null, 'hashed', 'hashed new', false, false, false], + ['password', 'hashed', 'hashed', false, false, false], + ['password', 'hashed', 'hashed new', false, true, false], + ['password', 'hashed', 'hashed new', true, false, true], + ['password', 'hashed', 'hashed', true, false, true], + ]; + } + + /** + * @dataProvider dataUpdateSendPassword + * + * @param string|null plainTextPassword + * @param string originalPassword + * @param string newPassword + * @param string originalSendPasswordByTalk + * @param string newSendPasswordByTalk + * @param bool sendMail + */ + public function testUpdateSendPassword($plainTextPassword, string $originalPassword, string $newPassword, string $originalSendPasswordByTalk, string $newSendPasswordByTalk, bool $sendMail) { + $node = $this->getMockBuilder(File::class)->getMock(); + $node->expects($this->any())->method('getName')->willReturn('filename'); + + $originalShare = $this->getMockBuilder(IShare::class)->getMock(); + $originalShare->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); + $originalShare->expects($this->any())->method('getNode')->willReturn($node); + $originalShare->expects($this->any())->method('getId')->willReturn(42); + $originalShare->expects($this->any())->method('getPassword')->willReturn($originalPassword); + $originalShare->expects($this->any())->method('getSendPasswordByTalk')->willReturn($originalSendPasswordByTalk); + + $share = $this->getMockBuilder(IShare::class)->getMock(); + $share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); + $share->expects($this->any())->method('getNode')->willReturn($node); + $share->expects($this->any())->method('getId')->willReturn(42); + $share->expects($this->any())->method('getPassword')->willReturn($newPassword); + $share->expects($this->any())->method('getSendPasswordByTalk')->willReturn($newSendPasswordByTalk); + + if ($sendMail) { + $this->mailer->expects($this->once())->method('send'); + } else { + $this->mailer->expects($this->never())->method('send'); + } + + $instance = $this->getInstance(['getShareById', 'createPasswordSendActivity']); + $instance->expects($this->once())->method('getShareById')->willReturn($originalShare); + + $this->assertSame($share, + $instance->update($share, $plainTextPassword) + ); + } + public function testDelete() { $instance = $this->getInstance(['removeShareFromTable']); $this->share->expects($this->once())->method('getId')->willReturn(42); diff --git a/core/Migrations/Version14000Date20180710092004.php b/core/Migrations/Version14000Date20180710092004.php new file mode 100644 index 0000000000..d2e364063c --- /dev/null +++ b/core/Migrations/Version14000Date20180710092004.php @@ -0,0 +1,48 @@ +. + * + */ + +namespace OC\Core\Migrations; + +use Doctrine\DBAL\Types\Type; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\SimpleMigrationStep; +use OCP\Migration\IOutput; + +class Version14000Date20180710092004 extends SimpleMigrationStep { + + public function changeSchema(IOutput $output, \Closure $schemaClosure, array $options) { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $table = $schema->getTable('share'); + + if (!$table->hasColumn('password_by_talk')) { + $table->addColumn('password_by_talk', Type::BOOLEAN, [ + 'default' => 0, + ]); + } + + return $schema; + } +} diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 574d47b4aa..8df7fe5ecd 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -112,6 +112,21 @@ ' ' + '' + '' + + '{{#if isTalkEnabled}}' + + '
  • ' + + '' + + '' + + '' + + '' + + '
  • ' + + '
  • ' + + '' + + ' ' + + ' ' + + ' ' + + '' + + '
  • ' + + '{{/if}}' + '{{/if}}' + '
  • ' + '' + @@ -183,6 +198,7 @@ 'click .permissions': 'onPermissionChange', 'click .expireDate' : 'onExpireDateChange', 'click .password' : 'onMailSharePasswordProtectChange', + 'click .passwordByTalk' : 'onMailSharePasswordProtectByTalkChange', 'click .secureDrop' : 'onSecureDropChange', 'keyup input.passwordField': 'onMailSharePasswordKeyUp', 'focusout input.passwordField': 'onMailSharePasswordEntered', @@ -260,7 +276,7 @@ var share = this.model.get('shares')[shareIndex]; var password = share.password; var hasPassword = password !== null && password !== ''; - + var sendPasswordByTalk = share.send_password_by_talk; return _.extend(hasPermissionOverride, { cid: this.cid, @@ -282,12 +298,22 @@ isMailShare: shareType === OC.Share.SHARE_TYPE_EMAIL, isCircleShare: shareType === OC.Share.SHARE_TYPE_CIRCLE, isFileSharedByMail: shareType === OC.Share.SHARE_TYPE_EMAIL && !this.model.isFolder(), - isPasswordSet: hasPassword, + isPasswordSet: hasPassword && !sendPasswordByTalk, + isPasswordByTalkSet: hasPassword && sendPasswordByTalk, + isTalkEnabled: oc_appswebroots['spreed'] !== undefined, secureDropMode: !this.model.hasReadPermission(shareIndex), hasExpireDate: this.model.getExpireDate(shareIndex) !== null, shareNote: this.model.getNote(shareIndex), expireDate: moment(this.model.getExpireDate(shareIndex), 'YYYY-MM-DD').format('DD-MM-YYYY'), + // The password placeholder does not take into account if + // sending the password by Talk is enabled or not; when + // switching from sending the password by Talk to sending the + // password by email the password is reused and the share + // updated, so the placeholder already shows the password in the + // brief time between disabling sending the password by email + // and receiving the updated share. passwordPlaceholder: hasPassword ? PASSWORD_PLACEHOLDER : PASSWORD_PLACEHOLDER_MESSAGE, + passwordByTalkPlaceholder: (hasPassword && sendPasswordByTalk)? PASSWORD_PLACEHOLDER : PASSWORD_PLACEHOLDER_MESSAGE, }); }, @@ -303,6 +329,7 @@ secureDropLabel: t('core', 'File drop (upload only)'), expireDateLabel: t('core', 'Set expiration date'), passwordLabel: t('core', 'Password protect'), + passwordByTalkLabel: t('core', 'Password protect by Talk'), crudsLabel: t('core', 'Access control'), expirationDatePlaceholder: t('core', 'Expiration date'), defaultExpireDate: moment().add(1, 'day').format('DD-MM-YYYY'), // Can't expire today @@ -674,8 +701,10 @@ var inputClass = '#passwordField-' + this.cid + '-' + shareId; var passwordField = $(inputClass); var state = element.prop('checked'); - if (!state) { - this.model.updateShare(shareId, {password: ''}); + var passwordByTalkElement = $('#passwordByTalk-' + this.cid + '-' + shareId); + var passwordByTalkState = passwordByTalkElement.prop('checked'); + if (!state && !passwordByTalkState) { + this.model.updateShare(shareId, {password: '', sendPasswordByTalk: false}); passwordField.attr('value', ''); passwordField.removeClass('error'); passwordField.tooltip('hide'); @@ -683,13 +712,67 @@ passwordField.attr('placeholder', PASSWORD_PLACEHOLDER_MESSAGE); // We first need to reset the password field before we hide it passwordContainer.toggleClass('hidden', !state); - } else { + } else if (state) { + if (passwordByTalkState) { + // Switching from sending the password by Talk to sending + // the password by mail can be done keeping the previous + // password sent by Talk. + this.model.updateShare(shareId, {sendPasswordByTalk: false}); + + var passwordByTalkContainerClass = '.passwordByTalkMenu-' + this.cid + '-' + shareId; + var passwordByTalkContainer = $(passwordByTalkContainerClass); + passwordByTalkContainer.addClass('hidden'); + passwordByTalkElement.prop('checked', false); + } + passwordContainer.toggleClass('hidden', !state); passwordField = '#passwordField-' + this.cid + '-' + shareId; this.$(passwordField).focus(); } }, + onMailSharePasswordProtectByTalkChange: function(event) { + var element = $(event.target); + var li = element.closest('li[data-share-id]'); + var shareId = li.data('share-id'); + var passwordByTalkContainerClass = '.passwordByTalkMenu-' + this.cid + '-' + shareId; + var passwordByTalkContainer = $(passwordByTalkContainerClass); + var loading = this.$el.find(passwordByTalkContainerClass + ' .icon-loading-small'); + var inputClass = '#passwordByTalkField-' + this.cid + '-' + shareId; + var passwordByTalkField = $(inputClass); + var state = element.prop('checked'); + var passwordElement = $('#password-' + this.cid + '-' + shareId); + var passwordState = passwordElement.prop('checked'); + if (!state) { + this.model.updateShare(shareId, {password: '', sendPasswordByTalk: false}); + passwordByTalkField.attr('value', ''); + passwordByTalkField.removeClass('error'); + passwordByTalkField.tooltip('hide'); + loading.addClass('hidden'); + passwordByTalkField.attr('placeholder', PASSWORD_PLACEHOLDER_MESSAGE); + // We first need to reset the password field before we hide it + passwordByTalkContainer.toggleClass('hidden', !state); + } else if (state) { + if (passwordState) { + // Enabling sending the password by Talk requires a new + // password to be given (the one sent by mail is not reused, + // as it would defeat the purpose of checking the identity + // of the sharee by Talk if it was already sent by mail), so + // the share is not updated until the user explicitly gives + // the new password. + + var passwordContainerClass = '.passwordMenu-' + this.cid + '-' + shareId; + var passwordContainer = $(passwordContainerClass); + passwordContainer.addClass('hidden'); + passwordElement.prop('checked', false); + } + + passwordByTalkContainer.toggleClass('hidden', !state); + passwordByTalkField = '#passwordByTalkField-' + this.cid + '-' + shareId; + this.$(passwordByTalkField).focus(); + } + }, + onMailSharePasswordKeyUp: function(event) { if(event.keyCode === 13) { this.onMailSharePasswordEntered(event); @@ -700,8 +783,15 @@ var passwordField = $(event.target); var li = passwordField.closest('li[data-share-id]'); var shareId = li.data('share-id'); - var passwordContainerClass = '.passwordContainer-' + this.cid + '-' + shareId; - var loading = this.$el.find(passwordContainerClass + ' .icon-loading-small'); + var passwordContainerClass = '.passwordMenu-' + this.cid + '-' + shareId; + var passwordByTalkContainerClass = '.passwordByTalkMenu-' + this.cid + '-' + shareId; + var sendPasswordByTalk = passwordField.attr('id').startsWith('passwordByTalk'); + var loading; + if (sendPasswordByTalk) { + loading = this.$el.find(passwordByTalkContainerClass + ' .icon-loading-small'); + } else { + loading = this.$el.find(passwordContainerClass + ' .icon-loading-small'); + } if (!loading.hasClass('hidden')) { // still in process return; @@ -720,7 +810,8 @@ this.model.updateShare(shareId, { - password: password + password: password, + sendPasswordByTalk: sendPasswordByTalk }, { error: function(model, msg) { // destroy old tooltips diff --git a/core/templates/publicshareauth.php b/core/templates/publicshareauth.php index adcc2853f8..22e2262229 100644 --- a/core/templates/publicshareauth.php +++ b/core/templates/publicshareauth.php @@ -20,6 +20,7 @@ placeholder="t('Password')); ?>" value="" autocomplete="new-password" autocapitalize="off" autocorrect="off" autofocus /> +

    diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index a060131979..44b7cd5244 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -611,6 +611,7 @@ return array( 'OC\\Core\\Migrations\\Version14000Date20180518120534' => $baseDir . '/core/Migrations/Version14000Date20180518120534.php', 'OC\\Core\\Migrations\\Version14000Date20180522074438' => $baseDir . '/core/Migrations/Version14000Date20180522074438.php', 'OC\\Core\\Migrations\\Version14000Date20180626223656' => $baseDir . '/core/Migrations/Version14000Date20180626223656.php', + 'OC\\Core\\Migrations\\Version14000Date20180710092004' => $baseDir . '/core/Migrations/Version14000Date20180710092004.php', 'OC\\Core\\Migrations\\Version14000Date20180712153140' => $baseDir . '/core/Migrations/Version14000Date20180712153140.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', 'OC\\DB\\AdapterMySQL' => $baseDir . '/lib/private/DB/AdapterMySQL.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 4c6c55a59a..815f94d571 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -641,6 +641,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Migrations\\Version14000Date20180518120534' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180518120534.php', 'OC\\Core\\Migrations\\Version14000Date20180522074438' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180522074438.php', 'OC\\Core\\Migrations\\Version14000Date20180626223656' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180626223656.php', + 'OC\\Core\\Migrations\\Version14000Date20180710092004' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180710092004.php', 'OC\\Core\\Migrations\\Version14000Date20180712153140' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180712153140.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', 'OC\\DB\\AdapterMySQL' => __DIR__ . '/../../..' . '/lib/private/DB/AdapterMySQL.php', diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 76b523afd1..d0316b44c1 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -827,10 +827,20 @@ class Manager implements IManager { $expirationDateUpdated = true; } } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL) { + // The new password is not set again if it is the same as the old + // one, unless when switching from sending by Talk to sending by + // mail. $plainTextPassword = $share->getPassword(); - if (!$this->updateSharePasswordIfNeeded($share, $originalShare)) { + if (!empty($plainTextPassword) && !$this->updateSharePasswordIfNeeded($share, $originalShare) && + !($originalShare->getSendPasswordByTalk() && !$share->getSendPasswordByTalk())) { $plainTextPassword = null; } + if (empty($plainTextPassword) && !$originalShare->getSendPasswordByTalk() && $share->getSendPasswordByTalk()) { + // If the same password was already sent by mail the recipient + // would already have access to the share without having to call + // the sharer to verify her identity + throw new \InvalidArgumentException('Can’t enable sending the password by Talk without setting a new password'); + } } $this->pathCreateChecks($share->getNode()); diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index e54497c9b5..71c0453d9e 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -63,6 +63,8 @@ class Share implements \OCP\Share\IShare { private $expireDate; /** @var string */ private $password; + /** @var bool */ + private $sendPasswordByTalk = false; /** @var string */ private $token; /** @var int */ @@ -402,6 +404,21 @@ class Share implements \OCP\Share\IShare { return $this->password; } + /** + * @inheritdoc + */ + public function setSendPasswordByTalk(bool $sendPasswordByTalk) { + $this->sendPasswordByTalk = $sendPasswordByTalk; + return $this; + } + + /** + * @inheritdoc + */ + public function getSendPasswordByTalk(): bool { + return $this->sendPasswordByTalk; + } + /** * @inheritdoc */ diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php index 5303cde45a..43543fdad4 100644 --- a/lib/public/Share/IShare.php +++ b/lib/public/Share/IShare.php @@ -312,6 +312,29 @@ interface IShare { */ public function getPassword(); + + /** + * Set if the recipient can start a conversation with the owner to get the + * password using Nextcloud Talk. + * + * @param bool $sendPasswordByTalk + * @return \OCP\Share\IShare The modified object + * @since 14.0.0 + */ + public function setSendPasswordByTalk(bool $sendPasswordByTalk); + + /** + * Get if the recipient can start a conversation with the owner to get the + * password using Nextcloud Talk. + * The returned value does not take into account other factors, like Talk + * being enabled for the owner of the share or not; it just cover whether + * the option is enabled for the share itself or not. + * + * @return bool + * @since 14.0.0 + */ + public function getSendPasswordByTalk(): bool; + /** * Set the public link token. * diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index cc98223d4b..7106e22b26 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -2716,6 +2716,442 @@ class ManagerTest extends \Test\TestCase { $manager->updateShare($share); } + public function testUpdateShareMailEnableSendPasswordByTalk() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'getShareById', + 'generalCreateChecks', + 'verifyPassword', + 'pathCreateChecks', + 'linkCreateChecks', + 'validateExpirationDate', + ]) + ->getMock(); + + $originalShare = $this->manager->newShare(); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setPassword(null) + ->setSendPasswordByTalk(false); + + $tomorrow = new \DateTime(); + $tomorrow->setTime(0,0,0); + $tomorrow->add(new \DateInterval('P1D')); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(100); + + $share = $this->manager->newShare(); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setToken('token') + ->setSharedBy('owner') + ->setShareOwner('owner') + ->setPassword('password') + ->setSendPasswordByTalk(true) + ->setExpirationDate($tomorrow) + ->setNode($file) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); + $manager->expects($this->once())->method('generalCreateChecks')->with($share); + $manager->expects($this->once())->method('verifyPassword')->with('password'); + $manager->expects($this->once())->method('pathCreateChecks')->with($file); + $manager->expects($this->never())->method('linkCreateChecks'); + $manager->expects($this->never())->method('validateExpirationDate'); + + $this->hasher->expects($this->once()) + ->method('hash') + ->with('password') + ->willReturn('hashed'); + + $this->defaultProvider->expects($this->once()) + ->method('update') + ->with($share, 'password') + ->willReturn($share); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); + $hookListner->expects($this->never())->method('post'); + + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post'); + $hookListner2->expects($this->once())->method('post')->with([ + 'itemType' => 'file', + 'itemSource' => 100, + 'uidOwner' => 'owner', + 'token' => 'token', + 'disabled' => false, + ]); + + $hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post'); + $hookListner3->expects($this->never())->method('post'); + + $manager->updateShare($share); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Can’t enable sending the password by Talk without setting a new password + */ + public function testUpdateShareMailEnableSendPasswordByTalkWithNoPassword() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'getShareById', + 'generalCreateChecks', + 'verifyPassword', + 'pathCreateChecks', + 'linkCreateChecks', + 'validateExpirationDate', + ]) + ->getMock(); + + $originalShare = $this->manager->newShare(); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setPassword(null) + ->setSendPasswordByTalk(false); + + $tomorrow = new \DateTime(); + $tomorrow->setTime(0,0,0); + $tomorrow->add(new \DateInterval('P1D')); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(100); + + $share = $this->manager->newShare(); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setToken('token') + ->setSharedBy('owner') + ->setShareOwner('owner') + ->setPassword(null) + ->setSendPasswordByTalk(true) + ->setExpirationDate($tomorrow) + ->setNode($file) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); + $manager->expects($this->once())->method('generalCreateChecks')->with($share); + $manager->expects($this->never())->method('verifyPassword'); + $manager->expects($this->never())->method('pathCreateChecks'); + $manager->expects($this->never())->method('linkCreateChecks'); + $manager->expects($this->never())->method('validateExpirationDate'); + + $this->hasher->expects($this->never()) + ->method('hash'); + + $this->defaultProvider->expects($this->never()) + ->method('update'); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); + $hookListner->expects($this->never())->method('post'); + + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post'); + $hookListner2->expects($this->never())->method('post'); + + $hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post'); + $hookListner3->expects($this->never())->method('post'); + + $manager->updateShare($share); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Can’t enable sending the password by Talk without setting a new password + */ + public function testUpdateShareMailEnableSendPasswordByTalkRemovingPassword() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'getShareById', + 'generalCreateChecks', + 'verifyPassword', + 'pathCreateChecks', + 'linkCreateChecks', + 'validateExpirationDate', + ]) + ->getMock(); + + $originalShare = $this->manager->newShare(); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setPassword('password') + ->setSendPasswordByTalk(false); + + $tomorrow = new \DateTime(); + $tomorrow->setTime(0,0,0); + $tomorrow->add(new \DateInterval('P1D')); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(100); + + $share = $this->manager->newShare(); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setToken('token') + ->setSharedBy('owner') + ->setShareOwner('owner') + ->setPassword(null) + ->setSendPasswordByTalk(true) + ->setExpirationDate($tomorrow) + ->setNode($file) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); + $manager->expects($this->once())->method('generalCreateChecks')->with($share); + $manager->expects($this->never())->method('verifyPassword'); + $manager->expects($this->never())->method('pathCreateChecks'); + $manager->expects($this->never())->method('linkCreateChecks'); + $manager->expects($this->never())->method('validateExpirationDate'); + + $this->hasher->expects($this->never()) + ->method('hash'); + + $this->defaultProvider->expects($this->never()) + ->method('update'); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); + $hookListner->expects($this->never())->method('post'); + + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post'); + $hookListner2->expects($this->never())->method('post'); + + $hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post'); + $hookListner3->expects($this->never())->method('post'); + + $manager->updateShare($share); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Can’t enable sending the password by Talk without setting a new password + */ + public function testUpdateShareMailEnableSendPasswordByTalkRemovingPasswordWithEmptyString() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'getShareById', + 'generalCreateChecks', + 'verifyPassword', + 'pathCreateChecks', + 'linkCreateChecks', + 'validateExpirationDate', + ]) + ->getMock(); + + $originalShare = $this->manager->newShare(); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setPassword('password') + ->setSendPasswordByTalk(false); + + $tomorrow = new \DateTime(); + $tomorrow->setTime(0,0,0); + $tomorrow->add(new \DateInterval('P1D')); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(100); + + $share = $this->manager->newShare(); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setToken('token') + ->setSharedBy('owner') + ->setShareOwner('owner') + ->setPassword('') + ->setSendPasswordByTalk(true) + ->setExpirationDate($tomorrow) + ->setNode($file) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); + $manager->expects($this->once())->method('generalCreateChecks')->with($share); + $manager->expects($this->never())->method('verifyPassword'); + $manager->expects($this->never())->method('pathCreateChecks'); + $manager->expects($this->never())->method('linkCreateChecks'); + $manager->expects($this->never())->method('validateExpirationDate'); + + $this->hasher->expects($this->never()) + ->method('hash'); + + $this->defaultProvider->expects($this->never()) + ->method('update'); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); + $hookListner->expects($this->never())->method('post'); + + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post'); + $hookListner2->expects($this->never())->method('post'); + + $hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post'); + $hookListner3->expects($this->never())->method('post'); + + $manager->updateShare($share); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Can’t enable sending the password by Talk without setting a new password + */ + public function testUpdateShareMailEnableSendPasswordByTalkWithPreviousPassword() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'getShareById', + 'generalCreateChecks', + 'verifyPassword', + 'pathCreateChecks', + 'linkCreateChecks', + 'validateExpirationDate', + ]) + ->getMock(); + + $originalShare = $this->manager->newShare(); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setPassword('password') + ->setSendPasswordByTalk(false); + + $tomorrow = new \DateTime(); + $tomorrow->setTime(0,0,0); + $tomorrow->add(new \DateInterval('P1D')); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(100); + + $share = $this->manager->newShare(); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setToken('token') + ->setSharedBy('owner') + ->setShareOwner('owner') + ->setPassword('password') + ->setSendPasswordByTalk(true) + ->setExpirationDate($tomorrow) + ->setNode($file) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); + $manager->expects($this->once())->method('generalCreateChecks')->with($share); + $manager->expects($this->never())->method('verifyPassword'); + $manager->expects($this->never())->method('pathCreateChecks'); + $manager->expects($this->never())->method('linkCreateChecks'); + $manager->expects($this->never())->method('validateExpirationDate'); + + $this->hasher->expects($this->never()) + ->method('hash'); + + $this->defaultProvider->expects($this->never()) + ->method('update'); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); + $hookListner->expects($this->never())->method('post'); + + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post'); + $hookListner2->expects($this->never())->method('post'); + + $hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post'); + $hookListner3->expects($this->never())->method('post'); + + $manager->updateShare($share); + } + + public function testUpdateShareMailDisableSendPasswordByTalkWithPreviousPassword() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'getShareById', + 'generalCreateChecks', + 'verifyPassword', + 'pathCreateChecks', + 'linkCreateChecks', + 'validateExpirationDate', + ]) + ->getMock(); + + $originalShare = $this->manager->newShare(); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setPassword('password') + ->setSendPasswordByTalk(true); + + $tomorrow = new \DateTime(); + $tomorrow->setTime(0,0,0); + $tomorrow->add(new \DateInterval('P1D')); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(100); + + $share = $this->manager->newShare(); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setToken('token') + ->setSharedBy('owner') + ->setShareOwner('owner') + ->setPassword('password') + ->setSendPasswordByTalk(false) + ->setExpirationDate($tomorrow) + ->setNode($file) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); + $manager->expects($this->once())->method('generalCreateChecks')->with($share); + $manager->expects($this->once())->method('pathCreateChecks')->with($file); + $manager->expects($this->never())->method('verifyPassword'); + $manager->expects($this->never())->method('linkCreateChecks'); + $manager->expects($this->never())->method('validateExpirationDate'); + + $this->hasher->expects($this->never()) + ->method('hash'); + + $this->defaultProvider->expects($this->once()) + ->method('update') + ->with($share, 'password') + ->willReturn($share); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); + $hookListner->expects($this->never())->method('post'); + + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post'); + $hookListner2->expects($this->never())->method('post'); + + $hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post'); + $hookListner3->expects($this->never())->method('post'); + + $manager->updateShare($share); + } + /** * @expectedException \InvalidArgumentException * @expectedExceptionMessage Can’t change target of link share diff --git a/version.php b/version.php index c18ca01286..429bfec990 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = array(14, 0, 0, 11); +$OC_Version = array(14, 0, 0, 12); // The human readable string $OC_VersionString = '14.0.0 alpha';