From 5e4a166365a601bf0133a97fda968a85744dfef5 Mon Sep 17 00:00:00 2001 From: Stefan Cherniakov Date: Thu, 22 Aug 2024 06:43:43 +0200 Subject: [PATCH] feat(files_sharing): reminder for link shares with expiration date Signed-off-by: Stefan Cherniakov --- apps/files_sharing/appinfo/info.xml | 3 +- .../composer/composer/autoload_classmap.php | 2 + .../composer/composer/autoload_static.php | 2 + .../Version31000Date20240821142813.php | 36 +++ apps/files_sharing/lib/SharesReminderJob.php | 261 ++++++++++++++++++ .../tests/SharesReminderJobTest.php | 196 +++++++++++++ lib/private/Share20/Share.php | 11 + lib/public/Share/IShare.php | 16 ++ 8 files changed, 526 insertions(+), 1 deletion(-) create mode 100644 apps/files_sharing/lib/Migration/Version31000Date20240821142813.php create mode 100644 apps/files_sharing/lib/SharesReminderJob.php create mode 100644 apps/files_sharing/tests/SharesReminderJobTest.php diff --git a/apps/files_sharing/appinfo/info.xml b/apps/files_sharing/appinfo/info.xml index 6979893d7e6..dcf6621b183 100644 --- a/apps/files_sharing/appinfo/info.xml +++ b/apps/files_sharing/appinfo/info.xml @@ -14,7 +14,7 @@ Turning the feature off removes shared files and folders on the server for all share recipients, and also on the sync clients and mobile apps. More information is available in the Nextcloud Documentation. - 1.23.0 + 1.23.1 agpl Michael Gapczynski Bjoern Schiessle @@ -33,6 +33,7 @@ Turning the feature off removes shared files and folders on the server for all s OCA\Files_Sharing\DeleteOrphanedSharesJob OCA\Files_Sharing\ExpireSharesJob + OCA\Files_Sharing\SharesReminderJob OCA\Files_Sharing\BackgroundJob\FederatedSharesDiscoverJob diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index e4aa9c7b16a..e3c94a7ac1a 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -76,6 +76,7 @@ return array( 'OCA\\Files_Sharing\\Migration\\Version22000Date20210216084241' => $baseDir . '/../lib/Migration/Version22000Date20210216084241.php', 'OCA\\Files_Sharing\\Migration\\Version24000Date20220208195521' => $baseDir . '/../lib/Migration/Version24000Date20220208195521.php', 'OCA\\Files_Sharing\\Migration\\Version24000Date20220404142216' => $baseDir . '/../lib/Migration/Version24000Date20220404142216.php', + 'OCA\\Files_Sharing\\Migration\\Version31000Date20240821142813' => $baseDir . '/../lib/Migration/Version31000Date20240821142813.php', 'OCA\\Files_Sharing\\MountProvider' => $baseDir . '/../lib/MountProvider.php', 'OCA\\Files_Sharing\\Notification\\Listener' => $baseDir . '/../lib/Notification/Listener.php', 'OCA\\Files_Sharing\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php', @@ -87,6 +88,7 @@ return array( 'OCA\\Files_Sharing\\ShareBackend\\Folder' => $baseDir . '/../lib/ShareBackend/Folder.php', 'OCA\\Files_Sharing\\SharedMount' => $baseDir . '/../lib/SharedMount.php', 'OCA\\Files_Sharing\\SharedStorage' => $baseDir . '/../lib/SharedStorage.php', + 'OCA\\Files_Sharing\\SharesReminderJob' => $baseDir . '/../lib/SharesReminderJob.php', 'OCA\\Files_Sharing\\Updater' => $baseDir . '/../lib/Updater.php', 'OCA\\Files_Sharing\\ViewOnly' => $baseDir . '/../lib/ViewOnly.php', ); diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php index 36a2622f56e..597e65d96a2 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -91,6 +91,7 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\Migration\\Version22000Date20210216084241' => __DIR__ . '/..' . '/../lib/Migration/Version22000Date20210216084241.php', 'OCA\\Files_Sharing\\Migration\\Version24000Date20220208195521' => __DIR__ . '/..' . '/../lib/Migration/Version24000Date20220208195521.php', 'OCA\\Files_Sharing\\Migration\\Version24000Date20220404142216' => __DIR__ . '/..' . '/../lib/Migration/Version24000Date20220404142216.php', + 'OCA\\Files_Sharing\\Migration\\Version31000Date20240821142813' => __DIR__ . '/..' . '/../lib/Migration/Version31000Date20240821142813.php', 'OCA\\Files_Sharing\\MountProvider' => __DIR__ . '/..' . '/../lib/MountProvider.php', 'OCA\\Files_Sharing\\Notification\\Listener' => __DIR__ . '/..' . '/../lib/Notification/Listener.php', 'OCA\\Files_Sharing\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php', @@ -102,6 +103,7 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\ShareBackend\\Folder' => __DIR__ . '/..' . '/../lib/ShareBackend/Folder.php', 'OCA\\Files_Sharing\\SharedMount' => __DIR__ . '/..' . '/../lib/SharedMount.php', 'OCA\\Files_Sharing\\SharedStorage' => __DIR__ . '/..' . '/../lib/SharedStorage.php', + 'OCA\\Files_Sharing\\SharesReminderJob' => __DIR__ . '/..' . '/../lib/SharesReminderJob.php', 'OCA\\Files_Sharing\\Updater' => __DIR__ . '/..' . '/../lib/Updater.php', 'OCA\\Files_Sharing\\ViewOnly' => __DIR__ . '/..' . '/../lib/ViewOnly.php', ); diff --git a/apps/files_sharing/lib/Migration/Version31000Date20240821142813.php b/apps/files_sharing/lib/Migration/Version31000Date20240821142813.php new file mode 100644 index 00000000000..b063902a380 --- /dev/null +++ b/apps/files_sharing/lib/Migration/Version31000Date20240821142813.php @@ -0,0 +1,36 @@ +getTable('share'); + $table->addColumn('reminder_sent', Types::BOOLEAN, [ + 'notnull' => false, + 'default' => false, + ]); + return $schema; + } + +} diff --git a/apps/files_sharing/lib/SharesReminderJob.php b/apps/files_sharing/lib/SharesReminderJob.php new file mode 100644 index 00000000000..214e555f173 --- /dev/null +++ b/apps/files_sharing/lib/SharesReminderJob.php @@ -0,0 +1,261 @@ +setInterval(3600); + } + + + /** + * Makes the background job do its work + * + * @param array $argument unused argument + * @throws Exception if a database error occurs + */ + public function run(mixed $argument): void { + $shares = $this->getShares(); + [$foldersByEmail, $langByEmail] = $this->prepareReminders($shares); + $this->sendReminders($foldersByEmail, $langByEmail); + } + + /** + * Finds all folder shares of type user or email with expiration dates within the specified timeframe. + * This method returns only those shares that have not yet received the reminder. + * + * @return array + * @throws Exception if a database error occurs + */ + private function getShares(): array { + $minDate = new \DateTime(); + $maxDate = new \DateTime(); + $maxDate->setTimestamp($maxDate->getTimestamp() + self::SECONDS_BEFORE_REMINDER); + + $qb = $this->db->getQueryBuilder(); + $qb->select('id', 'share_type') + ->from('share') + ->where( + $qb->expr()->andX( + $qb->expr()->orX( + $qb->expr()->eq('share_type', $qb->expr()->literal(IShare::TYPE_USER)), + $qb->expr()->eq('share_type', $qb->expr()->literal(IShare::TYPE_EMAIL)) + ), + $qb->expr()->eq('item_type', $qb->expr()->literal('folder')), + $qb->expr()->gte('expiration', $qb->createNamedParameter($minDate->format('Y-m-d H:i:s'))), + $qb->expr()->lt('expiration', $qb->createNamedParameter($maxDate->format('Y-m-d H:i:s'))), + $qb->expr()->eq('reminder_sent', $qb->createNamedParameter( + false, IQueryBuilder::PARAM_BOOL + )) + ) + ); + + $sharesResult = $qb->executeQuery(); + $shares = []; + while ($share = $sharesResult->fetch()) { + if ((int)$share['share_type'] === IShare::TYPE_EMAIL) { + $id = "ocMailShare:$share[id]"; + } else { + $id = "ocinternal:$share[id]"; + } + + try { + $shares[] = $this->shareManager->getShareById($id); + } catch (ShareNotFound) { + $this->logger->error("Share with ID $id not found."); + } + } + $sharesResult->closeCursor(); + return $shares; + } + + /** + * Checks if the user should be reminded about this share. + * If so, it will retrieve and return all the necessary data for this. + * It also updates the reminder sent flag for the affected shares (to avoid multiple reminders). + * + * @param array $shares Shares that were obtained with {@link getShares} + * @return array A tuple consisting of two dictionaries: folders and languages by email + * @throws Exception if the reminder sent flag could not be saved + */ + private function prepareReminders(array $shares): array { + // This dictionary stores email addresses as keys and folder lists as values. + // It is used to ensure that each user receives no more than one email notification. + // The email will include the names and links of the folders that the user should be reminded of. + $foldersByEmail = []; + // Similar to the previous one, this variable stores the language for each email (if provided) + $langByEmail = []; + + /** @var IShare $share */ + foreach ($shares as $share) { + if (!$this->shouldRemindOfThisShare($share)) { + continue; + } + + $sharedWith = $share->getSharedWith(); + if ($share->getShareType() == IShare::TYPE_USER) { + $user = $this->userManager->get($sharedWith); + $mailTo = $user->getEMailAddress(); + $lang = $this->l10nFactory->getUserLanguage($user); + $link = $this->urlGenerator->linkToRouteAbsolute('files.view.index', [ + 'dir' => $share->getTarget() + ]); + } else { + $mailTo = $sharedWith; + $lang = ''; + $link = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare', [ + 'token' => $share->getToken() + ]); + } + if (empty($mailTo)) { + continue; + } + + if (!empty($lang)) { + $langByEmail[$mailTo] ??= $lang; + } + if (!isset($foldersByEmail[$mailTo])) { + $foldersByEmail[$mailTo] = []; + } + $foldersByEmail[$mailTo][] = ['name' => $share->getNode()->getName(), 'link' => $link]; + + $share->setReminderSent(true); + $qb = $this->db->getQueryBuilder(); + $qb->update('share') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId()))) + ->set('reminder_sent', $qb->createNamedParameter($share->getReminderSent())) + ->execute(); + } + + return [$foldersByEmail, $langByEmail]; + } + + /** + * Checks if user has write permission and folder is empty + * + * @param IShare $share Share to check + * @return bool + */ + private function shouldRemindOfThisShare(IShare $share): bool { + try { + $folder = $share->getNode(); + $fileCount = count($folder->getDirectoryListing()); + } catch (NotFoundException) { + $id = $share->getFullId(); + $this->logger->debug("File by share ID $id not found."); + return false; + } + $permissions = $share->getPermissions(); + $hasCreatePermission = ($permissions & Constants::PERMISSION_CREATE) === Constants::PERMISSION_CREATE; + return ($fileCount == 0 && $hasCreatePermission); + } + + /** + * This method accepts data obtained by {@link prepareReminders} and sends reminder emails. + * + * @param array $foldersByEmail + * @param array $langByEmail + * @return void + */ + private function sendReminders(array $foldersByEmail, array $langByEmail): void { + $instanceName = $this->defaults->getName(); + $from = [Util::getDefaultEmailAddress($instanceName) => $instanceName]; + foreach ($foldersByEmail as $email => $folders) { + $l = $this->l10nFactory->get('files_sharing', $langByEmail[$email] ?? null); + $emailTemplate = $this->generateEMailTemplate($l, $folders); + $message = $this->mailer->createMessage(); + $message->setFrom($from); + $message->setTo([$email]); + $message->useTemplate($emailTemplate); + $errorText = "Sending email with share reminder to $email failed."; + try { + $failedRecipients = $this->mailer->send($message); + if (count($failedRecipients) > 0) { + $this->logger->error($errorText); + } + } catch (\Exception) { + $this->logger->error($errorText); + } + } + } + + /** + * Returns the reminder email template + * + * @param IL10N $l + * @param array $folders Folders the user should be reminded of + * @return IEMailTemplate + */ + private function generateEMailTemplate(IL10N $l, array $folders): IEMailTemplate { + $emailTemplate = $this->mailer->createEMailTemplate('files_sharing.SharesReminder', [ + 'folders' => $folders, + ]); + + $emailTemplate->addHeader(); + if (count($folders) == 1) { + $emailTemplate->setSubject( + $l->t('Remember to upload the files to %s', [$folders[0]['name']]) + ); + $emailTemplate->addBodyText($l->t( + 'We would like to kindly remind you that you have not yet uploaded any files to the shared folder.' + )); + } else { + $emailTemplate->setSubject( + $l->t('Remember to upload the files to shared folders') + ); + $emailTemplate->addBodyText($l->t( + 'We would like to kindly remind you that you have not yet uploaded any files to the shared folders.' + )); + } + + foreach ($folders as $folder) { + $emailTemplate->addBodyButton( + $l->t('Open "%s"', [$folder['name']]), + $folder['link'] + ); + } + $emailTemplate->addFooter(); + return $emailTemplate; + } +} diff --git a/apps/files_sharing/tests/SharesReminderJobTest.php b/apps/files_sharing/tests/SharesReminderJobTest.php new file mode 100644 index 00000000000..d35d88786f2 --- /dev/null +++ b/apps/files_sharing/tests/SharesReminderJobTest.php @@ -0,0 +1,196 @@ +db = \OC::$server->get(IDBConnection::class); + $this->shareManager = \OC::$server->get(IManager::class); + $this->userManager = \OC::$server->get(IUserManager::class); + $this->mailer = $this->createMock(IMailer::class); + + // Clear occasional leftover shares from other tests + $this->db->executeUpdate('DELETE FROM `*PREFIX*share`'); + + $this->user1 = $this->getUniqueID('user1_'); + $this->user2 = $this->getUniqueID('user2_'); + + $user1 = $this->userManager->createUser($this->user1, 'longrandompassword'); + $user2 = $this->userManager->createUser($this->user2, 'longrandompassword'); + $user1->setSystemEMailAddress('user1@test.com'); + $user2->setSystemEMailAddress('user2@test.com'); + + \OC::registerShareHooks(\OC::$server->get(SystemConfig::class)); + + $this->job = new SharesReminderJob( + \OC::$server->get(ITimeFactory::class), + $this->db, + \OC::$server->get(IManager::class), + $this->userManager, + \OC::$server->get(LoggerInterface::class), + \OC::$server->get(IURLGenerator::class), + \OC::$server->get(IFactory::class), + $this->mailer, + \OC::$server->get(Defaults::class), + ); + } + + protected function tearDown(): void { + $this->db->executeUpdate('DELETE FROM `*PREFIX*share`'); + + $userManager = \OC::$server->get(IUserManager::class); + $user1 = $userManager->get($this->user1); + if ($user1) { + $user1->delete(); + } + $user2 = $userManager->get($this->user2); + if ($user2) { + $user2->delete(); + } + + $this->logout(); + + parent::tearDown(); + } + + public function dataSharesReminder() { + $someMail = 'test@test.com'; + $noExpirationDate = null; + $today = new \DateTime(); + // For expiration dates, the time is always automatically set to zero by ShareAPIController + $today->setTime(0, 0); + $nearFuture = new \DateTime(); + $nearFuture->setTimestamp($today->getTimestamp() + 86400 * 1); + $farFuture = new \DateTime(); + $farFuture->setTimestamp($today->getTimestamp() + 86400 * 2); + $permissionRead = Constants::PERMISSION_READ; + $permissionCreate = $permissionRead | Constants::PERMISSION_CREATE; + $permissionUpdate = $permissionRead | Constants::PERMISSION_UPDATE; + $permissionDelete = $permissionRead | Constants::PERMISSION_DELETE; + $permissionAll = Constants::PERMISSION_ALL; + + return [ + // No reminders for folders without expiration date + [$noExpirationDate, '', false, $permissionRead, false], + [$noExpirationDate, '', false, $permissionCreate, false], + [$noExpirationDate, '', true, $permissionDelete, false], + [$noExpirationDate, '', true, $permissionCreate, false], + [$noExpirationDate, $someMail, false, $permissionUpdate, false], + [$noExpirationDate, $someMail, false, $permissionCreate, false], + [$noExpirationDate, $someMail, true, $permissionRead, false], + [$noExpirationDate, $someMail, true, $permissionAll, false], + // No reminders for folders with expiration date in the far future + [$farFuture, '', false, $permissionRead, false], + [$farFuture, '', false, $permissionCreate, false], + [$farFuture, '', true, $permissionDelete, false], + [$farFuture, '', true, $permissionCreate, false], + [$farFuture, $someMail, false, $permissionUpdate, false], + [$farFuture, $someMail, false, $permissionCreate, false], + [$farFuture, $someMail, true, $permissionRead, false], + [$farFuture, $someMail, true, $permissionAll, false], + /* Should send reminders for folders with expiration date in the near future + if the folder is empty and the user has write permission */ + [$nearFuture, '', false, $permissionRead, false], + [$nearFuture, '', false, $permissionCreate, false], + [$nearFuture, '', true, $permissionDelete, false], + [$nearFuture, '', true, $permissionCreate, true], + [$nearFuture, $someMail, false, $permissionUpdate, false], + [$nearFuture, $someMail, false, $permissionCreate, false], + [$nearFuture, $someMail, true, $permissionRead, false], + [$nearFuture, $someMail, true, $permissionAll, true], + ]; + } + + /** + * @dataProvider dataSharesReminder + * + * @param \DateTime|null $expirationDate Share expiration date + * @param string $email Share with this email. If empty, the share is of type TYPE_USER and the sharee is user2 + * @param bool $isEmpty Is share folder empty? + * @param int $permissions + * @param bool $shouldBeReminded + */ + public function testSharesReminder( + \DateTime|null $expirationDate, string $email, bool $isEmpty, int $permissions, bool $shouldBeReminded + ): void { + $this->loginAsUser($this->user1); + + $user1Folder = \OC::$server->get(IRootFolder::class)->getUserFolder($this->user1); + $testFolder = $user1Folder->newFolder('test'); + + if (!$isEmpty) { + $testFolder->newFile("some_file.txt"); + } + + $share = $this->shareManager->newShare(); + + $share->setNode($testFolder) + ->setShareType(($email ? IShare::TYPE_EMAIL : IShare::TYPE_USER)) + ->setPermissions($permissions) + ->setSharedBy($this->user1) + ->setSharedWith(($email ?: $this->user2)) + ->setExpirationDate($expirationDate); + $share = $this->shareManager->createShare($share); + + $this->logout(); + $messageMock = $this->createMock(IMessage::class); + $this->mailer->method('createMessage')->willReturn($messageMock); + $this->mailer + ->expects(($shouldBeReminded ? $this->once() : $this->never())) + ->method('send') + ->with($messageMock); + $messageMock + ->expects(($shouldBeReminded ? $this->once() : $this->never())) + ->method('setTo') + ->with([$email ?: $this->userManager->get($this->user2)->getSystemEMailAddress()]); + $this->assertSame(false, $share->getReminderSent()); + $this->job->run([]); + $qb = $this->db->getQueryBuilder(); + $reminderSent = $qb + ->select('reminder_sent') + ->from('share') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId()))) + ->executeQuery() + ->fetch()["reminder_sent"]; + $this->assertEquals($shouldBeReminded, $reminderSent); + } +} diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index b01d05ca4b9..92f801e3fc1 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -72,6 +72,8 @@ class Share implements IShare { private $nodeCacheEntry; /** @var bool */ private $hideDownload = false; + /** @var bool */ + private $reminderSent = false; private bool $noExpirationDate = false; @@ -613,4 +615,13 @@ class Share implements IShare { public function getHideDownload(): bool { return $this->hideDownload; } + + public function setReminderSent(bool $reminderSent): IShare { + $this->reminderSent = $reminderSent; + return $this; + } + + public function getReminderSent(): bool { + return $this->reminderSent; + } } diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php index fe5e7eb47dd..c2843c078e3 100644 --- a/lib/public/Share/IShare.php +++ b/lib/public/Share/IShare.php @@ -617,4 +617,20 @@ interface IShare { * @since 15.0.0 */ public function getHideDownload(): bool; + + /** + * Sets a flag that stores whether a reminder via email has been sent + * + * @return self The modified object + * @since 31.0.0 + */ + public function setReminderSent(bool $reminderSent): IShare; + + /** + * Gets a flag that stores whether a reminder via email has been sent + * + * @return bool + * @since 31.0.0 + */ + public function getReminderSent(): bool; } -- 2.39.5