Skip to content

Commit f94d30c

Browse files
nfebenfebe
committed
fix(sharing): Adapt share suggestions to match trusted servers configs
When `show_federated_shares_to_trusted_servers_as_internal` is enabled but `show_federated_shares_as_internal` is not, filter federated share suggestions to only include trusted servers. Previously, searching for an email address would suggest non-trusted federated servers. Resolved: #54511 Signed-off-by: nfebe <[email protected]>
1 parent 9f1027b commit f94d30c

File tree

3 files changed

+71
-16
lines changed

3 files changed

+71
-16
lines changed

‎apps/files_sharing/src/components/SharingInput.vue‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,20 @@ export default {
457457
}
458458
},
459459
460+
/**
461+
* Filter suggestion results based on trusted server configuration
462+
*
463+
* @param {object} result The raw suggestion result from API
464+
* @return {boolean} Whether to include this result in suggestions
465+
*/
466+
filterByTrustedServer(result) {
467+
const isRemoteEntity = result.value.shareType === ShareType.Remote || result.value.shareType === ShareType.RemoteGroup
468+
if (isRemoteEntity && this.config.showFederatedSharesToTrustedServersAsInternal) {
469+
return result.value.isTrustedServer === true
470+
}
471+
return true
472+
},
473+
460474
/**
461475
* Format shares for the multiselect options
462476
*

‎lib/private/Collaboration/Collaborators/RemotePlugin.php‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
*/
77
namespace OC\Collaboration\Collaborators;
88

9+
use OCA\Federation\TrustedServers;
910
use OCP\Collaboration\Collaborators\ISearchPlugin;
1011
use OCP\Collaboration\Collaborators\ISearchResult;
1112
use OCP\Collaboration\Collaborators\SearchResultType;
1213
use OCP\Contacts\IManager;
1314
use OCP\Federation\ICloudIdManager;
15+
use OCP\IAppConfig;
1416
use OCP\IConfig;
1517
use OCP\IUserManager;
1618
use OCP\IUserSession;
@@ -27,11 +29,14 @@ public function __construct(
2729
private IConfig $config,
2830
private IUserManager $userManager,
2931
IUserSession $userSession,
32+
private IAppConfig $appConfig,
33+
private TrustedServers $trustedServers,
3034
) {
3135
$this->userId = $userSession->getUser()?->getUID() ?? '';
3236
$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
3337
}
3438

39+
3540
public function search($search, $limit, $offset, ISearchResult $searchResult): bool {
3641
$result = ['wide' => [], 'exact' => []];
3742
$resultType = new SearchResultType('remotes');
@@ -67,9 +72,6 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
6772
}
6873

6974
$localUser = $this->userManager->get($remoteUser);
70-
/**
71-
* Add local share if remote cloud id matches a local user ones
72-
*/
7375
if ($localUser !== null && $remoteUser !== $this->userId && $cloudId === $localUser->getCloudId()) {
7476
$result['wide'][] = [
7577
'label' => $contact['FN'],
@@ -95,6 +97,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
9597
'shareType' => IShare::TYPE_REMOTE,
9698
'shareWith' => $cloudId,
9799
'server' => $serverUrl,
100+
'isTrustedServer' => $this->trustedServers->isTrustedServer($serverUrl),
98101
],
99102
];
100103
} else {
@@ -107,6 +110,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
107110
'shareType' => IShare::TYPE_REMOTE,
108111
'shareWith' => $cloudId,
109112
'server' => $serverUrl,
113+
'isTrustedServer' => $this->trustedServers->isTrustedServer($serverUrl),
110114
],
111115
];
112116
}
@@ -120,9 +124,6 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
120124
$result['wide'] = array_slice($result['wide'], $offset, $limit);
121125
}
122126

123-
/**
124-
* Add generic share with remote item for valid cloud ids that are not users of the local instance
125-
*/
126127
if (!$searchResult->hasExactIdMatch($resultType) && $this->cloudIdManager->isValidCloudId($search) && $offset === 0) {
127128
try {
128129
[$remoteUser, $serverUrl] = $this->splitUserRemote($search);
@@ -136,6 +137,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
136137
'shareType' => IShare::TYPE_REMOTE,
137138
'shareWith' => $search,
138139
'server' => $serverUrl,
140+
'isTrustedServer' => $this->trustedServers->isTrustedServer($serverUrl),
139141
],
140142
];
141143
}

‎tests/lib/Collaboration/Collaborators/RemotePluginTest.php‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,20 @@
1010
use OC\Collaboration\Collaborators\RemotePlugin;
1111
use OC\Collaboration\Collaborators\SearchResult;
1212
use OC\Federation\CloudIdManager;
13+
use OCA\Federation\TrustedServers;
1314
use OCP\Collaboration\Collaborators\SearchResultType;
1415
use OCP\Contacts\IManager;
1516
use OCP\EventDispatcher\IEventDispatcher;
1617
use OCP\Federation\ICloudIdManager;
18+
use OCP\IAppConfig;
1719
use OCP\ICacheFactory;
1820
use OCP\IConfig;
1921
use OCP\IURLGenerator;
2022
use OCP\IUser;
2123
use OCP\IUserManager;
2224
use OCP\IUserSession;
2325
use OCP\Share\IShare;
26+
use PHPUnit\Framework\MockObject\MockObject;
2427
use Test\TestCase;
2528

2629
class RemotePluginTest extends TestCase {
@@ -36,6 +39,9 @@ class RemotePluginTest extends TestCase {
3639
/** @var ICloudIdManager|\PHPUnit\Framework\MockObject\MockObject */
3740
protected $cloudIdManager;
3841

42+
protected IAppConfig|MockObject $appConfig;
43+
protected ICloudIdManager|MockObject $trustedServers;
44+
3945
/** @var RemotePlugin */
4046
protected $plugin;
4147

@@ -55,6 +61,8 @@ protected function setUp(): void {
5561
$this->createMock(IURLGenerator::class),
5662
$this->createMock(IUserManager::class),
5763
);
64+
$this->appConfig = $this->createMock(IAppConfig::class);
65+
$this->trustedServers = $this->createMock(TrustedServers::class);
5866
$this->searchResult = new SearchResult();
5967
}
6068

@@ -67,7 +75,7 @@ public function instantiatePlugin() {
6775
$userSession->expects($this->any())
6876
->method('getUser')
6977
->willReturn($user);
70-
$this->plugin = new RemotePlugin($this->contactsManager, $this->cloudIdManager, $this->config, $this->userManager, $userSession);
78+
$this->plugin = new RemotePlugin($this->contactsManager, $this->cloudIdManager, $this->config, $this->userManager, $userSession, $this->appConfig, $this->trustedServers);
7179
}
7280

7381
/**
@@ -141,6 +149,37 @@ public function testSplitUserRemoteError($id): void {
141149
$this->plugin->splitUserRemote($id);
142150
}
143151

152+
public function testTrustedServerMetadata(): void {
153+
$this->config->expects($this->any())
154+
->method('getAppValue')
155+
->willReturnCallback(
156+
function ($appName, $key, $default) {
157+
if ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') {
158+
return 'yes';
159+
}
160+
return $default;
161+
}
162+
);
163+
164+
$this->trustedServers->expects($this->any())
165+
->method('isTrustedServer')
166+
->willReturnCallback(function ($serverUrl) {
167+
return $serverUrl === 'trustedserver.com';
168+
});
169+
170+
$this->instantiatePlugin();
171+
172+
$this->contactsManager->expects($this->any())
173+
->method('search')
174+
->willReturn([]);
175+
176+
$this->plugin->search('[email protected]', 2, 0, $this->searchResult);
177+
$result = $this->searchResult->asArray();
178+
179+
$this->assertNotEmpty($result['exact']['remotes']);
180+
$this->assertTrue($result['exact']['remotes'][0]['value']['isTrustedServer']);
181+
}
182+
144183
public static function dataGetRemote() {
145184
return [
146185
['test', [], true, ['remotes' => [], 'exact' => ['remotes' => []]], false, true],
@@ -149,15 +188,15 @@ public static function dataGetRemote() {
149188
'test@remote',
150189
[],
151190
true,
152-
['remotes' => [], 'exact' => ['remotes' => [['label' => 'test (remote)', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'test@remote', 'server' => 'remote'], 'uuid' => 'test', 'name' => 'test']]]],
191+
['remotes' => [], 'exact' => ['remotes' => [['label' => 'test (remote)', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'test@remote', 'server' => 'remote', 'isTrustedServer' => false], 'uuid' => 'test', 'name' => 'test']]]],
153192
false,
154193
true,
155194
],
156195
[
157196
'test@remote',
158197
[],
159198
false,
160-
['remotes' => [], 'exact' => ['remotes' => [['label' => 'test (remote)', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'test@remote', 'server' => 'remote'], 'uuid' => 'test', 'name' => 'test']]]],
199+
['remotes' => [], 'exact' => ['remotes' => [['label' => 'test (remote)', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'test@remote', 'server' => 'remote', 'isTrustedServer' => false], 'uuid' => 'test', 'name' => 'test']]]],
161200
false,
162201
true,
163202
],
@@ -183,7 +222,7 @@ public static function dataGetRemote() {
183222
],
184223
],
185224
true,
186-
['remotes' => [['name' => 'User @ Localhost', 'label' => 'User @ Localhost (username@localhost)', 'uuid' => 'uid1', 'type' => '', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'username@localhost', 'server' => 'localhost']]], 'exact' => ['remotes' => []]],
225+
['remotes' => [['name' => 'User @ Localhost', 'label' => 'User @ Localhost (username@localhost)', 'uuid' => 'uid1', 'type' => '', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'username@localhost', 'server' => 'localhost', 'isTrustedServer' => false]]], 'exact' => ['remotes' => []]],
187226
false,
188227
true,
189228
],
@@ -235,7 +274,7 @@ public static function dataGetRemote() {
235274
],
236275
],
237276
true,
238-
['remotes' => [['name' => 'User @ Localhost', 'label' => 'User @ Localhost (username@localhost)', 'uuid' => 'uid', 'type' => '', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'username@localhost', 'server' => 'localhost']]], 'exact' => ['remotes' => [['label' => 'test (remote)', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'test@remote', 'server' => 'remote'], 'uuid' => 'test', 'name' => 'test']]]],
277+
['remotes' => [['name' => 'User @ Localhost', 'label' => 'User @ Localhost (username@localhost)', 'uuid' => 'uid', 'type' => '', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'username@localhost', 'server' => 'localhost', 'isTrustedServer' => false]]], 'exact' => ['remotes' => [['label' => 'test (remote)', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'test@remote', 'server' => 'remote', 'isTrustedServer' => false], 'uuid' => 'test', 'name' => 'test']]]],
239278
false,
240279
true,
241280
],
@@ -261,7 +300,7 @@ public static function dataGetRemote() {
261300
],
262301
],
263302
false,
264-
['remotes' => [], 'exact' => ['remotes' => [['label' => 'test (remote)', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'test@remote', 'server' => 'remote'], 'uuid' => 'test', 'name' => 'test']]]],
303+
['remotes' => [], 'exact' => ['remotes' => [['label' => 'test (remote)', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'test@remote', 'server' => 'remote', 'isTrustedServer' => false], 'uuid' => 'test', 'name' => 'test']]]],
265304
false,
266305
true,
267306
],
@@ -287,7 +326,7 @@ public static function dataGetRemote() {
287326
],
288327
],
289328
true,
290-
['remotes' => [], 'exact' => ['remotes' => [['name' => 'User @ Localhost', 'label' => 'User @ Localhost (username@localhost)', 'uuid' => 'uid1', 'type' => '', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'username@localhost', 'server' => 'localhost']]]]],
329+
['remotes' => [], 'exact' => ['remotes' => [['name' => 'User @ Localhost', 'label' => 'User @ Localhost (username@localhost)', 'uuid' => 'uid1', 'type' => '', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'username@localhost', 'server' => 'localhost', 'isTrustedServer' => false]]]]],
291330
true,
292331
true,
293332
],
@@ -313,7 +352,7 @@ public static function dataGetRemote() {
313352
],
314353
],
315354
false,
316-
['remotes' => [], 'exact' => ['remotes' => [['name' => 'User @ Localhost', 'label' => 'User @ Localhost (username@localhost)', 'uuid' => 'uid1', 'type' => '', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'username@localhost', 'server' => 'localhost']]]]],
355+
['remotes' => [], 'exact' => ['remotes' => [['name' => 'User @ Localhost', 'label' => 'User @ Localhost (username@localhost)', 'uuid' => 'uid1', 'type' => '', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'username@localhost', 'server' => 'localhost', 'isTrustedServer' => false]]]]],
317356
true,
318357
true,
319358
],
@@ -340,7 +379,7 @@ public static function dataGetRemote() {
340379
],
341380
],
342381
false,
343-
['remotes' => [], 'exact' => ['remotes' => [['name' => 'User Name @ Localhost', 'label' => 'User Name @ Localhost (user name@localhost)', 'uuid' => 'uid3', 'type' => '', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'user name@localhost', 'server' => 'localhost']]]]],
382+
['remotes' => [], 'exact' => ['remotes' => [['name' => 'User Name @ Localhost', 'label' => 'User Name @ Localhost (user name@localhost)', 'uuid' => 'uid3', 'type' => '', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'user name@localhost', 'server' => 'localhost', 'isTrustedServer' => false]]]]],
344383
true,
345384
true,
346385
],
@@ -367,7 +406,7 @@ public static function dataGetRemote() {
367406
],
368407
],
369408
false,
370-
['remotes' => [], 'exact' => ['remotes' => [['label' => 'user space (remote)', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'user space@remote', 'server' => 'remote'], 'uuid' => 'user space', 'name' => 'user space']]]],
409+
['remotes' => [], 'exact' => ['remotes' => [['label' => 'user space (remote)', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'user space@remote', 'server' => 'remote', 'isTrustedServer' => false], 'uuid' => 'user space', 'name' => 'user space']]]],
371410
false,
372411
true,
373412
],

0 commit comments

Comments
 (0)