diff options
26 files changed, 1154 insertions, 90 deletions
diff --git a/apps/files/lib/Command/TransferOwnership.php b/apps/files/lib/Command/TransferOwnership.php index d175f66d171..6113f6df54c 100644 --- a/apps/files/lib/Command/TransferOwnership.php +++ b/apps/files/lib/Command/TransferOwnership.php @@ -263,7 +263,8 @@ class TransferOwnership extends Command { foreach($this->shares as $share) { try { - if ($share->getSharedWith() === $this->destinationUser) { + if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER && + $share->getSharedWith() === $this->destinationUser) { // Unmount the shares before deleting, so we don't try to get the storage later on. $shareMountPoint = $this->mountManager->find('/' . $this->destinationUser . '/files' . $share->getTarget()); if ($shareMountPoint) { diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 33782d21b5f..bda0047e66c 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -30,6 +30,7 @@ declare(strict_types=1); namespace OCA\Files_Sharing\Controller; use OCA\Files\Helper; +use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSException; @@ -81,6 +82,8 @@ class ShareAPIController extends OCSController { private $lockedNode; /** @var IConfig */ private $config; + /** @var IAppManager */ + private $appManager; /** * Share20OCS constructor. @@ -95,6 +98,7 @@ class ShareAPIController extends OCSController { * @param string $userId * @param IL10N $l10n * @param IConfig $config + * @param IAppManager $appManager */ public function __construct( string $appName, @@ -106,7 +110,8 @@ class ShareAPIController extends OCSController { IURLGenerator $urlGenerator, string $userId, IL10N $l10n, - IConfig $config + IConfig $config, + IAppManager $appManager ) { parent::__construct($appName, $request); @@ -119,6 +124,7 @@ class ShareAPIController extends OCSController { $this->currentUser = $userId; $this->l = $l10n; $this->config = $config; + $this->appManager = $appManager; } /** @@ -206,6 +212,7 @@ class ShareAPIController extends OCSController { } else if ($share->getShareType() === Share::SHARE_TYPE_EMAIL) { $result['share_with'] = $share->getSharedWith(); $result['password'] = $share->getPassword(); + $result['send_password_by_talk'] = $share->getSendPasswordByTalk(); $result['share_with_displayname'] = $this->getDisplayNameFromAddressBook($share->getSharedWith(), 'EMAIL'); $result['token'] = $share->getToken(); } else if ($share->getShareType() === Share::SHARE_TYPE_CIRCLE) { @@ -328,6 +335,7 @@ class ShareAPIController extends OCSController { * @param string $shareWith * @param string $publicUpload * @param string $password + * @param bool $sendPasswordByTalk * @param string $expireDate * * @return DataResponse @@ -345,6 +353,7 @@ class ShareAPIController extends OCSController { string $shareWith = null, string $publicUpload = 'false', string $password = '', + string $sendPasswordByTalk = null, string $expireDate = '' ): DataResponse { $share = $this->shareManager->newShare(); @@ -485,6 +494,14 @@ class ShareAPIController extends OCSController { $share->setPermissions($permissions); } $share->setSharedWith($shareWith); + + if ($sendPasswordByTalk === 'true') { + if (!$this->appManager->isEnabledForUser('spreed')) { + throw new OCSForbiddenException($this->l->t('Sharing %s sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled', [$path->getPath()])); + } + + $share->setSendPasswordByTalk(true); + } } else if ($shareType === Share::SHARE_TYPE_CIRCLE) { if (!\OC::$server->getAppManager()->isEnabledForUser('circles') || !class_exists('\OCA\Circles\ShareByCircleProvider')) { throw new OCSNotFoundException($this->l->t('You cannot share to a Circle if the app is not enabled')); @@ -697,6 +714,7 @@ class ShareAPIController extends OCSController { * @param string $id * @param int $permissions * @param string $password + * @param string $sendPasswordByTalk * @param string $publicUpload * @param string $expireDate * @param string $note @@ -711,6 +729,7 @@ class ShareAPIController extends OCSController { string $id, int $permissions = null, string $password = null, + string $sendPasswordByTalk = null, string $publicUpload = null, string $expireDate = null, string $note = null @@ -727,7 +746,7 @@ class ShareAPIController extends OCSController { throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist')); } - if ($permissions === null && $password === null && $publicUpload === null && $expireDate === null && $note === null) { + if ($permissions === null && $password === null && $sendPasswordByTalk === null && $publicUpload === null && $expireDate === null && $note === null) { throw new OCSBadRequestException($this->l->t('Wrong or no update parameter given')); } @@ -816,6 +835,16 @@ class ShareAPIController extends OCSController { } else if ($password !== null) { $share->setPassword($password); } + + if ($sendPasswordByTalk === 'true') { + if (!$this->appManager->isEnabledForUser('spreed')) { + throw new OCSForbiddenException($this->l->t('Sharing sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled')); + } + + $share->setSendPasswordByTalk(true); + } else { + $share->setSendPasswordByTalk(false); + } } if ($expireDate === '') { diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index bd1331a0908..6ebffed2b58 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -61,6 +61,7 @@ use OCP\Files\NotFoundException; use OCP\Files\IRootFolder; use OCP\Share\Exceptions\ShareNotFound; use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\EventDispatcher\GenericEvent; use OCP\Share\IManager as ShareManager; /** @@ -143,6 +144,34 @@ class ShareController extends AuthPublicShareController { $this->shareManager = $shareManager; } + /** + * @PublicPage + * @NoCSRFRequired + * + * Show the authentication page + * The form has to submit to the authenticate method route + */ + public function showAuthenticate(): TemplateResponse { + $templateParameters = ['share' => $this->share]; + + $event = new GenericEvent(null, $templateParameters); + $this->eventDispatcher->dispatch('OCA\Files_Sharing::loadAdditionalScripts::publicShareAuth', $event); + + return new TemplateResponse('core', 'publicshareauth', $templateParameters, 'guest'); + } + + /** + * The template to show when authentication failed + */ + protected function showAuthFailed(): TemplateResponse { + $templateParameters = ['share' => $this->share, 'wrongpw' => true]; + + $event = new GenericEvent(null, $templateParameters); + $this->eventDispatcher->dispatch('OCA\Files_Sharing::loadAdditionalScripts::publicShareAuth', $event); + + return new TemplateResponse('core', 'publicshareauth', $templateParameters, 'guest'); + } + protected function verifyPassword(string $password): bool { return $this->shareManager->checkPassword($this->share, $password); } diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index bf4cca53891..a68ec7de1f0 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -33,6 +33,7 @@ namespace OCA\Files_Sharing\Tests; use OC\Files\Cache\Scanner; use OCA\Files_Sharing\Controller\ShareAPIController; +use OCP\App\IAppManager; use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSForbiddenException; @@ -107,6 +108,7 @@ class ApiTest extends TestCase { return vsprintf($text, $parameters); })); $config = $this->createMock(IConfig::class); + $appManager = $this->createMock(IAppManager::class); return new ShareAPIController( self::APP_NAME, @@ -118,7 +120,8 @@ class ApiTest extends TestCase { \OC::$server->getURLGenerator(), $userId, $l, - $config + $config, + $appManager ); } @@ -960,7 +963,7 @@ class ApiTest extends TestCase { // update public upload $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); - $ocs->updateShare($share1->getId(), null, null, 'true'); + $ocs->updateShare($share1->getId(), null, null, null, 'true'); $ocs->cleanup(); $share1 = $this->shareManager->getShareById($share1->getFullId()); @@ -1003,7 +1006,7 @@ class ApiTest extends TestCase { // update expire date to a valid value $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); - $ocs->updateShare($share1->getId(), null, null, null, $dateWithinRange->format('Y-m-d')); + $ocs->updateShare($share1->getId(), null, null, null, null, $dateWithinRange->format('Y-m-d')); $ocs->cleanup(); $share1 = $this->shareManager->getShareById($share1->getFullId()); @@ -1234,7 +1237,7 @@ class ApiTest extends TestCase { $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); try { - $result = $ocs->createShare($this->folder, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', $date); + $result = $ocs->createShare($this->folder, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, $date); $this->assertTrue($valid); } catch (OCSNotFoundException $e) { $this->assertFalse($valid); @@ -1270,7 +1273,7 @@ class ApiTest extends TestCase { $date->add(new \DateInterval('P5D')); $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); - $result = $ocs->createShare($this->filename, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', $date->format('Y-m-d')); + $result = $ocs->createShare($this->filename, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, $date->format('Y-m-d')); $ocs->cleanup(); $data = $result->getData(); @@ -1304,7 +1307,7 @@ class ApiTest extends TestCase { $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); try { - $ocs->createShare($this->filename, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', $date->format('Y-m-d')); + $ocs->createShare($this->filename, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, $date->format('Y-m-d')); $this->fail(); } catch (OCSException $e) { $this->assertEquals(404, $e->getCode()); @@ -1325,7 +1328,7 @@ class ApiTest extends TestCase { $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); try { - $ocs->createShare($this->filename, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', $date->format('Y-m-d')); + $ocs->createShare($this->filename, \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, $date->format('Y-m-d')); $this->fail(); } catch(OCSException $e) { $this->assertEquals(404, $e->getCode()); diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 30041c3a27b..4fd8162db3d 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -26,6 +26,7 @@ */ namespace OCA\Files_Sharing\Tests\Controller; +use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\Files\File; @@ -88,6 +89,9 @@ class ShareAPIControllerTest extends TestCase { /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ private $config; + /** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */ + private $appManager; + protected function setUp() { $this->shareManager = $this->createMock(IManager::class); $this->shareManager @@ -107,6 +111,7 @@ class ShareAPIControllerTest extends TestCase { return vsprintf($text, $parameters); })); $this->config = $this->createMock(IConfig::class); + $this->appManager = $this->createMock(IAppManager::class); $this->ocs = new ShareAPIController( $this->appName, @@ -118,7 +123,8 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ); } @@ -137,7 +143,8 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['formatShare']) ->getMock(); } @@ -453,7 +460,8 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['canAccessShare']) ->getMock(); @@ -724,7 +732,8 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['formatShare']) ->getMock(); @@ -822,7 +831,8 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['formatShare']) ->getMock(); @@ -1008,7 +1018,7 @@ class ShareAPIControllerTest extends TestCase { )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'true', '', ''); + $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'true', '', null, ''); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1042,7 +1052,7 @@ class ShareAPIControllerTest extends TestCase { )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', 'password', ''); + $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', 'password', null, ''); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1089,7 +1099,7 @@ class ShareAPIControllerTest extends TestCase { )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', '2000-01-01'); + $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, '2000-01-01'); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1115,7 +1125,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('shareApiAllowLinks')->willReturn(true); $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); - $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', 'a1b2d3'); + $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', null, 'a1b2d3'); } /** @@ -1138,7 +1148,8 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->appManager ])->setMethods(['formatShare']) ->getMock(); @@ -1264,7 +1275,7 @@ class ShareAPIControllerTest extends TestCase { ->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, '', 'false', ''); + $result = $ocs->updateShare(42, null, '', null, 'false', ''); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1299,7 +1310,7 @@ class ShareAPIControllerTest extends TestCase { ->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, 'password', 'true', '2000-01-01'); + $result = $ocs->updateShare(42, null, 'password', null, 'true', '2000-01-01'); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1333,7 +1344,7 @@ class ShareAPIControllerTest extends TestCase { )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, $permissions, $password, $publicUpload, $expireDate); + $result = $ocs->updateShare(42, $permissions, $password, null, $publicUpload, $expireDate); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1357,7 +1368,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); - $ocs->updateShare(42, null, 'password', 'true', '2000-01-a'); + $ocs->updateShare(42, null, 'password', null, 'true', '2000-01-a'); } public function publicUploadParamsProvider() { @@ -1395,7 +1406,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(false); - $ocs->updateShare(42, $permissions, $password, $publicUpload, $expireDate); + $ocs->updateShare(42, $permissions, $password, null, $publicUpload, $expireDate); } /** @@ -1416,7 +1427,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); - $ocs->updateShare(42, null, 'password', 'true', ''); + $ocs->updateShare(42, null, 'password', null, 'true', ''); } public function testUpdateLinkSharePasswordDoesNotChangeOther() { @@ -1450,7 +1461,7 @@ class ShareAPIControllerTest extends TestCase { )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, 'newpassword', null, null); + $result = $ocs->updateShare(42, null, 'newpassword', null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1487,7 +1498,7 @@ class ShareAPIControllerTest extends TestCase { )->will($this->returnArgument(0)); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, null, null, '2010-12-23'); + $result = $ocs->updateShare(42, null, null, null, null, '2010-12-23'); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1524,7 +1535,7 @@ class ShareAPIControllerTest extends TestCase { ->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, null, null, 'true', null); + $result = $ocs->updateShare(42, null, null, null, 'true', null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1560,7 +1571,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getSharedWith')->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, 7, null, null, null); + $result = $ocs->updateShare(42, 7, null, null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1596,7 +1607,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getSharedWith')->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, 31, null, null, null); + $result = $ocs->updateShare(42, 31, null, null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1625,7 +1636,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getSharedWith')->willReturn([]); $expected = new DataResponse([]); - $result = $ocs->updateShare(42, 31, null, null, null); + $result = $ocs->updateShare(42, 31, null, null, null, null); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData()); @@ -1722,7 +1733,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); try { - $ocs->updateShare(42, null, null, 'true'); + $ocs->updateShare(42, null, null, null, 'true'); $this->fail(); } catch (OCSNotFoundException $e) { $this->assertEquals('Cannot increase permissions', $e->getMessage()); @@ -2276,7 +2287,52 @@ class ShareAPIControllerTest extends TestCase { 'share_with_displayname' => 'mail display name', 'mail_send' => 0, 'mimetype' => 'myFolderMimeType', - 'password' => 'password' + 'password' => 'password', + 'send_password_by_talk' => false + ], $share, [], false + ]; + + $share = \OC::$server->getShareManager()->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setSharedBy('initiator') + ->setSharedWith('user@server.com') + ->setShareOwner('owner') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder) + ->setShareTime(new \DateTime('2000-01-01T00:01:02')) + ->setTarget('myTarget') + ->setId(42) + ->setPassword('password') + ->setSendPasswordByTalk(true); + + $result[] = [ + [ + 'id' => 42, + 'share_type' => \OCP\Share::SHARE_TYPE_EMAIL, + 'uid_owner' => 'initiator', + 'displayname_owner' => 'initiator', + 'permissions' => 1, + 'stime' => 946684862, + 'parent' => null, + 'expiration' => null, + 'token' => null, + 'uid_file_owner' => 'owner', + 'displayname_file_owner' => 'owner', + 'note' => '', + 'path' => 'folder', + 'item_type' => 'folder', + 'storage_id' => 'storageId', + 'storage' => 100, + 'item_source' => 2, + 'file_source' => 2, + 'file_parent' => 1, + 'file_target' => 'myTarget', + 'share_with' => 'user@server.com', + 'share_with_displayname' => 'mail display name', + 'mail_send' => 0, + 'mimetype' => 'myFolderMimeType', + 'password' => 'password', + 'send_password_by_talk' => true ], $share, [], false ]; diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 73e962e3292..6cc27957bbe 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -338,7 +338,8 @@ class ShareByMailProvider implements IShareProvider { $share->getShareOwner(), $share->getPermissions(), $share->getToken(), - $share->getPassword() + $share->getPassword(), + $share->getSendPasswordByTalk() ); try { @@ -453,7 +454,7 @@ class ShareByMailProvider implements IShareProvider { $initiator = $share->getSharedBy(); $shareWith = $share->getSharedWith(); - if ($password === '' || $this->settingsManager->sendPasswordByMail() === false) { + if ($password === '' || $this->settingsManager->sendPasswordByMail() === false || $share->getSendPasswordByTalk()) { return false; } @@ -660,9 +661,11 @@ class ShareByMailProvider implements IShareProvider { * @param string $uidOwner * @param int $permissions * @param string $token + * @param string $password + * @param bool $sendPasswordByTalk * @return int */ - protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password) { + protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password, $sendPasswordByTalk) { $qb = $this->dbConnection->getQueryBuilder(); $qb->insert('share') ->setValue('share_type', $qb->createNamedParameter(\OCP\Share::SHARE_TYPE_EMAIL)) @@ -675,6 +678,7 @@ class ShareByMailProvider implements IShareProvider { ->setValue('permissions', $qb->createNamedParameter($permissions)) ->setValue('token', $qb->createNamedParameter($token)) ->setValue('password', $qb->createNamedParameter($password)) + ->setValue('password_by_talk', $qb->createNamedParameter($sendPasswordByTalk, IQueryBuilder::PARAM_BOOL)) ->setValue('stime', $qb->createNamedParameter(time())); /* @@ -703,7 +707,8 @@ class ShareByMailProvider implements IShareProvider { // a real password was given $validPassword = $plainTextPassword !== null && $plainTextPassword !== ''; - if($validPassword && $originalShare->getPassword() !== $share->getPassword()) { + if($validPassword && ($originalShare->getPassword() !== $share->getPassword() || + ($originalShare->getSendPasswordByTalk() && !$share->getSendPasswordByTalk()))) { $this->sendPassword($share, $plainTextPassword); } /* @@ -716,6 +721,7 @@ class ShareByMailProvider implements IShareProvider { ->set('uid_owner', $qb->createNamedParameter($share->getShareOwner())) ->set('uid_initiator', $qb->createNamedParameter($share->getSharedBy())) ->set('password', $qb->createNamedParameter($share->getPassword())) + ->set('password_by_talk', $qb->createNamedParameter($share->getSendPasswordByTalk(), IQueryBuilder::PARAM_BOOL)) ->set('expiration', $qb->createNamedParameter($share->getExpirationDate(), IQueryBuilder::PARAM_DATE)) ->set('note', $qb->createNamedParameter($share->getNote())) ->execute(); @@ -972,6 +978,7 @@ class ShareByMailProvider implements IShareProvider { $share->setShareTime($shareTime); $share->setSharedWith($data['share_with']); $share->setPassword($data['password']); + $share->setSendPasswordByTalk($data['password_by_talk']); if ($data['uid_initiator'] !== null) { $share->setShareOwner($data['uid_owner']); diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index f0d99e6026c..4942dac7022 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -38,6 +38,7 @@ use OCP\IUser; use OCP\IUserManager; use OCP\Mail\IEMailTemplate; use OCP\Mail\IMailer; +use OCP\Mail\IMessage; use OCP\Security\IHasher; use OCP\Security\ISecureRandom; use OCP\Share\IManager; @@ -204,6 +205,107 @@ class ShareByMailProviderTest extends TestCase { ); } + public function testCreateSendPasswordByMailWithoutEnforcedPasswordProtection() { + $share = $this->getMockBuilder(IShare::class)->getMock(); + $share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); + $share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false); + $share->expects($this->any())->method('getSharedBy')->willReturn('owner'); + + $node = $this->getMockBuilder(File::class)->getMock(); + $node->expects($this->any())->method('getName')->willReturn('filename'); + + $instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity']); + + $instance->expects($this->once())->method('getSharedWith')->willReturn([]); + $instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42); + $instance->expects($this->once())->method('createShareActivity')->with($share); + $instance->expects($this->once())->method('getRawShare')->with(42)->willReturn('rawShare'); + $instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject'); + $share->expects($this->any())->method('getNode')->willReturn($node); + + // The autogenerated password should not be mailed. + $this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(false); + $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); + $instance->expects($this->never())->method('autoGeneratePassword'); + + $this->mailer->expects($this->never())->method('send'); + + $this->assertSame('shareObject', + $instance->create($share) + ); + } + + public function testCreateSendPasswordByMailWithEnforcedPasswordProtection() { + $share = $this->getMockBuilder(IShare::class)->getMock(); + $share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); + $share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false); + $share->expects($this->any())->method('getSharedBy')->willReturn('owner'); + + $node = $this->getMockBuilder(File::class)->getMock(); + $node->expects($this->any())->method('getName')->willReturn('filename'); + + $instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity']); + + $instance->expects($this->once())->method('getSharedWith')->willReturn([]); + $instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42); + $instance->expects($this->once())->method('createShareActivity')->with($share); + $instance->expects($this->once())->method('getRawShare')->with(42)->willReturn('rawShare'); + $instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject'); + $share->expects($this->any())->method('getNode')->willReturn($node); + + // The autogenerated password should be mailed to the receiver of the share. + $this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(true); + $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); + $instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('password'); + + $message = $this->createMock(IMessage::class); + $message->expects($this->once())->method('setTo')->with(['receiver@example.com']); + $this->mailer->expects($this->once())->method('createMessage')->willReturn($message); + $this->mailer->expects($this->once())->method('send'); + + $this->assertSame('shareObject', + $instance->create($share) + ); + } + + public function testCreateSendPasswordByTalkWithEnforcedPasswordProtection() { + $share = $this->getMockBuilder(IShare::class)->getMock(); + $share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); + $share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(true); + $share->expects($this->any())->method('getSharedBy')->willReturn('owner'); + + $node = $this->getMockBuilder(File::class)->getMock(); + $node->expects($this->any())->method('getName')->willReturn('filename'); + + $instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity']); + + $instance->expects($this->once())->method('getSharedWith')->willReturn([]); + $instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42); + $instance->expects($this->once())->method('createShareActivity')->with($share); + $instance->expects($this->once())->method('getRawShare')->with(42)->willReturn('rawShare'); + $instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject'); + $share->expects($this->any())->method('getNode')->willReturn($node); + + // The autogenerated password should be mailed to the owner of the share. + $this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(true); + $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); + $instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('password'); + + $message = $this->createMock(IMessage::class); + $message->expects($this->once())->method('setTo')->with(['owner@example.com' => 'Owner display name']); + $this->mailer->expects($this->once())->method('createMessage')->willReturn($message); + $this->mailer->expects($this->once())->method('send'); + + $user = $this->createMock(IUser::class); + $this->userManager->expects($this->once())->method('get')->with('owner')->willReturn($user); + $user->expects($this->once())->method('getDisplayName')->willReturn('Owner display name'); + $user->expects($this->once())->method('getEMailAddress')->willReturn('owner@example.com'); + + $this->assertSame('shareObject', + $instance->create($share) + ); + } + /** * @expectedException \Exception */ @@ -296,6 +398,7 @@ class ShareByMailProviderTest extends TestCase { $permissions = 1; $token = 'token'; $password = 'password'; + $sendPasswordByTalk = true; $instance = $this->getInstance(); @@ -310,7 +413,8 @@ class ShareByMailProviderTest extends TestCase { $uidOwner, $permissions, $token, - $password + $password, + $sendPasswordByTalk ] ); @@ -330,6 +434,7 @@ class ShareByMailProviderTest extends TestCase { $this->assertSame($permissions, (int)$result[0]['permissions']); $this->assertSame($token, $result[0]['token']); $this->assertSame($password, $result[0]['password']); + $this->assertSame($sendPasswordByTalk, (bool)$result[0]['password_by_talk']); } @@ -377,6 +482,60 @@ class ShareByMailProviderTest extends TestCase { $this->assertSame($note, $result[0]['note']); } + public function dataUpdateSendPassword() { + return [ + ['password', 'hashed', 'hashed new', false, false, true], + ['', 'hashed', 'hashed new', false, false, false], + [null, 'hashed', 'hashed new', false, false, false], + ['password', 'hashed', 'hashed', false, false, false], + ['password', 'hashed', 'hashed new', false, true, false], + ['password', 'hashed', 'hashed new', true, false, true], + ['password', 'hashed', 'hashed', true, false, true], + ]; + } + + /** + * @dataProvider dataUpdateSendPassword + * + * @param string|null plainTextPassword + * @param string originalPassword + * @param string newPassword + * @param string originalSendPasswordByTalk + * @param string newSendPasswordByTalk + * @param bool sendMail + */ + public function testUpdateSendPassword($plainTextPassword, string $originalPassword, string $newPassword, string $originalSendPasswordByTalk, string $newSendPasswordByTalk, bool $sendMail) { + $node = $this->getMockBuilder(File::class)->getMock(); + $node->expects($this->any())->method('getName')->willReturn('filename'); + + $originalShare = $this->getMockBuilder(IShare::class)->getMock(); + $originalShare->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); + $originalShare->expects($this->any())->method('getNode')->willReturn($node); + $originalShare->expects($this->any())->method('getId')->willReturn(42); + $originalShare->expects($this->any())->method('getPassword')->willReturn($originalPassword); + $originalShare->expects($this->any())->method('getSendPasswordByTalk')->willReturn($originalSendPasswordByTalk); + + $share = $this->getMockBuilder(IShare::class)->getMock(); + $share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); + $share->expects($this->any())->method('getNode')->willReturn($node); + $share->expects($this->any())->method('getId')->willReturn(42); + $share->expects($this->any())->method('getPassword')->willReturn($newPassword); + $share->expects($this->any())->method('getSendPasswordByTalk')->willReturn($newSendPasswordByTalk); + + if ($sendMail) { + $this->mailer->expects($this->once())->method('send'); + } else { + $this->mailer->expects($this->never())->method('send'); + } + + $instance = $this->getInstance(['getShareById', 'createPasswordSendActivity']); + $instance->expects($this->once())->method('getShareById')->willReturn($originalShare); + + $this->assertSame($share, + $instance->update($share, $plainTextPassword) + ); + } + public function testDelete() { $instance = $this->getInstance(['removeShareFromTable']); $this->share->expects($this->once())->method('getId')->willReturn(42); diff --git a/build/integration/features/transfer-ownership.feature b/build/integration/features/transfer-ownership.feature index 7990eac3280..3f10fa75a83 100644 --- a/build/integration/features/transfer-ownership.feature +++ b/build/integration/features/transfer-ownership.feature @@ -72,6 +72,19 @@ Feature: transfer-ownership And As an "user2" Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + Scenario: transfering ownership of file shares to user with the same id as the group + Given user "user0" exists + And user "test" exists + And user "user2" exists + And group "test" exists + And user "user2" belongs to group "test" + And User "user0" uploads file "data/textfile.txt" to "/somefile.txt" + And file "/somefile.txt" of user "user0" is shared with group "test" + When transfering ownership from "user0" to "test" + And the command was successful + And As an "user2" + Then Downloaded content when downloading file "/somefile.txt" with range "bytes=0-6" should be "This is" + Scenario: transfering ownership does not transfer received shares Given user "user0" exists And user "user1" exists diff --git a/core/Command/Db/ConvertType.php b/core/Command/Db/ConvertType.php index 7fdf1f39b8e..b6c2c66b3d3 100644 --- a/core/Command/Db/ConvertType.php +++ b/core/Command/Db/ConvertType.php @@ -245,7 +245,7 @@ class ConvertType extends Command implements CompletionAwareInterface { $currentMigration = $fromMS->getMigration('current'); if ($currentMigration !== '0') { $toMS = new MigrationService($app, $toDB); - $toMS->migrate($currentMigration); + $toMS->migrate($currentMigration, true); } } } diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index 7bf2555819d..5bd06ac7e66 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -58,6 +58,10 @@ use OC\Hooks\PublicEmitter; use OCP\Util; class LoginController extends Controller { + + const LOGIN_MSG_INVALIDPASSWORD = 'invalidpassword'; + const LOGIN_MSG_USERDISABLED = 'userdisabled'; + /** @var IUserManager */ private $userManager; /** @var IConfig */ @@ -171,19 +175,7 @@ class LoginController extends Controller { $parameters['redirect_url'] = $redirect_url; } - $parameters['canResetPassword'] = true; - $parameters['resetPasswordLink'] = $this->config->getSystemValue('lost_password_link', ''); - if (!$parameters['resetPasswordLink']) { - if ($user !== null && $user !== '') { - $userObj = $this->userManager->get($user); - if ($userObj instanceof IUser) { - $parameters['canResetPassword'] = $userObj->canChangePassword(); - } - } - } elseif ($parameters['resetPasswordLink'] === 'disabled') { - $parameters['canResetPassword'] = false; - } - + $parameters = $this->setPasswordResetParameters($user, $parameters); $parameters['alt_login'] = OC_App::getAlternativeLogIns(); if ($user !== null && $user !== '') { @@ -210,6 +202,40 @@ class LoginController extends Controller { } /** + * Sets the password reset params. + * + * Users may not change their passwords if: + * - The account is disabled + * - The backend doesn't support password resets + * - The password reset function is disabled + * + * @param string $user + * @param array $parameters + * @return array + */ + private function setPasswordResetParameters( + string $user = null, array $parameters): array { + if ($user !== null && $user !== '') { + $userObj = $this->userManager->get($user); + } else { + $userObj = null; + } + + $parameters['resetPasswordLink'] = $this->config + ->getSystemValue('lost_password_link', ''); + + if (!$parameters['resetPasswordLink'] && $userObj !== null) { + $parameters['canResetPassword'] = $userObj->canChangePassword(); + } else if ($userObj !== null && $userObj->isEnabled() === false) { + $parameters['canResetPassword'] = false; + } else { + $parameters['canResetPassword'] = true; + } + + return $parameters; + } + + /** * @param string $redirectUrl * @return RedirectResponse */ @@ -256,6 +282,17 @@ class LoginController extends Controller { } $originalUser = $user; + + $userObj = $this->userManager->get($user); + + if ($userObj !== null && $userObj->isEnabled() === false) { + $this->logger->warning('Login failed: \''. $user . '\' disabled' . + ' (Remote IP: \''. $this->request->getRemoteAddress(). '\')', + ['app' => 'core']); + return $this->createLoginFailedResponse($user, $originalUser, + $redirect_url, self::LOGIN_MSG_USERDISABLED); + } + // TODO: Add all the insane error handling /* @var $loginResult IUser */ $loginResult = $this->userManager->checkPasswordNoLogging($user, $password); @@ -270,20 +307,15 @@ class LoginController extends Controller { } } } + if ($loginResult === false) { - $this->logger->warning('Login failed: \''. $user .'\' (Remote IP: \''. $this->request->getRemoteAddress(). '\')', ['app' => 'core']); - // Read current user and append if possible - we need to return the unmodified user otherwise we will leak the login name - $args = !is_null($user) ? ['user' => $originalUser] : []; - if (!is_null($redirect_url)) { - $args['redirect_url'] = $redirect_url; - } - $response = new RedirectResponse($this->urlGenerator->linkToRoute('core.login.showLoginForm', $args)); - $response->throttle(['user' => $user]); - $this->session->set('loginMessages', [ - ['invalidpassword'], [] - ]); - return $response; + $this->logger->warning('Login failed: \''. $user . + '\' (Remote IP: \''. $this->request->getRemoteAddress(). '\')', + ['app' => 'core']); + return $this->createLoginFailedResponse($user, $originalUser, + $redirect_url, self::LOGIN_MSG_INVALIDPASSWORD); } + // TODO: remove password checks from above and let the user session handle failures // requires https://github.com/owncloud/core/pull/24616 $this->userSession->completeLogin($loginResult, ['loginName' => $user, 'password' => $password]); @@ -331,6 +363,33 @@ class LoginController extends Controller { } /** + * Creates a login failed response. + * + * @param string $user + * @param string $originalUser + * @param string $redirect_url + * @param string $loginMessage + * @return RedirectResponse + */ + private function createLoginFailedResponse( + $user, $originalUser, $redirect_url, string $loginMessage) { + // Read current user and append if possible we need to + // return the unmodified user otherwise we will leak the login name + $args = !is_null($user) ? ['user' => $originalUser] : []; + if (!is_null($redirect_url)) { + $args['redirect_url'] = $redirect_url; + } + $response = new RedirectResponse( + $this->urlGenerator->linkToRoute('core.login.showLoginForm', $args) + ); + $response->throttle(['user' => $user]); + $this->session->set('loginMessages', [ + [$loginMessage], [] + ]); + return $response; + } + + /** * @NoAdminRequired * @UseSession * @BruteForceProtection(action=sudo) diff --git a/core/Migrations/Version14000Date20180710092004.php b/core/Migrations/Version14000Date20180710092004.php new file mode 100644 index 00000000000..d2e364063ce --- /dev/null +++ b/core/Migrations/Version14000Date20180710092004.php @@ -0,0 +1,48 @@ +<?php +declare(strict_types=1); + +/** + * + * @copyright Copyright (c) 2018, Daniel Calviño Sánchez (danxuliu@gmail.com) + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +namespace OC\Core\Migrations; + +use Doctrine\DBAL\Types\Type; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\SimpleMigrationStep; +use OCP\Migration\IOutput; + +class Version14000Date20180710092004 extends SimpleMigrationStep { + + public function changeSchema(IOutput $output, \Closure $schemaClosure, array $options) { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $table = $schema->getTable('share'); + + if (!$table->hasColumn('password_by_talk')) { + $table->addColumn('password_by_talk', Type::BOOLEAN, [ + 'default' => 0, + ]); + } + + return $schema; + } +} diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 574d47b4aac..8df7fe5ecd7 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -112,6 +112,21 @@ ' <span class="icon-loading-small hidden"></span>' + '</span>' + '</li>' + + '{{#if isTalkEnabled}}' + + '<li>' + + '<span class="shareOption menuitem">' + + '<input id="passwordByTalk-{{cid}}-{{shareId}}" type="checkbox" name="passwordByTalk" class="passwordByTalk checkbox" {{#if isPasswordByTalkSet}}checked="checked"{{/if}} />' + + '<label for="passwordByTalk-{{cid}}-{{shareId}}">{{passwordByTalkLabel}}</label>' + + '</span>' + + '</li>' + + '<li class="passwordByTalkMenu-{{cid}}-{{shareId}} {{#unless isPasswordByTalkSet}}hidden{{/unless}}">' + + '<span class="passwordByTalkContainer-{{cid}}-{{shareId}} icon-passwordtalk menuitem">' + + ' <label for="passwordByTalkField-{{cid}}-{{shareId}}" class="hidden-visually" value="{{password}}">{{passwordByTalkLabel}}</label>' + + ' <input id="passwordByTalkField-{{cid}}-{{shareId}}" class="passwordField" type="password" placeholder="{{passwordByTalkPlaceholder}}" value="{{passwordValue}}" autocomplete="new-password" />' + + ' <span class="icon-loading-small hidden"></span>' + + '</span>' + + '</li>' + + '{{/if}}' + '{{/if}}' + '<li>' + '<span class="shareOption menuitem">' + @@ -183,6 +198,7 @@ 'click .permissions': 'onPermissionChange', 'click .expireDate' : 'onExpireDateChange', 'click .password' : 'onMailSharePasswordProtectChange', + 'click .passwordByTalk' : 'onMailSharePasswordProtectByTalkChange', 'click .secureDrop' : 'onSecureDropChange', 'keyup input.passwordField': 'onMailSharePasswordKeyUp', 'focusout input.passwordField': 'onMailSharePasswordEntered', @@ -260,7 +276,7 @@ var share = this.model.get('shares')[shareIndex]; var password = share.password; var hasPassword = password !== null && password !== ''; - + var sendPasswordByTalk = share.send_password_by_talk; return _.extend(hasPermissionOverride, { cid: this.cid, @@ -282,12 +298,22 @@ isMailShare: shareType === OC.Share.SHARE_TYPE_EMAIL, isCircleShare: shareType === OC.Share.SHARE_TYPE_CIRCLE, isFileSharedByMail: shareType === OC.Share.SHARE_TYPE_EMAIL && !this.model.isFolder(), - isPasswordSet: hasPassword, + isPasswordSet: hasPassword && !sendPasswordByTalk, + isPasswordByTalkSet: hasPassword && sendPasswordByTalk, + isTalkEnabled: oc_appswebroots['spreed'] !== undefined, secureDropMode: !this.model.hasReadPermission(shareIndex), hasExpireDate: this.model.getExpireDate(shareIndex) !== null, shareNote: this.model.getNote(shareIndex), expireDate: moment(this.model.getExpireDate(shareIndex), 'YYYY-MM-DD').format('DD-MM-YYYY'), + // The password placeholder does not take into account if + // sending the password by Talk is enabled or not; when + // switching from sending the password by Talk to sending the + // password by email the password is reused and the share + // updated, so the placeholder already shows the password in the + // brief time between disabling sending the password by email + // and receiving the updated share. passwordPlaceholder: hasPassword ? PASSWORD_PLACEHOLDER : PASSWORD_PLACEHOLDER_MESSAGE, + passwordByTalkPlaceholder: (hasPassword && sendPasswordByTalk)? PASSWORD_PLACEHOLDER : PASSWORD_PLACEHOLDER_MESSAGE, }); }, @@ -303,6 +329,7 @@ secureDropLabel: t('core', 'File drop (upload only)'), expireDateLabel: t('core', 'Set expiration date'), passwordLabel: t('core', 'Password protect'), + passwordByTalkLabel: t('core', 'Password protect by Talk'), crudsLabel: t('core', 'Access control'), expirationDatePlaceholder: t('core', 'Expiration date'), defaultExpireDate: moment().add(1, 'day').format('DD-MM-YYYY'), // Can't expire today @@ -674,8 +701,10 @@ var inputClass = '#passwordField-' + this.cid + '-' + shareId; var passwordField = $(inputClass); var state = element.prop('checked'); - if (!state) { - this.model.updateShare(shareId, {password: ''}); + var passwordByTalkElement = $('#passwordByTalk-' + this.cid + '-' + shareId); + var passwordByTalkState = passwordByTalkElement.prop('checked'); + if (!state && !passwordByTalkState) { + this.model.updateShare(shareId, {password: '', sendPasswordByTalk: false}); passwordField.attr('value', ''); passwordField.removeClass('error'); passwordField.tooltip('hide'); @@ -683,13 +712,67 @@ passwordField.attr('placeholder', PASSWORD_PLACEHOLDER_MESSAGE); // We first need to reset the password field before we hide it passwordContainer.toggleClass('hidden', !state); - } else { + } else if (state) { + if (passwordByTalkState) { + // Switching from sending the password by Talk to sending + // the password by mail can be done keeping the previous + // password sent by Talk. + this.model.updateShare(shareId, {sendPasswordByTalk: false}); + + var passwordByTalkContainerClass = '.passwordByTalkMenu-' + this.cid + '-' + shareId; + var passwordByTalkContainer = $(passwordByTalkContainerClass); + passwordByTalkContainer.addClass('hidden'); + passwordByTalkElement.prop('checked', false); + } + passwordContainer.toggleClass('hidden', !state); passwordField = '#passwordField-' + this.cid + '-' + shareId; this.$(passwordField).focus(); } }, + onMailSharePasswordProtectByTalkChange: function(event) { + var element = $(event.target); + var li = element.closest('li[data-share-id]'); + var shareId = li.data('share-id'); + var passwordByTalkContainerClass = '.passwordByTalkMenu-' + this.cid + '-' + shareId; + var passwordByTalkContainer = $(passwordByTalkContainerClass); + var loading = this.$el.find(passwordByTalkContainerClass + ' .icon-loading-small'); + var inputClass = '#passwordByTalkField-' + this.cid + '-' + shareId; + var passwordByTalkField = $(inputClass); + var state = element.prop('checked'); + var passwordElement = $('#password-' + this.cid + '-' + shareId); + var passwordState = passwordElement.prop('checked'); + if (!state) { + this.model.updateShare(shareId, {password: '', sendPasswordByTalk: false}); + passwordByTalkField.attr('value', ''); + passwordByTalkField.removeClass('error'); + passwordByTalkField.tooltip('hide'); + loading.addClass('hidden'); + passwordByTalkField.attr('placeholder', PASSWORD_PLACEHOLDER_MESSAGE); + // We first need to reset the password field before we hide it + passwordByTalkContainer.toggleClass('hidden', !state); + } else if (state) { + if (passwordState) { + // Enabling sending the password by Talk requires a new + // password to be given (the one sent by mail is not reused, + // as it would defeat the purpose of checking the identity + // of the sharee by Talk if it was already sent by mail), so + // the share is not updated until the user explicitly gives + // the new password. + + var passwordContainerClass = '.passwordMenu-' + this.cid + '-' + shareId; + var passwordContainer = $(passwordContainerClass); + passwordContainer.addClass('hidden'); + passwordElement.prop('checked', false); + } + + passwordByTalkContainer.toggleClass('hidden', !state); + passwordByTalkField = '#passwordByTalkField-' + this.cid + '-' + shareId; + this.$(passwordByTalkField).focus(); + } + }, + onMailSharePasswordKeyUp: function(event) { if(event.keyCode === 13) { this.onMailSharePasswordEntered(event); @@ -700,8 +783,15 @@ var passwordField = $(event.target); var li = passwordField.closest('li[data-share-id]'); var shareId = li.data('share-id'); - var passwordContainerClass = '.passwordContainer-' + this.cid + '-' + shareId; - var loading = this.$el.find(passwordContainerClass + ' .icon-loading-small'); + var passwordContainerClass = '.passwordMenu-' + this.cid + '-' + shareId; + var passwordByTalkContainerClass = '.passwordByTalkMenu-' + this.cid + '-' + shareId; + var sendPasswordByTalk = passwordField.attr('id').startsWith('passwordByTalk'); + var loading; + if (sendPasswordByTalk) { + loading = this.$el.find(passwordByTalkContainerClass + ' .icon-loading-small'); + } else { + loading = this.$el.find(passwordContainerClass + ' .icon-loading-small'); + } if (!loading.hasClass('hidden')) { // still in process return; @@ -720,7 +810,8 @@ this.model.updateShare(shareId, { - password: password + password: password, + sendPasswordByTalk: sendPasswordByTalk }, { error: function(model, msg) { // destroy old tooltips diff --git a/core/templates/login.php b/core/templates/login.php index 8fc68edb92d..989ea1eaad5 100644 --- a/core/templates/login.php +++ b/core/templates/login.php @@ -2,6 +2,8 @@ <?php vendor_script('jsTimezoneDetect/jstz'); script('core', 'merged-login'); + +use OC\Core\Controller\LoginController; ?> <!--[if IE 8]><style>input[type="checkbox"]{padding:0;}</style><![endif]--> @@ -34,7 +36,7 @@ script('core', 'merged-login'); <!-- the following div ensures that the spinner is always inside the #message div --> <div style="clear: both;"></div> </div> - <p class="grouptop<?php if (!empty($_['invalidpassword'])) { ?> shake<?php } ?>"> + <p class="grouptop<?php if (!empty($_[LoginController::LOGIN_MSG_INVALIDPASSWORD])) { ?> shake<?php } ?>"> <input type="text" name="user" id="user" placeholder="<?php p($l->t('Username or email')); ?>" aria-label="<?php p($l->t('Username or email')); ?>" @@ -44,7 +46,7 @@ script('core', 'merged-login'); <label for="user" class="infield"><?php p($l->t('Username or email')); ?></label> </p> - <p class="groupbottom<?php if (!empty($_['invalidpassword'])) { ?> shake<?php } ?>"> + <p class="groupbottom<?php if (!empty($_[LoginController::LOGIN_MSG_INVALIDPASSWORD])) { ?> shake<?php } ?>"> <input type="password" name="password" id="password" value="" placeholder="<?php p($l->t('Password')); ?>" aria-label="<?php p($l->t('Password')); ?>" @@ -58,10 +60,14 @@ script('core', 'merged-login'); <div class="submit-icon icon-confirm-white"></div> </div> - <?php if (!empty($_['invalidpassword'])) { ?> + <?php if (!empty($_[LoginController::LOGIN_MSG_INVALIDPASSWORD])) { ?> <p class="warning wrongPasswordMsg"> <?php p($l->t('Wrong password.')); ?> </p> + <?php } else if (!empty($_[LoginController::LOGIN_MSG_USERDISABLED])) { ?> + <p class="warning userDisabledMsg"> + <?php p(\OC::$server->getL10N('lib')->t('User disabled')); ?> + </p> <?php } ?> <?php if ($_['throttle_delay'] > 5000) { ?> diff --git a/core/templates/publicshareauth.php b/core/templates/publicshareauth.php index adcc2853f87..22e22622291 100644 --- a/core/templates/publicshareauth.php +++ b/core/templates/publicshareauth.php @@ -20,6 +20,7 @@ placeholder="<?php p($l->t('Password')); ?>" value="" autocomplete="new-password" autocapitalize="off" autocorrect="off" autofocus /> + <input type="hidden" name="sharingToken" value="<?php p($_['share']->getToken()) ?>" id="sharingToken"> <input type="submit" id="password-submit" class="svg icon-confirm input-button-inline" value="" disabled="disabled" /> </p> diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index a060131979d..44b7cd5244f 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -611,6 +611,7 @@ return array( 'OC\\Core\\Migrations\\Version14000Date20180518120534' => $baseDir . '/core/Migrations/Version14000Date20180518120534.php', 'OC\\Core\\Migrations\\Version14000Date20180522074438' => $baseDir . '/core/Migrations/Version14000Date20180522074438.php', 'OC\\Core\\Migrations\\Version14000Date20180626223656' => $baseDir . '/core/Migrations/Version14000Date20180626223656.php', + 'OC\\Core\\Migrations\\Version14000Date20180710092004' => $baseDir . '/core/Migrations/Version14000Date20180710092004.php', 'OC\\Core\\Migrations\\Version14000Date20180712153140' => $baseDir . '/core/Migrations/Version14000Date20180712153140.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', 'OC\\DB\\AdapterMySQL' => $baseDir . '/lib/private/DB/AdapterMySQL.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 4c6c55a59ad..815f94d5711 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -641,6 +641,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Migrations\\Version14000Date20180518120534' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180518120534.php', 'OC\\Core\\Migrations\\Version14000Date20180522074438' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180522074438.php', 'OC\\Core\\Migrations\\Version14000Date20180626223656' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180626223656.php', + 'OC\\Core\\Migrations\\Version14000Date20180710092004' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180710092004.php', 'OC\\Core\\Migrations\\Version14000Date20180712153140' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180712153140.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', 'OC\\DB\\AdapterMySQL' => __DIR__ . '/../../..' . '/lib/private/DB/AdapterMySQL.php', diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php index cc2889dae0c..6f5a74103a5 100644 --- a/lib/private/DB/MigrationService.php +++ b/lib/private/DB/MigrationService.php @@ -376,13 +376,14 @@ class MigrationService { * Applies all not yet applied versions up to $to * * @param string $to + * @param bool $schemaOnly * @throws \InvalidArgumentException */ - public function migrate($to = 'latest') { + public function migrate($to = 'latest', $schemaOnly = false) { // read known migrations $toBeExecuted = $this->getMigrationsToExecute($to); foreach ($toBeExecuted as $version) { - $this->executeStep($version); + $this->executeStep($version, $schemaOnly); } } @@ -432,14 +433,17 @@ class MigrationService { * Executes one explicit version * * @param string $version + * @param bool $schemaOnly * @throws \InvalidArgumentException */ - public function executeStep($version) { + public function executeStep($version, $schemaOnly = false) { $instance = $this->createInstance($version); - $instance->preSchemaChange($this->output, function() { - return new SchemaWrapper($this->connection); - }, ['tablePrefix' => $this->connection->getPrefix()]); + if (!$schemaOnly) { + $instance->preSchemaChange($this->output, function() { + return new SchemaWrapper($this->connection); + }, ['tablePrefix' => $this->connection->getPrefix()]); + } $toSchema = $instance->changeSchema($this->output, function() { return new SchemaWrapper($this->connection); @@ -450,9 +454,11 @@ class MigrationService { $toSchema->performDropTableCalls(); } - $instance->postSchemaChange($this->output, function() { - return new SchemaWrapper($this->connection); - }, ['tablePrefix' => $this->connection->getPrefix()]); + if (!$schemaOnly) { + $instance->postSchemaChange($this->output, function() { + return new SchemaWrapper($this->connection); + }, ['tablePrefix' => $this->connection->getPrefix()]); + } $this->markAsExecuted($version); } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 76b523afd10..d0316b44c1a 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -827,10 +827,20 @@ class Manager implements IManager { $expirationDateUpdated = true; } } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL) { + // The new password is not set again if it is the same as the old + // one, unless when switching from sending by Talk to sending by + // mail. $plainTextPassword = $share->getPassword(); - if (!$this->updateSharePasswordIfNeeded($share, $originalShare)) { + if (!empty($plainTextPassword) && !$this->updateSharePasswordIfNeeded($share, $originalShare) && + !($originalShare->getSendPasswordByTalk() && !$share->getSendPasswordByTalk())) { $plainTextPassword = null; } + if (empty($plainTextPassword) && !$originalShare->getSendPasswordByTalk() && $share->getSendPasswordByTalk()) { + // If the same password was already sent by mail the recipient + // would already have access to the share without having to call + // the sharer to verify her identity + throw new \InvalidArgumentException('Can’t enable sending the password by Talk without setting a new password'); + } } $this->pathCreateChecks($share->getNode()); diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index e54497c9b55..71c0453d9e5 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -63,6 +63,8 @@ class Share implements \OCP\Share\IShare { private $expireDate; /** @var string */ private $password; + /** @var bool */ + private $sendPasswordByTalk = false; /** @var string */ private $token; /** @var int */ @@ -405,6 +407,21 @@ class Share implements \OCP\Share\IShare { /** * @inheritdoc */ + public function setSendPasswordByTalk(bool $sendPasswordByTalk) { + $this->sendPasswordByTalk = $sendPasswordByTalk; + return $this; + } + + /** + * @inheritdoc + */ + public function getSendPasswordByTalk(): bool { + return $this->sendPasswordByTalk; + } + + /** + * @inheritdoc + */ public function setToken($token) { $this->token = $token; return $this; diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php index 5303cde45a6..43543fdad47 100644 --- a/lib/public/Share/IShare.php +++ b/lib/public/Share/IShare.php @@ -312,6 +312,29 @@ interface IShare { */ public function getPassword(); + + /** + * Set if the recipient can start a conversation with the owner to get the + * password using Nextcloud Talk. + * + * @param bool $sendPasswordByTalk + * @return \OCP\Share\IShare The modified object + * @since 14.0.0 + */ + public function setSendPasswordByTalk(bool $sendPasswordByTalk); + + /** + * Get if the recipient can start a conversation with the owner to get the + * password using Nextcloud Talk. + * The returned value does not take into account other factors, like Talk + * being enabled for the owner of the share or not; it just cover whether + * the option is enabled for the share itself or not. + * + * @return bool + * @since 14.0.0 + */ + public function getSendPasswordByTalk(): bool; + /** * Set the public link token. * diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index 1e26d86a039..7ebd6ee8340 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -286,7 +286,52 @@ class LoginControllerTest extends TestCase { $this->assertEquals($expectedResponse, $this->loginController->showLoginForm('LdapUser', '', '')); } - public function testShowLoginFormForUserNamedNull() { + /** + * Asserts that a disabled user can't login and gets the expected response. + */ + public function testLoginForDisabledUser() { + /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ + $user = $this->createMock(IUser::class); + $user->method('getUID') + ->willReturn('uid'); + $user->method('isEnabled') + ->willReturn(false); + + $this->request + ->expects($this->once()) + ->method('passesCSRFCheck') + ->willReturn(true); + + $this->userSession + ->method('isLoggedIn') + ->willReturn(false); + + $loginName = 'iMDisabled'; + $password = 'secret'; + + $this->session + ->expects($this->once()) + ->method('set') + ->with('loginMessages', [ + [LoginController::LOGIN_MSG_USERDISABLED], [] + ]); + + $this->userManager + ->expects($this->once()) + ->method('get') + ->with($loginName) + ->willReturn($user); + + $expected = new RedirectResponse(''); + $expected->throttle(['user' => $loginName]); + + $response = $this->loginController->tryLogin( + $loginName, $password, null, false, 'Europe/Berlin', '1' + ); + $this->assertEquals($expected, $response); + } + + public function testShowLoginFormForUserNamed0() { $this->userSession ->expects($this->once()) ->method('isLoggedIn') @@ -297,8 +342,7 @@ class LoginControllerTest extends TestCase { ->with('lost_password_link') ->willReturn(false); $user = $this->createMock(IUser::class); - $user - ->expects($this->once()) + $user->expects($this->once()) ->method('canChangePassword') ->willReturn(false); $this->userManager diff --git a/tests/acceptance/features/bootstrap/LoginPageContext.php b/tests/acceptance/features/bootstrap/LoginPageContext.php index 1496e3030c2..df7944aa912 100644 --- a/tests/acceptance/features/bootstrap/LoginPageContext.php +++ b/tests/acceptance/features/bootstrap/LoginPageContext.php @@ -71,6 +71,14 @@ class LoginPageContext implements Context, ActorAwareInterface { } /** + * @return Locator + */ + public static function userDisabledMessage() { + return Locator::forThe()->xpath("//*[@class = 'warning userDisabledMsg' and normalize-space() = 'User disabled']")-> + describedAs('User disabled message on login page'); + } + + /** * @When I log in with user :user and password :password */ public function iLogInWithUserAndPassword($user, $password) { @@ -97,6 +105,14 @@ class LoginPageContext implements Context, ActorAwareInterface { } /** + * @Then I see that the disabled user message is shown + */ + public function iSeeThatTheDisabledUserMessageIsShown() { + PHPUnit_Framework_Assert::assertTrue( + $this->actor->find(self::userDisabledMessage(), 10)->isVisible()); + } + + /** * @BeforeScenario */ public function getOtherRequiredSiblingContexts(BeforeScenarioScope $scope) { diff --git a/tests/acceptance/features/login.feature b/tests/acceptance/features/login.feature index 3a31d3f88bd..44353d37c65 100644 --- a/tests/acceptance/features/login.feature +++ b/tests/acceptance/features/login.feature @@ -28,6 +28,12 @@ Feature: login Then I see that the current page is the Login page And I see that a wrong password message is shown + Scenario: try to log in as disabled user + Given I visit the Home page + When I log in with user disabledUser and password 123456acb + Then I see that the current page is the Login page + And I see that the disabled user message is shown + Scenario: log in with invalid user once fixed by admin Given I act as John And I can not log in with user unknownUser and password 123456acb diff --git a/tests/acceptance/installAndConfigureServer.sh b/tests/acceptance/installAndConfigureServer.sh index c61faeda238..98de72bf45e 100755 --- a/tests/acceptance/installAndConfigureServer.sh +++ b/tests/acceptance/installAndConfigureServer.sh @@ -35,6 +35,8 @@ fi php occ maintenance:install --admin-pass=admin OC_PASS=123456acb php occ user:add --password-from-env user0 +OC_PASS=123456acb php occ user:add --password-from-env disabledUser +php occ user:disable disabledUser if [ "$NEXTCLOUD_SERVER_DOMAIN" != "" ]; then # Default first trusted domain is "localhost"; replace it with given domain. diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index cc98223d4b5..7106e22b264 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -2716,6 +2716,442 @@ class ManagerTest extends \Test\TestCase { $manager->updateShare($share); } + public function testUpdateShareMailEnableSendPasswordByTalk() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'getShareById', + 'generalCreateChecks', + 'verifyPassword', + 'pathCreateChecks', + 'linkCreateChecks', + 'validateExpirationDate', + ]) + ->getMock(); + + $originalShare = $this->manager->newShare(); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setPassword(null) + ->setSendPasswordByTalk(false); + + $tomorrow = new \DateTime(); + $tomorrow->setTime(0,0,0); + $tomorrow->add(new \DateInterval('P1D')); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(100); + + $share = $this->manager->newShare(); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setToken('token') + ->setSharedBy('owner') + ->setShareOwner('owner') + ->setPassword('password') + ->setSendPasswordByTalk(true) + ->setExpirationDate($tomorrow) + ->setNode($file) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); + $manager->expects($this->once())->method('generalCreateChecks')->with($share); + $manager->expects($this->once())->method('verifyPassword')->with('password'); + $manager->expects($this->once())->method('pathCreateChecks')->with($file); + $manager->expects($this->never())->method('linkCreateChecks'); + $manager->expects($this->never())->method('validateExpirationDate'); + + $this->hasher->expects($this->once()) + ->method('hash') + ->with('password') + ->willReturn('hashed'); + + $this->defaultProvider->expects($this->once()) + ->method('update') + ->with($share, 'password') + ->willReturn($share); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); + $hookListner->expects($this->never())->method('post'); + + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post'); + $hookListner2->expects($this->once())->method('post')->with([ + 'itemType' => 'file', + 'itemSource' => 100, + 'uidOwner' => 'owner', + 'token' => 'token', + 'disabled' => false, + ]); + + $hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post'); + $hookListner3->expects($this->never())->method('post'); + + $manager->updateShare($share); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Can’t enable sending the password by Talk without setting a new password + */ + public function testUpdateShareMailEnableSendPasswordByTalkWithNoPassword() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'getShareById', + 'generalCreateChecks', + 'verifyPassword', + 'pathCreateChecks', + 'linkCreateChecks', + 'validateExpirationDate', + ]) + ->getMock(); + + $originalShare = $this->manager->newShare(); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setPassword(null) + ->setSendPasswordByTalk(false); + + $tomorrow = new \DateTime(); + $tomorrow->setTime(0,0,0); + $tomorrow->add(new \DateInterval('P1D')); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(100); + + $share = $this->manager->newShare(); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setToken('token') + ->setSharedBy('owner') + ->setShareOwner('owner') + ->setPassword(null) + ->setSendPasswordByTalk(true) + ->setExpirationDate($tomorrow) + ->setNode($file) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); + $manager->expects($this->once())->method('generalCreateChecks')->with($share); + $manager->expects($this->never())->method('verifyPassword'); + $manager->expects($this->never())->method('pathCreateChecks'); + $manager->expects($this->never())->method('linkCreateChecks'); + $manager->expects($this->never())->method('validateExpirationDate'); + + $this->hasher->expects($this->never()) + ->method('hash'); + + $this->defaultProvider->expects($this->never()) + ->method('update'); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); + $hookListner->expects($this->never())->method('post'); + + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post'); + $hookListner2->expects($this->never())->method('post'); + + $hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post'); + $hookListner3->expects($this->never())->method('post'); + + $manager->updateShare($share); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Can’t enable sending the password by Talk without setting a new password + */ + public function testUpdateShareMailEnableSendPasswordByTalkRemovingPassword() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'getShareById', + 'generalCreateChecks', + 'verifyPassword', + 'pathCreateChecks', + 'linkCreateChecks', + 'validateExpirationDate', + ]) + ->getMock(); + + $originalShare = $this->manager->newShare(); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setPassword('password') + ->setSendPasswordByTalk(false); + + $tomorrow = new \DateTime(); + $tomorrow->setTime(0,0,0); + $tomorrow->add(new \DateInterval('P1D')); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(100); + + $share = $this->manager->newShare(); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setToken('token') + ->setSharedBy('owner') + ->setShareOwner('owner') + ->setPassword(null) + ->setSendPasswordByTalk(true) + ->setExpirationDate($tomorrow) + ->setNode($file) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); + $manager->expects($this->once())->method('generalCreateChecks')->with($share); + $manager->expects($this->never())->method('verifyPassword'); + $manager->expects($this->never())->method('pathCreateChecks'); + $manager->expects($this->never())->method('linkCreateChecks'); + $manager->expects($this->never())->method('validateExpirationDate'); + + $this->hasher->expects($this->never()) + ->method('hash'); + + $this->defaultProvider->expects($this->never()) + ->method('update'); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); + $hookListner->expects($this->never())->method('post'); + + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post'); + $hookListner2->expects($this->never())->method('post'); + + $hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post'); + $hookListner3->expects($this->never())->method('post'); + + $manager->updateShare($share); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Can’t enable sending the password by Talk without setting a new password + */ + public function testUpdateShareMailEnableSendPasswordByTalkRemovingPasswordWithEmptyString() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'getShareById', + 'generalCreateChecks', + 'verifyPassword', + 'pathCreateChecks', + 'linkCreateChecks', + 'validateExpirationDate', + ]) + ->getMock(); + + $originalShare = $this->manager->newShare(); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setPassword('password') + ->setSendPasswordByTalk(false); + + $tomorrow = new \DateTime(); + $tomorrow->setTime(0,0,0); + $tomorrow->add(new \DateInterval('P1D')); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(100); + + $share = $this->manager->newShare(); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setToken('token') + ->setSharedBy('owner') + ->setShareOwner('owner') + ->setPassword('') + ->setSendPasswordByTalk(true) + ->setExpirationDate($tomorrow) + ->setNode($file) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); + $manager->expects($this->once())->method('generalCreateChecks')->with($share); + $manager->expects($this->never())->method('verifyPassword'); + $manager->expects($this->never())->method('pathCreateChecks'); + $manager->expects($this->never())->method('linkCreateChecks'); + $manager->expects($this->never())->method('validateExpirationDate'); + + $this->hasher->expects($this->never()) + ->method('hash'); + + $this->defaultProvider->expects($this->never()) + ->method('update'); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); + $hookListner->expects($this->never())->method('post'); + + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post'); + $hookListner2->expects($this->never())->method('post'); + + $hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post'); + $hookListner3->expects($this->never())->method('post'); + + $manager->updateShare($share); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Can’t enable sending the password by Talk without setting a new password + */ + public function testUpdateShareMailEnableSendPasswordByTalkWithPreviousPassword() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'getShareById', + 'generalCreateChecks', + 'verifyPassword', + 'pathCreateChecks', + 'linkCreateChecks', + 'validateExpirationDate', + ]) + ->getMock(); + + $originalShare = $this->manager->newShare(); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setPassword('password') + ->setSendPasswordByTalk(false); + + $tomorrow = new \DateTime(); + $tomorrow->setTime(0,0,0); + $tomorrow->add(new \DateInterval('P1D')); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(100); + + $share = $this->manager->newShare(); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setToken('token') + ->setSharedBy('owner') + ->setShareOwner('owner') + ->setPassword('password') + ->setSendPasswordByTalk(true) + ->setExpirationDate($tomorrow) + ->setNode($file) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); + $manager->expects($this->once())->method('generalCreateChecks')->with($share); + $manager->expects($this->never())->method('verifyPassword'); + $manager->expects($this->never())->method('pathCreateChecks'); + $manager->expects($this->never())->method('linkCreateChecks'); + $manager->expects($this->never())->method('validateExpirationDate'); + + $this->hasher->expects($this->never()) + ->method('hash'); + + $this->defaultProvider->expects($this->never()) + ->method('update'); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); + $hookListner->expects($this->never())->method('post'); + + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post'); + $hookListner2->expects($this->never())->method('post'); + + $hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post'); + $hookListner3->expects($this->never())->method('post'); + + $manager->updateShare($share); + } + + public function testUpdateShareMailDisableSendPasswordByTalkWithPreviousPassword() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'getShareById', + 'generalCreateChecks', + 'verifyPassword', + 'pathCreateChecks', + 'linkCreateChecks', + 'validateExpirationDate', + ]) + ->getMock(); + + $originalShare = $this->manager->newShare(); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setPassword('password') + ->setSendPasswordByTalk(true); + + $tomorrow = new \DateTime(); + $tomorrow->setTime(0,0,0); + $tomorrow->add(new \DateInterval('P1D')); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(100); + + $share = $this->manager->newShare(); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setToken('token') + ->setSharedBy('owner') + ->setShareOwner('owner') + ->setPassword('password') + ->setSendPasswordByTalk(false) + ->setExpirationDate($tomorrow) + ->setNode($file) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); + $manager->expects($this->once())->method('generalCreateChecks')->with($share); + $manager->expects($this->once())->method('pathCreateChecks')->with($file); + $manager->expects($this->never())->method('verifyPassword'); + $manager->expects($this->never())->method('linkCreateChecks'); + $manager->expects($this->never())->method('validateExpirationDate'); + + $this->hasher->expects($this->never()) + ->method('hash'); + + $this->defaultProvider->expects($this->once()) + ->method('update') + ->with($share, 'password') + ->willReturn($share); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); + $hookListner->expects($this->never())->method('post'); + + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post'); + $hookListner2->expects($this->never())->method('post'); + + $hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post'); + $hookListner3->expects($this->never())->method('post'); + + $manager->updateShare($share); + } + /** * @expectedException \InvalidArgumentException * @expectedExceptionMessage Can’t change target of link share diff --git a/version.php b/version.php index c18ca01286b..429bfec990f 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = array(14, 0, 0, 11); +$OC_Version = array(14, 0, 0, 12); // The human readable string $OC_VersionString = '14.0.0 alpha'; |