fix(caldav): Do not load IMipPlugin before user auth and session is c…

…reated Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
nextcloud · Sep 22, 2024 · 2ac9a1b · 2ac9a1b
1 parent fa949da
commit 2ac9a1b
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 39 deletions.
diff --git 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
@@ -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
@@ -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())