From 9d0319446e1da38cf4f38a1cf22242bc525d21a4 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Mon, 25 Jan 2021 09:02:38 +0100 Subject: [PATCH] [CalDAV] Validate notified emails Signed-off-by: Thomas Citharel --- .../NotificationProvider/EmailProvider.php | 9 + .../EmailProviderTest.php | 191 +++++++++++------- 2 files changed, 126 insertions(+), 74 deletions(-) diff --git a/apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php b/apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php index 6b026753d6..db0633e230 100644 --- a/apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php +++ b/apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php @@ -114,6 +114,11 @@ class EmailProvider extends AbstractProvider { $template->addFooter(); foreach ($emailAddresses as $emailAddress) { + if (!$this->mailer->validateMailAddress($emailAddress)) { + $this->logger->error('Email address {address} for reminder notification is incorrect', ['app' => 'dav', 'address' => $emailAddress]); + continue; + } + $message = $this->mailer->createMessage(); $message->setFrom([$fromEMail]); if ($organizer) { @@ -196,6 +201,10 @@ class EmailProvider extends AbstractProvider { $organizerEMail = substr($organizer->getValue(), 7); + if (!$this->mailer->validateMailAddress($organizerEMail)) { + return null; + } + $name = $organizer->offsetGet('CN'); if ($name instanceof Parameter) { return [$organizerEMail => $name]; diff --git a/apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/EmailProviderTest.php b/apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/EmailProviderTest.php index 76b2a0c4f6..2639b5e514 100644 --- a/apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/EmailProviderTest.php +++ b/apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/EmailProviderTest.php @@ -80,28 +80,7 @@ class EmailProviderTest extends AbstractNotificationProviderTest { } public function testSendWithoutAttendees():void { - $user1 = $this->createMock(IUser::class); - $user1->method('getUID') - ->willReturn('uid1'); - $user1->method('getEMailAddress') - ->willReturn('uid1@example.com'); - $user2 = $this->createMock(IUser::class); - $user2->method('getUID') - ->willReturn('uid2'); - $user2->method('getEMailAddress') - ->willReturn('uid2@example.com'); - $user3 = $this->createMock(IUser::class); - $user3->method('getUID') - ->willReturn('uid3'); - $user3->method('getEMailAddress') - ->willReturn('uid3@example.com'); - $user4 = $this->createMock(IUser::class); - $user4->method('getUID') - ->willReturn('uid4'); - $user4->method('getEMailAddress') - ->willReturn(null); - - $users = [$user1, $user2, $user3, $user4]; + list($user1, $user2, $user3, , $user5) = $users = $this->getUsers(); $enL10N = $this->createMock(IL10N::class); $enL10N->method('t') @@ -121,6 +100,7 @@ class EmailProviderTest extends AbstractNotificationProviderTest { [$user1, 'en'], [$user2, 'de'], [$user3, 'de'], + [$user5, 'de'], ]); $this->l10nFactory @@ -153,36 +133,56 @@ class EmailProviderTest extends AbstractNotificationProviderTest { ->willReturn($template1); $this->mailer->expects($this->at(1)) + ->method('validateMailAddress') + ->with('uid1@example.com') + ->willReturn(true); + + $this->mailer->expects($this->at(2)) ->method('createMessage') ->with() ->willReturn($message11); - $this->mailer->expects($this->at(2)) + $this->mailer->expects($this->at(3)) ->method('send') ->with($message11) ->willReturn([]); - $this->mailer->expects($this->at(3)) + $this->mailer->expects($this->at(4)) ->method('createEMailTemplate') ->with('dav.calendarReminder') ->willReturn($template2); - $this->mailer->expects($this->at(4)) - ->method('createMessage') - ->with() - ->willReturn($message21); $this->mailer->expects($this->at(5)) - ->method('send') - ->with($message21) - ->willReturn([]); + ->method('validateMailAddress') + ->with('uid2@example.com') + ->willReturn(true); + $this->mailer->expects($this->at(6)) ->method('createMessage') ->with() - ->willReturn($message22); + ->willReturn($message21); $this->mailer->expects($this->at(7)) + ->method('send') + ->with($message21) + ->willReturn([]); + $this->mailer->expects($this->at(8)) + ->method('validateMailAddress') + ->with('uid3@example.com') + ->willReturn(true); + + $this->mailer->expects($this->at(9)) + ->method('createMessage') + ->with() + ->willReturn($message22); + $this->mailer->expects($this->at(10)) ->method('send') ->with($message22) ->willReturn([]); + $this->mailer->expects($this->at(11)) + ->method('validateMailAddress') + ->with('invalid') + ->willReturn(false); + $this->setupURLGeneratorMock(2); $vcalendar = $this->getNoAttendeeVCalendar(); @@ -190,28 +190,7 @@ class EmailProviderTest extends AbstractNotificationProviderTest { } public function testSendWithAttendees(): void { - $user1 = $this->createMock(IUser::class); - $user1->method('getUID') - ->willReturn('uid1'); - $user1->method('getEMailAddress') - ->willReturn('uid1@example.com'); - $user2 = $this->createMock(IUser::class); - $user2->method('getUID') - ->willReturn('uid2'); - $user2->method('getEMailAddress') - ->willReturn('uid2@example.com'); - $user3 = $this->createMock(IUser::class); - $user3->method('getUID') - ->willReturn('uid3'); - $user3->method('getEMailAddress') - ->willReturn('uid3@example.com'); - $user4 = $this->createMock(IUser::class); - $user4->method('getUID') - ->willReturn('uid4'); - $user4->method('getEMailAddress') - ->willReturn(null); - - $users = [$user1, $user2, $user3, $user4]; + list($user1, $user2, $user3, , $user5) = $users = $this->getUsers(); $enL10N = $this->createMock(IL10N::class); $enL10N->method('t') @@ -231,6 +210,7 @@ class EmailProviderTest extends AbstractNotificationProviderTest { [$user1, 'en'], [$user2, 'de'], [$user3, 'de'], + [$user5, 'de'], ]); $this->l10nFactory @@ -266,56 +246,89 @@ class EmailProviderTest extends AbstractNotificationProviderTest { ->willReturn($template1); $this->mailer->expects($this->at(1)) + ->method('validateMailAddress') + ->with('foo1@example.org') + ->willReturn(true); + + $this->mailer->expects($this->at(2)) ->method('createMessage') ->with() ->willReturn($message11); - $this->mailer->expects($this->at(2)) + $this->mailer->expects($this->at(3)) ->method('send') ->with($message11) ->willReturn([]); - $this->mailer->expects($this->at(3)) - ->method('createMessage') - ->with() - ->willReturn($message12); $this->mailer->expects($this->at(4)) - ->method('send') - ->with($message12) - ->willReturn([]); + ->method('validateMailAddress') + ->with('uid2@example.com') + ->willReturn(true); $this->mailer->expects($this->at(5)) ->method('createMessage') ->with() - ->willReturn($message13); + ->willReturn($message12); $this->mailer->expects($this->at(6)) ->method('send') - ->with($message13) + ->with($message12) ->willReturn([]); $this->mailer->expects($this->at(7)) - ->method('createEMailTemplate') - ->with('dav.calendarReminder') - ->willReturn($template2); + ->method('validateMailAddress') + ->with('uid3@example.com') + ->willReturn(true); $this->mailer->expects($this->at(8)) ->method('createMessage') ->with() - ->willReturn($message21); + ->willReturn($message13); $this->mailer->expects($this->at(9)) + ->method('send') + ->with($message13) + ->willReturn([]); + + $this->mailer->expects($this->at(10)) + ->method('validateMailAddress') + ->with('invalid') + ->willReturn(false); + + $this->mailer->expects($this->at(11)) + ->method('createEMailTemplate') + ->with('dav.calendarReminder') + ->willReturn($template2); + + $this->mailer->expects($this->at(12)) + ->method('validateMailAddress') + ->with('foo3@example.org') + ->willReturn(true); + + $this->mailer->expects($this->at(13)) + ->method('createMessage') + ->with() + ->willReturn($message21); + $this->mailer->expects($this->at(14)) ->method('send') ->with($message21) ->willReturn([]); - $this->mailer->expects($this->at(10)) + $this->mailer->expects($this->at(15)) + ->method('validateMailAddress') + ->with('foo4@example.org') + ->willReturn(true); + $this->mailer->expects($this->at(16)) ->method('createMessage') ->with() ->willReturn($message22); - $this->mailer->expects($this->at(11)) + $this->mailer->expects($this->at(17)) ->method('send') ->with($message22) ->willReturn([]); - $this->mailer->expects($this->at(12)) + $this->mailer->expects($this->at(18)) + ->method('validateMailAddress') + ->with('uid1@example.com') + ->willReturn(true); + $this->mailer->expects($this->at(19)) ->method('createMessage') ->with() ->willReturn($message23); - $this->mailer->expects($this->at(13)) + $this->mailer->expects($this->at(20)) ->method('send') ->with($message23) ->willReturn([]); @@ -376,9 +389,9 @@ class EmailProviderTest extends AbstractNotificationProviderTest { } /** - * @param array $toMail + * @param string $toMail * @param IEMailTemplate $templateMock - * @param array $replyTo + * @param array|null $replyTo * @return IMessage */ private function getMessageMock(string $toMail, IEMailTemplate $templateMock, array $replyTo=null):IMessage { @@ -524,4 +537,34 @@ class EmailProviderTest extends AbstractNotificationProviderTest { ->willReturn('AbsURL4'); } } + + private function getUsers(): array { + $user1 = $this->createMock(IUser::class); + $user1->method('getUID') + ->willReturn('uid1'); + $user1->method('getEMailAddress') + ->willReturn('uid1@example.com'); + $user2 = $this->createMock(IUser::class); + $user2->method('getUID') + ->willReturn('uid2'); + $user2->method('getEMailAddress') + ->willReturn('uid2@example.com'); + $user3 = $this->createMock(IUser::class); + $user3->method('getUID') + ->willReturn('uid3'); + $user3->method('getEMailAddress') + ->willReturn('uid3@example.com'); + $user4 = $this->createMock(IUser::class); + $user4->method('getUID') + ->willReturn('uid4'); + $user4->method('getEMailAddress') + ->willReturn(null); + $user5 = $this->createMock(IUser::class); + $user5->method('getUID') + ->willReturn('uid5'); + $user5->method('getEMailAddress') + ->willReturn('invalid'); + + return [$user1, $user2, $user3, $user4, $user5]; + } }