upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-06-08 08:16:43 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 10

Fix idn emails not working in shares

Browse source 
And add check before sending email that email address is valid

Fix #30595

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
This commit is contained in:
Carl Schwan 2022-01-12 14:15:08 +01:00
parent 09c350c576
commit 2f10bc14f7
No known key found for this signature in database
GPG key ID: 06B35D38387B67BE
4 changed files with 76 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
apps/sharebymail/lib/ShareByMailProvider.php
View file

@ -325,6 +325,16 @@ class ShareByMailProvider implements IShareProvider {
$share->getExpirationDate()
);
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('Failed to send share by mail. Got an invalid email address ' . $share->getSharedWith(), [
'app' => 'sharebymail',
'exception' => $e,
]);
}
try {
$link = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare',
['token' => $share->getToken()]);
@ -653,7 +663,7 @@ class ShareByMailProvider implements IShareProvider {
* @param \DateTime|null $expirationTime
* @return int
*/
protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password, $sendPasswordByTalk, $hideDownload, $expirationTime) {
protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password, $sendPasswordByTalk, $hideDownload, $expirationTime): int {
$qb = $this->dbConnection->getQueryBuilder();
$qb->insert('share')
->setValue('share_type', $qb->createNamedParameter(IShare::TYPE_EMAIL))
@ -747,7 +757,7 @@ class ShareByMailProvider implements IShareProvider {
} catch (\Exception $e) {
}
$this->removeShareFromTable($share->getId());
$this->removeShareFromTable((int)$share->getId());
}
/**
@ -943,9 +953,9 @@ class ShareByMailProvider implements IShareProvider {
/**
* remove share from table
*
* @param string $shareId
* @param int $shareId
*/
protected function removeShareFromTable($shareId) {
protected function removeShareFromTable(int $shareId): void {
$qb = $this->dbConnection->getQueryBuilder();
$qb->delete('share')
->where($qb->expr()->eq('id', $qb->createNamedParameter($shareId)));

6
apps/sharebymail/tests/ShareByMailProviderTest.php
View file

@ -214,7 +214,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');
@ -456,6 +456,7 @@ class ShareByMailProviderTest extends TestCase {
public function testCreateMailShare() {
$this->share->expects($this->any())->method('getToken')->willReturn('token');
$this->share->expects($this->once())->method('setToken')->with('token');
$this->share->expects($this->any())->method('getSharedWith')->willReturn('valid@valid.com');
$node = $this->getMockBuilder('OCP\Files\Node')->getMock();
$node->expects($this->any())->method('getName')->willReturn('fileName');
$this->share->expects($this->any())->method('getNode')->willReturn($node);
@ -480,6 +481,7 @@ class ShareByMailProviderTest extends TestCase {
$this->share->expects($this->any())->method('getToken')->willReturn('token');
$this->share->expects($this->once())->method('setToken')->with('token');
$this->share->expects($this->any())->method('getSharedWith')->willReturn('valid@valid.com');
$node = $this->getMockBuilder('OCP\Files\Node')->getMock();
$node->expects($this->any())->method('getName')->willReturn('fileName');
$this->share->expects($this->any())->method('getNode')->willReturn($node);
@ -977,6 +979,7 @@ class ShareByMailProviderTest extends TestCase {
$rootFolder = \OC::$server->getRootFolder();
$provider = $this->getInstance(['sendMailNotification', 'createShareActivity']);
$this->mailer->expects($this->any())->method('validateMailAddress')->willReturn(true);
$u1 = $userManager->createUser('testFed', md5(time()));
$u2 = $userManager->createUser('testFed2', md5(time()));
@ -1019,6 +1022,7 @@ class ShareByMailProviderTest extends TestCase {
$rootFolder = \OC::$server->getRootFolder();
$provider = $this->getInstance(['sendMailNotification', 'createShareActivity']);
$this->mailer->expects($this->any())->method('validateMailAddress')->willReturn(true);
$u1 = $userManager->createUser('testFed', md5(time()));
$u2 = $userManager->createUser('testFed2', md5(time()));

10
lib/private/Collaboration/Collaborators/MailPlugin.php
View file

@ -39,6 +39,7 @@ use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserSession;
use OCP\Share\IShare;
use OCP\Mail\IMailer;
class MailPlugin implements ISearchPlugin {
/* @var bool */
@ -65,19 +66,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';
@ -251,8 +256,7 @@ class MailPlugin implements ISearchPlugin {
$userResults['wide'] = array_slice($userResults['wide'], $offset, $limit);
}
if (!$searchResult->hasExactIdMatch($emailType) && filter_var($search, FILTER_VALIDATE_EMAIL)) {
if (!$searchResult->hasExactIdMatch($emailType) && $this->mailer->validateMailAddress($search)) {
$result['exact'][] = [
'label' => $search,
'uuid' => $search,

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

@ -35,6 +35,7 @@ use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserSession;
use OCP\Share\IShare;
use OCP\Mail\IMailer;
use Test\TestCase;
class MailPluginTest extends TestCase {
@ -62,6 +63,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();
@ -70,6 +74,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->searchResult = new SearchResult();
@ -82,7 +87,8 @@ class MailPluginTest extends TestCase {
$this->config,
$this->groupManager,
$this->knownUserService,
$this->userSession
$this->userSession,
$this->mailer
);
}
@ -95,7 +101,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(
@ -115,6 +121,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) {
@ -135,9 +144,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',
@ -146,6 +155,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
@ -155,6 +165,7 @@ class MailPluginTest extends TestCase {
['emails' => [], 'exact' => ['emails' => []]],
false,
false,
false,
],
// data set 4
[
@ -164,6 +175,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
[
@ -191,6 +203,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
[
@ -218,6 +231,7 @@ class MailPluginTest extends TestCase {
['emails' => [], 'exact' => ['emails' => []]],
false,
false,
false,
],
// data set 7
[
@ -245,6 +259,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
[
@ -272,6 +287,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
[
@ -299,6 +315,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
[
@ -326,6 +343,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
@ -354,6 +372,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
@ -382,6 +401,7 @@ class MailPluginTest extends TestCase {
['emails' => [], 'exact' => ['emails' => []]],
false,
false,
false,
],
// data set 13
// Local user found by email
@ -400,6 +420,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
@ -418,6 +439,7 @@ class MailPluginTest extends TestCase {
['exact' => []],
false,
false,
true,
],
// data set 15
// Pagination and "more results" for user matches byyyyyyy emails
@ -460,6 +482,7 @@ class MailPluginTest extends TestCase {
], 'emails' => [], 'exact' => ['users' => [], 'emails' => []]],
false,
true,
false,
],
// data set 16
// Pagination and "more results" for normal emails
@ -498,6 +521,7 @@ class MailPluginTest extends TestCase {
], 'exact' => ['emails' => []]],
false,
true,
false,
],
// data set 17
// multiple email addresses with type
@ -531,6 +555,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,
],
];
}
@ -545,7 +581,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(
@ -568,6 +604,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) {
@ -621,7 +660,8 @@ class MailPluginTest extends TestCase {
[
"currentUser" => ["group1"],
"User" => ["group1"]
]
],
false,
],
// The user `User` cannot share with the current user
[
@ -641,7 +681,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
[
@ -661,7 +702,8 @@ class MailPluginTest extends TestCase {
[
"currentUser" => ["group1"],
"User" => ["group2"]
]
],
true,
]
];
}