Skip to content

Commit a97d9e8

Browse files
st3inyst3iny
authored
Merge pull request #35019 from nextcloud/backport/34909/stable24
[stable24] Fix duplicate event email notifications
2 parents e9e248d + af21230 commit a97d9e8

11 files changed

Lines changed: 218 additions & 19 deletions

File tree

‎apps/dav/lib/CalDAV/Reminder/INotificationProvider.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
99
* @author Georg Ehrke <oc.list@georgehrke.com>
1010
* @author Roeland Jago Douma <roeland@famdouma.nl>
11+
* @author Richard Steinmetz <richard@steinmetz.cloud>
1112
*
1213
* @license GNU AGPL version 3 or any later version
1314
*
@@ -42,10 +43,12 @@ interface INotificationProvider {
4243
*
4344
* @param VEvent $vevent
4445
* @param string $calendarDisplayName
46+
* @param string[] $principalEmailAddresses All email addresses associated to the principal owning the calendar object
4547
* @param IUser[] $users
4648
* @return void
4749
*/
4850
public function send(VEvent $vevent,
4951
string $calendarDisplayName,
52+
array $principalEmailAddresses,
5053
array $users = []): void;
5154
}

‎apps/dav/lib/CalDAV/Reminder/NotificationProvider/AbstractProvider.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* @author Georg Ehrke <oc.list@georgehrke.com>
1111
* @author Joas Schilling <coding@schilljs.com>
1212
* @author Roeland Jago Douma <roeland@famdouma.nl>
13+
* @author Richard Steinmetz <richard@steinmetz.cloud>
1314
*
1415
* @license GNU AGPL version 3 or any later version
1516
*
@@ -89,11 +90,13 @@ public function __construct(ILogger $logger,
8990
*
9091
* @param VEvent $vevent
9192
* @param string $calendarDisplayName
93+
* @param string[] $principalEmailAddresses
9294
* @param IUser[] $users
9395
* @return void
9496
*/
9597
abstract public function send(VEvent $vevent,
9698
string $calendarDisplayName,
99+
array $principalEmailAddresses,
97100
array $users = []): void;
98101

99102
/**

‎apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,16 +78,28 @@ public function __construct(IConfig $config,
7878
*
7979
* @param VEvent $vevent
8080
* @param string $calendarDisplayName
81+
* @param string[] $principalEmailAddresses
8182
* @param array $users
8283
* @throws \Exception
8384
*/
8485
public function send(VEvent $vevent,
8586
string $calendarDisplayName,
87+
array $principalEmailAddresses,
8688
array $users = []):void {
8789
$fallbackLanguage = $this->getFallbackLanguage();
8890

91+
$organizerEmailAddress = null;
92+
if (isset($vevent->ORGANIZER)) {
93+
$organizerEmailAddress = $this->getEMailAddressOfAttendee($vevent->ORGANIZER);
94+
}
95+
8996
$emailAddressesOfSharees = $this->getEMailAddressesOfAllUsersWithWriteAccessToCalendar($users);
90-
$emailAddressesOfAttendees = $this->getAllEMailAddressesFromEvent($vevent);
97+
$emailAddressesOfAttendees = [];
98+
if (count($principalEmailAddresses) === 0
99+
|| ($organizerEmailAddress && in_array($organizerEmailAddress, $principalEmailAddresses, true))
100+
) {
101+
$emailAddressesOfAttendees = $this->getAllEMailAddressesFromEvent($vevent);
102+
}
91103

92104
// Quote from php.net:
93105
// If the input arrays have the same string keys, then the later value for that key will overwrite the previous one.

‎apps/dav/lib/CalDAV/Reminder/NotificationProvider/PushProvider.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* @author Georg Ehrke <oc.list@georgehrke.com>
1111
* @author Roeland Jago Douma <roeland@famdouma.nl>
1212
* @author Thomas Citharel <nextcloud@tcit.fr>
13+
* @author Richard Steinmetz <richard@steinmetz.cloud>
1314
*
1415
* @license GNU AGPL version 3 or any later version
1516
*
@@ -81,11 +82,13 @@ public function __construct(IConfig $config,
8182
*
8283
* @param VEvent $vevent
8384
* @param string $calendarDisplayName
85+
* @param string[] $principalEmailAddresses
8486
* @param IUser[] $users
8587
* @throws \Exception
8688
*/
8789
public function send(VEvent $vevent,
88-
string $calendarDisplayName = null,
90+
string $calendarDisplayName,
91+
array $principalEmailAddresses,
8992
array $users = []):void {
9093
if ($this->config->getAppValue('dav', 'sendEventRemindersPush', 'no') !== 'yes') {
9194
return;

‎apps/dav/lib/CalDAV/Reminder/ReminderService.php‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
* @author Joas Schilling <coding@schilljs.com>
1212
* @author Roeland Jago Douma <roeland@famdouma.nl>
1313
* @author Thomas Citharel <nextcloud@tcit.fr>
14+
* @author Richard Steinmetz <richard@steinmetz.cloud>
1415
*
1516
* @license GNU AGPL version 3 or any later version
1617
*
@@ -32,6 +33,7 @@
3233

3334
use DateTimeImmutable;
3435
use OCA\DAV\CalDAV\CalDavBackend;
36+
use OCA\DAV\Connector\Sabre\Principal;
3537
use OCP\AppFramework\Utility\ITimeFactory;
3638
use OCP\IConfig;
3739
use OCP\IGroup;
@@ -70,6 +72,9 @@ class ReminderService {
7072
/** @var IConfig */
7173
private $config;
7274

75+
/** @var Principal */
76+
private $principalConnector;
77+
7378
public const REMINDER_TYPE_EMAIL = 'EMAIL';
7479
public const REMINDER_TYPE_DISPLAY = 'DISPLAY';
7580
public const REMINDER_TYPE_AUDIO = 'AUDIO';
@@ -102,14 +107,16 @@ public function __construct(Backend $backend,
102107
IGroupManager $groupManager,
103108
CalDavBackend $caldavBackend,
104109
ITimeFactory $timeFactory,
105-
IConfig $config) {
110+
IConfig $config,
111+
Principal $principalConnector) {
106112
$this->backend = $backend;
107113
$this->notificationProviderManager = $notificationProviderManager;
108114
$this->userManager = $userManager;
109115
$this->groupManager = $groupManager;
110116
$this->caldavBackend = $caldavBackend;
111117
$this->timeFactory = $timeFactory;
112118
$this->config = $config;
119+
$this->principalConnector = $principalConnector;
113120
}
114121

115122
/**
@@ -163,8 +170,14 @@ public function processReminders():void {
163170
$users[] = $user;
164171
}
165172

173+
$userPrincipalEmailAddresses = [];
174+
$userPrincipal = $this->principalConnector->getPrincipalByPath($reminder['principaluri']);
175+
if ($userPrincipal) {
176+
$userPrincipalEmailAddresses = $this->principalConnector->getEmailAddressesOfPrincipal($userPrincipal);
177+
}
178+
166179
$notificationProvider = $this->notificationProviderManager->getProvider($reminder['type']);
167-
$notificationProvider->send($vevent, $reminder['displayname'], $users);
180+
$notificationProvider->send($vevent, $reminder['displayname'], $userPrincipalEmailAddresses, $users);
168181

169182
$this->deleteOrProcessNext($reminder, $vevent);
170183
}

‎apps/dav/lib/CalDAV/Schedule/Plugin.php‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* @author Joas Schilling <coding@schilljs.com>
1010
* @author Roeland Jago Douma <roeland@famdouma.nl>
1111
* @author Thomas Citharel <nextcloud@tcit.fr>
12+
* @author Richard Steinmetz <richard@steinmetz.cloud>
1213
*
1314
* @license GNU AGPL version 3 or any later version
1415
*
@@ -45,6 +46,7 @@
4546
use Sabre\VObject\Component\VCalendar;
4647
use Sabre\VObject\Component\VEvent;
4748
use Sabre\VObject\DateTimeParser;
49+
use Sabre\VObject\Document;
4850
use Sabre\VObject\FreeBusyGenerator;
4951
use Sabre\VObject\ITip;
5052
use Sabre\VObject\Parameter;
@@ -163,6 +165,14 @@ public function calendarObjectChange(RequestInterface $request, ResponseInterfac
163165
* @inheritDoc
164166
*/
165167
public function scheduleLocalDelivery(ITip\Message $iTipMessage):void {
168+
/** @var Component|null $vevent */
169+
$vevent = $iTipMessage->message->VEVENT ?? null;
170+
171+
// Strip VALARMs from incoming VEVENT
172+
if ($vevent && isset($vevent->VALARM)) {
173+
$vevent->remove('VALARM');
174+
}
175+
166176
parent::scheduleLocalDelivery($iTipMessage);
167177

168178
// We only care when the message was successfully delivered locally
@@ -199,18 +209,10 @@ public function scheduleLocalDelivery(ITip\Message $iTipMessage):void {
199209
return;
200210
}
201211

202-
if (!isset($iTipMessage->message)) {
212+
if (!$vevent) {
203213
return;
204214
}
205215

206-
$vcalendar = $iTipMessage->message;
207-
if (!isset($vcalendar->VEVENT)) {
208-
return;
209-
}
210-
211-
/** @var Component $vevent */
212-
$vevent = $vcalendar->VEVENT;
213-
214216
// We don't support autoresponses for recurrencing events for now
215217
if (isset($vevent->RRULE) || isset($vevent->RDATE)) {
216218
return;

‎apps/dav/lib/Connector/Sabre/Principal.php‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,4 +591,44 @@ public function getCircleMembership($principal):array {
591591

592592
return [];
593593
}
594+
595+
/**
596+
* Get all email addresses associated to a principal.
597+
*
598+
* @param array $principal Data from getPrincipal*()
599+
* @return string[] All email addresses without the mailto: prefix
600+
*/
601+
public function getEmailAddressesOfPrincipal(array $principal): array {
602+
$emailAddresses = [];
603+
604+
if (($primaryAddress = $principal['{http://sabredav.org/ns}email-address'])) {
605+
$emailAddresses[] = $primaryAddress;
606+
}
607+
608+
if (isset($principal['{DAV:}alternate-URI-set'])) {
609+
foreach ($principal['{DAV:}alternate-URI-set'] as $address) {
610+
if (str_starts_with($address, 'mailto:')) {
611+
$emailAddresses[] = substr($address, 7);
612+
}
613+
}
614+
}
615+
616+
if (isset($principal['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set'])) {
617+
foreach ($principal['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set'] as $address) {
618+
if (str_starts_with($address, 'mailto:')) {
619+
$emailAddresses[] = substr($address, 7);
620+
}
621+
}
622+
}
623+
624+
if (isset($principal['{http://calendarserver.org/ns/}email-address-set'])) {
625+
foreach ($principal['{http://calendarserver.org/ns/}email-address-set'] as $address) {
626+
if (str_starts_with($address, 'mailto:')) {
627+
$emailAddresses[] = substr($address, 7);
628+
}
629+
}
630+
}
631+
632+
return array_values(array_unique($emailAddresses));
633+
}
594634
}

‎apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/EmailProviderTest.php‎

Lines changed: 87 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ protected function setUp(): void {
8181

8282
public function testSendWithoutAttendees():void {
8383
[$user1, $user2, $user3, , $user5] = $users = $this->getUsers();
84+
$principalEmailAddresses = [$user1->getEmailAddress()];
8485

8586
$enL10N = $this->createMock(IL10N::class);
8687
$enL10N->method('t')
@@ -186,11 +187,12 @@ public function testSendWithoutAttendees():void {
186187
$this->setupURLGeneratorMock(2);
187188

188189
$vcalendar = $this->getNoAttendeeVCalendar();
189-
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users);
190+
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
190191
}
191192

192-
public function testSendWithAttendees(): void {
193+
public function testSendWithAttendeesWhenOwnerIsOrganizer(): void {
193194
[$user1, $user2, $user3, , $user5] = $users = $this->getUsers();
195+
$principalEmailAddresses = [$user1->getEmailAddress()];
194196

195197
$enL10N = $this->createMock(IL10N::class);
196198
$enL10N->method('t')
@@ -282,7 +284,81 @@ public function testSendWithAttendees(): void {
282284
$this->setupURLGeneratorMock(2);
283285

284286
$vcalendar = $this->getAttendeeVCalendar();
285-
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users);
287+
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
288+
}
289+
290+
public function testSendWithAttendeesWhenOwnerIsAttendee(): void {
291+
[$user1, $user2, $user3] = $this->getUsers();
292+
$users = [$user2, $user3];
293+
$principalEmailAddresses = [$user2->getEmailAddress()];
294+
295+
$deL10N = $this->createMock(IL10N::class);
296+
$deL10N->method('t')
297+
->willReturnArgument(0);
298+
$deL10N->method('l')
299+
->willReturnArgument(0);
300+
301+
$this->l10nFactory
302+
->method('getUserLanguage')
303+
->willReturnMap([
304+
[$user2, 'de'],
305+
[$user3, 'de'],
306+
]);
307+
308+
$this->l10nFactory
309+
->method('findGenericLanguage')
310+
->willReturn('en');
311+
312+
$this->l10nFactory
313+
->method('languageExists')
314+
->willReturnMap([
315+
['dav', 'de', true],
316+
]);
317+
318+
$this->l10nFactory
319+
->method('get')
320+
->willReturnMap([
321+
['dav', 'de', null, $deL10N],
322+
]);
323+
324+
$template1 = $this->getTemplateMock();
325+
$message12 = $this->getMessageMock('uid2@example.com', $template1);
326+
$message13 = $this->getMessageMock('uid3@example.com', $template1);
327+
328+
$this->mailer->expects(self::once())
329+
->method('createEMailTemplate')
330+
->with('dav.calendarReminder')
331+
->willReturnOnConsecutiveCalls(
332+
$template1,
333+
);
334+
$this->mailer->expects($this->atLeastOnce())
335+
->method('validateMailAddress')
336+
->willReturnMap([
337+
['foo1@example.org', true],
338+
['foo3@example.org', true],
339+
['foo4@example.org', true],
340+
['uid1@example.com', true],
341+
['uid2@example.com', true],
342+
['uid3@example.com', true],
343+
['invalid', false],
344+
]);
345+
$this->mailer->expects($this->exactly(2))
346+
->method('createMessage')
347+
->with()
348+
->willReturnOnConsecutiveCalls(
349+
$message12,
350+
$message13,
351+
);
352+
$this->mailer->expects($this->exactly(2))
353+
->method('send')
354+
->withConsecutive(
355+
[$message12],
356+
[$message13],
357+
)->willReturn([]);
358+
$this->setupURLGeneratorMock(1);
359+
360+
$vcalendar = $this->getAttendeeVCalendar();
361+
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
286362
}
287363

288364
/**
@@ -392,6 +468,14 @@ private function getAttendeeVCalendar():VCalendar {
392468
'DESCRIPTION' => 'DESCRIPTION 456',
393469
]);
394470

471+
$vcalendar->VEVENT->add(
472+
'ORGANIZER',
473+
'mailto:uid1@example.com',
474+
[
475+
'LANG' => 'en'
476+
]
477+
);
478+
395479
$vcalendar->VEVENT->add(
396480
'ATTENDEE',
397481
'mailto:foo1@example.org',

‎apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/PushProviderTest.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* @author Georg Ehrke <oc.list@georgehrke.com>
1111
* @author Roeland Jago Douma <roeland@famdouma.nl>
1212
* @author Thomas Citharel <nextcloud@tcit.fr>
13+
* @author Richard Steinmetz <richard@steinmetz.cloud>
1314
*
1415
* @license GNU AGPL version 3 or any later version
1516
*
@@ -107,7 +108,7 @@ public function testNotSend(): void {
107108

108109
$users = [$user1, $user2, $user3];
109110

110-
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, $users);
111+
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, [], $users);
111112
}
112113

113114
public function testSend(): void {
@@ -160,7 +161,7 @@ public function testSend(): void {
160161
->method('notify')
161162
->with($notification3);
162163

163-
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, $users);
164+
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, [], $users);
164165
}
165166

166167
/**

0 commit comments

Comments
 (0)