Merge pull request #45098 from nextcloud/fix/issue-45081

Do not load IMipPlugin before user session is initialized also use userSession instead of userId in constructor.
nextcloud · May 4, 2024 · c17fcc4 · c17fcc4
2 parents 461f6d4 + 51d338b
commit c17fcc4
Show file tree

Hide file tree

Showing 3 changed files with 95 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\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,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,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())