Skip to content

GitLab

Commit 659b5849 authored by David Toledo's avatar David Toledo Committed by GitHub
Browse files
Options

Merge pull request #26648 from owncloud/fedshare-dedup 

Skip generated remote share if exact user match found
parents 105c201e 49516b72
Hide whitespace changes
Inline Side-by-side
Showing with 121 additions and 11 deletions
apps/files_sharing/lib/Controller/ShareesController.php
View file @ 659b5849
......@@ -334,7 +334,14 @@ class ShareesController extends OCSController {
$this->result['remotes'] = [];
}
if (!$foundRemoteById && substr_count($search, '@') >= 1 && $this->offset === 0) {
if (!$foundRemoteById && substr_count($search, '@') >= 1 && $this->offset === 0
// if an exact local user is found, only keep the remote entry if
// its domain does not matches the trusted domains
// (if it does, it is a user whose local login domain matches the ownCloud
// instance domain)
&& (empty($this->result['exact']['users'])
|| !$this->isInstanceDomain($search))
) {
$this->result['exact']['remotes'][] = [
'label' => $search,
'value' => [
......@@ -554,4 +561,27 @@ class ShareesController extends OCSController {
protected function isV2() {
return $this->request->getScriptName() === '/ocs/v2.php';
}
/**
* Checks whether the given target's domain part matches one of the server's
* trusted domain entries
*
* @param string $target target
* @return true if one match was found, false otherwise
*/
protected function isInstanceDomain($target) {
if (strpos($target, '/') !== false) {
// not a proper email-like format with domain name
return false;
}
$parts = explode('@', $target);
if (count($parts) === 1) {
// no "@" sign
return false;
}
$domainName = $parts[count($parts) - 1];
$trustedDomains = $this->config->getSystemValue('trusted_domains', []);
return in_array($domainName, $trustedDomains, true);
}
}
apps/files_sharing/tests/API/ShareesTest.php
View file @ 659b5849
......@@ -64,6 +64,9 @@ class ShareesTest extends TestCase {
/** @var \OCP\IUserSession|\PHPUnit_Framework_MockObject_MockObject */
protected $session;
/** @var \OCP\IConfig|\PHPUnit_Framework_MockObject_MockObject */
protected $config;
/** @var \OCP\IRequest|\PHPUnit_Framework_MockObject_MockObject */
protected $request;
......@@ -97,13 +100,17 @@ class ShareesTest extends TestCase {
->disableOriginalConstructor()
->getMock();
$this->config = $this->getMockBuilder(IConfig::class)
->disableOriginalConstructor()
->getMock();
$this->sharees = new ShareesController(
'files_sharing',
$this->request,
$this->groupManager,
$this->userManager,
$this->contactsManager,
$this->getMockBuilder(IConfig::class)->disableOriginalConstructor()->getMock(),
$this->config,
$this->session,
$this->getMockBuilder(IURLGenerator::class)->disableOriginalConstructor()->getMock(),
$this->getMockBuilder(ILogger::class)->disableOriginalConstructor()->getMock(),
......@@ -823,8 +830,11 @@ class ShareesTest extends TestCase {
public function dataGetRemote() {
return [
// #0
['test', [], true, [], [], true],
// #1
['test', [], false, [], [], true],
// #2
[
'test@remote',
[],
......@@ -835,6 +845,7 @@ class ShareesTest extends TestCase {
[],
true,
],
// #3
[
'test@remote',
[],
......@@ -845,6 +856,7 @@ class ShareesTest extends TestCase {
[],
true,
],
// #4
[
'test',
[
......@@ -870,6 +882,7 @@ class ShareesTest extends TestCase {
],
true,
],
// #5
[
'test',
[
......@@ -893,6 +906,7 @@ class ShareesTest extends TestCase {
[],
true,
],
// #6
[
'test@remote',
[
......@@ -920,6 +934,7 @@ class ShareesTest extends TestCase {
],
true,
],
// #7
[
'test@remote',
[
......@@ -945,6 +960,7 @@ class ShareesTest extends TestCase {
[],
true,
],
// #8
[
'username@localhost',
[
......@@ -970,6 +986,7 @@ class ShareesTest extends TestCase {
[],
true,
],
// #9
[
'username@localhost',
[
......@@ -995,7 +1012,7 @@ class ShareesTest extends TestCase {
[],
true,
],
// contact with space
// #10: contact with space
[
'user name@localhost',
[
......@@ -1021,7 +1038,7 @@ class ShareesTest extends TestCase {
[],
true,
],
// remote with space, no contact
// #11: remote with space, no contact
[
'user space@remote',
[
......@@ -1047,6 +1064,60 @@ class ShareesTest extends TestCase {
[],
true,
],
// #12: if no user found and domain name matches local domain, keep remote entry
[
'test@trusted.domain.tld',
[],
true,
[
['label' => 'test@trusted.domain.tld', 'value' => ['shareType' => Share::SHARE_TYPE_REMOTE, 'shareWith' => 'test@trusted.domain.tld']],
],
[],
true,
[]
],
// #13: if no user found and domain name does not match local domain, keep remote entry
[
'test@domain.tld',
[],
true,
[
['label' => 'test@domain.tld', 'value' => ['shareType' => Share::SHARE_TYPE_REMOTE, 'shareWith' => 'test@domain.tld']],
],
[],
true,
[]
],
// #14: if exact user found and domain name matches local domain, skip generated remote entry
[
'test@trusted.domain.tld',
[],
true,
[],
[],
true,
[
'users' => [
['label' => 'test@domain.tld', 'value' => ['shareType' => Share::SHARE_TYPE_USER, 'shareWith' => 'test@domain.tld']],
]
]
],
// #15: if exact user found but with a non-local domain, keep the remote entry
[
'test@domain.tld',
[],
true,
[
['label' => 'test@domain.tld', 'value' => ['shareType' => Share::SHARE_TYPE_REMOTE, 'shareWith' => 'test@domain.tld']],
],
[],
true,
[
'users' => [
['label' => 'test@domain.tld', 'value' => ['shareType' => Share::SHARE_TYPE_USER, 'shareWith' => 'test@domain.tld']],
]
]
],
];
}
......@@ -1059,8 +1130,20 @@ class ShareesTest extends TestCase {
* @param array $exactExpected
* @param array $expected
* @param bool $reachedEnd
* @param array $previousExact
*/
public function testGetRemote($searchTerm, $contacts, $shareeEnumeration, $exactExpected, $expected, $reachedEnd) {
public function testGetRemote($searchTerm, $contacts, $shareeEnumeration, $exactExpected, $expected, $reachedEnd, $previousExact = []) {
$this->config->expects($this->any())
->method('getSystemValue')
->with('trusted_domains')
->willReturn(['trusted.domain.tld', 'trusted2.domain.tld']);
// inject previous results if needed
if (!empty($previousExact)) {
$result = $this->invokePrivate($this->sharees, 'result');
$result['exact'] = array_merge($result['exact'], $previousExact);
$this->invokePrivate($this->sharees, 'result', [$result]);
}
$this->invokePrivate($this->sharees, 'shareeEnumeration', [$shareeEnumeration]);
$this->contactsManager->expects($this->any())
->method('search')
......@@ -1191,10 +1274,7 @@ class ShareesTest extends TestCase {
* @param string $message
*/
public function testSearchInvalid($message, $search = '', $itemType = null, $page = 1, $perPage = 200) {
$config = $this->getMockBuilder(IConfig::class)
->disableOriginalConstructor()
->getMock();
$config->expects($this->never())
$this->config->expects($this->never())
->method('getAppValue');
/** @var ShareesController | \PHPUnit_Framework_MockObject_MockObject $sharees */
......@@ -1205,7 +1285,7 @@ class ShareesTest extends TestCase {
$this->groupManager,
$this->userManager,
$this->contactsManager,
$config,
$this->config,
$this->session,
$this->getMockBuilder('OCP\IURLGenerator')->disableOriginalConstructor()->getMock(),
$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock(),
......@@ -1354,7 +1434,7 @@ class ShareesTest extends TestCase {
$this->groupManager,
$this->userManager,
$this->contactsManager,
$this->getMockBuilder(IConfig::class)->disableOriginalConstructor()->getMock(),
$this->config,
$this->session,
$this->getMockBuilder(IURLGenerator::class)->disableOriginalConstructor()->getMock(),
$this->getMockBuilder(ILogger::class)->disableOriginalConstructor()->getMock(),
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment