From 9d0319446e1da38cf4f38a1cf22242bc525d21a4 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Mon, 25 Jan 2021 09:02:38 +0100 Subject: [CalDAV] Validate notified emails Signed-off-by: Thomas Citharel --- .../NotificationProvider/EmailProvider.php | 9 ++ .../NotificationProvider/EmailProviderTest.php | 171 +++++++++++++-------- 2 files changed, 116 insertions(+), 64 deletions(-) (limited to 'apps/dav') diff --git a/apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php b/apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php index 6b026753d62..db0633e230f 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 76b2a0c4f65..2639b5e5145 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)) + $this->mailer->expects($this->at(5)) + ->method('validateMailAddress') + ->with('uid2@example.com') + ->willReturn(true); + + $this->mailer->expects($this->at(6)) ->method('createMessage') ->with() ->willReturn($message21); - $this->mailer->expects($this->at(5)) + $this->mailer->expects($this->at(7)) ->method('send') ->with($message21) ->willReturn([]); - $this->mailer->expects($this->at(6)) + $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(7)) + $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)) + $this->mailer->expects($this->at(4)) + ->method('validateMailAddress') + ->with('uid2@example.com') + ->willReturn(true); + $this->mailer->expects($this->at(5)) ->method('createMessage') ->with() ->willReturn($message12); - $this->mailer->expects($this->at(4)) + $this->mailer->expects($this->at(6)) ->method('send') ->with($message12) ->willReturn([]); - $this->mailer->expects($this->at(5)) + + $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(6)) + $this->mailer->expects($this->at(9)) ->method('send') ->with($message13) ->willReturn([]); - $this->mailer->expects($this->at(7)) + $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(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([]); @@ -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]; + } } -- cgit v1.2.3