Mirrors
/
nextcloud-server
mirror of https://github.com/nextcloud/server
3
0
Fork 0
Code Releases Activity
Browse Source

Add check before sending email that email address is valid 

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
pull/30600/head
Carl Schwan 4 years ago
parent
d4a5e480ab
commit
7ecb65f725
No known key found for this signature in database GPG Key ID: 6B35D38387B67BE
4 changed files with 69 additions and 16 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 10
      apps/sharebymail/lib/ShareByMailProvider.php
  2. 2
      apps/sharebymail/tests/ShareByMailProviderTest.php
  3. 15
      lib/private/Collaboration/Collaborators/MailPlugin.php
  4. 58
      tests/lib/Collaboration/Collaborators/MailPluginTest.php

10
apps/sharebymail/lib/ShareByMailProvider.php
View File

@ -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()]);

2
apps/sharebymail/tests/ShareByMailProviderTest.php
View File

@ -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');

15
lib/private/Collaboration/Collaborators/MailPlugin.php
View File

@ -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,

58
tests/lib/Collaboration/Collaborators/MailPluginTest.php
View File

@ -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,
]
];
}

Write Preview
Loading…
Cancel
Save