From 9859a449081711d9cab764e728858d8abf678859 Mon Sep 17 00:00:00 2001 From: SebastianKrupinski Date: Mon, 29 Apr 2024 14:29:17 -0400 Subject: [PATCH 1/5] fix(caldav): Do not load IMipPlugin before user auth and session is created Signed-off-by: SebastianKrupinski --- apps/dav/lib/Server.php | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index deee381d24cfe..2465a8253b23a 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\CardDAV\HasPhotoPlugin; use OCA\DAV\CardDAV\ImageExportPlugin; @@ -176,12 +177,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)); @@ -190,7 +189,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() @@ -304,6 +302,19 @@ 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->getConfig(), + \OC::$server->getMailer(), + \OC::$server->get(LoggerInterface::class), + \OC::$server->get(\OCP\AppFramework\Utility\ITimeFactory::class), + \OC::$server->get(\OCP\Defaults::class), + \OC::$server->get(\OCP\IUserManager::class), + $user->getUid(), + \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( From f44b73e2db6d82ceb7ef9e075fce77d8e5555b0f Mon Sep 17 00:00:00 2001 From: SebastianKrupinski Date: Tue, 30 Apr 2024 10:30:12 -0400 Subject: [PATCH 2/5] fix(caldav): Use userSession instead of userId Signed-off-by: SebastianKrupinski --- apps/dav/lib/CalDAV/Schedule/IMipPlugin.php | 30 +++++++++------------ apps/dav/lib/Server.php | 7 +++-- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php index fcc2ae1e166c5..472f745fefe88 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; + // 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 = $iTipMessage->senderName->getValue(); } - - // 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); + elseif (is_string($iTipMessage->senderName) && !empty(trim($iTipMessage->senderName))) { + $senderName = $iTipMessage->senderName; + } + else { + $senderName = $this->userSession->getUser()->getDisplayName(); } $sender = substr($iTipMessage->sender, 7); diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 2465a8253b23a..37d04ba819048 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -304,13 +304,12 @@ public function __construct(IRequest $request, string $baseUri) { )); if (\OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes') { $this->server->addPlugin(new IMipPlugin( - \OC::$server->getConfig(), - \OC::$server->getMailer(), + \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), - \OC::$server->get(\OCP\IUserManager::class), - $user->getUid(), + $userSession, \OC::$server->get(\OCA\DAV\CalDAV\Schedule\IMipService::class), \OC::$server->get(\OCA\DAV\CalDAV\EventComparisonService::class) )); From d25039ec1ad8560d6948cb3650e0f92ab4d17e15 Mon Sep 17 00:00:00 2001 From: SebastianKrupinski Date: Wed, 1 May 2024 19:51:45 -0400 Subject: [PATCH 3/5] fix(caldav): Test if user object is not null and trim senderName Signed-off-by: SebastianKrupinski --- apps/dav/lib/CalDAV/Schedule/IMipPlugin.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php index 472f745fefe88..d27dcf5520642 100644 --- a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php +++ b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php @@ -206,13 +206,16 @@ public function schedule(Message $iTipMessage) { // 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 = $iTipMessage->senderName->getValue(); + $senderName = trim($iTipMessage->senderName->getValue()); } elseif (is_string($iTipMessage->senderName) && !empty(trim($iTipMessage->senderName))) { - $senderName = $iTipMessage->senderName; + $senderName = trim($iTipMessage->senderName); + } + elseif ($this->userSession->getUser() !== null) { + $senderName = trim($this->userSession->getUser()->getDisplayName()); } else { - $senderName = $this->userSession->getUser()->getDisplayName(); + $senderName = ''; } $sender = substr($iTipMessage->sender, 7); From 2088b492bfd345d8e0c9f9d71e15224befffa291 Mon Sep 17 00:00:00 2001 From: SebastianKrupinski Date: Thu, 2 May 2024 11:50:27 -0400 Subject: [PATCH 4/5] fix(caldav): Fixed phpUnit to use userSession instead of userId and userManager Signed-off-by: SebastianKrupinski --- .../unit/CalDAV/Schedule/IMipPluginTest.php | 84 +++++++++++++++---- 1 file changed, 67 insertions(+), 17 deletions(-) diff --git a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php index 0e459418ae0af..c659d52ae5871 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,16 @@ 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->user->method('getUID'); + $this->user->method('getDisplayName'); + */ - $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 +136,7 @@ protected function setUp(): void { $this->logger, $this->timeFactory, $this->defaults, - $this->userManager, - 'user123', + $this->userSession, $this->service, $this->eventComparisonService ); @@ -213,8 +224,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 +325,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 +356,6 @@ public function testAttendeeIsResource(): void { $this->assertEquals('1.0', $message->getScheduleStatus()); } - public function testParsingRecurrence(): void { $message = new Message(); $message->method = 'REQUEST'; @@ -404,9 +428,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 +559,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 +655,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 +748,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()) From 51d338b9cc32b487b7ca8939fef5ef221828cc67 Mon Sep 17 00:00:00 2001 From: SebastianKrupinski Date: Fri, 3 May 2024 09:01:07 -0400 Subject: [PATCH 5/5] fix(caldav): Fixed formatting to comply with php-cs Signed-off-by: SebastianKrupinski --- apps/dav/lib/CalDAV/Schedule/IMipPlugin.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php index d27dcf5520642..dcac8db2bbd88 100644 --- a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php +++ b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php @@ -207,14 +207,11 @@ public function schedule(Message $iTipMessage) { // 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))) { + } elseif (is_string($iTipMessage->senderName) && !empty(trim($iTipMessage->senderName))) { $senderName = trim($iTipMessage->senderName); - } - elseif ($this->userSession->getUser() !== null) { + } elseif ($this->userSession->getUser() !== null) { $senderName = trim($this->userSession->getUser()->getDisplayName()); - } - else { + } else { $senderName = ''; }