Merge pull request #25310 from nextcloud/dav-validate-email-addresses

[CalDAV] Validate notified emails

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

@@ -115,6 +115,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) {
@@ -197,6 +202,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];

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

@@ -81,28 +81,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')
@@ -122,6 +101,7 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
 				[$user1, 'en'],
 				[$user2, 'de'],
 				[$user3, 'de'],
+				[$user5, 'de'],
 			]);
 
 		$this->l10nFactory
@@ -154,36 +134,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();
@@ -191,28 +191,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')
@@ -232,6 +211,7 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
 				[$user1, 'en'],
 				[$user2, 'de'],
 				[$user3, 'de'],
+				[$user5, 'de'],
 			]);
 
 		$this->l10nFactory
@@ -267,56 +247,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([]);
@@ -377,9 +390,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 {
@@ -525,4 +538,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];
+	}
 }