Skip to content

Commit

Permalink
fix(caldav): Do not load IMipPlugin before user auth and session is c…
Browse files Browse the repository at this point in the history
…reated

Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
  • Loading branch information
SebastianKrupinski authored and solracsf committed Sep 22, 2024
1 parent fa949da commit 2ac9a1b
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 39 deletions.
32 changes: 14 additions & 18 deletions apps/dav/lib/CalDAV/Schedule/IMipPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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';
Expand All @@ -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;
}
Expand Down Expand Up @@ -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()))) {

Check notice

Code scanning / Psalm

PossiblyNullArgument Note

Argument 1 of trim cannot be null, possibly null value provided
$senderName = trim($iTipMessage->senderName->getValue());

Check notice

Code scanning / Psalm

PossiblyNullArgument Note

Argument 1 of trim cannot be null, possibly null value provided
} elseif (is_string($iTipMessage->senderName) && !empty(trim($iTipMessage->senderName))) {
$senderName = trim($iTipMessage->senderName);
} elseif ($this->userSession->getUser() !== null) {
$senderName = trim($this->userSession->getUser()->getDisplayName());

Check notice

Code scanning / Psalm

PossiblyNullReference Note

Cannot call method getDisplayName on possibly null value
} else {
$senderName = '';
}

$sender = substr($iTipMessage->sender, 7);
Expand Down
18 changes: 14 additions & 4 deletions apps/dav/lib/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()));

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OC\Server::getRequest has been marked as deprecated

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OC\Server::getConfig has been marked as deprecated
$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));
Expand All @@ -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()
Expand Down Expand Up @@ -310,6 +308,18 @@ public function __construct(IRequest $request, string $baseUri) {
\OC::$server->getCommentsManager(),
$userSession
));
if (\OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes') {

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OC\Server::getConfig has been marked as deprecated
$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(
Expand Down
80 changes: 63 additions & 17 deletions apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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')
Expand All @@ -124,8 +132,7 @@ protected function setUp(): void {
$this->logger,
$this->timeFactory,
$this->defaults,
$this->userManager,
'user123',
$this->userSession,
$this->service,
$this->eventComparisonService
);
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand All @@ -331,7 +352,6 @@ public function testAttendeeIsResource(): void {
$this->assertEquals('1.0', $message->getScheduleStatus());
}


public function testParsingRecurrence(): void {
$message = new Message();
$message->method = 'REQUEST';
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down

0 comments on commit 2ac9a1b

Please sign in to comment.