From a720bdb83dd4382c3c7c617508691277ffc28612 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 | 198 +++++++++++------- 2 files changed, 131 insertions(+), 76 deletions(-) diff --git a/apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php b/apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php index d0dc8f7f55..ea5ac1ded8 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 93673758ad..97162d0e37 100644 --- a/apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/EmailProviderTest.php +++ b/apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/EmailProviderTest.php @@ -78,29 +78,6 @@ 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]; - $this->config->expects($this->at(0)) ->method('getUserValue') ->with('uid1', 'core', 'lang', null) @@ -113,6 +90,10 @@ class EmailProviderTest extends AbstractNotificationProviderTest { ->method('getUserValue') ->with('uid3', 'core', 'lang', null) ->willReturn('de'); + $this->config->expects($this->at(3)) + ->method('getUserValue') + ->with('uid5', 'core', 'lang', null) + ->willReturn('de'); $enL10N = $this->createMock(IL10N::class); $enL10N->method('t') @@ -163,66 +144,63 @@ 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(); - $this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users); + $this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $this->getUsers()); } 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]; - $this->config->expects($this->at(0)) ->method('getUserValue') ->with('uid1', 'core', 'lang', null) @@ -235,6 +213,10 @@ class EmailProviderTest extends AbstractNotificationProviderTest { ->method('getUserValue') ->with('uid3', 'core', 'lang', null) ->willReturn('de'); + $this->config->expects($this->at(3)) + ->method('getUserValue') + ->with('uid5', 'core', 'lang', null) + ->willReturn('de'); $enL10N = $this->createMock(IL10N::class); $enL10N->method('t') @@ -273,10 +255,12 @@ class EmailProviderTest extends AbstractNotificationProviderTest { ->with('dav', 'en') ->willReturn($deL10N); + // German $template1 = $this->getTemplateMock(); $message11 = $this->getMessageMock('foo1@example.org', $template1); $message12 = $this->getMessageMock('uid2@example.com', $template1); $message13 = $this->getMessageMock('uid3@example.com', $template1); + // English $template2 = $this->getTemplateMock(); $message21 = $this->getMessageMock('foo3@example.org', $template2); $message22 = $this->getMessageMock('foo4@example.org', $template2); @@ -288,56 +272,88 @@ 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($message12) + ->willReturn([]); + $this->mailer->expects($this->at(7)) + ->method('validateMailAddress') + ->with('uid3@example.com') + ->willReturn(true); + $this->mailer->expects($this->at(8)) + ->method('createMessage') + ->with() + ->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(7)) + $this->mailer->expects($this->at(11)) ->method('createEMailTemplate') ->with('dav.calendarReminder') ->willReturn($template2); - $this->mailer->expects($this->at(8)) + $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(9)) + $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([]); @@ -345,7 +361,7 @@ 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, $this->getUsers()); } /** @@ -398,9 +414,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 { @@ -546,4 +562,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]; + } }