diff --git a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php index 6d70d6fac4739..942bb3f326d77 100644 --- a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php +++ b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php @@ -38,9 +38,11 @@ use OCA\DAV\CalDAV\CalendarObject; use OCA\DAV\CalDAV\EventComparisonService; +use OCP\Accounts\IAccountManager; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; use OCP\IConfig; +use OCP\IUser; use OCP\IUserSession; use OCP\Mail\IMailer; use OCP\Util; @@ -83,6 +85,7 @@ class IMipPlugin extends SabreIMipPlugin { public const METHOD_CANCEL = 'cancel'; public const IMIP_INDENT = 15; // Enough for the length of all body bullet items, in all languages private EventComparisonService $eventComparisonService; + private IAccountManager $accountManager; public function __construct(IConfig $config, IMailer $mailer, @@ -91,7 +94,8 @@ public function __construct(IConfig $config, Defaults $defaults, IUserSession $userSession, IMipService $imipService, - EventComparisonService $eventComparisonService) { + EventComparisonService $eventComparisonService, + IAccountManager $accountManager) { parent::__construct(''); $this->userSession = $userSession; $this->config = $config; @@ -101,6 +105,7 @@ public function __construct(IConfig $config, $this->defaults = $defaults; $this->imipService = $imipService; $this->eventComparisonService = $eventComparisonService; + $this->accountManager = $accountManager; } public function initialize(DAV\Server $server): void { @@ -202,21 +207,17 @@ public function schedule(Message $iTipMessage) { } $this->imipService->setL10n($attendee); - // Build the sender name. + $sender = substr($iTipMessage->sender, 7); + // Due to a bug in sabre, the senderName property for an iTIP message can actually also be a VObject Property - // If the iTIP message senderName is null or empty use the user session name as the senderName if (($iTipMessage->senderName instanceof Parameter) && !empty(trim($iTipMessage->senderName->getValue()))) { $senderName = trim($iTipMessage->senderName->getValue()); } elseif (is_string($iTipMessage->senderName) && !empty(trim($iTipMessage->senderName))) { $senderName = trim($iTipMessage->senderName); - } elseif ($this->userSession->getUser() !== null) { - $senderName = trim($this->userSession->getUser()->getDisplayName()); } else { - $senderName = ''; + $senderName = $this->getSenderNameFor($sender); } - $sender = substr($iTipMessage->sender, 7); - $replyingAttendee = null; switch (strtolower($iTipMessage->method)) { case self::METHOD_REPLY: @@ -319,6 +320,40 @@ public function schedule(Message $iTipMessage) { } } + /** + * Messages are regularly brokered on behalf of somebody else, so the + * session user's name is only used when the sender address is one of + * theirs. + */ + private function getSenderNameFor(string $sender): ?string { + $user = $this->userSession->getUser(); + if ($user !== null && $this->isAddressOfUser($sender, $user)) { + return trim($user->getDisplayName()) ?: null; + } + + return null; + } + + /** + * Profile email addresses are part of the user's calendar-user-address-set + * and therefore valid sender addresses next to the system email address. + */ + private function isAddressOfUser(string $address, IUser $user): bool { + if (strcasecmp((string)$user->getEMailAddress(), $address) === 0) { + return true; + } + + $emailCollection = $this->accountManager->getAccount($user) + ->getPropertyCollection(IAccountManager::COLLECTION_EMAIL); + foreach ($emailCollection->getProperties() as $property) { + if (strcasecmp($property->getValue(), $address) === 0) { + return true; + } + } + + return false; + } + /** * @return ?VCalendar */ diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 3b7459058e80c..54753122f0274 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -324,7 +324,8 @@ public function __construct(IRequest $request, string $baseUri) { \OC::$server->get(\OCP\Defaults::class), $userSession, \OC::$server->get(\OCA\DAV\CalDAV\Schedule\IMipService::class), - \OC::$server->get(\OCA\DAV\CalDAV\EventComparisonService::class) + \OC::$server->get(\OCA\DAV\CalDAV\EventComparisonService::class), + \OC::$server->get(\OCP\Accounts\IAccountManager::class), )); } $this->server->addPlugin(new \OCA\DAV\CalDAV\Search\SearchPlugin()); diff --git a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php index de6218bf5f536..0653a1680a731 100644 --- a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php +++ b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php @@ -32,6 +32,10 @@ use OCA\DAV\CalDAV\EventComparisonService; use OCA\DAV\CalDAV\Schedule\IMipPlugin; use OCA\DAV\CalDAV\Schedule\IMipService; +use OCP\Accounts\IAccount; +use OCP\Accounts\IAccountManager; +use OCP\Accounts\IAccountProperty; +use OCP\Accounts\IAccountPropertyCollection; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; use OCP\IConfig; @@ -90,6 +94,9 @@ class IMipPluginTest extends TestCase { /** @var EventComparisonService|MockObject */ private $eventComparisonService; + /** @var IAccountManager|MockObject */ + private $accountManager; + protected function setUp(): void { $this->mailMessage = $this->createMock(IMessage::class); $this->mailMessage->method('setFrom')->willReturn($this->mailMessage); @@ -126,6 +133,8 @@ protected function setUp(): void { $this->eventComparisonService = $this->createMock(EventComparisonService::class); + $this->accountManager = $this->createMock(IAccountManager::class); + $this->plugin = new IMipPlugin( $this->config, $this->mailer, @@ -134,7 +143,8 @@ protected function setUp(): void { $this->defaults, $this->userSession, $this->service, - $this->eventComparisonService + $this->eventComparisonService, + $this->accountManager, ); } @@ -226,6 +236,9 @@ public function testParsingSingle(): void { $this->user->expects(self::any()) ->method('getDisplayName') ->willReturn('Mr. Wizard'); + $this->user->expects(self::any()) + ->method('getEMailAddress') + ->willReturn('gandalf@wiz.ard'); $this->userSession->expects(self::any()) ->method('getUser') ->willReturn($this->user); @@ -430,6 +443,9 @@ public function testParsingRecurrence(): void { $this->user->expects(self::any()) ->method('getDisplayName') ->willReturn('Mr. Wizard'); + $this->user->expects(self::any()) + ->method('getEMailAddress') + ->willReturn('gandalf@wiz.ard'); $this->userSession->expects(self::any()) ->method('getUser') ->willReturn($this->user); @@ -561,6 +577,9 @@ public function testFailedDelivery(): void { $this->user->expects(self::any()) ->method('getDisplayName') ->willReturn('Mr. Wizard'); + $this->user->expects(self::any()) + ->method('getEMailAddress') + ->willReturn('gandalf@wiz.ard'); $this->userSession->expects(self::any()) ->method('getUser') ->willReturn($this->user); @@ -657,6 +676,9 @@ public function testNoOldEvent(): void { $this->user->expects(self::any()) ->method('getDisplayName') ->willReturn('Mr. Wizard'); + $this->user->expects(self::any()) + ->method('getEMailAddress') + ->willReturn('gandalf@wiz.ard'); $this->userSession->expects(self::any()) ->method('getUser') ->willReturn($this->user); @@ -750,6 +772,9 @@ public function testNoButtons(): void { $this->user->expects(self::any()) ->method('getDisplayName') ->willReturn('Mr. Wizard'); + $this->user->expects(self::any()) + ->method('getEMailAddress') + ->willReturn('gandalf@wiz.ard'); $this->userSession->expects(self::any()) ->method('getUser') ->willReturn($this->user); @@ -783,4 +808,161 @@ public function testNoButtons(): void { $this->plugin->schedule($message); $this->assertEquals('1.1', $message->getScheduleStatus()); } + + /** + * Runs schedule() for an organizer sourced REQUEST whose iTip message + * carries no sender name and captures the resulting From and Reply-To + * headers. + * + * @return array{from: ?array, replyTo: ?array} + */ + private function scheduleWithoutSenderName(string $organizer, string $recipient): array { + $vCalendar = new VCalendar(); + $vEvent = new VEvent($vCalendar, 'VEVENT', [ + 'UID' => 'uid-1234', + 'SEQUENCE' => 1, + 'SUMMARY' => 'Meeting', + 'DTSTART' => new \DateTime('2017-01-01 00:00:00'), + ]); + $vEvent->add('ORGANIZER', 'mailto:' . $organizer); + $vEvent->add('ATTENDEE', 'mailto:' . $recipient, ['RSVP' => 'TRUE', 'CN' => 'Frodo']); + + $message = new Message(); + $message->method = 'REQUEST'; + $message->message = $vCalendar; + $message->sender = 'mailto:' . $organizer; + $message->senderName = null; + $message->recipient = 'mailto:' . $recipient; + + $capturedFrom = null; + $capturedReplyTo = null; + $mailMessage = $this->createMock(IMessage::class); + $mailMessage->method('setTo')->willReturn($mailMessage); + $mailMessage->method('setFrom') + ->willReturnCallback(function (array $from) use (&$capturedFrom, $mailMessage) { + $capturedFrom = $from; + return $mailMessage; + }); + $mailMessage->method('setReplyTo') + ->willReturnCallback(function (array $replyTo) use (&$capturedReplyTo, $mailMessage) { + $capturedReplyTo = $replyTo; + return $mailMessage; + }); + + $mailer = $this->createMock(IMailer::class); + $mailer->method('createMessage')->willReturn($mailMessage); + $mailer->method('createEMailTemplate')->willReturn($this->emailTemplate); + $mailer->method('validateMailAddress')->with($recipient)->willReturn(true); + $mailer->method('send')->willReturn([]); + + $this->service->method('getLastOccurrence')->willReturn(1496912700); + $this->service->method('getCurrentAttendee')->willReturn($vEvent->select('ATTENDEE')[0]); + $this->service->method('isRoomOrResource')->willReturn(false); + $this->service->method('getAttendeeRsvpOrReqForParticipant')->willReturn(false); + $this->service->method('buildBodyData')->willReturn([ + 'meeting_title' => 'Meeting', + 'invitee_name' => '', + 'attendee_name' => $recipient, + ]); + // Mirrors the real IMipService::getFrom() so assertions read like the + // actual mail header. + $this->service->method('getFrom') + ->willReturnCallback(static function (?string $senderName, string $default): string { + return ($senderName === null || $senderName === '') ? $default : $senderName . ' via ' . $default; + }); + + $this->eventComparisonService->method('findModified') + ->willReturn(['old' => [], 'new' => [$vEvent]]); + + $plugin = new IMipPlugin( + $this->config, + $mailer, + $this->logger, + $this->timeFactory, + $this->defaults, + $this->userSession, + $this->service, + $this->eventComparisonService, + $this->accountManager, + ); + $plugin->schedule($message); + self::assertSame('1.1', $message->getScheduleStatus()); + + return ['from' => $capturedFrom, 'replyTo' => $capturedReplyTo]; + } + + /** + * Messages are regularly brokered on behalf of somebody else, so headers + * must not fall back to the session user's name when the sender address + * is not theirs. + */ + public function testSenderNameIsNotTakenFromAnUnrelatedSessionUser(): void { + $this->user->method('getUID')->willReturn('bilbo'); + $this->user->method('getDisplayName')->willReturn('Bilbo Baggins'); + $this->user->method('getEMailAddress')->willReturn('bilbo@hobb.it'); + + $result = $this->scheduleWithoutSenderName('a@example.com', 'frodo@hobb.it'); + + self::assertSame(['Instance Name 123'], array_values($result['from'])); + self::assertSame(['a@example.com'], $result['replyTo']); + } + + public function testSenderNameFallsBackToSessionUserWhenTheyAreTheSender(): void { + $this->user->method('getUID')->willReturn('gandalf'); + $this->user->method('getDisplayName')->willReturn('Mr. Wizard'); + $this->user->method('getEMailAddress')->willReturn('gandalf@wiz.ard'); + + $result = $this->scheduleWithoutSenderName('gandalf@wiz.ard', 'frodo@hobb.it'); + + self::assertSame(['Mr. Wizard via Instance Name 123'], array_values($result['from'])); + self::assertSame(['gandalf@wiz.ard' => 'Mr. Wizard'], $result['replyTo']); + } + + /** + * The session user must also be recognized as the sender when sending + * under one of their profile email aliases. + */ + public function testSenderNameUsesSessionUserForTheirAliasAddress(): void { + $this->user->method('getUID')->willReturn('carl'); + $this->user->method('getDisplayName')->willReturn('Carl Session'); + $this->user->method('getEMailAddress')->willReturn('carl@example.com'); + + $aliasProperty = $this->createMock(IAccountProperty::class); + $aliasProperty->method('getValue')->willReturn('Shared@Corp.example'); + $emailCollection = $this->createMock(IAccountPropertyCollection::class); + $emailCollection->method('getProperties')->willReturn([$aliasProperty]); + $account = $this->createMock(IAccount::class); + $account->method('getPropertyCollection') + ->with(IAccountManager::COLLECTION_EMAIL) + ->willReturn($emailCollection); + $this->accountManager->method('getAccount')->with($this->user)->willReturn($account); + + $result = $this->scheduleWithoutSenderName('shared@corp.example', 'frodo@hobb.it'); + + self::assertSame(['Carl Session via Instance Name 123'], array_values($result['from'])); + self::assertSame(['shared@corp.example' => 'Carl Session'], $result['replyTo']); + } + + public function testSenderNameStaysNeutralForBlankDisplayNames(): void { + $this->user->method('getUID')->willReturn('gandalf'); + $this->user->method('getDisplayName')->willReturn(' '); + $this->user->method('getEMailAddress')->willReturn('gandalf@wiz.ard'); + + $result = $this->scheduleWithoutSenderName('gandalf@wiz.ard', 'frodo@hobb.it'); + + self::assertSame(['Instance Name 123'], array_values($result['from'])); + self::assertSame(['gandalf@wiz.ard'], $result['replyTo']); + } + + /** + * Sessionless contexts: invitation link responses and background jobs. + */ + public function testSenderNameStaysNeutralWithoutSessionUser(): void { + $this->userSession = $this->createMock(IUserSession::class); + + $result = $this->scheduleWithoutSenderName('a@example.com', 'frodo@hobb.it'); + + self::assertSame(['Instance Name 123'], array_values($result['from'])); + self::assertSame(['a@example.com'], $result['replyTo']); + } }