Add check before sending email that email address is valid

Signed-off-by: Carl Schwan <carl@carlschwan.eu>

4 files changed, 69 insertions, 16 deletions

diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php
index 62ba9d35f3f..65a7498bcd6 100644
--- a/apps/sharebymail/lib/ShareByMailProvider.php
+++ b/apps/sharebymail/lib/ShareByMailProvider.php
@@ -334,6 +334,16 @@ class ShareByMailProvider implements IShareProvider {
 			$share->getNote()
 		);
 
+		if ($this->mailer->validateMailAddress($share->getSharedWith())) {
+			$this->removeShareFromTable($shareId);
+			$e = new HintException('Failed to send share by mail. Got an invalid email address: ' . $share->getSharedWith(),
+				$this->l->t('Failed to send share by email. Got an invalid email address'));
+			$this->logger->error($e->getMessage(), [
+				'message' => 'Failed to send share by mail. Got an invalid email address ' . $share->getSharedWith(),
+				'app' => 'sharebymail',
+			]);
+		}
+
 		try {
 			$link = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare',
 				['token' => $share->getToken()]);
diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php
index bbe5516408d..45cab303669 100644
--- a/apps/sharebymail/tests/ShareByMailProviderTest.php
+++ b/apps/sharebymail/tests/ShareByMailProviderTest.php
@@ -217,7 +217,7 @@ class ShareByMailProviderTest extends TestCase {
 
 	public function testCreateSendPasswordByMailWithoutEnforcedPasswordProtection() {
 		$share = $this->getMockBuilder(IShare::class)->getMock();
-		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
+		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@examplelölöl.com');
 		$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
 		$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
 
diff --git a/lib/private/Collaboration/Collaborators/MailPlugin.php b/lib/private/Collaboration/Collaborators/MailPlugin.php
index 53a223b38dd..aae6f305981 100644
--- a/lib/private/Collaboration/Collaborators/MailPlugin.php
+++ b/lib/private/Collaboration/Collaborators/MailPlugin.php
@@ -38,6 +38,7 @@ use OCP\IGroupManager;
 use OCP\IUser;
 use OCP\IUserSession;
 use OCP\Share\IShare;
+use OCP\Mail\IMailer;
 
 class MailPlugin implements ISearchPlugin {
 	/* @var bool */
@@ -64,19 +65,23 @@ class MailPlugin implements ISearchPlugin {
 	private $knownUserService;
 	/** @var IUserSession */
 	private $userSession;
+	/** @var IMailer */
+	private $mailer;
 
 	public function __construct(IManager $contactsManager,
 								ICloudIdManager $cloudIdManager,
 								IConfig $config,
 								IGroupManager $groupManager,
 								KnownUserService $knownUserService,
-								IUserSession $userSession) {
+								IUserSession $userSession,
+								IMailer $mailer) {
 		$this->contactsManager = $contactsManager;
 		$this->cloudIdManager = $cloudIdManager;
 		$this->config = $config;
 		$this->groupManager = $groupManager;
 		$this->knownUserService = $knownUserService;
 		$this->userSession = $userSession;
+		$this->mailer = $mailer;
 
 		$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
 		$this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
@@ -247,14 +252,10 @@ class MailPlugin implements ISearchPlugin {
 			$userResults['wide'] = array_slice($userResults['wide'], $offset, $limit);
 		}
 
-		[$username, $domain] = explode('@', $search);
-		$domain = idn_to_ascii($domain);
-		$searchIdn = $username . '@' . $domain;
-
-		if (!$searchResult->hasExactIdMatch($emailType) && filter_var($searchIdn, FILTER_VALIDATE_EMAIL)) {
+		if (!$searchResult->hasExactIdMatch($emailType) && $this->mailer->validateMailAddress($search)) {
 			$result['exact'][] = [
 				'label' => $search,
-				'uuid' => $searchIdn,
+				'uuid' => $search,
 				'value' => [
 					'shareType' => IShare::TYPE_EMAIL,
 					'shareWith' => $search,
diff --git a/tests/lib/Collaboration/Collaborators/MailPluginTest.php b/tests/lib/Collaboration/Collaborators/MailPluginTest.php
index 2b71820af52..702c1d6be6e 100644
--- a/tests/lib/Collaboration/Collaborators/MailPluginTest.php
+++ b/tests/lib/Collaboration/Collaborators/MailPluginTest.php
@@ -37,6 +37,7 @@ use OCP\IUser;
 use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\Share\IShare;
+use OCP\Mail\IMailer;
 use Test\TestCase;
 
 class MailPluginTest extends TestCase {
@@ -64,6 +65,9 @@ class MailPluginTest extends TestCase {
 	/** @var  IUserSession|\PHPUnit\Framework\MockObject\MockObject */
 	protected $userSession;
 
+	/** @var IMailer|\PHPUnit\Framework\MockObject\MockObject */
+	protected $mailer;
+
 	protected function setUp(): void {
 		parent::setUp();
 
@@ -72,6 +76,7 @@ class MailPluginTest extends TestCase {
 		$this->groupManager = $this->createMock(IGroupManager::class);
 		$this->knownUserService = $this->createMock(KnownUserService::class);
 		$this->userSession = $this->createMock(IUserSession::class);
+		$this->mailer = $this->createMock(IMailer::class);
 		$this->cloudIdManager = new CloudIdManager($this->contactsManager, $this->createMock(IURLGenerator::class), $this->createMock(IUserManager::class));
 
 		$this->searchResult = new SearchResult();
@@ -84,7 +89,8 @@ class MailPluginTest extends TestCase {
 			$this->config,
 			$this->groupManager,
 			$this->knownUserService,
-			$this->userSession
+			$this->userSession,
+			$this->mailer
 		);
 	}
 
@@ -97,7 +103,7 @@ class MailPluginTest extends TestCase {
 	 * @param array $expected
 	 * @param bool $reachedEnd
 	 */
-	public function testSearch($searchTerm, $contacts, $shareeEnumeration, $expected, $exactIdMatch, $reachedEnd) {
+	public function testSearch($searchTerm, $contacts, $shareeEnumeration, $expected, $exactIdMatch, $reachedEnd, $validEmail) {
 		$this->config->expects($this->any())
 			->method('getAppValue')
 			->willReturnCallback(
@@ -117,6 +123,9 @@ class MailPluginTest extends TestCase {
 		$this->userSession->method('getUser')
 			->willReturn($currentUser);
 
+		$this->mailer->method('validateMailAddress')
+			->willReturn($validEmail);
+
 		$this->contactsManager->expects($this->any())
 			->method('search')
 			->willReturnCallback(function ($search, $searchAttributes) use ($searchTerm, $contacts) {
@@ -137,9 +146,9 @@ class MailPluginTest extends TestCase {
 	public function dataGetEmail() {
 		return [
 			// data set 0
-			['test', [], true, ['emails' => [], 'exact' => ['emails' => []]], false, false],
+			['test', [], true, ['emails' => [], 'exact' => ['emails' => []]], false, false, false],
 			// data set 1
-			['test', [], false, ['emails' => [], 'exact' => ['emails' => []]], false, false],
+			['test', [], false, ['emails' => [], 'exact' => ['emails' => []]], false, false, false],
 			// data set 2
 			[
 				'test@remote.com',
@@ -148,6 +157,7 @@ class MailPluginTest extends TestCase {
 				['emails' => [], 'exact' => ['emails' => [['uuid' => 'test@remote.com', 'label' => 'test@remote.com', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'test@remote.com']]]]],
 				false,
 				false,
+				true,
 			],
 			// data set 3
 			[ // no valid email address
@@ -157,6 +167,7 @@ class MailPluginTest extends TestCase {
 				['emails' => [], 'exact' => ['emails' => []]],
 				false,
 				false,
+				false,
 			],
 			// data set 4
 			[
@@ -166,6 +177,7 @@ class MailPluginTest extends TestCase {
 				['emails' => [], 'exact' => ['emails' => [['uuid' => 'test@remote.com', 'label' => 'test@remote.com', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'test@remote.com']]]]],
 				false,
 				false,
+				true,
 			],
 			// data set 5
 			[
@@ -193,6 +205,7 @@ class MailPluginTest extends TestCase {
 				['emails' => [['uuid' => 'uid1', 'name' => 'User @ Localhost', 'type' => '', 'label' => 'User @ Localhost (username@localhost)', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'username@localhost']]], 'exact' => ['emails' => []]],
 				false,
 				false,
+				false,
 			],
 			// data set 6
 			[
@@ -221,6 +234,7 @@ class MailPluginTest extends TestCase {
 				['emails' => [], 'exact' => ['emails' => []]],
 				false,
 				false,
+				false,
 			],
 			// data set 7
 			[
@@ -248,6 +262,7 @@ class MailPluginTest extends TestCase {
 				['emails' => [['uuid' => 'uid1', 'name' => 'User @ Localhost', 'type' => '', 'label' => 'User @ Localhost (username@localhost)', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'username@localhost']]], 'exact' => ['emails' => [['label' => 'test@remote.com', 'uuid' => 'test@remote.com', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'test@remote.com']]]]],
 				false,
 				false,
+				true,
 			],
 			// data set 8
 			[
@@ -276,6 +291,7 @@ class MailPluginTest extends TestCase {
 				['emails' => [], 'exact' => ['emails' => [['label' => 'test@remote.com', 'uuid' => 'test@remote.com', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'test@remote.com']]]]],
 				false,
 				false,
+				true,
 			],
 			// data set 9
 			[
@@ -303,6 +319,7 @@ class MailPluginTest extends TestCase {
 				['emails' => [], 'exact' => ['emails' => [['name' => 'User @ Localhost', 'uuid' => 'uid1', 'type' => '', 'label' => 'User @ Localhost (username@localhost)', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'username@localhost']]]]],
 				true,
 				false,
+				false,
 			],
 			// data set 10
 			[
@@ -330,6 +347,7 @@ class MailPluginTest extends TestCase {
 				['emails' => [], 'exact' => ['emails' => [['name' => 'User @ Localhost', 'uuid' => 'uid1', 'type' => '', 'label' => 'User @ Localhost (username@localhost)', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'username@localhost']]]]],
 				true,
 				false,
+				false,
 			],
 			// data set 11
 			// contact with space
@@ -358,6 +376,7 @@ class MailPluginTest extends TestCase {
 				['emails' => [], 'exact' => ['emails' => [['name' => 'User Name @ Localhost', 'uuid' => 'uid1', 'type' => '', 'label' => 'User Name @ Localhost (user name@localhost)', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'user name@localhost']]]]],
 				true,
 				false,
+				false,
 			],
 			// data set 12
 			// remote with space, no contact
@@ -387,6 +406,7 @@ class MailPluginTest extends TestCase {
 				['emails' => [], 'exact' => ['emails' => []]],
 				false,
 				false,
+				false,
 			],
 			// data set 13
 			// Local user found by email
@@ -405,6 +425,7 @@ class MailPluginTest extends TestCase {
 				['users' => [], 'exact' => ['users' => [['uuid' => 'uid1', 'name' => 'User', 'label' => 'User (test@example.com)','value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test'], 'shareWithDisplayNameUnique' => 'test@example.com']]]],
 				true,
 				false,
+				true,
 			],
 			// data set 14
 			// Current local user found by email => no result
@@ -423,6 +444,7 @@ class MailPluginTest extends TestCase {
 				['exact' => []],
 				false,
 				false,
+				true,
 			],
 			// data set 15
 			// Pagination and "more results" for user matches byyyyyyy emails
@@ -465,6 +487,7 @@ class MailPluginTest extends TestCase {
 				], 'emails' => [], 'exact' => ['users' => [], 'emails' => []]],
 				false,
 				true,
+				false,
 			],
 			// data set 16
 			// Pagination and "more results" for normal emails
@@ -503,6 +526,7 @@ class MailPluginTest extends TestCase {
 				], 'exact' => ['emails' => []]],
 				false,
 				true,
+				false,
 			],
 			// data set 17
 			// multiple email addresses with type
@@ -536,6 +560,18 @@ class MailPluginTest extends TestCase {
 				]]],
 				false,
 				false,
+				false,
+			],
+			// data set 18
+			// idn email
+			[
+				'test@lölölölölölölöl.com',
+				[],
+				true,
+				['emails' => [], 'exact' => ['emails' => [['uuid' => 'test@lölölölölölölöl.com', 'label' => 'test@lölölölölölölöl.com', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'test@lölölölölölölöl.com']]]]],
+				false,
+				false,
+				true,
 			],
 		];
 	}
@@ -550,7 +586,7 @@ class MailPluginTest extends TestCase {
 	 * @param bool $reachedEnd
 	 * @param array groups
 	 */
-	public function testSearchGroupsOnly($searchTerm, $contacts, $expected, $exactIdMatch, $reachedEnd, $userToGroupMapping) {
+	public function testSearchGroupsOnly($searchTerm, $contacts, $expected, $exactIdMatch, $reachedEnd, $userToGroupMapping, $validEmail) {
 		$this->config->expects($this->any())
 			->method('getAppValue')
 			->willReturnCallback(
@@ -573,6 +609,9 @@ class MailPluginTest extends TestCase {
 			->method('getUID')
 			->willReturn('currentUser');
 
+		$this->mailer->method('validateMailAddress')
+			->willReturn($validEmail);
+
 		$this->contactsManager->expects($this->any())
 			->method('search')
 			->willReturnCallback(function ($search, $searchAttributes) use ($searchTerm, $contacts) {
@@ -626,7 +665,8 @@ class MailPluginTest extends TestCase {
 				[
 					"currentUser" => ["group1"],
 					"User" => ["group1"]
-				]
+				],
+				false,
 			],
 			// The user `User` cannot share with the current user
 			[
@@ -646,7 +686,8 @@ class MailPluginTest extends TestCase {
 				[
 					"currentUser" => ["group1"],
 					"User" => ["group2"]
-				]
+				],
+				false,
 			],
 			// The user `User` cannot share with the current user, but there is an exact match on the e-mail address -> share by e-mail
 			[
@@ -666,7 +707,8 @@ class MailPluginTest extends TestCase {
 				[
 					"currentUser" => ["group1"],
 					"User" => ["group2"]
-				]
+				],
+				true,
 			]
 		];
 	}