From db2359187838461b2867639c82289be9bcd2f744 Mon Sep 17 00:00:00 2001 From: Divyam Date: Tue, 22 Sep 2026 12:20:39 +0530 Subject: [PATCH 1/5] feat(sharebymail): allow user-level sender addresses via Mail Provider API Resolves #56904 This commit updates the ShareByMailProvider to support sending share emails using the user's personal email address via the Mail Provider API, mirroring the functionality available for Calendar invitations (IMipPlugin). - Added an admin setting ('useUserEmail') to enable/disable user-level senders. - Modified ShareByMailProvider to try to find an appropriate Mail Provider service. - If a Mail Provider is available and user-level sender is enabled, emails are sent from the sharing user's address. - If no provider is available, or an error occurs during sending, it cleanly falls back to the existing system mailer logic. - Updated relevant UI components and unit tests. Signed-off-by: Divyam --- apps/sharebymail/lib/Settings/Admin.php | 2 + .../lib/Settings/SettingsManager.php | 18 ++ apps/sharebymail/lib/ShareByMailProvider.php | 214 +++++++++++++++++- .../src/components/AdminSettings.vue | 21 ++ .../tests/ShareByMailProviderTest.php | 12 + tests/lib/Share20/ShareByMailProviderTest.php | 14 ++ 6 files changed, 273 insertions(+), 8 deletions(-) diff --git a/apps/sharebymail/lib/Settings/Admin.php b/apps/sharebymail/lib/Settings/Admin.php index 74242c9e35641..fef33723e2542 100644 --- a/apps/sharebymail/lib/Settings/Admin.php +++ b/apps/sharebymail/lib/Settings/Admin.php @@ -30,6 +30,7 @@ public function __construct( public function getForm() { $this->initialState->provideInitialState('sendPasswordMail', $this->settingsManager->sendPasswordByMail()); $this->initialState->provideInitialState('replyToInitiator', $this->settingsManager->replyToInitiator()); + $this->initialState->provideInitialState('useUserEmail', $this->settingsManager->useUserEmail()); Util::addStyle('sharebymail', 'admin-settings'); Util::addScript('sharebymail', 'admin-settings'); @@ -67,6 +68,7 @@ public function getAuthorizedAppConfig(): array { 'sharebymail' => [ 'sendpasswordmail', 'replyToInitiator', + 'useUserEmail', ], ]; } diff --git a/apps/sharebymail/lib/Settings/SettingsManager.php b/apps/sharebymail/lib/Settings/SettingsManager.php index 00480380d2b8c..04127d47512b9 100644 --- a/apps/sharebymail/lib/Settings/SettingsManager.php +++ b/apps/sharebymail/lib/Settings/SettingsManager.php @@ -17,6 +17,8 @@ class SettingsManager { private $replyToInitiatorDefault = 'yes'; + private $useUserEmailDefault = 'no'; + public function __construct( private IConfig $config, ) { @@ -41,4 +43,20 @@ public function replyToInitiator(): bool { $replyToInitiator = $this->config->getAppValue('sharebymail', 'replyToInitiator', $this->replyToInitiatorDefault); return $replyToInitiator === 'yes'; } + + /** + * Should share-by-mail emails be sent using the user's personal email + * address (via Mail Provider) instead of the global system email address. + * + * When enabled and a Mail Provider is available for the user, share + * notification emails will be sent from the user's own address, similar + * to how calendar invitations work. + * + * @return bool + * @psalm-suppress DeprecatedMethod + */ + public function useUserEmail(): bool { + $useUserEmail = $this->config->getAppValue('sharebymail', 'useUserEmail', $this->useUserEmailDefault); + return $useUserEmail === 'yes'; + } } diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index b96a61288517f..4f251bb06cfb4 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -19,6 +19,7 @@ use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\HintException; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IDBConnection; use OCP\IL10N; @@ -27,6 +28,9 @@ use OCP\IUserManager; use OCP\Mail\IEmailValidator; use OCP\Mail\IMailer; +use OCP\Mail\Provider\Address; +use OCP\Mail\Provider\IManager as IMailManager; +use OCP\Mail\Provider\IMessageSend; use OCP\Security\Events\GenerateSecurePasswordEvent; use OCP\Security\IHasher; use OCP\Security\ISecureRandom; @@ -74,6 +78,8 @@ public function __construct( private IEventDispatcher $eventDispatcher, private IShareManager $shareManager, private IEmailValidator $emailValidator, + private IMailManager $mailManager, + private IAppConfig $appConfig, ) { } @@ -328,6 +334,79 @@ private function trySendPasswordToOwner(IShare $share): void { 'exception' => $e, ]); } + + } + + /** + * Try to find a Mail Provider service for the given user that can send mail. + * + * This follows the same pattern as IMipPlugin for calendar invitations: + * if mail providers are enabled globally and the admin has enabled + * user-level sending for share emails, look up the user's mail service. + * + * @param string $userId The user ID of the share initiator + * @return IMessageSend|null A mail service that can send, or null to fall back to the system mailer + */ + protected function findMailService(string $userId): ?IMessageSend { + if (!$this->settingsManager->useUserEmail()) { + return null; + } + + if (!$this->appConfig->getValueBool('core', 'mail_providers_enabled', true)) { + return null; + } + + $user = $this->userManager->get($userId); + if ($user === null) { + return null; + } + + $userEmail = $user->getEMailAddress(); + if ($userEmail === null) { + return null; + } + + $mailService = $this->mailManager->findServiceByAddress($userId, $userEmail); + if ($mailService instanceof IMessageSend) { + return $mailService; + } + + return null; + } + + /** + * Send the share notification email via Mail Provider if available, + * otherwise fall back to the system mailer. + * + * @param IMessageSend $mailService The mail provider service + * @param string $senderEmail The sender's email address + * @param string $senderName The sender's display name + * @param array $recipientEmails The recipient email addresses + * @param \OCP\Mail\IEMailTemplate $emailTemplate The email template + */ + protected function sendViaMailProvider( + IMessageSend $mailService, + string $senderEmail, + string $senderName, + array $recipientEmails, + \OCP\Mail\IEMailTemplate $emailTemplate, + ): void { + /** @psalm-suppress UndefinedInterfaceMethod */ + $message = $mailService->initiateMessage(); + $message->setFrom(new Address($senderEmail, $senderName)); + + $recipients = array_map(fn (string $email) => new Address($email), $recipientEmails); + if (count($recipients) > 1) { + $message->setBcc(...$recipients); + } else { + $message->setTo(...$recipients); + } + + $message->setSubject($emailTemplate->renderSubject()); + $message->setBodyPlain($emailTemplate->renderText()); + $message->setBodyHtml($emailTemplate->renderHtml()); + + $mailService->sendMessage($message); } /** @@ -347,7 +426,6 @@ protected function sendEmail(IShare $share, array $emails): void { $initiatorUser = $this->userManager->get($initiator); $initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator; - $message = $this->mailer->createMessage(); $emailTemplate = $this->mailer->createEMailTemplate('sharebymail.RecipientNotification', [ 'filename' => $filename, @@ -385,6 +463,62 @@ protected function sendEmail(IShare $share, array $emails): void { $link ); + $instanceName = $this->defaults->getName(); + + // Try to send via the user's Mail Provider + $mailService = $this->findMailService($initiator); + if ($mailService !== null && $initiatorUser instanceof IUser) { + $initiatorEmail = $initiatorUser->getEMailAddress(); + if ($initiatorEmail !== null) { + $emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : '')); + try { + $this->sendViaMailProvider($mailService, $initiatorEmail, $initiatorDisplayName, $emails, $emailTemplate); + return; + } catch (\Exception $e) { + $this->logger->warning('Failed to send share email via Mail Provider, falling back to system mailer.', [ + 'app' => 'sharebymail', + 'exception' => $e, + ]); + // Fall through to system mailer + // Re-create template since footer was already added + $emailTemplate = $this->mailer->createEMailTemplate('sharebymail.RecipientNotification', [ + 'filename' => $filename, + 'link' => $link, + 'initiator' => $initiatorDisplayName, + 'expiration' => $expiration, + 'shareWith' => $shareWith, + 'note' => $note + ]); + $emailTemplate->setSubject($this->l->t('%1$s shared %2$s with you', [$initiatorDisplayName, $filename])); + $emailTemplate->addHeader(); + $emailTemplate->addHeading($this->l->t('%1$s shared %2$s with you', [$initiatorDisplayName, $filename]), false); + if ($note !== '') { + $emailTemplate->addBodyListItem( + htmlspecialchars($note), + $this->l->t('Note:'), + $this->getAbsoluteImagePath('caldav/description.png'), + $note + ); + } + if ($expiration !== null) { + $dateString = (string)$this->l->l('date', $expiration, ['width' => 'medium']); + $emailTemplate->addBodyListItem( + $this->l->t('This share is valid until %s at midnight', [$dateString]), + $this->l->t('Expiration:'), + $this->getAbsoluteImagePath('caldav/time.png'), + ); + } + $emailTemplate->addBodyButton( + $this->l->t('Open shared item'), + $link + ); + } + } + } + + // Fall back to the system mailer + $message = $this->mailer->createMessage(); + // If multiple recipients are given, we send the mail to all of them if (count($emails) > 1) { // We do not want to expose the email addresses of the other recipients @@ -394,7 +528,6 @@ protected function sendEmail(IShare $share, array $emails): void { } // The "From" contains the sharers name - $instanceName = $this->defaults->getName(); $senderName = $instanceName; if ($this->settingsManager->replyToInitiator()) { $senderName = $this->l->t( @@ -457,8 +590,6 @@ protected function sendPassword(IShare $share, string $password, array $emails): $plainBodyPart = $this->l->t('%1$s shared %2$s with you. You should have already received a separate mail with a link to access it.', [$initiatorDisplayName, $filename]); $htmlBodyPart = $this->l->t('%1$s shared %2$s with you. You should have already received a separate mail with a link to access it.', [$initiatorDisplayName, $filename]); - $message = $this->mailer->createMessage(); - $emailTemplate = $this->mailer->createEMailTemplate('sharebymail.RecipientPasswordNotification', [ 'filename' => $filename, 'password' => $password, @@ -481,6 +612,47 @@ protected function sendPassword(IShare $share, string $password, array $emails): $emailTemplate->addBodyText($this->l->t('This password will expire at %s', [$expirationTime->format('r')])); } + $instanceName = $this->defaults->getName(); + + // Try to send via the user's Mail Provider + $mailService = $this->findMailService($initiator); + if ($mailService !== null && $initiatorEmailAddress !== null) { + $emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : '')); + try { + $this->sendViaMailProvider($mailService, $initiatorEmailAddress, $initiatorDisplayName, $emails, $emailTemplate); + $this->createPasswordSendActivity($share, $shareWith, false); + return true; + } catch (\Exception $e) { + $this->logger->warning('Failed to send share password email via Mail Provider, falling back to system mailer.', [ + 'app' => 'sharebymail', + 'exception' => $e, + ]); + // Re-create template for fallback + $emailTemplate = $this->mailer->createEMailTemplate('sharebymail.RecipientPasswordNotification', [ + 'filename' => $filename, + 'password' => $password, + 'initiator' => $initiatorDisplayName, + 'initiatorEmail' => $initiatorEmailAddress, + 'shareWith' => $shareWith, + ]); + $emailTemplate->setSubject($this->l->t('Password to access %1$s shared to you by %2$s', [$filename, $initiatorDisplayName])); + $emailTemplate->addHeader(); + $emailTemplate->addHeading($this->l->t('Password to access %s', [$filename]), false); + $emailTemplate->addBodyText(htmlspecialchars($htmlBodyPart), $plainBodyPart); + $emailTemplate->addBodyText($this->l->t('It is protected with the following password:')); + $emailTemplate->addBodyText($password); + if ($this->config->getSystemValue('sharing.enable_mail_link_password_expiration', false) === true) { + $expirationTime = new \DateTime(); + $expirationInterval = $this->config->getSystemValue('sharing.mail_link_password_expiration_interval', 3600); + $expirationTime = $expirationTime->add(new \DateInterval('PT' . $expirationInterval . 'S')); + $emailTemplate->addBodyText($this->l->t('This password will expire at %s', [$expirationTime->format('r')])); + } + } + } + + // Fall back to the system mailer + $message = $this->mailer->createMessage(); + // If multiple recipients are given, we send the mail to all of them if (count($emails) > 1) { // We do not want to expose the email addresses of the other recipients @@ -490,7 +662,6 @@ protected function sendPassword(IShare $share, string $password, array $emails): } // The "From" contains the sharers name - $instanceName = $this->defaults->getName(); $senderName = $instanceName; if ($this->settingsManager->replyToInitiator()) { $senderName = $this->l->t( @@ -542,8 +713,6 @@ protected function sendNote(IShare $share): void { $plainHeading = $this->l->t('%1$s shared %2$s with you and wants to add:', [$initiatorDisplayName, $filename]); $htmlHeading = $this->l->t('%1$s shared %2$s with you and wants to add', [$initiatorDisplayName, $filename]); - $message = $this->mailer->createMessage(); - $emailTemplate = $this->mailer->createEMailTemplate('shareByMail.sendNote'); $emailTemplate->setSubject($this->l->t('%s added a note to a file shared with you', [$initiatorDisplayName])); @@ -558,8 +727,37 @@ protected function sendNote(IShare $share): void { $link ); - // The "From" contains the sharers name $instanceName = $this->defaults->getName(); + + // Try to send via the user's Mail Provider + $mailService = $this->findMailService($initiator); + if ($mailService !== null && $initiatorEmailAddress !== null) { + $emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : '')); + try { + $this->sendViaMailProvider($mailService, $initiatorEmailAddress, $initiatorDisplayName, [$recipient], $emailTemplate); + return; + } catch (\Exception $e) { + $this->logger->warning('Failed to send share note email via Mail Provider, falling back to system mailer.', [ + 'app' => 'sharebymail', + 'exception' => $e, + ]); + // Re-create template for fallback + $emailTemplate = $this->mailer->createEMailTemplate('shareByMail.sendNote'); + $emailTemplate->setSubject($this->l->t('%s added a note to a file shared with you', [$initiatorDisplayName])); + $emailTemplate->addHeader(); + $emailTemplate->addHeading(htmlspecialchars($htmlHeading), $plainHeading); + $emailTemplate->addBodyText(htmlspecialchars($note), $note); + $emailTemplate->addBodyButton( + $this->l->t('Open shared item'), + $link + ); + } + } + + // Fall back to the system mailer + $message = $this->mailer->createMessage(); + + // The "From" contains the sharers name $senderName = $instanceName; if ($this->settingsManager->replyToInitiator()) { $senderName = $this->l->t( diff --git a/apps/sharebymail/src/components/AdminSettings.vue b/apps/sharebymail/src/components/AdminSettings.vue index 24832d9fc76f1..aa0a6e227c3f3 100644 --- a/apps/sharebymail/src/components/AdminSettings.vue +++ b/apps/sharebymail/src/components/AdminSettings.vue @@ -14,6 +14,13 @@ {{ t('sharebymail', 'Reply to initiator') }} + + + {{ t('sharebymail', 'Send share emails from user\'s email address') }} + +

+ {{ t('sharebymail', 'When enabled, share notification emails will be sent from the user\'s personal email address via their Mail Provider (e.g. Nextcloud Mail), similar to calendar invitations. Falls back to the system email if no Mail Provider is available.') }} +

@@ -43,6 +50,7 @@ export default { return { sendPasswordMail: loadState('sharebymail', 'sendPasswordMail'), replyToInitiator: loadState('sharebymail', 'replyToInitiator'), + useUserEmail: loadState('sharebymail', 'useUserEmail'), } }, @@ -54,6 +62,10 @@ export default { replyToInitiator(newValue) { this.update('replyToInitiator', newValue) }, + + useUserEmail(newValue) { + this.update('useUserEmail', newValue) + }, }, methods: { @@ -88,3 +100,12 @@ export default { }, } + + diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index c87748f6657ce..0ae210940f355 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -20,6 +20,7 @@ use OCP\Files\File; use OCP\Files\IRootFolder; use OCP\Files\Node; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IDBConnection; use OCP\IL10N; @@ -29,6 +30,7 @@ use OCP\Mail\IEMailTemplate; use OCP\Mail\IMailer; use OCP\Mail\IMessage; +use OCP\Mail\Provider\IManager as IMailManager; use OCP\Security\Events\GenerateSecurePasswordEvent; use OCP\Security\IHasher; use OCP\Security\ISecureRandom; @@ -70,6 +72,8 @@ class ShareByMailProviderTest extends TestCase { private SettingsManager&MockObject $settingsManager; private IActivityManager&MockObject $activityManager; private IEventDispatcher&MockObject $eventDispatcher; + private IMailManager&MockObject $mailManager; + private IAppConfig&MockObject $appConfig; protected function setUp(): void { parent::setUp(); @@ -91,10 +95,14 @@ protected function setUp(): void { $this->share = $this->createMock(IShare::class); $this->activityManager = $this->createMock('OCP\Activity\IManager'); $this->settingsManager = $this->createMock(SettingsManager::class); + $this->settingsManager->expects($this->any())->method('useUserEmail')->willReturn(true); $this->defaults = $this->createMock(Defaults::class); $this->hasher = $this->createMock(IHasher::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->shareManager = $this->createMock(IManager::class); + $this->mailManager = $this->createMock(IMailManager::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->appConfig->expects($this->any())->method('getValueBool')->willReturn(true); $this->userManager->expects($this->any())->method('userExists')->willReturn(true); $this->config->expects($this->any())->method('getAppValue')->with('core', 'enforce_strict_email_check')->willReturn('yes'); @@ -126,6 +134,8 @@ private function getInstance(array $mockedMethods = []) { $this->eventDispatcher, $this->shareManager, $this->getEmailValidatorWithStrictEmailCheck(), + $this->mailManager, + $this->appConfig, ]) ->onlyMethods($mockedMethods) ->getMock(); @@ -148,6 +158,8 @@ private function getInstance(array $mockedMethods = []) { $this->eventDispatcher, $this->shareManager, $this->getEmailValidatorWithStrictEmailCheck(), + $this->mailManager, + $this->appConfig, ); } diff --git a/tests/lib/Share20/ShareByMailProviderTest.php b/tests/lib/Share20/ShareByMailProviderTest.php index 50ed5a53445ee..b14ccd86bce44 100644 --- a/tests/lib/Share20/ShareByMailProviderTest.php +++ b/tests/lib/Share20/ShareByMailProviderTest.php @@ -16,12 +16,14 @@ use OCP\Defaults; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\IRootFolder; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IDBConnection; use OCP\IL10N; use OCP\IURLGenerator; use OCP\IUserManager; use OCP\Mail\IMailer; +use OCP\Mail\Provider\IManager as IMailManager; use OCP\Security\IHasher; use OCP\Security\ISecureRandom; use OCP\Server; @@ -88,6 +90,12 @@ class ShareByMailProviderTest extends TestCase { /** @var SettingsManager|MockObject */ private $settingsManager; + /** @var IMailManager|MockObject */ + private $mailManager; + + /** @var IAppConfig|MockObject */ + private $appConfig; + #[\Override] protected function setUp(): void { parent::setUp(); @@ -102,11 +110,15 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->activityManager = $this->createMock(\OCP\Activity\IManager::class); $this->settingsManager = $this->createMock(SettingsManager::class); + $this->settingsManager->expects($this->any())->method('useUserEmail')->willReturn(true); $this->hasher = $this->createMock(IHasher::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->shareManager = $this->createMock(\OCP\Share\IManager::class); $this->secureRandom = $this->createMock(ISecureRandom::class); $this->config = $this->createMock(IConfig::class); + $this->mailManager = $this->createMock(IMailManager::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->appConfig->expects($this->any())->method('getValueBool')->willReturn(true); // Empty share table $this->dbConn->getQueryBuilder()->delete('share')->executeStatement(); @@ -128,6 +140,8 @@ protected function setUp(): void { $this->eventDispatcher, $this->shareManager, $this->getEmailValidatorWithStrictEmailCheck(), + $this->mailManager, + $this->appConfig, ); } From 198e859bec1a963ae67e65161559718a1cc1eff5 Mon Sep 17 00:00:00 2001 From: Divyam Date: Tue, 22 Sep 2026 20:41:46 +0530 Subject: [PATCH 2/5] fix(sharebymail): strictly type findMailService and fix user mock error Signed-off-by: Divyam --- apps/sharebymail/lib/ShareByMailProvider.php | 29 ++++++++------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 4f251bb06cfb4..51ce9d2fcd7b3 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -344,25 +344,19 @@ private function trySendPasswordToOwner(IShare $share): void { * if mail providers are enabled globally and the admin has enabled * user-level sending for share emails, look up the user's mail service. * - * @param string $userId The user ID of the share initiator + * @param ?string $userId The user ID of the share initiator + * @param ?string $userEmail The email address of the share initiator * @return IMessageSend|null A mail service that can send, or null to fall back to the system mailer */ - protected function findMailService(string $userId): ?IMessageSend { - if (!$this->settingsManager->useUserEmail()) { - return null; - } - - if (!$this->appConfig->getValueBool('core', 'mail_providers_enabled', true)) { + protected function findMailService(?string $userId, ?string $userEmail): ?IMessageSend { + if ($userId === null || $userEmail === null) { return null; } - - $user = $this->userManager->get($userId); - if ($user === null) { + if (!$this->settingsManager->useUserEmail()) { return null; } - $userEmail = $user->getEMailAddress(); - if ($userEmail === null) { + if (!$this->appConfig->getValueBool('core', 'mail_providers_enabled', true)) { return null; } @@ -466,10 +460,9 @@ protected function sendEmail(IShare $share, array $emails): void { $instanceName = $this->defaults->getName(); // Try to send via the user's Mail Provider - $mailService = $this->findMailService($initiator); - if ($mailService !== null && $initiatorUser instanceof IUser) { - $initiatorEmail = $initiatorUser->getEMailAddress(); - if ($initiatorEmail !== null) { + $initiatorEmail = ($initiatorUser instanceof IUser) ? $initiatorUser->getEMailAddress() : null; + $mailService = $this->findMailService($initiator, $initiatorEmail); + if ($mailService !== null && $initiatorEmail !== null) { $emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : '')); try { $this->sendViaMailProvider($mailService, $initiatorEmail, $initiatorDisplayName, $emails, $emailTemplate); @@ -615,7 +608,7 @@ protected function sendPassword(IShare $share, string $password, array $emails): $instanceName = $this->defaults->getName(); // Try to send via the user's Mail Provider - $mailService = $this->findMailService($initiator); + $mailService = $this->findMailService($initiator, $initiatorEmailAddress); if ($mailService !== null && $initiatorEmailAddress !== null) { $emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : '')); try { @@ -730,7 +723,7 @@ protected function sendNote(IShare $share): void { $instanceName = $this->defaults->getName(); // Try to send via the user's Mail Provider - $mailService = $this->findMailService($initiator); + $mailService = $this->findMailService($initiator, $initiatorEmailAddress); if ($mailService !== null && $initiatorEmailAddress !== null) { $emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : '')); try { From 10147bf73fd10bd608220cb8ef4815e520b97a71 Mon Sep 17 00:00:00 2001 From: Divyam Date: Tue, 22 Sep 2026 20:46:03 +0530 Subject: [PATCH 3/5] fix(sharebymail): strictly typehint mail service as IMessageSend and IService Signed-off-by: Divyam --- apps/sharebymail/lib/ShareByMailProvider.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 51ce9d2fcd7b3..d06fb2184485f 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -31,6 +31,7 @@ use OCP\Mail\Provider\Address; use OCP\Mail\Provider\IManager as IMailManager; use OCP\Mail\Provider\IMessageSend; +use OCP\Mail\Provider\IService; use OCP\Security\Events\GenerateSecurePasswordEvent; use OCP\Security\IHasher; use OCP\Security\ISecureRandom; @@ -346,9 +347,9 @@ private function trySendPasswordToOwner(IShare $share): void { * * @param ?string $userId The user ID of the share initiator * @param ?string $userEmail The email address of the share initiator - * @return IMessageSend|null A mail service that can send, or null to fall back to the system mailer + * @return (IMessageSend&IService)|null A mail service that can send, or null to fall back to the system mailer */ - protected function findMailService(?string $userId, ?string $userEmail): ?IMessageSend { + protected function findMailService(?string $userId, ?string $userEmail): (IMessageSend&IService)|null { if ($userId === null || $userEmail === null) { return null; } @@ -372,20 +373,19 @@ protected function findMailService(?string $userId, ?string $userEmail): ?IMessa * Send the share notification email via Mail Provider if available, * otherwise fall back to the system mailer. * - * @param IMessageSend $mailService The mail provider service + * @param IMessageSend&IService $mailService The mail provider service * @param string $senderEmail The sender's email address * @param string $senderName The sender's display name * @param array $recipientEmails The recipient email addresses * @param \OCP\Mail\IEMailTemplate $emailTemplate The email template */ protected function sendViaMailProvider( - IMessageSend $mailService, + IMessageSend&IService $mailService, string $senderEmail, string $senderName, array $recipientEmails, \OCP\Mail\IEMailTemplate $emailTemplate, ): void { - /** @psalm-suppress UndefinedInterfaceMethod */ $message = $mailService->initiateMessage(); $message->setFrom(new Address($senderEmail, $senderName)); From 0035814de4bad356d2420c84c27c708b1a3ca1b9 Mon Sep 17 00:00:00 2001 From: Divyam Date: Tue, 22 Sep 2026 20:54:05 +0530 Subject: [PATCH 4/5] refactor(sharebymail): factor out mail provider sending and template building Signed-off-by: Divyam --- apps/sharebymail/lib/ShareByMailProvider.php | 298 ++++++++----------- 1 file changed, 131 insertions(+), 167 deletions(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index d06fb2184485f..cdb827b3eba3a 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -379,6 +379,45 @@ protected function findMailService(?string $userId, ?string $userEmail): (IMessa * @param array $recipientEmails The recipient email addresses * @param \OCP\Mail\IEMailTemplate $emailTemplate The email template */ + /** + * Send the share email via Mail Provider if available, + * otherwise return false to fall back to the system mailer. + * + * @param string $initiator The user ID of the share initiator + * @param ?string $initiatorEmail The email address of the share initiator + * @param string $initiatorDisplayName The display name of the share initiator + * @param array $recipientEmails The recipient email addresses + * @param callable $templateFactory A factory function that returns an \OCP\Mail\IEMailTemplate + * @return bool True if successfully sent via Mail Provider, false if falling back + */ + protected function sendViaMailProviderWithFallback( + string $initiator, + ?string $initiatorEmail, + string $initiatorDisplayName, + array $recipientEmails, + callable $templateFactory, + ): bool { + $mailService = $this->findMailService($initiator, $initiatorEmail); + if ($mailService === null || $initiatorEmail === null) { + return false; + } + + $emailTemplate = $templateFactory(); + $instanceName = $this->defaults->getName(); + $emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : '')); + + try { + $this->sendViaMailProvider($mailService, $initiatorEmail, $initiatorDisplayName, $recipientEmails, $emailTemplate); + return true; + } catch (\Exception $e) { + $this->logger->warning('Failed to send share email via Mail Provider, falling back to system mailer.', [ + 'app' => 'sharebymail', + 'exception' => $e, + ]); + return false; + } + } + protected function sendViaMailProvider( IMessageSend&IService $mailService, string $senderEmail, @@ -421,94 +460,56 @@ protected function sendEmail(IShare $share, array $emails): void { $initiatorUser = $this->userManager->get($initiator); $initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator; - $emailTemplate = $this->mailer->createEMailTemplate('sharebymail.RecipientNotification', [ - 'filename' => $filename, - 'link' => $link, - 'initiator' => $initiatorDisplayName, - 'expiration' => $expiration, - 'shareWith' => $shareWith, - 'note' => $note - ]); + $templateFactory = function () use ($filename, $link, $initiatorDisplayName, $expiration, $shareWith, $note): \OCP\Mail\IEMailTemplate { + $emailTemplate = $this->mailer->createEMailTemplate('sharebymail.RecipientNotification', [ + 'filename' => $filename, + 'link' => $link, + 'initiator' => $initiatorDisplayName, + 'expiration' => $expiration, + 'shareWith' => $shareWith, + 'note' => $note + ]); - $emailTemplate->setSubject($this->l->t('%1$s shared %2$s with you', [$initiatorDisplayName, $filename])); - $emailTemplate->addHeader(); - $emailTemplate->addHeading($this->l->t('%1$s shared %2$s with you', [$initiatorDisplayName, $filename]), false); - - if ($note !== '') { - $emailTemplate->addBodyListItem( - htmlspecialchars($note), - $this->l->t('Note:'), - $this->getAbsoluteImagePath('caldav/description.png'), - $note - ); - } + $emailTemplate->setSubject($this->l->t('%1$s shared %2$s with you', [$initiatorDisplayName, $filename])); + $emailTemplate->addHeader(); + $emailTemplate->addHeading($this->l->t('%1$s shared %2$s with you', [$initiatorDisplayName, $filename]), false); - if ($expiration !== null) { - $dateString = (string)$this->l->l('date', $expiration, ['width' => 'medium']); - $emailTemplate->addBodyListItem( - $this->l->t('This share is valid until %s at midnight', [$dateString]), - $this->l->t('Expiration:'), - $this->getAbsoluteImagePath('caldav/time.png'), - ); - } + if ($note !== '') { + $emailTemplate->addBodyListItem( + htmlspecialchars($note), + $this->l->t('Note:'), + $this->getAbsoluteImagePath('caldav/description.png'), + $note + ); + } - $emailTemplate->addBodyButton( - $this->l->t('Open shared item'), - $link - ); + if ($expiration !== null) { + $dateString = (string)$this->l->l('date', $expiration, ['width' => 'medium']); + $emailTemplate->addBodyListItem( + $this->l->t('This share is valid until %s at midnight', [$dateString]), + $this->l->t('Expiration:'), + $this->getAbsoluteImagePath('caldav/time.png'), + ); + } + + $emailTemplate->addBodyButton( + $this->l->t('Open shared item'), + $link + ); + + return $emailTemplate; + }; $instanceName = $this->defaults->getName(); // Try to send via the user's Mail Provider $initiatorEmail = ($initiatorUser instanceof IUser) ? $initiatorUser->getEMailAddress() : null; - $mailService = $this->findMailService($initiator, $initiatorEmail); - if ($mailService !== null && $initiatorEmail !== null) { - $emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : '')); - try { - $this->sendViaMailProvider($mailService, $initiatorEmail, $initiatorDisplayName, $emails, $emailTemplate); - return; - } catch (\Exception $e) { - $this->logger->warning('Failed to send share email via Mail Provider, falling back to system mailer.', [ - 'app' => 'sharebymail', - 'exception' => $e, - ]); - // Fall through to system mailer - // Re-create template since footer was already added - $emailTemplate = $this->mailer->createEMailTemplate('sharebymail.RecipientNotification', [ - 'filename' => $filename, - 'link' => $link, - 'initiator' => $initiatorDisplayName, - 'expiration' => $expiration, - 'shareWith' => $shareWith, - 'note' => $note - ]); - $emailTemplate->setSubject($this->l->t('%1$s shared %2$s with you', [$initiatorDisplayName, $filename])); - $emailTemplate->addHeader(); - $emailTemplate->addHeading($this->l->t('%1$s shared %2$s with you', [$initiatorDisplayName, $filename]), false); - if ($note !== '') { - $emailTemplate->addBodyListItem( - htmlspecialchars($note), - $this->l->t('Note:'), - $this->getAbsoluteImagePath('caldav/description.png'), - $note - ); - } - if ($expiration !== null) { - $dateString = (string)$this->l->l('date', $expiration, ['width' => 'medium']); - $emailTemplate->addBodyListItem( - $this->l->t('This share is valid until %s at midnight', [$dateString]), - $this->l->t('Expiration:'), - $this->getAbsoluteImagePath('caldav/time.png'), - ); - } - $emailTemplate->addBodyButton( - $this->l->t('Open shared item'), - $link - ); - } - } + if ($this->sendViaMailProviderWithFallback($initiator, $initiatorEmail, $initiatorDisplayName, $emails, $templateFactory)) { + return; } + $emailTemplate = $templateFactory(); + // Fall back to the system mailer $message = $this->mailer->createMessage(); @@ -583,67 +584,42 @@ protected function sendPassword(IShare $share, string $password, array $emails): $plainBodyPart = $this->l->t('%1$s shared %2$s with you. You should have already received a separate mail with a link to access it.', [$initiatorDisplayName, $filename]); $htmlBodyPart = $this->l->t('%1$s shared %2$s with you. You should have already received a separate mail with a link to access it.', [$initiatorDisplayName, $filename]); - $emailTemplate = $this->mailer->createEMailTemplate('sharebymail.RecipientPasswordNotification', [ - 'filename' => $filename, - 'password' => $password, - 'initiator' => $initiatorDisplayName, - 'initiatorEmail' => $initiatorEmailAddress, - 'shareWith' => $shareWith, - ]); + $templateFactory = function () use ($filename, $password, $initiatorDisplayName, $initiatorEmailAddress, $shareWith, $htmlBodyPart, $plainBodyPart): \OCP\Mail\IEMailTemplate { + $emailTemplate = $this->mailer->createEMailTemplate('sharebymail.RecipientPasswordNotification', [ + 'filename' => $filename, + 'password' => $password, + 'initiator' => $initiatorDisplayName, + 'initiatorEmail' => $initiatorEmailAddress, + 'shareWith' => $shareWith, + ]); - $emailTemplate->setSubject($this->l->t('Password to access %1$s shared to you by %2$s', [$filename, $initiatorDisplayName])); - $emailTemplate->addHeader(); - $emailTemplate->addHeading($this->l->t('Password to access %s', [$filename]), false); - $emailTemplate->addBodyText(htmlspecialchars($htmlBodyPart), $plainBodyPart); - $emailTemplate->addBodyText($this->l->t('It is protected with the following password:')); - $emailTemplate->addBodyText($password); + $emailTemplate->setSubject($this->l->t('Password to access %1$s shared to you by %2$s', [$filename, $initiatorDisplayName])); + $emailTemplate->addHeader(); + $emailTemplate->addHeading($this->l->t('Password to access %s', [$filename]), false); + $emailTemplate->addBodyText(htmlspecialchars($htmlBodyPart), $plainBodyPart); + $emailTemplate->addBodyText($this->l->t('It is protected with the following password:')); + $emailTemplate->addBodyText($password); + + if ($this->config->getSystemValue('sharing.enable_mail_link_password_expiration', false) === true) { + $expirationTime = new \DateTime(); + $expirationInterval = $this->config->getSystemValue('sharing.mail_link_password_expiration_interval', 3600); + $expirationTime = $expirationTime->add(new \DateInterval('PT' . $expirationInterval . 'S')); + $emailTemplate->addBodyText($this->l->t('This password will expire at %s', [$expirationTime->format('r')])); + } + + return $emailTemplate; + }; - if ($this->config->getSystemValue('sharing.enable_mail_link_password_expiration', false) === true) { - $expirationTime = new \DateTime(); - $expirationInterval = $this->config->getSystemValue('sharing.mail_link_password_expiration_interval', 3600); - $expirationTime = $expirationTime->add(new \DateInterval('PT' . $expirationInterval . 'S')); - $emailTemplate->addBodyText($this->l->t('This password will expire at %s', [$expirationTime->format('r')])); + // Try to send via the user's Mail Provider + if ($this->sendViaMailProviderWithFallback($initiator, $initiatorEmailAddress, $initiatorDisplayName, $emails, $templateFactory)) { + $this->createPasswordSendActivity($share, $shareWith, false); + return true; } $instanceName = $this->defaults->getName(); - // Try to send via the user's Mail Provider - $mailService = $this->findMailService($initiator, $initiatorEmailAddress); - if ($mailService !== null && $initiatorEmailAddress !== null) { - $emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : '')); - try { - $this->sendViaMailProvider($mailService, $initiatorEmailAddress, $initiatorDisplayName, $emails, $emailTemplate); - $this->createPasswordSendActivity($share, $shareWith, false); - return true; - } catch (\Exception $e) { - $this->logger->warning('Failed to send share password email via Mail Provider, falling back to system mailer.', [ - 'app' => 'sharebymail', - 'exception' => $e, - ]); - // Re-create template for fallback - $emailTemplate = $this->mailer->createEMailTemplate('sharebymail.RecipientPasswordNotification', [ - 'filename' => $filename, - 'password' => $password, - 'initiator' => $initiatorDisplayName, - 'initiatorEmail' => $initiatorEmailAddress, - 'shareWith' => $shareWith, - ]); - $emailTemplate->setSubject($this->l->t('Password to access %1$s shared to you by %2$s', [$filename, $initiatorDisplayName])); - $emailTemplate->addHeader(); - $emailTemplate->addHeading($this->l->t('Password to access %s', [$filename]), false); - $emailTemplate->addBodyText(htmlspecialchars($htmlBodyPart), $plainBodyPart); - $emailTemplate->addBodyText($this->l->t('It is protected with the following password:')); - $emailTemplate->addBodyText($password); - if ($this->config->getSystemValue('sharing.enable_mail_link_password_expiration', false) === true) { - $expirationTime = new \DateTime(); - $expirationInterval = $this->config->getSystemValue('sharing.mail_link_password_expiration_interval', 3600); - $expirationTime = $expirationTime->add(new \DateInterval('PT' . $expirationInterval . 'S')); - $emailTemplate->addBodyText($this->l->t('This password will expire at %s', [$expirationTime->format('r')])); - } - } - } - // Fall back to the system mailer + $emailTemplate = $templateFactory(); $message = $this->mailer->createMessage(); // If multiple recipients are given, we send the mail to all of them @@ -706,48 +682,36 @@ protected function sendNote(IShare $share): void { $plainHeading = $this->l->t('%1$s shared %2$s with you and wants to add:', [$initiatorDisplayName, $filename]); $htmlHeading = $this->l->t('%1$s shared %2$s with you and wants to add', [$initiatorDisplayName, $filename]); - $emailTemplate = $this->mailer->createEMailTemplate('shareByMail.sendNote'); + $templateFactory = function () use ($initiatorDisplayName, $htmlHeading, $plainHeading, $note, $share): \OCP\Mail\IEMailTemplate { + $emailTemplate = $this->mailer->createEMailTemplate('shareByMail.sendNote'); - $emailTemplate->setSubject($this->l->t('%s added a note to a file shared with you', [$initiatorDisplayName])); - $emailTemplate->addHeader(); - $emailTemplate->addHeading(htmlspecialchars($htmlHeading), $plainHeading); - $emailTemplate->addBodyText(htmlspecialchars($note), $note); - - $link = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare', - ['token' => $share->getToken()]); - $emailTemplate->addBodyButton( - $this->l->t('Open shared item'), - $link - ); + $emailTemplate->setSubject($this->l->t('%s added a note to a file shared with you', [$initiatorDisplayName])); + $emailTemplate->addHeader(); + $emailTemplate->addHeading(htmlspecialchars($htmlHeading), $plainHeading); + $emailTemplate->addBodyText(htmlspecialchars($note), $note); - $instanceName = $this->defaults->getName(); + $link = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare', + ['token' => $share->getToken()]); + $emailTemplate->addBodyButton( + $this->l->t('Open shared item'), + $link + ); + + return $emailTemplate; + }; // Try to send via the user's Mail Provider - $mailService = $this->findMailService($initiator, $initiatorEmailAddress); - if ($mailService !== null && $initiatorEmailAddress !== null) { - $emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : '')); - try { - $this->sendViaMailProvider($mailService, $initiatorEmailAddress, $initiatorDisplayName, [$recipient], $emailTemplate); - return; - } catch (\Exception $e) { - $this->logger->warning('Failed to send share note email via Mail Provider, falling back to system mailer.', [ - 'app' => 'sharebymail', - 'exception' => $e, - ]); - // Re-create template for fallback - $emailTemplate = $this->mailer->createEMailTemplate('shareByMail.sendNote'); - $emailTemplate->setSubject($this->l->t('%s added a note to a file shared with you', [$initiatorDisplayName])); - $emailTemplate->addHeader(); - $emailTemplate->addHeading(htmlspecialchars($htmlHeading), $plainHeading); - $emailTemplate->addBodyText(htmlspecialchars($note), $note); - $emailTemplate->addBodyButton( - $this->l->t('Open shared item'), - $link - ); - } + if ($this->sendViaMailProviderWithFallback($initiator, $initiatorEmailAddress, $initiatorDisplayName, [$recipient], $templateFactory)) { + return; } + $instanceName = $this->defaults->getName(); + + $link = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare', + ['token' => $share->getToken()]); + // Fall back to the system mailer + $emailTemplate = $templateFactory(); $message = $this->mailer->createMessage(); // The "From" contains the sharers name From ace1d9ea317714a739b635e9a10a7b9be49a7e13 Mon Sep 17 00:00:00 2001 From: Divyam Date: Tue, 22 Sep 2026 22:06:34 +0530 Subject: [PATCH 5/5] style(sharebymail): fix trailing whitespace Signed-off-by: Divyam --- apps/sharebymail/lib/ShareByMailProvider.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index cdb827b3eba3a..ebc0f6264bac9 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -496,7 +496,7 @@ protected function sendEmail(IShare $share, array $emails): void { $this->l->t('Open shared item'), $link ); - + return $emailTemplate; }; @@ -509,7 +509,7 @@ protected function sendEmail(IShare $share, array $emails): void { } $emailTemplate = $templateFactory(); - + // Fall back to the system mailer $message = $this->mailer->createMessage(); @@ -606,7 +606,7 @@ protected function sendPassword(IShare $share, string $password, array $emails): $expirationTime = $expirationTime->add(new \DateInterval('PT' . $expirationInterval . 'S')); $emailTemplate->addBodyText($this->l->t('This password will expire at %s', [$expirationTime->format('r')])); } - + return $emailTemplate; }; @@ -696,7 +696,7 @@ protected function sendNote(IShare $share): void { $this->l->t('Open shared item'), $link ); - + return $emailTemplate; }; @@ -706,7 +706,7 @@ protected function sendNote(IShare $share): void { } $instanceName = $this->defaults->getName(); - + $link = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare', ['token' => $share->getToken()]);