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 0000000000000..d2e364063ced3 --- /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 a060131979d01..44b7cd5244f4e 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -611,6 +611,7 @@ '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 4c6c55a59ad64..815f94d57116e 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 c18ca01286ba1..429bfec990fea 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 e54497c9b5517..71c0453d9e578 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 @@ public function getPassword() { 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 5303cde45a6eb..43543fdad4761 100644 --- a/lib/public/Share/IShare.php +++ b/lib/public/Share/IShare.php @@ -312,6 +312,29 @@ public function setPassword($password); */ 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 73e962e3292be..d26938a3242a3 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -338,7 +338,8 @@ protected function createMailShare(IShare $share) { $share->getShareOwner(), $share->getPermissions(), $share->getToken(), - $share->getPassword() + $share->getPassword(), + $share->getSendPasswordByTalk() ); try { @@ -660,9 +661,11 @@ public function getChildren(IShare $parent) { * @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 @@ protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ ->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 @@ public function update(IShare $share, $plainTextPassword = null) { ->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 @@ protected function createShareObject($data) { $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 f0d99e6026cda..48b5de62146cf 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -296,6 +296,7 @@ public function testAddShareToDB() { $permissions = 1; $token = 'token'; $password = 'password'; + $sendPasswordByTalk = true; $instance = $this->getInstance(); @@ -310,7 +311,8 @@ public function testAddShareToDB() { $uidOwner, $permissions, $token, - $password + $password, + $sendPasswordByTalk ] ); @@ -330,6 +332,7 @@ public function testAddShareToDB() { $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 d26938a3242a3..6cc27957bbe61 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -454,7 +454,7 @@ protected function sendPassword(IShare $share, $password) { $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 @@ public function update(IShare $share, $plainTextPassword = null) { // 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 48b5de62146cf..4942dac702276 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -38,6 +38,7 @@ 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 @@ public function testCreate() { ); } + 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 @@ public function testUpdate() { $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 76b523afd10b3..d0316b44c1a4d 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -827,10 +827,20 @@ public function updateShare(\OCP\Share\IShare $share) { $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 cc98223d4b5f6..7106e22b264d7 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -2716,6 +2716,442 @@ public function testUpdateShareMail() { $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 33782d21b5f0d..bda0047e66c94 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -30,6 +30,7 @@ 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 @@ public function __construct( IURLGenerator $urlGenerator, string $userId, IL10N $l10n, - IConfig $config + IConfig $config, + IAppManager $appManager ) { parent::__construct($appName, $request); @@ -119,6 +124,7 @@ public function __construct( $this->currentUser = $userId; $this->l = $l10n; $this->config = $config; + $this->appManager = $appManager; } /** @@ -206,6 +212,7 @@ protected function formatShare(\OCP\Share\IShare $share, Node $recipientNode = n } 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 @@ public function deleteShare(string $id): DataResponse { * @param string $shareWith * @param string $publicUpload * @param string $password + * @param bool $sendPasswordByTalk * @param string $expireDate * * @return DataResponse @@ -345,6 +353,7 @@ public function createShare( string $shareWith = null, string $publicUpload = 'false', string $password = '', + string $sendPasswordByTalk = null, string $expireDate = '' ): DataResponse { $share = $this->shareManager->newShare(); @@ -485,6 +494,14 @@ public function createShare( $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 @@ public function getShares( * @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 @@ public function updateShare( string $id, int $permissions = null, string $password = null, + string $sendPasswordByTalk = null, string $publicUpload = null, string $expireDate = null, string $note = null @@ -727,7 +746,7 @@ public function updateShare( 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 @@ public function updateShare( } 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 bf4cca53891e3..a68ec7de1f079 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -33,6 +33,7 @@ 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 @@ private function createOCS($userId) { return vsprintf($text, $parameters); })); $config = $this->createMock(IConfig::class); + $appManager = $this->createMock(IAppManager::class); return new ShareAPIController( self::APP_NAME, @@ -118,7 +120,8 @@ private function createOCS($userId) { \OC::$server->getURLGenerator(), $userId, $l, - $config + $config, + $appManager ); } @@ -960,7 +963,7 @@ function testUpdateShareUpload() { // 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 @@ function testUpdateShareExpireDate() { // 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 @@ public function testPublicLinkExpireDate($date, $valid) { $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 @@ public function testCreatePublicLinkExpireDateValid() { $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 @@ public function testCreatePublicLinkExpireDateInvalidFuture() { $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 @@ public function XtestCreatePublicLinkExpireDateInvalidPast() { $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 30041c3a27b99..4fd8162db3d4e 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 @@ protected function setUp() { 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 @@ protected function setUp() { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ); } @@ -137,7 +143,8 @@ private function mockFormatShare() { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['formatShare']) ->getMock(); } @@ -453,7 +460,8 @@ public function testGetShare(\OCP\Share\IShare $share, array $result) { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['canAccessShare']) ->getMock(); @@ -724,7 +732,8 @@ public function testCreateShareUser() { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['formatShare']) ->getMock(); @@ -822,7 +831,8 @@ public function testCreateShareGroup() { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['formatShare']) ->getMock(); @@ -1008,7 +1018,7 @@ public function testCreateShareLinkPublicUploadFolder() { )->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 @@ public function testCreateShareLinkPassword() { )->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 @@ public function testCreateShareValidExpireDate() { )->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 @@ public function testCreateShareInvalidExpireDate() { $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 @@ public function testCreateReshareOfFederatedMountNoDeletePermissions() { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['formatShare']) ->getMock(); @@ -1264,7 +1275,7 @@ public function testUpdateLinkShareClear() { ->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 @@ public function testUpdateLinkShareSet() { ->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 @@ public function testUpdateLinkShareEnablePublicUpload($permissions, $publicUploa )->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 @@ public function testUpdateLinkShareInvalidDate() { $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 @@ public function testUpdateLinkSharePublicUploadNotAllowed($permissions, $publicU $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 @@ public function testUpdateLinkSharePublicUploadOnFile() { $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 @@ public function testUpdateLinkSharePasswordDoesNotChangeOther() { )->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 @@ public function testUpdateLinkShareExpireDateDoesNotChangeOther() { )->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 @@ public function testUpdateLinkSharePublicUploadDoesNotChangeOther() { ->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 @@ public function testUpdateLinkSharePermissions() { $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 @@ public function testUpdateLinkSharePermissionsShare() { $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 @@ public function testUpdateOtherPermissions() { $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 @@ public function testUpdateShareCannotIncreasePermissionsLinkShare() { $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 @@ public function dataFormatShare() { '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 bd1331a090884..6ebffed2b5828 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -61,6 +61,7 @@ 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 @@ public function __construct(string $appName, $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 adcc2853f873f..22e2262229119 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 574d47b4aacf7..8df7fe5ecd7ea 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