diff options
author | Cyrille Bollu <cyrpub@bollu.be> | 2022-02-05 22:49:17 +0300 |
---|---|---|
committer | Cyrille Bollu <cyrpub@bollu.be> | 2022-04-11 22:58:24 +0300 |
commit | c6a5c07041d2e5d20771409aede8b755d28372ac (patch) | |
tree | 71051efd25c16bed5a419eb1670477f1f5471933 /apps/sharebymail | |
parent | 60f946aba5862102a81100b09e26b37b6d59a3fa (diff) |
Adds a "Request password" button to the public share authentication page for shares
of type TYPE_EMAIL, when the "video verification" checkbox isn't checked. Users accessing
non-anonymous public shares (TYPE_EMAIL shares) can now request a temporary password themselves.
- Creates a migration step for the files_sharing app to add the 'password_expiration_time'
attribute to the oc_shares table.
- Makes share temporary passwords' expiration time configurable via a system value.
- Adds a system config value to allow permanent share passwords
-Fixes a typo in a comment in apps/files_sharing/src/components/SharingEntryLink.vue
See https://github.com/nextcloud/server/issues/31005
Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
Diffstat (limited to 'apps/sharebymail')
-rw-r--r-- | apps/sharebymail/lib/ShareByMailProvider.php | 57 | ||||
-rw-r--r-- | apps/sharebymail/tests/ShareByMailProviderTest.php | 55 |
2 files changed, 88 insertions, 24 deletions
diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index c1438c01125..1e767b7f859 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -52,6 +52,7 @@ use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\HintException; +use OCP\IConfig; use OCP\IDBConnection; use OCP\IL10N; use OCP\ILogger; @@ -75,6 +76,8 @@ use OCP\Share\IShareProvider; */ class ShareByMailProvider implements IShareProvider { + private IConfig $config; + /** @var IDBConnection */ private $dbConnection; @@ -126,7 +129,8 @@ class ShareByMailProvider implements IShareProvider { return 'ocMailShare'; } - public function __construct(IDBConnection $connection, + public function __construct(IConfig $config, + IDBConnection $connection, ISecureRandom $secureRandom, IUserManager $userManager, IRootFolder $rootFolder, @@ -140,6 +144,7 @@ class ShareByMailProvider implements IShareProvider { IHasher $hasher, IEventDispatcher $eventDispatcher, IShareManager $shareManager) { + $this->config = $config; $this->dbConnection = $connection; $this->secureRandom = $secureRandom; $this->userManager = $userManager; @@ -190,9 +195,14 @@ class ShareByMailProvider implements IShareProvider { } $shareId = $this->createMailShare($share); - $send = $this->sendPassword($share, $password); - if ($passwordEnforced && $send === false) { - $this->sendPasswordToOwner($share, $password); + + // Sends share password to receiver when it's a permanent one (otherwise she will have to request it via the showShare UI) + // or to owner when the password shall be given during a Talk session + if ($this->config->getSystemValue('allow_mail_share_permanent_password') || $share->getSendPasswordByTalk()) { + $send = $this->sendPassword($share, $password); + if ($passwordEnforced && $send === false) { + $this->sendPasswordToOwner($share, $password); + } } $this->createShareActivity($share); @@ -327,6 +337,7 @@ class ShareByMailProvider implements IShareProvider { $share->getPermissions(), $share->getToken(), $share->getPassword(), + $share->getPasswordExpirationTime(), $share->getSendPasswordByTalk(), $share->getHideDownload(), $share->getLabel(), @@ -673,23 +684,24 @@ class ShareByMailProvider implements IShareProvider { } /** - * add share to the database and return the ID - * - * @param int $itemSource - * @param string $itemType - * @param string $shareWith - * @param string $sharedBy - * @param string $uidOwner - * @param int $permissions - * @param string $token - * @param string $password - * @param bool $sendPasswordByTalk - * @param bool $hideDownload - * @param string $label - * @param \DateTime|null $expirationTime - * @return int + * Add share to the database and return the ID */ - protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password, $sendPasswordByTalk, $hideDownload, $label, $expirationTime, $note = ''): int { + protected function addShareToDB( + ?int $itemSource, + ?string $itemType, + ?string $shareWith, + ?string $sharedBy, + ?string $uidOwner, + ?int $permissions, + ?string $token, + ?string $password, + ?\DateTimeInterface $passwordExpirationTime, + ?bool $sendPasswordByTalk, + ?bool $hideDownload, + ?string $label, + ?\DateTimeInterface $expirationTime, + ?string $note = '' + ): int { $qb = $this->dbConnection->getQueryBuilder(); $qb->insert('share') ->setValue('share_type', $qb->createNamedParameter(IShare::TYPE_EMAIL)) @@ -702,6 +714,7 @@ class ShareByMailProvider implements IShareProvider { ->setValue('permissions', $qb->createNamedParameter($permissions)) ->setValue('token', $qb->createNamedParameter($token)) ->setValue('password', $qb->createNamedParameter($password)) + ->setValue('password_expiration_time', $qb->createNamedParameter($passwordExpirationTime, IQueryBuilder::PARAM_DATE)) ->setValue('password_by_talk', $qb->createNamedParameter($sendPasswordByTalk, IQueryBuilder::PARAM_BOOL)) ->setValue('stime', $qb->createNamedParameter(time())) ->setValue('hide_download', $qb->createNamedParameter((int)$hideDownload, IQueryBuilder::PARAM_INT)) @@ -739,6 +752,7 @@ class ShareByMailProvider implements IShareProvider { ($originalShare->getSendPasswordByTalk() && !$share->getSendPasswordByTalk()))) { $this->sendPassword($share, $plainTextPassword); } + /* * We allow updating the permissions and password of mail shares */ @@ -749,6 +763,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_expiration_time', $qb->createNamedParameter($share->getPasswordExpirationTime(), IQueryBuilder::PARAM_DATE)) ->set('label', $qb->createNamedParameter($share->getLabel())) ->set('password_by_talk', $qb->createNamedParameter($share->getSendPasswordByTalk(), IQueryBuilder::PARAM_BOOL)) ->set('expiration', $qb->createNamedParameter($share->getExpirationDate(), IQueryBuilder::PARAM_DATE)) @@ -1012,6 +1027,8 @@ class ShareByMailProvider implements IShareProvider { $share->setShareTime($shareTime); $share->setSharedWith($data['share_with']); $share->setPassword($data['password']); + $passwordExpirationTime = \DateTime::createFromFormat('Y-m-d H:i:s', $data['password_expiration_time']); + $share->setPasswordExpirationTime($passwordExpirationTime !== false? $passwordExpirationTime : null); $share->setLabel($data['label']); $share->setSendPasswordByTalk((bool)$data['password_by_talk']); $share->setHideDownload((bool)$data['hide_download']); diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index af2d5bad57c..b00ef13630f 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -39,6 +39,7 @@ use OCP\Defaults; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\File; use OCP\Files\IRootFolder; +use OCP\IConfig; use OCP\IDBConnection; use OCP\IL10N; use OCP\ILogger; @@ -63,6 +64,9 @@ use Test\TestCase; */ class ShareByMailProviderTest extends TestCase { + /** @var IConfig */ + private $config; + /** @var IDBConnection */ private $connection; @@ -111,6 +115,7 @@ class ShareByMailProviderTest extends TestCase { protected function setUp(): void { parent::setUp(); + $this->config = $this->getMockBuilder(IConfig::class)->getMock(); $this->connection = \OC::$server->getDatabaseConnection(); $this->l = $this->getMockBuilder(IL10N::class)->getMock(); @@ -145,6 +150,7 @@ class ShareByMailProviderTest extends TestCase { $instance = $this->getMockBuilder('OCA\ShareByMail\ShareByMailProvider') ->setConstructorArgs( [ + $this->config, $this->connection, $this->secureRandom, $this->userManager, @@ -168,6 +174,7 @@ class ShareByMailProviderTest extends TestCase { } return new ShareByMailProvider( + $this->config, $this->connection, $this->secureRandom, $this->userManager, @@ -267,9 +274,43 @@ class ShareByMailProviderTest extends TestCase { $this->hasher->expects($this->once())->method('hash')->with('password')->willReturn('passwordHashed'); $share->expects($this->once())->method('setPassword')->with('passwordHashed'); + // The given password (but not the autogenerated password) should not be + // mailed to the receiver of the share because permanent passwords are not enforced. + $this->shareManager->expects($this->any())->method('shareApiLinkEnforcePassword')->willReturn(false); + $this->config->expects($this->once())->method('getSystemValue')->with('allow_mail_share_permanent_password')->willReturn(false); + $instance->expects($this->never())->method('autoGeneratePassword'); + + $this->assertSame('shareObject', + $instance->create($share) + ); + } + + public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswordProtectionWithPermanentPassword() { + $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); + + $share->expects($this->once())->method('getPassword')->willReturn('password'); + $this->hasher->expects($this->once())->method('hash')->with('password')->willReturn('passwordHashed'); + $share->expects($this->once())->method('setPassword')->with('passwordHashed'); + // The given password (but not the autogenerated password) should be - // mailed to the receiver of the share. + // mailed to the receiver of the share because permanent passwords are enforced. $this->shareManager->expects($this->any())->method('shareApiLinkEnforcePassword')->willReturn(false); + $this->config->expects($this->once())->method('getSystemValue')->with('allow_mail_share_permanent_password')->willReturn(true); $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); $instance->expects($this->never())->method('autoGeneratePassword'); @@ -290,7 +331,7 @@ class ShareByMailProviderTest extends TestCase { ); } - public function testCreateSendPasswordByMailWithEnforcedPasswordProtection() { + public function testCreateSendPasswordByMailWithEnforcedPasswordProtectionWithPermanentPassword() { $share = $this->getMockBuilder(IShare::class)->getMock(); $share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); $share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false); @@ -320,8 +361,9 @@ class ShareByMailProviderTest extends TestCase { $this->hasher->expects($this->once())->method('hash')->with('autogeneratedPassword')->willReturn('autogeneratedPasswordHashed'); $share->expects($this->once())->method('setPassword')->with('autogeneratedPasswordHashed'); - // The autogenerated password should be mailed to the receiver of the share. + // The autogenerated password should be mailed to the receiver of the share because permanent passwords are enforced. $this->shareManager->expects($this->any())->method('shareApiLinkEnforcePassword')->willReturn(true); + $this->config->expects($this->once())->method('getSystemValue')->with('allow_mail_share_permanent_password')->willReturn(true); $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); $message = $this->createMock(IMessage::class); @@ -341,7 +383,7 @@ class ShareByMailProviderTest extends TestCase { ); } - public function testCreateSendPasswordByMailWithPasswordAndWithEnforcedPasswordProtection() { + public function testCreateSendPasswordByMailWithPasswordAndWithEnforcedPasswordProtectionWithPermanentPassword() { $share = $this->getMockBuilder(IShare::class)->getMock(); $share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); $share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false); @@ -366,6 +408,7 @@ class ShareByMailProviderTest extends TestCase { // The given password (but not the autogenerated password) should be // mailed to the receiver of the share. $this->shareManager->expects($this->any())->method('shareApiLinkEnforcePassword')->willReturn(true); + $this->config->expects($this->once())->method('getSystemValue')->with('allow_mail_share_permanent_password')->willReturn(true); $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); $instance->expects($this->never())->method('autoGeneratePassword'); @@ -410,6 +453,7 @@ class ShareByMailProviderTest extends TestCase { // The autogenerated password should be mailed to the owner of the share. $this->shareManager->expects($this->any())->method('shareApiLinkEnforcePassword')->willReturn(true); + $this->config->expects($this->once())->method('getSystemValue')->with('allow_mail_share_permanent_password')->willReturn(false); $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); $instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('autogeneratedPassword'); @@ -531,6 +575,7 @@ class ShareByMailProviderTest extends TestCase { $hideDownload = true; $label = 'label'; $expiration = new \DateTime(); + $passwordExpirationTime = new \DateTime(); $instance = $this->getInstance(); @@ -546,6 +591,7 @@ class ShareByMailProviderTest extends TestCase { $permissions, $token, $password, + $passwordExpirationTime, $sendPasswordByTalk, $hideDownload, $label, @@ -572,6 +618,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($passwordExpirationTime->getTimestamp(), \DateTime::createFromFormat('Y-m-d H:i:s', $result[0]['password_expiration_time'])->getTimestamp()); $this->assertSame($sendPasswordByTalk, (bool)$result[0]['password_by_talk']); $this->assertSame($hideDownload, (bool)$result[0]['hide_download']); $this->assertSame($label, $result[0]['label']); |