Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from May 4, 2024

Conversation

SebastianKrupinski
Copy link
Contributor

Summary

Do not load IMipPlugin before user is authenticated and user session is initilized

@@ -176,12 +177,10 @@

// 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
@@ -176,12 +177,10 @@

// 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::getConfig has been marked as deprecated
@@ -304,6 +302,19 @@
\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
@@ -304,6 +302,19 @@
\OC::$server->getCommentsManager(),
$userSession
));
if (\OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes') {

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::getAppValue has been marked as deprecated
apps/dav/lib/Server.php Fixed Show fixed Hide fixed
apps/dav/lib/Server.php Fixed Show fixed Hide fixed
$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()))) {

Check notice

Code scanning / Psalm

PossiblyNullArgument Note

Argument 1 of trim cannot be null, possibly null value provided
apps/dav/lib/CalDAV/Schedule/IMipPlugin.php Fixed Show fixed Hide fixed
$senderName = $iTipMessage->senderName;
}
else {
$senderName = $this->userSession->getUser()->getDisplayName();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be an opportunity to trim $senderName afterwards for all cases.

$senderName = $iTipMessage->senderName;
}
else {
$senderName = $this->userSession->getUser()->getDisplayName();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though this should be safe when registered through dav server, it might not be when used through apps/dav/appinfo/v1/caldav.php (very legacy stuff, but still). I'd add a check to see if getUser() isn't null (and psalm would be happy).

@SebastianKrupinski
Copy link
Contributor Author

@tcitworld thank you for the input. I will apply your recommendations.

@tcitworld
Copy link
Member

OCA\DAV\Tests\unit\CalDAV\Schedule\IMipPluginTest needs to be changed with the new constructor.

Copy link
Member

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint fix, otherwise ✅

// 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());

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
@SebastianKrupinski SebastianKrupinski merged commit c17fcc4 into master May 4, 2024
157 checks passed
@SebastianKrupinski SebastianKrupinski deleted the fix/issue-45081 branch May 4, 2024 13:36
Copy link

welcome bot commented May 4, 2024

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: From Name is not setting properly in Calendar invitations created from Thunderbird
3 participants