From 2ac9a1bac44fd68b501615643971925b71b15ce3 Mon Sep 17 00:00:00 2001 From: SebastianKrupinski Date: Mon, 29 Apr 2024 14:29:17 -0400 Subject: [PATCH] fix(caldav): Do not load IMipPlugin before user auth and session is created Signed-off-by: SebastianKrupinski --- apps/dav/lib/CalDAV/Schedule/IMipPlugin.php | 32 ++++---- apps/dav/lib/Server.php | 18 ++++- .../unit/CalDAV/Schedule/IMipPluginTest.php | 80 +++++++++++++++---- 3 files changed, 91 insertions(+), 39 deletions(-) diff --git a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php index fcc2ae1e166c5..dcac8db2bbd88 100644 --- a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php +++ b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php @@ -41,7 +41,7 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; use OCP\IConfig; -use OCP\IUserManager; +use OCP\IUserSession; use OCP\Mail\IMailer; use OCP\Util; use Psr\Log\LoggerInterface; @@ -69,13 +69,12 @@ * @license http://sabre.io/license/ Modified BSD License */ class IMipPlugin extends SabreIMipPlugin { - private ?string $userId; + private IUserSession $userSession; private IConfig $config; private IMailer $mailer; private LoggerInterface $logger; private ITimeFactory $timeFactory; private Defaults $defaults; - private IUserManager $userManager; private ?VCalendar $vCalendar = null; private IMipService $imipService; public const MAX_DATE = '2038-01-01'; @@ -90,18 +89,16 @@ public function __construct(IConfig $config, LoggerInterface $logger, ITimeFactory $timeFactory, Defaults $defaults, - IUserManager $userManager, - $userId, + IUserSession $userSession, IMipService $imipService, EventComparisonService $eventComparisonService) { parent::__construct(''); - $this->userId = $userId; + $this->userSession = $userSession; $this->config = $config; $this->mailer = $mailer; $this->logger = $logger; $this->timeFactory = $timeFactory; $this->defaults = $defaults; - $this->userManager = $userManager; $this->imipService = $imipService; $this->eventComparisonService = $eventComparisonService; } @@ -206,17 +203,16 @@ public function schedule(Message $iTipMessage) { $this->imipService->setL10n($attendee); // Build the sender name. - // Due to a bug in sabre, the senderName property for an iTIP message - // can actually also be a VObject Property - /** @var Parameter|string|null $senderName */ - $senderName = $iTipMessage->senderName ?: null; - if($senderName instanceof Parameter) { - $senderName = $senderName->getValue() ?? null; - } - - // Try to get the sender name from the current user id if available. - if ($this->userId !== null && ($senderName === null || empty(trim($senderName)))) { - $senderName = $this->userManager->getDisplayName($this->userId); + // 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 = ''; } $sender = substr($iTipMessage->sender, 7); diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 6efcd50ecd35d..3b6a3a257898b 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -39,6 +39,7 @@ use OCA\DAV\AppInfo\PluginManager; use OCA\DAV\BulkUpload\BulkUploadPlugin; use OCA\DAV\CalDAV\BirthdayService; +use OCA\DAV\CalDAV\Schedule\IMipPlugin; use OCA\DAV\CalDAV\Security\RateLimitingPlugin; use OCA\DAV\CalDAV\Validation\CalDavValidatePlugin; use OCA\DAV\CardDAV\HasPhotoPlugin; @@ -179,12 +180,10 @@ public function __construct(IRequest $request, string $baseUri) { // calendar plugins if ($this->requestIsForSubtree(['calendars', 'public-calendars', 'system-calendars', 'principals'])) { + $this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest(), \OC::$server->getConfig())); $this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin()); $this->server->addPlugin(new \OCA\DAV\CalDAV\ICSExportPlugin\ICSExportPlugin(\OC::$server->getConfig(), $logger)); $this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin(\OC::$server->getConfig(), \OC::$server->get(LoggerInterface::class))); - if (\OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes') { - $this->server->addPlugin(\OC::$server->query(\OCA\DAV\CalDAV\Schedule\IMipPlugin::class)); - } $this->server->addPlugin(\OC::$server->get(\OCA\DAV\CalDAV\Trashbin\Plugin::class)); $this->server->addPlugin(new \OCA\DAV\CalDAV\WebcalCaching\Plugin($request)); @@ -193,7 +192,6 @@ public function __construct(IRequest $request, string $baseUri) { } $this->server->addPlugin(new \Sabre\CalDAV\Notifications\Plugin()); - $this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest(), \OC::$server->getConfig())); $this->server->addPlugin(new \OCA\DAV\CalDAV\Publishing\PublishPlugin( \OC::$server->getConfig(), \OC::$server->getURLGenerator() @@ -310,6 +308,18 @@ public function __construct(IRequest $request, string $baseUri) { \OC::$server->getCommentsManager(), $userSession )); + if (\OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes') { + $this->server->addPlugin(new IMipPlugin( + \OC::$server->get(\OCP\IConfig::class), + \OC::$server->get(\OCP\Mail\IMailer::class), + \OC::$server->get(LoggerInterface::class), + \OC::$server->get(\OCP\AppFramework\Utility\ITimeFactory::class), + \OC::$server->get(\OCP\Defaults::class), + $userSession, + \OC::$server->get(\OCA\DAV\CalDAV\Schedule\IMipService::class), + \OC::$server->get(\OCA\DAV\CalDAV\EventComparisonService::class) + )); + } $this->server->addPlugin(new \OCA\DAV\CalDAV\Search\SearchPlugin()); if ($view !== null) { $this->server->addPlugin(new FilesReportPlugin( diff --git a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php index 0e459418ae0af..8bd1bf22bcc95 100644 --- a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php +++ b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php @@ -35,7 +35,8 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; use OCP\IConfig; -use OCP\IUserManager; +use OCP\IUser; +use OCP\IUserSession; use OCP\Mail\IAttachment; use OCP\Mail\IEMailTemplate; use OCP\Mail\IMailer; @@ -68,8 +69,11 @@ class IMipPluginTest extends TestCase { /** @var IConfig|MockObject */ private $config; - /** @var IUserManager|MockObject */ - private $userManager; + /** @var IUserSession|MockObject */ + private $userSession; + + /** @var IUser|MockObject */ + private $user; /** @var IMipPlugin */ private $plugin; @@ -107,8 +111,12 @@ protected function setUp(): void { $this->timeFactory->method('getTime')->willReturn(1496912528); // 2017-01-01 $this->config = $this->createMock(IConfig::class); + + $this->user = $this->createMock(IUser::class); - $this->userManager = $this->createMock(IUserManager::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->userSession->method('getUser') + ->willReturn($this->user); $this->defaults = $this->createMock(Defaults::class); $this->defaults->method('getName') @@ -124,8 +132,7 @@ protected function setUp(): void { $this->logger, $this->timeFactory, $this->defaults, - $this->userManager, - 'user123', + $this->userSession, $this->service, $this->eventComparisonService ); @@ -213,8 +220,15 @@ public function testParsingSingle(): void { ->method('buildBodyData') ->with($newVevent, $oldVEvent) ->willReturn($data); - $this->userManager->expects(self::never()) - ->method('getDisplayName'); + $this->user->expects(self::any()) + ->method('getUID') + ->willReturn('user1'); + $this->user->expects(self::any()) + ->method('getDisplayName') + ->willReturn('Mr. Wizard'); + $this->userSession->expects(self::any()) + ->method('getUser') + ->willReturn($this->user); $this->service->expects(self::once()) ->method('getFrom'); $this->service->expects(self::once()) @@ -307,8 +321,15 @@ public function testAttendeeIsResource(): void { ->willReturn(true); $this->service->expects(self::never()) ->method('buildBodyData'); - $this->userManager->expects(self::never()) - ->method('getDisplayName'); + $this->user->expects(self::any()) + ->method('getUID') + ->willReturn('user1'); + $this->user->expects(self::any()) + ->method('getDisplayName') + ->willReturn('Mr. Wizard'); + $this->userSession->expects(self::any()) + ->method('getUser') + ->willReturn($this->user); $this->service->expects(self::never()) ->method('getFrom'); $this->service->expects(self::never()) @@ -331,7 +352,6 @@ public function testAttendeeIsResource(): void { $this->assertEquals('1.0', $message->getScheduleStatus()); } - public function testParsingRecurrence(): void { $message = new Message(); $message->method = 'REQUEST'; @@ -404,9 +424,15 @@ public function testParsingRecurrence(): void { ->method('buildBodyData') ->with($newVevent, null) ->willReturn($data); - $this->userManager->expects(self::once()) + $this->user->expects(self::any()) + ->method('getUID') + ->willReturn('user1'); + $this->user->expects(self::any()) ->method('getDisplayName') ->willReturn('Mr. Wizard'); + $this->userSession->expects(self::any()) + ->method('getUser') + ->willReturn($this->user); $this->service->expects(self::once()) ->method('getFrom'); $this->service->expects(self::once()) @@ -529,8 +555,15 @@ public function testFailedDelivery(): void { ->method('buildBodyData') ->with($newVevent, null) ->willReturn($data); - $this->userManager->expects(self::never()) - ->method('getDisplayName'); + $this->user->expects(self::any()) + ->method('getUID') + ->willReturn('user1'); + $this->user->expects(self::any()) + ->method('getDisplayName') + ->willReturn('Mr. Wizard'); + $this->userSession->expects(self::any()) + ->method('getUser') + ->willReturn($this->user); $this->service->expects(self::once()) ->method('getFrom'); $this->service->expects(self::once()) @@ -618,8 +651,15 @@ public function testNoOldEvent(): void { ->method('buildBodyData') ->with($newVevent, null) ->willReturn($data); - $this->userManager->expects(self::never()) - ->method('getDisplayName'); + $this->user->expects(self::any()) + ->method('getUID') + ->willReturn('user1'); + $this->user->expects(self::any()) + ->method('getDisplayName') + ->willReturn('Mr. Wizard'); + $this->userSession->expects(self::any()) + ->method('getUser') + ->willReturn($this->user); $this->service->expects(self::once()) ->method('getFrom'); $this->service->expects(self::once()) @@ -704,9 +744,15 @@ public function testNoButtons(): void { ->method('buildBodyData') ->with($newVevent, null) ->willReturn($data); - $this->userManager->expects(self::once()) + $this->user->expects(self::any()) + ->method('getUID') + ->willReturn('user1'); + $this->user->expects(self::any()) ->method('getDisplayName') ->willReturn('Mr. Wizard'); + $this->userSession->expects(self::any()) + ->method('getUser') + ->willReturn($this->user); $this->service->expects(self::once()) ->method('getFrom'); $this->service->expects(self::once())