Merge pull request #35021 from nextcloud/backport/34909/stable22

[stable22] Fix duplicate event email notifications
pull/35439/head
Vincent Petry 2 years ago committed by GitHub
commit 5c0c34654f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -8,6 +8,7 @@ declare(strict_types=1);
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
* @author Georg Ehrke <oc.list@georgehrke.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -42,10 +43,12 @@ interface INotificationProvider {
*
* @param VEvent $vevent
* @param string $calendarDisplayName
* @param string[] $principalEmailAddresses All email addresses associated to the principal owning the calendar object
* @param IUser[] $users
* @return void
*/
public function send(VEvent $vevent,
string $calendarDisplayName,
array $principalEmailAddresses,
array $users = []): void;
}

@ -10,6 +10,7 @@ declare(strict_types=1);
* @author Georg Ehrke <oc.list@georgehrke.com>
* @author Joas Schilling <coding@schilljs.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -89,11 +90,13 @@ abstract class AbstractProvider implements INotificationProvider {
*
* @param VEvent $vevent
* @param string $calendarDisplayName
* @param string[] $principalEmailAddresses
* @param IUser[] $users
* @return void
*/
abstract public function send(VEvent $vevent,
string $calendarDisplayName,
array $principalEmailAddresses,
array $users = []): void;
/**

@ -78,16 +78,28 @@ class EmailProvider extends AbstractProvider {
*
* @param VEvent $vevent
* @param string $calendarDisplayName
* @param string[] $principalEmailAddresses
* @param array $users
* @throws \Exception
*/
public function send(VEvent $vevent,
string $calendarDisplayName,
array $principalEmailAddresses,
array $users = []):void {
$fallbackLanguage = $this->getFallbackLanguage();
$organizerEmailAddress = null;
if (isset($vevent->ORGANIZER)) {
$organizerEmailAddress = $this->getEMailAddressOfAttendee($vevent->ORGANIZER);
}
$emailAddressesOfSharees = $this->getEMailAddressesOfAllUsersWithWriteAccessToCalendar($users);
$emailAddressesOfAttendees = $this->getAllEMailAddressesFromEvent($vevent);
$emailAddressesOfAttendees = [];
if (count($principalEmailAddresses) === 0
|| ($organizerEmailAddress && in_array($organizerEmailAddress, $principalEmailAddresses, true))
) {
$emailAddressesOfAttendees = $this->getAllEMailAddressesFromEvent($vevent);
}
// Quote from php.net:
// If the input arrays have the same string keys, then the later value for that key will overwrite the previous one.

@ -10,6 +10,7 @@ declare(strict_types=1);
* @author Georg Ehrke <oc.list@georgehrke.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Citharel <nextcloud@tcit.fr>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -81,11 +82,13 @@ class PushProvider extends AbstractProvider {
*
* @param VEvent $vevent
* @param string $calendarDisplayName
* @param string[] $principalEmailAddresses
* @param IUser[] $users
* @throws \Exception
*/
public function send(VEvent $vevent,
string $calendarDisplayName = null,
string $calendarDisplayName,
array $principalEmailAddresses,
array $users = []):void {
if ($this->config->getAppValue('dav', 'sendEventRemindersPush', 'no') !== 'yes') {
return;

@ -11,6 +11,7 @@ declare(strict_types=1);
* @author Joas Schilling <coding@schilljs.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Citharel <nextcloud@tcit.fr>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -32,6 +33,7 @@ namespace OCA\DAV\CalDAV\Reminder;
use DateTimeImmutable;
use OCA\DAV\CalDAV\CalDavBackend;
use OCA\DAV\Connector\Sabre\Principal;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IGroup;
use OCP\IGroupManager;
@ -66,6 +68,9 @@ class ReminderService {
/** @var ITimeFactory */
private $timeFactory;
/** @var Principal */
private $principalConnector;
public const REMINDER_TYPE_EMAIL = 'EMAIL';
public const REMINDER_TYPE_DISPLAY = 'DISPLAY';
public const REMINDER_TYPE_AUDIO = 'AUDIO';
@ -90,19 +95,22 @@ class ReminderService {
* @param IGroupManager $groupManager
* @param CalDavBackend $caldavBackend
* @param ITimeFactory $timeFactory
* @param Principal $principalConnector
*/
public function __construct(Backend $backend,
NotificationProviderManager $notificationProviderManager,
IUserManager $userManager,
IGroupManager $groupManager,
CalDavBackend $caldavBackend,
ITimeFactory $timeFactory) {
ITimeFactory $timeFactory,
Principal $principalConnector) {
$this->backend = $backend;
$this->notificationProviderManager = $notificationProviderManager;
$this->userManager = $userManager;
$this->groupManager = $groupManager;
$this->caldavBackend = $caldavBackend;
$this->timeFactory = $timeFactory;
$this->principalConnector = $principalConnector;
}
/**
@ -151,8 +159,14 @@ class ReminderService {
$users[] = $user;
}
$userPrincipalEmailAddresses = [];
$userPrincipal = $this->principalConnector->getPrincipalByPath($reminder['principaluri']);
if ($userPrincipal) {
$userPrincipalEmailAddresses = $this->principalConnector->getEmailAddressesOfPrincipal($userPrincipal);
}
$notificationProvider = $this->notificationProviderManager->getProvider($reminder['type']);
$notificationProvider->send($vevent, $reminder['displayname'], $users);
$notificationProvider->send($vevent, $reminder['displayname'], $userPrincipalEmailAddresses, $users);
$this->deleteOrProcessNext($reminder, $vevent);
}

@ -9,6 +9,7 @@
* @author Joas Schilling <coding@schilljs.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Citharel <nextcloud@tcit.fr>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -45,6 +46,7 @@ use Sabre\VObject\Component;
use Sabre\VObject\Component\VCalendar;
use Sabre\VObject\Component\VEvent;
use Sabre\VObject\DateTimeParser;
use Sabre\VObject\Document;
use Sabre\VObject\FreeBusyGenerator;
use Sabre\VObject\ITip;
use Sabre\VObject\Parameter;
@ -153,6 +155,14 @@ class Plugin extends \Sabre\CalDAV\Schedule\Plugin {
* @inheritDoc
*/
public function scheduleLocalDelivery(ITip\Message $iTipMessage):void {
/** @var Component|null $vevent */
$vevent = $iTipMessage->message->VEVENT ?? null;
// Strip VALARMs from incoming VEVENT
if ($vevent && isset($vevent->VALARM)) {
$vevent->remove('VALARM');
}
parent::scheduleLocalDelivery($iTipMessage);
// We only care when the message was successfully delivered locally
@ -189,18 +199,10 @@ class Plugin extends \Sabre\CalDAV\Schedule\Plugin {
return;
}
if (!isset($iTipMessage->message)) {
if (!$vevent) {
return;
}
$vcalendar = $iTipMessage->message;
if (!isset($vcalendar->VEVENT)) {
return;
}
/** @var Component $vevent */
$vevent = $vcalendar->VEVENT;
// We don't support autoresponses for recurrencing events for now
if (isset($vevent->RRULE) || isset($vevent->RDATE)) {
return;

@ -585,4 +585,44 @@ class Principal implements BackendInterface {
return [];
}
/**
* Get all email addresses associated to a principal.
*
* @param array $principal Data from getPrincipal*()
* @return string[] All email addresses without the mailto: prefix
*/
public function getEmailAddressesOfPrincipal(array $principal): array {
$emailAddresses = [];
if (($primaryAddress = $principal['{http://sabredav.org/ns}email-address'])) {
$emailAddresses[] = $primaryAddress;
}
if (isset($principal['{DAV:}alternate-URI-set'])) {
foreach ($principal['{DAV:}alternate-URI-set'] as $address) {
if (str_starts_with($address, 'mailto:')) {
$emailAddresses[] = substr($address, 7);
}
}
}
if (isset($principal['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set'])) {
foreach ($principal['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set'] as $address) {
if (str_starts_with($address, 'mailto:')) {
$emailAddresses[] = substr($address, 7);
}
}
}
if (isset($principal['{http://calendarserver.org/ns/}email-address-set'])) {
foreach ($principal['{http://calendarserver.org/ns/}email-address-set'] as $address) {
if (str_starts_with($address, 'mailto:')) {
$emailAddresses[] = substr($address, 7);
}
}
}
return array_values(array_unique($emailAddresses));
}
}

@ -81,6 +81,7 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
public function testSendWithoutAttendees():void {
[$user1, $user2, $user3, , $user5] = $users = $this->getUsers();
$principalEmailAddresses = [$user1->getEmailAddress()];
$enL10N = $this->createMock(IL10N::class);
$enL10N->method('t')
@ -186,11 +187,12 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
$this->setupURLGeneratorMock(2);
$vcalendar = $this->getNoAttendeeVCalendar();
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users);
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
}
public function testSendWithAttendees(): void {
public function testSendWithAttendeesWhenOwnerIsOrganizer(): void {
[$user1, $user2, $user3, , $user5] = $users = $this->getUsers();
$principalEmailAddresses = [$user1->getEmailAddress()];
$enL10N = $this->createMock(IL10N::class);
$enL10N->method('t')
@ -282,7 +284,81 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
$this->setupURLGeneratorMock(2);
$vcalendar = $this->getAttendeeVCalendar();
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users);
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
}
public function testSendWithAttendeesWhenOwnerIsAttendee(): void {
[$user1, $user2, $user3] = $this->getUsers();
$users = [$user2, $user3];
$principalEmailAddresses = [$user2->getEmailAddress()];
$deL10N = $this->createMock(IL10N::class);
$deL10N->method('t')
->willReturnArgument(0);
$deL10N->method('l')
->willReturnArgument(0);
$this->l10nFactory
->method('getUserLanguage')
->willReturnMap([
[$user2, 'de'],
[$user3, 'de'],
]);
$this->l10nFactory
->method('findGenericLanguage')
->willReturn('en');
$this->l10nFactory
->method('languageExists')
->willReturnMap([
['dav', 'de', true],
]);
$this->l10nFactory
->method('get')
->willReturnMap([
['dav', 'de', null, $deL10N],
]);
$template1 = $this->getTemplateMock();
$message12 = $this->getMessageMock('uid2@example.com', $template1);
$message13 = $this->getMessageMock('uid3@example.com', $template1);
$this->mailer->expects(self::once())
->method('createEMailTemplate')
->with('dav.calendarReminder')
->willReturnOnConsecutiveCalls(
$template1,
);
$this->mailer->expects($this->atLeastOnce())
->method('validateMailAddress')
->willReturnMap([
['foo1@example.org', true],
['foo3@example.org', true],
['foo4@example.org', true],
['uid1@example.com', true],
['uid2@example.com', true],
['uid3@example.com', true],
['invalid', false],
]);
$this->mailer->expects($this->exactly(2))
->method('createMessage')
->with()
->willReturnOnConsecutiveCalls(
$message12,
$message13,
);
$this->mailer->expects($this->exactly(2))
->method('send')
->withConsecutive(
[$message12],
[$message13],
)->willReturn([]);
$this->setupURLGeneratorMock(1);
$vcalendar = $this->getAttendeeVCalendar();
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
}
/**
@ -392,6 +468,14 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
'DESCRIPTION' => 'DESCRIPTION 456',
]);
$vcalendar->VEVENT->add(
'ORGANIZER',
'mailto:uid1@example.com',
[
'LANG' => 'en'
]
);
$vcalendar->VEVENT->add(
'ATTENDEE',
'mailto:foo1@example.org',

@ -10,6 +10,7 @@ declare(strict_types=1);
* @author Georg Ehrke <oc.list@georgehrke.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Citharel <nextcloud@tcit.fr>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -107,7 +108,7 @@ class PushProviderTest extends AbstractNotificationProviderTest {
$users = [$user1, $user2, $user3];
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, $users);
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, [], $users);
}
public function testSend(): void {
@ -160,7 +161,7 @@ class PushProviderTest extends AbstractNotificationProviderTest {
->method('notify')
->with($notification3);
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, $users);
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, [], $users);
}
/**

@ -9,6 +9,7 @@ declare(strict_types=1);
* @author Georg Ehrke <oc.list@georgehrke.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Citharel <nextcloud@tcit.fr>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -33,6 +34,7 @@ use OCA\DAV\CalDAV\Reminder\Backend;
use OCA\DAV\CalDAV\Reminder\INotificationProvider;
use OCA\DAV\CalDAV\Reminder\NotificationProviderManager;
use OCA\DAV\CalDAV\Reminder\ReminderService;
use OCA\DAV\Connector\Sabre\Principal;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IGroupManager;
use OCP\IUser;
@ -66,6 +68,9 @@ class ReminderServiceTest extends TestCase {
/** @var ReminderService */
private $reminderService;
/** @var Principal|\PHPUnit\Framework\MockObject\MockObject */
private $principalConnector;
public const CALENDAR_DATA = <<<EOD
BEGIN:VCALENDAR
PRODID:-//Nextcloud calendar v1.6.4
@ -194,6 +199,7 @@ EOD;
$this->groupManager = $this->createMock(IGroupManager::class);
$this->caldavBackend = $this->createMock(CalDavBackend::class);
$this->timeFactory = $this->createMock(ITimeFactory::class);
$this->principalConnector = $this->createMock(Principal::class);
$this->caldavBackend->method('getShares')->willReturn([]);
@ -202,7 +208,8 @@ EOD;
$this->userManager,
$this->groupManager,
$this->caldavBackend,
$this->timeFactory);
$this->timeFactory,
$this->principalConnector);
}
public function testOnCalendarObjectDelete():void {

@ -11,6 +11,7 @@
* @author Morris Jobke <hey@morrisjobke.de>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Müller <thomas.mueller@tmit.eu>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license AGPL-3.0
*
@ -896,4 +897,34 @@ class PrincipalTest extends TestCase {
['mailto:user3@foo.bar', 'user3@foo.bar', 'principals/users/user3'],
];
}
public function testGetEmailAddressesOfPrincipal(): void {
$principal = [
'{http://sabredav.org/ns}email-address' => 'bar@company.org',
'{DAV:}alternate-URI-set' => [
'/some/url',
'mailto:foo@bar.com',
'mailto:duplicate@example.com',
],
'{urn:ietf:params:xml:ns:caldav}calendar-user-address-set' => [
'mailto:bernard@example.com',
'mailto:bernard.desruisseaux@example.com',
],
'{http://calendarserver.org/ns/}email-address-set' => [
'mailto:duplicate@example.com',
'mailto:user@some.org',
],
];
$expected = [
'bar@company.org',
'foo@bar.com',
'duplicate@example.com',
'bernard@example.com',
'bernard.desruisseaux@example.com',
'user@some.org',
];
$actual = $this->connector->getEmailAddressesOfPrincipal($principal);
$this->assertEquals($expected, $actual);
}
}

Loading…
Cancel
Save