From 49fd17ff145c20d987a355518e36f2dfded2fc96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 10 Jul 2018 12:01:17 +0200 Subject: [PATCH 1/7] Add "password_by_talk" column to "share" table in the database MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../Version14000Date20180710092004.php | 48 +++++++++++++++++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + version.php | 2 +- 4 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 core/Migrations/Version14000Date20180710092004.php 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/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/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'; From 88600f4ecf6358f5279327cc8ed8471ae7d33b2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 10 Jul 2018 12:32:12 +0200 Subject: [PATCH 2/7] Add "sendPasswordByTalk" property to shares 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/Share20/Share.php | 17 +++++++++++++++++ lib/public/Share/IShare.php | 23 +++++++++++++++++++++++ 2 files changed, 40 insertions(+) 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. * From dd0c5e297e3190b1f4324fb2e88e08760d8d71b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 10 Jul 2018 12:33:25 +0200 Subject: [PATCH 3/7] Store "sendPasswordByTalk" property of mail shares in the database MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- apps/sharebymail/lib/ShareByMailProvider.php | 10 ++++++++-- apps/sharebymail/tests/ShareByMailProviderTest.php | 5 ++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 73e962e329..d26938a324 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 { @@ -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())); /* @@ -716,6 +720,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 +977,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..48b5de6214 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -296,6 +296,7 @@ class ShareByMailProviderTest extends TestCase { $permissions = 1; $token = 'token'; $password = 'password'; + $sendPasswordByTalk = true; $instance = $this->getInstance(); @@ -310,7 +311,8 @@ class ShareByMailProviderTest extends TestCase { $uidOwner, $permissions, $token, - $password + $password, + $sendPasswordByTalk ] ); @@ -330,6 +332,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']); } From d582a66bea10e3a7ab41c1e475c5eec3eb218ba0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 10 Jul 2018 12:36:28 +0200 Subject: [PATCH 4/7] Honour "sendPasswordByTalk" property in mail shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a password was set for a mail share an e-mail was sent to the recipient with the password. Now the e-mail is no longer sent if the password is meant to be sent by Talk. However, before the e-mail was not sent when the share was updated but the password was not changed. Now an e-mail is sent in that case too if switching from a password sent by Talk to a password sent by mail. On the other hand, when switching from a password sent by mail to a password sent by Talk it is mandatory to change the password; otherwise the recipient would already have access to the share without having to call the sharer to verify her identity. Signed-off-by: Daniel Calviño Sánchez --- apps/sharebymail/lib/ShareByMailProvider.php | 5 +- .../tests/ShareByMailProviderTest.php | 156 +++++++ lib/private/Share20/Manager.php | 12 +- tests/lib/Share20/ManagerTest.php | 436 ++++++++++++++++++ 4 files changed, 606 insertions(+), 3 deletions(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index d26938a324..6cc27957bb 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -454,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; } @@ -707,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); } /* diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index 48b5de6214..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 */ @@ -380,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/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/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 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 5/7] 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 ]; From 96108ab8588aa63a242df4a435d26e4ea3aecfdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 12 Jul 2018 14:52:36 +0200 Subject: [PATCH 6/7] Add event to load additional scripts in the auth page for public shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before the public share authentication page is rendered now an event to load additional scripts is dispatched. Thanks to this any app can load its own scripts that, when run on the browser, adjust as needed the page generated by the server. Note, however, that during the handling of the event apps are only able to add scripts or styles to be loaded; they can not render arbitrary content on the page, or change how the content is rendered by the original template; all those changes have to be done by the scripts at run-time. This implies that the scripts of the apps can use only those parameters, like the token of the share, added to the page when it is generated by the "publicshareauth" template. Due to this, and given that the event is being introduced to be used by Talk to inject the UI needed to request the password for a share, the token of the share is now provided in the generated page, just like done in the public share page. Signed-off-by: Daniel Calviño Sánchez --- .../lib/Controller/ShareController.php | 29 +++++++++++++++++++ core/templates/publicshareauth.php | 1 + 2 files changed, 30 insertions(+) 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/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 /> +

From 911093549ebe3068dc7df918c552dc45a4ed43f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 13 Jul 2018 13:00:10 +0200 Subject: [PATCH 7/7] Make possible to enable "sending password by Talk" from the share menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until now the password to be sent by mail was set by enabling a checkbox in the share menu and then entering the password in an input field shown below. Now, when Talk is enabled, another item is added to the share menu to do the same for a password to be sent by Talk. Sending the password by mail and sending it by Talk are mutually exclusive actions, so when one of the checkboxes is enabled the other one is automatically disabled. Note that the icon set for the field, "icon-passwordtalk", does not currently exist; it simply mimics the "icon-passwordmail" (which does not exist either) used for the field of the password protect by mail to get the right padding in the menu. Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogshareelistview.js | 107 +++++++++++++++++++++++++-- 1 file changed, 99 insertions(+), 8 deletions(-) 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