From 00e4c8aee4cbf2cf7ba71da8a1321dadf08f3409 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 12 Oct 2018 19:55:12 +0200 Subject: [PATCH 1/7] Fix update share tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The update share tests only checked that the share returned by "update()" had the expected values. However, as "update()" returns the same share that was given as a parameter the tests were not really verifying that the values were updated in the database. In a similar way, the test that checked that a password was removed did not set a password first, so even if the database returned null it could be simply returning the default value for the share; a password must be set first to ensure that it is removed. Besides that, a typo was fixed too that made the checks on the original share instead of on the one returned by "update()"; right now it is the same share, so the change makes no difference, but it is how the check should be done anyway. Signed-off-by: Daniel Calviño Sánchez --- .../lib/Share20/DefaultShareProviderTest.php | 52 ++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index 19f3716062717..d368304453f33 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -1788,6 +1788,14 @@ public function testUpdateUser() { $this->assertSame('user4', $share2->getSharedBy()); $this->assertSame('user5', $share2->getShareOwner()); $this->assertSame(1, $share2->getPermissions()); + + $share2 = $this->provider->getShareById($id); + + $this->assertEquals($id, $share2->getId()); + $this->assertSame('user3', $share2->getSharedWith()); + $this->assertSame('user4', $share2->getSharedBy()); + $this->assertSame('user5', $share2->getShareOwner()); + $this->assertSame(1, $share2->getPermissions()); } public function testUpdateLink() { @@ -1833,7 +1841,15 @@ public function testUpdateLink() { $share2 = $this->provider->update($share); $this->assertEquals($id, $share2->getId()); - $this->assertEquals('password', $share->getPassword()); + $this->assertEquals('password', $share2->getPassword()); + $this->assertSame('user4', $share2->getSharedBy()); + $this->assertSame('user5', $share2->getShareOwner()); + $this->assertSame(1, $share2->getPermissions()); + + $share2 = $this->provider->getShareById($id); + + $this->assertEquals($id, $share2->getId()); + $this->assertEquals('password', $share2->getPassword()); $this->assertSame('user4', $share2->getSharedBy()); $this->assertSame('user5', $share2->getShareOwner()); $this->assertSame(1, $share2->getPermissions()); @@ -1843,6 +1859,12 @@ public function testUpdateLinkRemovePassword() { $id = $this->addShareToDB(\OCP\Share::SHARE_TYPE_LINK, 'foo', 'user1', 'user2', 'file', 42, 'target', 31, null, null); + $qb = $this->dbConn->getQueryBuilder(); + $qb->update('share'); + $qb->where($qb->expr()->eq('id', $qb->createNamedParameter($id))); + $qb->set('password', $qb->createNamedParameter('password')); + $this->assertEquals(1, $qb->execute()); + $users = []; for($i = 0; $i < 6; $i++) { $user = $this->createMock(IUser::class); @@ -1882,7 +1904,15 @@ public function testUpdateLinkRemovePassword() { $share2 = $this->provider->update($share); $this->assertEquals($id, $share2->getId()); - $this->assertEquals(null, $share->getPassword()); + $this->assertEquals(null, $share2->getPassword()); + $this->assertSame('user4', $share2->getSharedBy()); + $this->assertSame('user5', $share2->getShareOwner()); + $this->assertSame(1, $share2->getPermissions()); + + $share2 = $this->provider->getShareById($id); + + $this->assertEquals($id, $share2->getId()); + $this->assertEquals(null, $share2->getPassword()); $this->assertSame('user4', $share2->getSharedBy()); $this->assertSame('user5', $share2->getShareOwner()); $this->assertSame(1, $share2->getPermissions()); @@ -1949,6 +1979,15 @@ public function testUpdateGroupNoSub() { $this->assertSame('user4', $share2->getSharedBy()); $this->assertSame('user5', $share2->getShareOwner()); $this->assertSame(1, $share2->getPermissions()); + + $share2 = $this->provider->getShareById($id); + + $this->assertEquals($id, $share2->getId()); + // Group shares do not allow updating the recipient + $this->assertSame('group0', $share2->getSharedWith()); + $this->assertSame('user4', $share2->getSharedBy()); + $this->assertSame('user5', $share2->getShareOwner()); + $this->assertSame(1, $share2->getPermissions()); } public function testUpdateGroupSubShares() { @@ -2019,6 +2058,15 @@ public function testUpdateGroupSubShares() { $this->assertSame('user5', $share2->getShareOwner()); $this->assertSame(1, $share2->getPermissions()); + $share2 = $this->provider->getShareById($id); + + $this->assertEquals($id, $share2->getId()); + // Group shares do not allow updating the recipient + $this->assertSame('group0', $share2->getSharedWith()); + $this->assertSame('user4', $share2->getSharedBy()); + $this->assertSame('user5', $share2->getShareOwner()); + $this->assertSame(1, $share2->getPermissions()); + $qb = $this->dbConn->getQueryBuilder(); $stmt = $qb->select('*') ->from('share') From 52cada951ba2f1849c0034e06520719c21fed9b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 1 Nov 2018 18:18:06 +0100 Subject: [PATCH 2/7] Check "note", "label" and "hide download" too in update share tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../Controller/ShareAPIControllerTest.php | 69 +++++++++++++++---- 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index eb39dd7926fa1..b4d28fe986e88 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -1512,6 +1512,9 @@ public function testUpdateLinkShareClear() { ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') ->setExpirationDate(new \DateTime()) + ->setNote('note') + ->setLabel('label') + ->setHideDownload(true) ->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setNode($node); @@ -1525,7 +1528,12 @@ public function testUpdateLinkShareClear() { $this->callback(function (\OCP\Share\IShare $share) { return $share->getPermissions() === \OCP\Constants::PERMISSION_READ && $share->getPassword() === null && - $share->getExpirationDate() === null; + $share->getExpirationDate() === null && + // Once set a note or a label are never back to null, only to an + // empty string. + $share->getNote() === '' && + $share->getLabel() === '' && + $share->getHideDownload() === false; }) )->will($this->returnArgument(0)); @@ -1533,7 +1541,7 @@ public function testUpdateLinkShareClear() { ->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, '', null, 'false', ''); + $result = $ocs->updateShare(42, null, '', null, 'false', '', '', '', 'false'); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1560,7 +1568,10 @@ public function testUpdateLinkShareSet() { return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) && $share->getPassword() === 'password' && - $share->getExpirationDate() == $date; + $share->getExpirationDate() == $date && + $share->getNote() === 'note' && + $share->getLabel() === 'label' && + $share->getHideDownload() === true; }) )->will($this->returnArgument(0)); @@ -1568,7 +1579,7 @@ public function testUpdateLinkShareSet() { ->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, 'password', null, 'true', '2000-01-01'); + $result = $ocs->updateShare(42, null, 'password', null, 'true', '2000-01-01', 'note', 'label', 'true'); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1701,6 +1712,9 @@ public function testUpdateLinkSharePasswordDoesNotChangeOther() { ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') ->setExpirationDate($date) + ->setNote('note') + ->setLabel('label') + ->setHideDownload(true) ->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setNode($node); @@ -1714,12 +1728,15 @@ public function testUpdateLinkSharePasswordDoesNotChangeOther() { $this->callback(function (\OCP\Share\IShare $share) use ($date) { return $share->getPermissions() === \OCP\Constants::PERMISSION_ALL && $share->getPassword() === 'newpassword' && - $share->getExpirationDate() === $date; + $share->getExpirationDate() === $date && + $share->getNote() === 'note' && + $share->getLabel() === 'label' && + $share->getHideDownload() === true; }) )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, 'newpassword', null, null, null); + $result = $ocs->updateShare(42, null, 'newpassword', null, null, null, null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1735,6 +1752,9 @@ public function testUpdateLinkShareExpireDateDoesNotChangeOther() { ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') ->setExpirationDate(new \DateTime()) + ->setNote('note') + ->setLabel('label') + ->setHideDownload(true) ->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setNode($node); @@ -1751,12 +1771,15 @@ public function testUpdateLinkShareExpireDateDoesNotChangeOther() { return $share->getPermissions() === \OCP\Constants::PERMISSION_ALL && $share->getPassword() === 'password' && - $share->getExpirationDate() == $date; + $share->getExpirationDate() == $date && + $share->getNote() === 'note' && + $share->getLabel() === 'label' && + $share->getHideDownload() === true; }) )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, null, null, null, '2010-12-23'); + $result = $ocs->updateShare(42, null, null, null, null, '2010-12-23', null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1775,6 +1798,9 @@ public function testUpdateLinkSharePublicUploadDoesNotChangeOther() { ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') ->setExpirationDate($date) + ->setNote('note') + ->setLabel('label') + ->setHideDownload(true) ->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setNode($folder); @@ -1785,7 +1811,10 @@ public function testUpdateLinkSharePublicUploadDoesNotChangeOther() { $this->callback(function (\OCP\Share\IShare $share) use ($date) { return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) && $share->getPassword() === 'password' && - $share->getExpirationDate() === $date; + $share->getExpirationDate() === $date && + $share->getNote() === 'note' && + $share->getLabel() === 'label' && + $share->getHideDownload() === true; }) )->will($this->returnArgument(0)); @@ -1793,7 +1822,7 @@ public function testUpdateLinkSharePublicUploadDoesNotChangeOther() { ->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, null, null, 'true', null); + $result = $ocs->updateShare(42, null, null, null, 'true', null, null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1812,6 +1841,9 @@ public function testUpdateLinkSharePermissions() { ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') ->setExpirationDate($date) + ->setNote('note') + ->setLabel('label') + ->setHideDownload(true) ->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setNode($folder); @@ -1822,14 +1854,17 @@ public function testUpdateLinkSharePermissions() { $this->callback(function (\OCP\Share\IShare $share) use ($date) { return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) && $share->getPassword() === 'password' && - $share->getExpirationDate() === $date; + $share->getExpirationDate() === $date && + $share->getNote() === 'note' && + $share->getLabel() === 'label' && + $share->getHideDownload() === true; }) )->will($this->returnArgument(0)); $this->shareManager->method('getSharedWith')->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, 7, null, null, null, null); + $result = $ocs->updateShare(42, 7, null, null, null, null, null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1848,6 +1883,9 @@ public function testUpdateLinkSharePermissionsShare() { ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') ->setExpirationDate($date) + ->setNote('note') + ->setLabel('label') + ->setHideDownload(true) ->setPermissions(\OCP\Constants::PERMISSION_READ) ->setNode($folder); @@ -1858,14 +1896,17 @@ public function testUpdateLinkSharePermissionsShare() { $this->callback(function (\OCP\Share\IShare $share) use ($date) { return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) && $share->getPassword() === 'password' && - $share->getExpirationDate() === $date; + $share->getExpirationDate() === $date && + $share->getNote() === 'note' && + $share->getLabel() === 'label' && + $share->getHideDownload() === true; }) )->will($this->returnArgument(0)); $this->shareManager->method('getSharedWith')->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, 31, null, null, null, null); + $result = $ocs->updateShare(42, 31, null, null, null, null, null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); From fe8a67f5175b687ec6cc8230c7759ddddbe25b0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 12 Oct 2018 20:15:40 +0200 Subject: [PATCH 3/7] Store "sendPasswordByTalk" property of link 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 --- lib/private/Share20/DefaultShareProvider.php | 4 ++++ tests/lib/Share20/DefaultShareProviderTest.php | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 50111054546c8..589b64fc58a6a 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -156,6 +156,8 @@ public function create(\OCP\Share\IShare $share) { $qb->setValue('password', $qb->createNamedParameter($share->getPassword())); } + $qb->setValue('password_by_talk', $qb->createNamedParameter($share->getSendPasswordByTalk(), IQueryBuilder::PARAM_BOOL)); + //If an expiration date is set store it if ($share->getExpirationDate() !== null) { $qb->setValue('expiration', $qb->createNamedParameter($share->getExpirationDate(), 'datetime')); @@ -293,6 +295,7 @@ public function update(\OCP\Share\IShare $share) { $qb->update('share') ->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId()))) ->set('password', $qb->createNamedParameter($share->getPassword())) + ->set('password_by_talk', $qb->createNamedParameter($share->getSendPasswordByTalk(), IQueryBuilder::PARAM_BOOL)) ->set('uid_owner', $qb->createNamedParameter($share->getShareOwner())) ->set('uid_initiator', $qb->createNamedParameter($share->getSharedBy())) ->set('permissions', $qb->createNamedParameter($share->getPermissions())) @@ -938,6 +941,7 @@ private function createShare($data) { $share->setSharedWith($data['share_with']); } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) { $share->setPassword($data['password']); + $share->setSendPasswordByTalk((bool)$data['password_by_talk']); $share->setToken($data['token']); } diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index d368304453f33..578225840b6ca 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -363,6 +363,7 @@ public function testGetShareByIdLinkShare() { ->values([ 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_LINK), 'password' => $qb->expr()->literal('password'), + 'password_by_talk' => $qb->expr()->literal(true), 'uid_owner' => $qb->expr()->literal('shareOwner'), 'uid_initiator' => $qb->expr()->literal('sharedBy'), 'item_type' => $qb->expr()->literal('file'), @@ -392,6 +393,7 @@ public function testGetShareByIdLinkShare() { $this->assertEquals(\OCP\Share::SHARE_TYPE_LINK, $share->getShareType()); $this->assertNull($share->getSharedWith()); $this->assertEquals('password', $share->getPassword()); + $this->assertEquals(true, $share->getSendPasswordByTalk()); $this->assertEquals('sharedBy', $share->getSharedBy()); $this->assertEquals('shareOwner', $share->getShareOwner()); $this->assertEquals($ownerPath, $share->getNode()); @@ -775,6 +777,7 @@ public function testCreateLinkShare() { $share->setNode($path); $share->setPermissions(1); $share->setPassword('password'); + $share->setSendPasswordByTalk(true); $share->setToken('token'); $expireDate = new \DateTime(); $share->setExpirationDate($expireDate); @@ -792,6 +795,7 @@ public function testCreateLinkShare() { $this->assertLessThanOrEqual(new \DateTime(), $share2->getShareTime()); $this->assertSame($path, $share2->getNode()); $this->assertSame('password', $share2->getPassword()); + $this->assertSame(true, $share2->getSendPasswordByTalk()); $this->assertSame('token', $share2->getToken()); $this->assertEquals($expireDate->getTimestamp(), $share2->getExpirationDate()->getTimestamp()); } @@ -803,6 +807,7 @@ public function testGetShareByToken() { ->values([ 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_LINK), 'password' => $qb->expr()->literal('password'), + 'password_by_talk' => $qb->expr()->literal(true), 'uid_owner' => $qb->expr()->literal('shareOwner'), 'uid_initiator' => $qb->expr()->literal('sharedBy'), 'item_type' => $qb->expr()->literal('file'), @@ -825,6 +830,7 @@ public function testGetShareByToken() { $this->assertSame('sharedBy', $share->getSharedBy()); $this->assertSame('secrettoken', $share->getToken()); $this->assertSame('password', $share->getPassword()); + $this->assertSame(true, $share->getSendPasswordByTalk()); $this->assertSame(null, $share->getSharedWith()); } @@ -1833,6 +1839,7 @@ public function testUpdateLink() { $share = $this->provider->getShareById($id); $share->setPassword('password'); + $share->setSendPasswordByTalk(true); $share->setSharedBy('user4'); $share->setShareOwner('user5'); $share->setNode($file2); @@ -1842,6 +1849,7 @@ public function testUpdateLink() { $this->assertEquals($id, $share2->getId()); $this->assertEquals('password', $share2->getPassword()); + $this->assertSame(true, $share2->getSendPasswordByTalk()); $this->assertSame('user4', $share2->getSharedBy()); $this->assertSame('user5', $share2->getShareOwner()); $this->assertSame(1, $share2->getPermissions()); @@ -1850,6 +1858,7 @@ public function testUpdateLink() { $this->assertEquals($id, $share2->getId()); $this->assertEquals('password', $share2->getPassword()); + $this->assertSame(true, $share2->getSendPasswordByTalk()); $this->assertSame('user4', $share2->getSharedBy()); $this->assertSame('user5', $share2->getShareOwner()); $this->assertSame(1, $share2->getPermissions()); From adf80aa8b329cf08e3f21a1ed8f722ab066d868d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 15 Oct 2018 12:27:56 +0200 Subject: [PATCH 4/7] Add sending the password by Talk for a link share 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 | 20 ++ .../Controller/ShareAPIControllerTest.php | 304 ++++++++++++++++++ 2 files changed, 324 insertions(+) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 61fad5d2b148b..42d0218de8cd3 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -212,6 +212,8 @@ protected function formatShare(\OCP\Share\IShare $share, Node $recipientNode = n $result['share_with'] = $share->getPassword(); $result['share_with_displayname'] = $share->getPassword(); + $result['send_password_by_talk'] = $share->getSendPasswordByTalk(); + $result['token'] = $share->getToken(); $result['url'] = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare', ['token' => $share->getToken()]); @@ -477,10 +479,19 @@ public function createShare( $share->setPassword($password); } + if (!empty($label)) { $share->setLabel($label); } + 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); + } + //Expire date if ($expireDate !== '') { try { @@ -873,6 +884,15 @@ public function updateShare( $share->setLabel($label); } + 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 if ($sendPasswordByTalk !== null) { + $share->setSendPasswordByTalk(false); + } } else { if ($permissions !== null) { $permissions = (int)$permissions; diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index b4d28fe986e88..efc252d49d72a 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -430,6 +430,7 @@ public function dataGetShare() { 'share_type' => \OCP\Share::SHARE_TYPE_LINK, 'share_with' => 'password', 'share_with_displayname' => 'password', + 'send_password_by_talk' => false, 'uid_owner' => 'initiatorId', 'displayname_owner' => 'initiatorDisplay', 'item_type' => 'folder', @@ -1135,6 +1136,71 @@ public function testCreateShareLinkPassword() { $this->assertEquals($expected->getData(), $result->getData()); } + public function testCreateShareLinkSendPasswordByTalk() { + $ocs = $this->mockFormatShare(); + + $path = $this->getMockBuilder(Folder::class)->getMock(); + $storage = $this->getMockBuilder(Storage::class)->getMock(); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); + $this->rootFolder->method('getUserFolder')->with($this->currentUser)->will($this->returnSelf()); + $this->rootFolder->method('get')->with('valid-path')->willReturn($path); + + $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare()); + $this->shareManager->method('shareApiAllowLinks')->willReturn(true); + $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); + + $this->appManager->method('isEnabledForUser')->with('spreed')->willReturn(true); + + $this->shareManager->expects($this->once())->method('createShare')->with( + $this->callback(function (\OCP\Share\IShare $share) use ($path) { + return $share->getNode() === $path && + $share->getShareType() === \OCP\Share::SHARE_TYPE_LINK && + $share->getPermissions() === \OCP\Constants::PERMISSION_READ && + $share->getSharedBy() === 'currentUser' && + $share->getPassword() === 'password' && + $share->getSendPasswordByTalk() === true && + $share->getExpirationDate() === null; + }) + )->will($this->returnArgument(0)); + + $expected = new DataResponse([]); + $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', 'password', 'true', ''); + + $this->assertInstanceOf(get_class($expected), $result); + $this->assertEquals($expected->getData(), $result->getData()); + } + + /** + * @expectedException \OCP\AppFramework\OCS\OCSForbiddenException + * @expectedExceptionMessage Sharing valid-path sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled + */ + public function testCreateShareLinkSendPasswordByTalkWithTalkDisabled() { + $ocs = $this->mockFormatShare(); + + $path = $this->getMockBuilder(Folder::class)->getMock(); + $storage = $this->getMockBuilder(Storage::class)->getMock(); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); + $path->method('getPath')->willReturn('valid-path'); + $this->rootFolder->method('getUserFolder')->with($this->currentUser)->will($this->returnSelf()); + $this->rootFolder->method('get')->with('valid-path')->willReturn($path); + + $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare()); + $this->shareManager->method('shareApiAllowLinks')->willReturn(true); + $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); + + $this->appManager->method('isEnabledForUser')->with('spreed')->willReturn(false); + + $this->shareManager->expects($this->never())->method('createShare'); + + $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', 'password', 'true', ''); + } + public function testCreateShareValidExpireDate() { $ocs = $this->mockFormatShare(); @@ -1711,6 +1777,7 @@ public function testUpdateLinkSharePasswordDoesNotChangeOther() { ->setSharedBy($this->currentUser) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') + ->setSendPasswordByTalk(true) ->setExpirationDate($date) ->setNote('note') ->setLabel('label') @@ -1728,6 +1795,7 @@ public function testUpdateLinkSharePasswordDoesNotChangeOther() { $this->callback(function (\OCP\Share\IShare $share) use ($date) { return $share->getPermissions() === \OCP\Constants::PERMISSION_ALL && $share->getPassword() === 'newpassword' && + $share->getSendPasswordByTalk() === true && $share->getExpirationDate() === $date && $share->getNote() === 'note' && $share->getLabel() === 'label' && @@ -1742,6 +1810,184 @@ public function testUpdateLinkSharePasswordDoesNotChangeOther() { $this->assertEquals($expected->getData(), $result->getData()); } + public function testUpdateLinkShareSendPasswordByTalkDoesNotChangeOther() { + $ocs = $this->mockFormatShare(); + + $date = new \DateTime('2000-01-01'); + $date->setTime(0,0,0); + + $node = $this->getMockBuilder(File::class)->getMock(); + $share = $this->newShare(); + $share->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setSharedBy($this->currentUser) + ->setShareType(\OCP\Share::SHARE_TYPE_LINK) + ->setPassword('password') + ->setSendPasswordByTalk(false) + ->setExpirationDate($date) + ->setNote('note') + ->setLabel('label') + ->setHideDownload(true) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setNode($node); + + $node->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + + $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); + + $this->appManager->method('isEnabledForUser')->with('spreed')->willReturn(true); + + $this->shareManager->expects($this->once())->method('updateShare')->with( + $this->callback(function (\OCP\Share\IShare $share) use ($date) { + return $share->getPermissions() === \OCP\Constants::PERMISSION_ALL && + $share->getPassword() === 'password' && + $share->getSendPasswordByTalk() === true && + $share->getExpirationDate() === $date && + $share->getNote() === 'note' && + $share->getLabel() === 'label' && + $share->getHideDownload() === true; + }) + )->will($this->returnArgument(0)); + + $expected = new DataResponse([]); + $result = $ocs->updateShare(42, null, null, 'true', null, null, null, null, null); + + $this->assertInstanceOf(get_class($expected), $result); + $this->assertEquals($expected->getData(), $result->getData()); + } + + /** + * @expectedException \OCP\AppFramework\OCS\OCSForbiddenException + * @expectedExceptionMessage Sharing sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled + */ + public function testUpdateLinkShareSendPasswordByTalkWithTalkDisabledDoesNotChangeOther() { + $ocs = $this->mockFormatShare(); + + $date = new \DateTime('2000-01-01'); + $date->setTime(0,0,0); + + $node = $this->getMockBuilder(File::class)->getMock(); + $share = $this->newShare(); + $share->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setSharedBy($this->currentUser) + ->setShareType(\OCP\Share::SHARE_TYPE_LINK) + ->setPassword('password') + ->setSendPasswordByTalk(false) + ->setExpirationDate($date) + ->setNote('note') + ->setLabel('label') + ->setHideDownload(true) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setNode($node); + + $node->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + + $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); + + $this->appManager->method('isEnabledForUser')->with('spreed')->willReturn(false); + + $this->shareManager->expects($this->never())->method('updateShare'); + + $ocs->updateShare(42, null, null, 'true', null, null, null, null, null); + } + + public function testUpdateLinkShareDoNotSendPasswordByTalkDoesNotChangeOther() { + $ocs = $this->mockFormatShare(); + + $date = new \DateTime('2000-01-01'); + $date->setTime(0,0,0); + + $node = $this->getMockBuilder(File::class)->getMock(); + $share = $this->newShare(); + $share->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setSharedBy($this->currentUser) + ->setShareType(\OCP\Share::SHARE_TYPE_LINK) + ->setPassword('password') + ->setSendPasswordByTalk(true) + ->setExpirationDate($date) + ->setNote('note') + ->setLabel('label') + ->setHideDownload(true) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setNode($node); + + $node->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + + $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); + + $this->appManager->method('isEnabledForUser')->with('spreed')->willReturn(true); + + $this->shareManager->expects($this->once())->method('updateShare')->with( + $this->callback(function (\OCP\Share\IShare $share) use ($date) { + return $share->getPermissions() === \OCP\Constants::PERMISSION_ALL && + $share->getPassword() === 'password' && + $share->getSendPasswordByTalk() === false && + $share->getExpirationDate() === $date && + $share->getNote() === 'note' && + $share->getLabel() === 'label' && + $share->getHideDownload() === true; + }) + )->will($this->returnArgument(0)); + + $expected = new DataResponse([]); + $result = $ocs->updateShare(42, null, null, 'false', null, null, null, null, null); + + $this->assertInstanceOf(get_class($expected), $result); + $this->assertEquals($expected->getData(), $result->getData()); + } + + public function testUpdateLinkShareDoNotSendPasswordByTalkWithTalkDisabledDoesNotChangeOther() { + $ocs = $this->mockFormatShare(); + + $date = new \DateTime('2000-01-01'); + $date->setTime(0,0,0); + + $node = $this->getMockBuilder(File::class)->getMock(); + $share = $this->newShare(); + $share->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setSharedBy($this->currentUser) + ->setShareType(\OCP\Share::SHARE_TYPE_LINK) + ->setPassword('password') + ->setSendPasswordByTalk(true) + ->setExpirationDate($date) + ->setNote('note') + ->setLabel('label') + ->setHideDownload(true) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setNode($node); + + $node->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + + $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); + + $this->appManager->method('isEnabledForUser')->with('spreed')->willReturn(false); + + $this->shareManager->expects($this->once())->method('updateShare')->with( + $this->callback(function (\OCP\Share\IShare $share) use ($date) { + return $share->getPermissions() === \OCP\Constants::PERMISSION_ALL && + $share->getPassword() === 'password' && + $share->getSendPasswordByTalk() === false && + $share->getExpirationDate() === $date && + $share->getNote() === 'note' && + $share->getLabel() === 'label' && + $share->getHideDownload() === true; + }) + )->will($this->returnArgument(0)); + + $expected = new DataResponse([]); + $result = $ocs->updateShare(42, null, null, 'false', null, null, null, null, null); + + $this->assertInstanceOf(get_class($expected), $result); + $this->assertEquals($expected->getData(), $result->getData()); + } + public function testUpdateLinkShareExpireDateDoesNotChangeOther() { $ocs = $this->mockFormatShare(); @@ -1751,6 +1997,7 @@ public function testUpdateLinkShareExpireDateDoesNotChangeOther() { ->setSharedBy($this->currentUser) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') + ->setSendPasswordByTalk(true) ->setExpirationDate(new \DateTime()) ->setNote('note') ->setLabel('label') @@ -1771,6 +2018,7 @@ public function testUpdateLinkShareExpireDateDoesNotChangeOther() { return $share->getPermissions() === \OCP\Constants::PERMISSION_ALL && $share->getPassword() === 'password' && + $share->getSendPasswordByTalk() === true && $share->getExpirationDate() == $date && $share->getNote() === 'note' && $share->getLabel() === 'label' && @@ -1797,6 +2045,7 @@ public function testUpdateLinkSharePublicUploadDoesNotChangeOther() { ->setSharedBy($this->currentUser) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') + ->setSendPasswordByTalk(true) ->setExpirationDate($date) ->setNote('note') ->setLabel('label') @@ -1811,6 +2060,7 @@ public function testUpdateLinkSharePublicUploadDoesNotChangeOther() { $this->callback(function (\OCP\Share\IShare $share) use ($date) { return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) && $share->getPassword() === 'password' && + $share->getSendPasswordByTalk() === true && $share->getExpirationDate() === $date && $share->getNote() === 'note' && $share->getLabel() === 'label' && @@ -1840,6 +2090,7 @@ public function testUpdateLinkSharePermissions() { ->setSharedBy($this->currentUser) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') + ->setSendPasswordByTalk(true) ->setExpirationDate($date) ->setNote('note') ->setLabel('label') @@ -1854,6 +2105,7 @@ public function testUpdateLinkSharePermissions() { $this->callback(function (\OCP\Share\IShare $share) use ($date) { return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) && $share->getPassword() === 'password' && + $share->getSendPasswordByTalk() === true && $share->getExpirationDate() === $date && $share->getNote() === 'note' && $share->getLabel() === 'label' && @@ -1882,6 +2134,7 @@ public function testUpdateLinkSharePermissionsShare() { ->setSharedBy($this->currentUser) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') + ->setSendPasswordByTalk(true) ->setExpirationDate($date) ->setNote('note') ->setLabel('label') @@ -1896,6 +2149,7 @@ public function testUpdateLinkSharePermissionsShare() { $this->callback(function (\OCP\Share\IShare $share) use ($date) { return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) && $share->getPassword() === 'password' && + $share->getSendPasswordByTalk() === true && $share->getExpirationDate() === $date && $share->getNote() === 'note' && $share->getLabel() === 'label' && @@ -2435,6 +2689,56 @@ public function dataFormatShare() { 'file_target' => 'myTarget', 'share_with' => 'mypassword', 'share_with_displayname' => 'mypassword', + 'send_password_by_talk' => false, + 'mail_send' => 0, + 'url' => 'myLink', + 'mimetype' => 'myMimeType', + 'hide_download' => 0, + ], $share, [], false + ]; + + $share = \OC::$server->getShareManager()->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_LINK) + ->setSharedBy('initiator') + ->setShareOwner('owner') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($file) + ->setShareTime(new \DateTime('2000-01-01T00:01:02')) + ->setTarget('myTarget') + ->setPassword('mypassword') + ->setSendPasswordByTalk(true) + ->setExpirationDate(new \DateTime('2001-01-02T00:00:00')) + ->setToken('myToken') + ->setNote('personal note') + ->setLabel('new link share') + ->setId(42); + + $result[] = [ + [ + 'id' => 42, + 'share_type' => \OCP\Share::SHARE_TYPE_LINK, + 'uid_owner' => 'initiator', + 'displayname_owner' => 'initiator', + 'permissions' => 1, + 'stime' => 946684862, + 'parent' => null, + 'expiration' => '2001-01-02 00:00:00', + 'token' => 'myToken', + 'uid_file_owner' => 'owner', + 'displayname_file_owner' => 'owner', + 'note' => 'personal note', + 'label' => 'new link share', + 'path' => 'file', + 'item_type' => 'file', + 'storage_id' => 'storageId', + 'storage' => 100, + 'item_source' => 3, + 'file_source' => 3, + 'file_parent' => 1, + 'file_target' => 'myTarget', + 'share_with' => 'mypassword', + 'share_with_displayname' => 'mypassword', + 'send_password_by_talk' => true, 'mail_send' => 0, 'url' => 'myLink', 'mimetype' => 'myMimeType', From 376704e83413d093d596f1b9168daf24907189aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 15 Oct 2018 17:09:46 +0200 Subject: [PATCH 5/7] Add "Password protect by Talk" to the menu of link shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Talk is enabled the menu for link shares now shows a checkbox to protect the password by Talk (that is, to show the "Request password by Talk" UI in the authentication page for the link share). Although in e-mail shares protecting the share with a password and protecting the password by Talk are mutually exclusive actions (as when the password is set it is sent to the sharee, so it must be set again when protecting it by Talk to be able to verify the identity of the sharee), in the case of link shares protecting the password by Talk is an additional step to protecting the share with a password (as just setting the password does not disclose it to anyone). As such, the checkbox is shown only when there is a password set for the link share (even if the field itself for the password is not shown, like when they are enforced in the settings). Note that the icon set for the field, "icon-passwordtalk", does not currently exist; it is the same used for e-mail shares, and it is needed simply to get the right padding in the menu. Signed-off-by: Daniel Calviño Sánchez --- ...ialoglinkshareview_popover_menu.handlebars | 10 ++ core/js/sharedialoglinkshareview.js | 64 ++++++++++ core/js/shareitemmodel.js | 5 +- core/js/sharetemplates.js | 29 +++-- .../tests/specs/sharedialoglinkshareview.js | 113 ++++++++++++++++++ core/js/tests/specs/shareitemmodelSpec.js | 13 +- 6 files changed, 222 insertions(+), 12 deletions(-) diff --git a/core/js/share/sharedialoglinkshareview_popover_menu.handlebars b/core/js/share/sharedialoglinkshareview_popover_menu.handlebars index cc951ce047d32..59312bc70b00d 100644 --- a/core/js/share/sharedialoglinkshareview_popover_menu.handlebars +++ b/core/js/share/sharedialoglinkshareview_popover_menu.handlebars @@ -62,6 +62,16 @@ {{/if}} + {{#if showPasswordByTalkCheckBox}} +
  • + + + + + +
  • + {{/if}}
  • \n \n \n \n \n \n
  • \n"; },"15":function(container,depth0,helpers,partials,data) { + return "datepicker"; +},"17":function(container,depth0,helpers,partials,data) { var helper; return container.escapeExpression(((helper = (helper = helpers.expireDate || (depth0 != null ? depth0.expireDate : depth0)) != null ? helper : helpers.helperMissing),(typeof helper === "function" ? helper.call(depth0 != null ? depth0 : (container.nullContext || {}),{"name":"expireDate","hash":{},"data":data}) : helper))); -},"17":function(container,depth0,helpers,partials,data) { +},"19":function(container,depth0,helpers,partials,data) { var helper; return container.escapeExpression(((helper = (helper = helpers.defaultExpireDate || (depth0 != null ? depth0.defaultExpireDate : depth0)) != null ? helper : helpers.helperMissing),(typeof helper === "function" ? helper.call(depth0 != null ? depth0 : (container.nullContext || {}),{"name":"defaultExpireDate","hash":{},"data":data}) : helper))); -},"19":function(container,depth0,helpers,partials,data) { - return "readonly"; },"21":function(container,depth0,helpers,partials,data) { + return "readonly"; +},"23":function(container,depth0,helpers,partials,data) { var helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=helpers.helperMissing, alias3="function", alias4=container.escapeExpression; return "
  • \n \n \n \n \n \n \n
  • \n
  • \n \n \n \n " + alias4(((helper = (helper = helpers.addNoteLabel || (depth0 != null ? depth0.addNoteLabel : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"addNoteLabel","hash":{},"data":data}) : helper))) + "\n \n \n \n
  • \n" - + ((stack1 = helpers.each.call(alias1,(depth0 != null ? depth0.social : depth0),{"name":"each","hash":{},"fn":container.program(21, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers.each.call(alias1,(depth0 != null ? depth0.social : depth0),{"name":"each","hash":{},"fn":container.program(23, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "
  • \n " + alias4(((helper = (helper = helpers.unshareLinkLabel || (depth0 != null ? depth0.unshareLinkLabel : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"unshareLinkLabel","hash":{},"data":data}) : helper))) + "\n
  • \n
  • \n \n \n \n " diff --git a/core/js/tests/specs/sharedialoglinkshareview.js b/core/js/tests/specs/sharedialoglinkshareview.js index f5fe8725c03f5..c2d84fd2e87fd 100644 --- a/core/js/tests/specs/sharedialoglinkshareview.js +++ b/core/js/tests/specs/sharedialoglinkshareview.js @@ -235,4 +235,117 @@ describe('OC.Share.ShareDialogLinkShareView', function () { }); + describe('protect password by Talk', function () { + + var $passwordByTalkCheckbox; + var $workingIcon; + + beforeEach(function () { + // Needed to render the view + configModel.isShareWithLinkAllowed.returns(true); + + // "Enable" Talk + window.oc_appswebroots['spreed'] = window.oc_webroot + '/apps/files/'; + + shareModel.set({ + linkShares: [{ + id: 123, + password: 'password' + }] + }); + view.render(); + + $passwordByTalkCheckbox = view.$el.find('.passwordByTalkCheckbox'); + $workingIcon = $passwordByTalkCheckbox.prev('.icon-loading-small'); + + sinon.stub(shareModel, 'saveLinkShare'); + + expect($workingIcon.hasClass('hidden')).toBeTruthy(); + }); + + afterEach(function () { + shareModel.saveLinkShare.restore(); + }); + + it('is shown if Talk is enabled and there is a password set', function() { + expect($passwordByTalkCheckbox.length).toBeTruthy(); + }); + + it('is not shown if Talk is enabled but there is no password set', function() { + // Changing the password value also triggers the rendering + shareModel.set({ + linkShares: [{ + id: 123 + }] + }); + + $passwordByTalkCheckbox = view.$el.find('.passwordByTalkCheckbox'); + + expect($passwordByTalkCheckbox.length).toBeFalsy(); + }); + + it('is not shown if there is a password set but Talk is not enabled', function() { + // "Disable" Talk + delete window.oc_appswebroots['spreed']; + + view.render(); + + $passwordByTalkCheckbox = view.$el.find('.passwordByTalkCheckbox'); + + expect($passwordByTalkCheckbox.length).toBeFalsy(); + }); + + it('checkbox is checked when the setting is enabled', function () { + shareModel.set({ + linkShares: [{ + id: 123, + password: 'password', + sendPasswordByTalk: true + }] + }); + view.render(); + + $passwordByTalkCheckbox = view.$el.find('.passwordByTalkCheckbox'); + + expect($passwordByTalkCheckbox.is(':checked')).toEqual(true); + }); + + it('checkbox is not checked when the setting is disabled', function () { + expect($passwordByTalkCheckbox.is(':checked')).toEqual(false); + }); + + it('enables the setting if clicked when unchecked', function () { + // Simulate the click by checking the checkbox and then triggering + // the "change" event. + $passwordByTalkCheckbox.prop('checked', true); + $passwordByTalkCheckbox.change(); + + expect($workingIcon.hasClass('hidden')).toBeFalsy(); + expect(shareModel.saveLinkShare.withArgs({ sendPasswordByTalk: true, cid: 123 }).calledOnce).toBeTruthy(); + }); + + it('disables the setting if clicked when checked', function () { + shareModel.set({ + linkShares: [{ + id: 123, + password: 'password', + sendPasswordByTalk: true + }] + }); + view.render(); + + $passwordByTalkCheckbox = view.$el.find('.passwordByTalkCheckbox'); + $workingIcon = $passwordByTalkCheckbox.prev('.icon-loading-small'); + + // Simulate the click by unchecking the checkbox and then triggering + // the "change" event. + $passwordByTalkCheckbox.prop('checked', false); + $passwordByTalkCheckbox.change(); + + expect($workingIcon.hasClass('hidden')).toBeFalsy(); + expect(shareModel.saveLinkShare.withArgs({ sendPasswordByTalk: false, cid: 123 }).calledOnce).toBeTruthy(); + }); + + }); + }); diff --git a/core/js/tests/specs/shareitemmodelSpec.js b/core/js/tests/specs/shareitemmodelSpec.js index 3b4dc5a960fb0..e8016950094d9 100644 --- a/core/js/tests/specs/shareitemmodelSpec.js +++ b/core/js/tests/specs/shareitemmodelSpec.js @@ -169,7 +169,8 @@ describe('OC.Share.ShareItemModel', function() { storage: 1, token: 'tehtoken', uid_owner: 'root', - hide_download: 1 + hide_download: 1, + send_password_by_talk: true } ])); @@ -189,6 +190,7 @@ describe('OC.Share.ShareItemModel', function() { expect(linkShares.length).toEqual(1); var linkShare = linkShares[0]; expect(linkShare.hideDownload).toEqual(true); + expect(linkShare.sendPasswordByTalk).toEqual(true); // TODO: check more attributes }); @@ -293,7 +295,8 @@ describe('OC.Share.ShareItemModel', function() { storage: 1, token: 'tehtoken', uid_owner: 'root', - hide_download: 0 + hide_download: 0, + send_password_by_talk: false }, { displayname_owner: 'root', expiration: '2015-10-15 00:00:00', @@ -312,7 +315,8 @@ describe('OC.Share.ShareItemModel', function() { storage: 1, token: 'anothertoken', uid_owner: 'root', - hide_download: 1 + hide_download: 1, + send_password_by_talk: true }] )); OC.currentUser = 'root'; @@ -327,6 +331,7 @@ describe('OC.Share.ShareItemModel', function() { var linkShare = linkShares[0]; expect(linkShare.token).toEqual('tehtoken'); expect(linkShare.hideDownload).toEqual(false); + expect(linkShare.sendPasswordByTalk).toEqual(false); // TODO: check child too }); @@ -588,6 +593,7 @@ describe('OC.Share.ShareItemModel', function() { hideDownload: false, password: '', passwordChanged: false, + sendPasswordByTalk: false, permissions: OC.PERMISSION_READ, expireDate: '', shareType: OC.Share.SHARE_TYPE_LINK @@ -612,6 +618,7 @@ describe('OC.Share.ShareItemModel', function() { hideDownload: false, password: '', passwordChanged: false, + sendPasswordByTalk: false, permissions: OC.PERMISSION_READ, expireDate: '2015-07-24 00:00:00', shareType: OC.Share.SHARE_TYPE_LINK From a784e1fcc12b75cb905429a8953840152e39bbbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 16 Oct 2018 14:20:56 +0200 Subject: [PATCH 6/7] Test that "Password protect by Talk" is not shown if Talk is not enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- tests/acceptance/features/app-files.feature | 4 ++ .../features/bootstrap/FilesAppContext.php | 44 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/tests/acceptance/features/app-files.feature b/tests/acceptance/features/app-files.feature index 70e085ca6657b..33bed4a3ef21a 100644 --- a/tests/acceptance/features/app-files.feature +++ b/tests/acceptance/features/app-files.feature @@ -218,6 +218,10 @@ Feature: app-files When I protect the shared link with the password "abcdef" Then I see that the working icon for password protect is shown And I see that the working icon for password protect is eventually not shown + And I see that the link share is password protected + # As Talk is not enabled in the acceptance tests of the server the checkbox + # is never shown. + And I see that the checkbox to protect the password of the link share by Talk is not shown Scenario: access a shared link protected by password with a valid password Given I act as John diff --git a/tests/acceptance/features/bootstrap/FilesAppContext.php b/tests/acceptance/features/bootstrap/FilesAppContext.php index 459028813b5ca..03bf371095d4d 100644 --- a/tests/acceptance/features/bootstrap/FilesAppContext.php +++ b/tests/acceptance/features/bootstrap/FilesAppContext.php @@ -276,6 +276,15 @@ public static function passwordProtectCheckbox() { describedAs("Password protect checkbox in the details view in Files app"); } + /** + * @return Locator + */ + public static function passwordProtectCheckboxInput() { + return Locator::forThe()->checkbox("Password protect")-> + descendantOf(self::shareLinkMenu())-> + describedAs("Password protect checkbox input in the details view in Files app"); + } + /** * @return Locator */ @@ -292,6 +301,18 @@ public static function passwordProtectWorkingIcon() { describedAs("Password protect working icon in the details view in Files app"); } + /** + * @return Locator + */ + public static function passwordProtectByTalkCheckbox() { + // forThe()->checkbox("Password protect by Talk") can not be used here; + // that would return the checkbox itself, but the element that the user + // interacts with is the label. + return Locator::forThe()->xpath("//label[normalize-space() = 'Password protect by Talk']")-> + descendantOf(self::shareLinkMenu())-> + describedAs("Password protect by Talk checkbox in the details view in Files app"); + } + /** * @Given I close the details view */ @@ -537,6 +558,29 @@ public function iSeeThatTheWorkingIconForPasswordProtectIsEventuallyNotShown() { } } + /** + * @Then I see that the link share is password protected + */ + public function iSeeThatTheLinkShareIsPasswordProtected() { + $this->showShareLinkMenuIfNeeded(); + + PHPUnit_Framework_Assert::assertTrue($this->actor->find(self::passwordProtectCheckboxInput(), 10)->isChecked(), "Password protect checkbox is checked"); + PHPUnit_Framework_Assert::assertTrue($this->actor->find(self::passwordProtectField(), 10)->isVisible(), "Password protect field is visible"); + } + + /** + * @Then I see that the checkbox to protect the password of the link share by Talk is not shown + */ + public function iSeeThatTheCheckboxToProtectThePasswordOfTheLinkShareByTalkIsNotShown() { + $this->showShareLinkMenuIfNeeded(); + + try { + PHPUnit_Framework_Assert::assertFalse( + $this->actor->find(self::passwordProtectByTalkCheckbox())->isVisible()); + } catch (NoSuchElementException $exception) { + } + } + /** * @Given I share the link for :fileName protected by the password :password */ From f57b07e2c2c6a2ff7cc027eeeeab6ab8da123ad5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 16 Oct 2018 14:26:35 +0200 Subject: [PATCH 7/7] Add acceptance test steps to be used from Talk acceptance tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../features/bootstrap/FilesAppContext.php | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/acceptance/features/bootstrap/FilesAppContext.php b/tests/acceptance/features/bootstrap/FilesAppContext.php index 03bf371095d4d..4a7af441e5767 100644 --- a/tests/acceptance/features/bootstrap/FilesAppContext.php +++ b/tests/acceptance/features/bootstrap/FilesAppContext.php @@ -313,6 +313,15 @@ public static function passwordProtectByTalkCheckbox() { describedAs("Password protect by Talk checkbox in the details view in Files app"); } + /** + * @return Locator + */ + public static function passwordProtectByTalkCheckboxInput() { + return Locator::forThe()->checkbox("Password protect by Talk")-> + descendantOf(self::shareLinkMenu())-> + describedAs("Password protect by Talk checkbox input in the details view in Files app"); + } + /** * @Given I close the details view */ @@ -415,6 +424,28 @@ public function iProtectTheSharedLinkWithThePassword($password) { $this->actor->find(self::passwordProtectField(), 2)->setValue($password . "\r"); } + /** + * @When I set the password of the shared link as protected by Talk + */ + public function iSetThePasswordOfTheSharedLinkAsProtectedByTalk() { + $this->showShareLinkMenuIfNeeded(); + + $this->iSeeThatThePasswordOfTheLinkShareIsNotProtectedByTalk(); + + $this->actor->find(self::passwordProtectByTalkCheckbox(), 2)->click(); + } + + /** + * @When I set the password of the shared link as not protected by Talk + */ + public function iSetThePasswordOfTheSharedLinkAsNotProtectedByTalk() { + $this->showShareLinkMenuIfNeeded(); + + $this->iSeeThatThePasswordOfTheLinkShareIsProtectedByTalk(); + + $this->actor->find(self::passwordProtectByTalkCheckbox(), 2)->click(); + } + /** * @Then I see that the current page is the Files app */ @@ -568,6 +599,24 @@ public function iSeeThatTheLinkShareIsPasswordProtected() { PHPUnit_Framework_Assert::assertTrue($this->actor->find(self::passwordProtectField(), 10)->isVisible(), "Password protect field is visible"); } + /** + * @Then I see that the password of the link share is protected by Talk + */ + public function iSeeThatThePasswordOfTheLinkShareIsProtectedByTalk() { + $this->showShareLinkMenuIfNeeded(); + + PHPUnit_Framework_Assert::assertTrue($this->actor->find(self::passwordProtectByTalkCheckboxInput(), 10)->isChecked()); + } + + /** + * @Then I see that the password of the link share is not protected by Talk + */ + public function iSeeThatThePasswordOfTheLinkShareIsNotProtectedByTalk() { + $this->showShareLinkMenuIfNeeded(); + + PHPUnit_Framework_Assert::assertFalse($this->actor->find(self::passwordProtectByTalkCheckboxInput(), 10)->isChecked()); + } + /** * @Then I see that the checkbox to protect the password of the link share by Talk is not shown */