sascha
/
nextcloud-server
mirror of https://github.com/nextcloud/server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #30600 from nextcloud/fix/30595/idn-email-share 

Fix idn emails not working in shares
pull/30597/head
Carl Schwan 2022-01-13 09:24:26 +07:00 committed by GitHub
parent 094fbb9c0d fffc19f5c3
commit b1048864f0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
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
Unescape Escape 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('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()]);
@ -679,7 +689,7 @@ class ShareByMailProvider implements IShareProvider {
* @param \DateTime|null $expirationTime
* @return int
*/
protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password, $sendPasswordByTalk, $hideDownload, $label, $expirationTime, $note = '') {
protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password, $sendPasswordByTalk, $hideDownload, $label, $expirationTime, $note = ''): int {
$qb = $this->dbConnection->getQueryBuilder();
$qb->insert('share')
->setValue('share_type', $qb->createNamedParameter(IShare::TYPE_EMAIL))
@ -774,7 +784,7 @@ class ShareByMailProvider implements IShareProvider {
} catch (\Exception $e) {
}
$this->removeShareFromTable($share->getId());
$this->removeShareFromTable((int)$share->getId());
}
/**
@ -970,9 +980,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
Unescape Escape 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');
@ -459,6 +459,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);
@ -483,6 +484,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);
@ -987,6 +989,7 @@ class ShareByMailProviderTest extends TestCase {
->willReturn(new \OC\Share20\Share($rootFolder, $userManager));
$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()));
@ -1033,6 +1036,7 @@ class ShareByMailProviderTest extends TestCase {
->willReturn(new \OC\Share20\Share($rootFolder, $userManager));
$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
Unescape Escape 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,8 +252,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
Unescape Escape 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,
]
];
}