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 19f3716062..d368304453 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -1788,6 +1788,14 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 eb39dd7926..b4d28fe986 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -1512,6 +1512,9 @@ class ShareAPIControllerTest extends TestCase { ->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 @@ class ShareAPIControllerTest extends TestCase { $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 @@ class ShareAPIControllerTest extends TestCase { ->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 @@ class ShareAPIControllerTest extends TestCase { 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 @@ class ShareAPIControllerTest extends TestCase { ->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 @@ class ShareAPIControllerTest extends TestCase { ->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 @@ class ShareAPIControllerTest extends TestCase { $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 @@ class ShareAPIControllerTest extends TestCase { ->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 @@ class ShareAPIControllerTest extends TestCase { 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 @@ class ShareAPIControllerTest extends TestCase { ->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 @@ class ShareAPIControllerTest extends TestCase { $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 @@ class ShareAPIControllerTest extends TestCase { ->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 @@ class ShareAPIControllerTest extends TestCase { ->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 @@ class ShareAPIControllerTest extends TestCase { $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 @@ class ShareAPIControllerTest extends TestCase { ->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 @@ class ShareAPIControllerTest extends TestCase { $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 5011105454..589b64fc58 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -156,6 +156,8 @@ class DefaultShareProvider implements IShareProvider { $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 @@ class DefaultShareProvider implements IShareProvider { $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 @@ class DefaultShareProvider implements IShareProvider { $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 d368304453..578225840b 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -363,6 +363,7 @@ class DefaultShareProviderTest extends \Test\TestCase { ->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 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 @@ class DefaultShareProviderTest extends \Test\TestCase { ->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 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 @@ class DefaultShareProviderTest extends \Test\TestCase { $share = $this->provider->getShareById($id); $share->setPassword('password'); + $share->setSendPasswordByTalk(true); $share->setSharedBy('user4'); $share->setShareOwner('user5'); $share->setNode($file2); @@ -1842,6 +1849,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 61fad5d2b1..42d0218de8 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -212,6 +212,8 @@ class ShareAPIController extends OCSController { $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 @@ class ShareAPIController extends OCSController { $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 @@ class ShareAPIController extends OCSController { $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 b4d28fe986..efc252d49d 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -430,6 +430,7 @@ class ShareAPIControllerTest extends TestCase { '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 @@ class ShareAPIControllerTest extends TestCase { $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 @@ class ShareAPIControllerTest extends TestCase { ->setSharedBy($this->currentUser) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') + ->setSendPasswordByTalk(true) ->setExpirationDate($date) ->setNote('note') ->setLabel('label') @@ -1728,6 +1795,7 @@ class ShareAPIControllerTest extends TestCase { $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 @@ class ShareAPIControllerTest extends TestCase { $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 @@ class ShareAPIControllerTest extends TestCase { ->setSharedBy($this->currentUser) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') + ->setSendPasswordByTalk(true) ->setExpirationDate(new \DateTime()) ->setNote('note') ->setLabel('label') @@ -1771,6 +2018,7 @@ class ShareAPIControllerTest extends TestCase { 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 @@ class ShareAPIControllerTest extends TestCase { ->setSharedBy($this->currentUser) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') + ->setSendPasswordByTalk(true) ->setExpirationDate($date) ->setNote('note') ->setLabel('label') @@ -1811,6 +2060,7 @@ class ShareAPIControllerTest extends TestCase { $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 @@ class ShareAPIControllerTest extends TestCase { ->setSharedBy($this->currentUser) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') + ->setSendPasswordByTalk(true) ->setExpirationDate($date) ->setNote('note') ->setLabel('label') @@ -1854,6 +2105,7 @@ class ShareAPIControllerTest extends TestCase { $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 @@ class ShareAPIControllerTest extends TestCase { ->setSharedBy($this->currentUser) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') + ->setSendPasswordByTalk(true) ->setExpirationDate($date) ->setNote('note') ->setLabel('label') @@ -1896,6 +2149,7 @@ class ShareAPIControllerTest extends TestCase { $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 @@ class ShareAPIControllerTest extends TestCase { '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 | 33 +++-- .../tests/specs/sharedialoglinkshareview.js | 113 ++++++++++++++++++ core/js/tests/specs/shareitemmodelSpec.js | 13 +- 6 files changed, 224 insertions(+), 14 deletions(-) diff --git a/core/js/share/sharedialoglinkshareview_popover_menu.handlebars b/core/js/share/sharedialoglinkshareview_popover_menu.handlebars index cc951ce047..59312bc70b 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.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))); + 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))); },"19":function(container,depth0,helpers,partials,data) { - return "readonly"; + 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))); },"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 f5fe8725c0..c2d84fd2e8 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 3b4dc5a960..e801695009 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 70e085ca66..33bed4a3ef 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 459028813b..03bf371095 100644 --- a/tests/acceptance/features/bootstrap/FilesAppContext.php +++ b/tests/acceptance/features/bootstrap/FilesAppContext.php @@ -276,6 +276,15 @@ class FilesAppContext implements Context, ActorAwareInterface { 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 @@ class FilesAppContext implements Context, ActorAwareInterface { 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 @@ class FilesAppContext implements Context, ActorAwareInterface { } } + /** + * @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 03bf371095..4a7af441e5 100644 --- a/tests/acceptance/features/bootstrap/FilesAppContext.php +++ b/tests/acceptance/features/bootstrap/FilesAppContext.php @@ -313,6 +313,15 @@ class FilesAppContext implements Context, ActorAwareInterface { 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 @@ class FilesAppContext implements Context, ActorAwareInterface { $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 @@ class FilesAppContext implements Context, ActorAwareInterface { 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 */