Skip to content

Commit dd038fb

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
1 parent 1e7b45d commit dd038fb

2 files changed

Lines changed: 175 additions & 40 deletions

File tree

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

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

9+
use OCA\Federation\TrustedServers;
10+
use OCA\Files_Sharing\Config\ConfigLexicon;
911
use OCP\Collaboration\Collaborators\ISearchPlugin;
1012
use OCP\Collaboration\Collaborators\ISearchResult;
1113
use OCP\Collaboration\Collaborators\SearchResultType;
1214
use OCP\Contacts\IManager;
1315
use OCP\Federation\ICloudIdManager;
16+
use OCP\IAppConfig;
1417
use OCP\IConfig;
1518
use OCP\IUserManager;
1619
use OCP\IUserSession;
@@ -27,11 +30,45 @@ public function __construct(
2730
private IConfig $config,
2831
private IUserManager $userManager,
2932
IUserSession $userSession,
33+
private ?IAppConfig $appConfig = null,
34+
private ?TrustedServers $trustedServers = null,
3035
) {
3136
$this->userId = $userSession->getUser()?->getUID() ?? '';
3237
$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
3338
}
3439

40+
private function shouldOnlyShowTrustedServers(): bool {
41+
if ($this->appConfig === null) {
42+
return false;
43+
}
44+
45+
$showFederatedToTrustedAsInternal = $this->appConfig->getValueBool(
46+
'files_sharing',
47+
ConfigLexicon::SHOW_FEDERATED_TO_TRUSTED_AS_INTERNAL,
48+
false
49+
);
50+
$showFederatedAsInternal = $this->appConfig->getValueBool(
51+
'files_sharing',
52+
ConfigLexicon::SHOW_FEDERATED_AS_INTERNAL,
53+
false
54+
);
55+
56+
return $showFederatedToTrustedAsInternal && !$showFederatedAsInternal;
57+
}
58+
59+
private function isServerTrusted(string $serverUrl): bool {
60+
if ($this->trustedServers === null) {
61+
return true;
62+
}
63+
64+
$normalizedUrl = $serverUrl;
65+
if (!str_contains($normalizedUrl, '://')) {
66+
$normalizedUrl = 'https://' . $normalizedUrl;
67+
}
68+
69+
return $this->trustedServers->isTrustedServer($normalizedUrl);
70+
}
71+
3572
public function search($search, $limit, $offset, ISearchResult $searchResult): bool {
3673
$result = ['wide' => [], 'exact' => []];
3774
$resultType = new SearchResultType('remotes');
@@ -82,33 +119,37 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
82119
];
83120
}
84121

85-
if (strtolower($contact['FN']) === $lowerSearch || strtolower($cloudId) === $lowerSearch) {
86-
if (strtolower($cloudId) === $lowerSearch) {
87-
$searchResult->markExactIdMatch($resultType);
122+
$shouldFilter = $this->shouldOnlyShowTrustedServers();
123+
124+
if (!$shouldFilter || $this->isServerTrusted($serverUrl)) {
125+
if (strtolower($contact['FN']) === $lowerSearch || strtolower($cloudId) === $lowerSearch) {
126+
if (strtolower($cloudId) === $lowerSearch) {
127+
$searchResult->markExactIdMatch($resultType);
128+
}
129+
$result['exact'][] = [
130+
'label' => $contact['FN'] . " ($cloudId)",
131+
'uuid' => $contact['UID'],
132+
'name' => $contact['FN'],
133+
'type' => $cloudIdType,
134+
'value' => [
135+
'shareType' => IShare::TYPE_REMOTE,
136+
'shareWith' => $cloudId,
137+
'server' => $serverUrl,
138+
],
139+
];
140+
} else {
141+
$result['wide'][] = [
142+
'label' => $contact['FN'] . " ($cloudId)",
143+
'uuid' => $contact['UID'],
144+
'name' => $contact['FN'],
145+
'type' => $cloudIdType,
146+
'value' => [
147+
'shareType' => IShare::TYPE_REMOTE,
148+
'shareWith' => $cloudId,
149+
'server' => $serverUrl,
150+
],
151+
];
88152
}
89-
$result['exact'][] = [
90-
'label' => $contact['FN'] . " ($cloudId)",
91-
'uuid' => $contact['UID'],
92-
'name' => $contact['FN'],
93-
'type' => $cloudIdType,
94-
'value' => [
95-
'shareType' => IShare::TYPE_REMOTE,
96-
'shareWith' => $cloudId,
97-
'server' => $serverUrl,
98-
],
99-
];
100-
} else {
101-
$result['wide'][] = [
102-
'label' => $contact['FN'] . " ($cloudId)",
103-
'uuid' => $contact['UID'],
104-
'name' => $contact['FN'],
105-
'type' => $cloudIdType,
106-
'value' => [
107-
'shareType' => IShare::TYPE_REMOTE,
108-
'shareWith' => $cloudId,
109-
'server' => $serverUrl,
110-
],
111-
];
112153
}
113154
}
114155
}
@@ -120,24 +161,25 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
120161
$result['wide'] = array_slice($result['wide'], $offset, $limit);
121162
}
122163

123-
/**
124-
* Add generic share with remote item for valid cloud ids that are not users of the local instance
125-
*/
126164
if (!$searchResult->hasExactIdMatch($resultType) && $this->cloudIdManager->isValidCloudId($search) && $offset === 0) {
127165
try {
128166
[$remoteUser, $serverUrl] = $this->splitUserRemote($search);
129167
$localUser = $this->userManager->get($remoteUser);
130168
if ($localUser === null || $search !== $localUser->getCloudId()) {
131-
$result['exact'][] = [
132-
'label' => $remoteUser . " ($serverUrl)",
133-
'uuid' => $remoteUser,
134-
'name' => $remoteUser,
135-
'value' => [
136-
'shareType' => IShare::TYPE_REMOTE,
137-
'shareWith' => $search,
138-
'server' => $serverUrl,
139-
],
140-
];
169+
$shouldFilter = $this->shouldOnlyShowTrustedServers();
170+
171+
if (!$shouldFilter || $this->isServerTrusted($serverUrl)) {
172+
$result['exact'][] = [
173+
'label' => $remoteUser . " ($serverUrl)",
174+
'uuid' => $remoteUser,
175+
'name' => $remoteUser,
176+
'value' => [
177+
'shareType' => IShare::TYPE_REMOTE,
178+
'shareWith' => $search,
179+
'server' => $serverUrl,
180+
],
181+
];
182+
}
141183
}
142184
} catch (\InvalidArgumentException $e) {
143185
}

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

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@
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;
@@ -36,6 +38,12 @@ class RemotePluginTest extends TestCase {
3638
/** @var ICloudIdManager|\PHPUnit\Framework\MockObject\MockObject */
3739
protected $cloudIdManager;
3840

41+
/** @var IAppConfig|\PHPUnit\Framework\MockObject\MockObject */
42+
protected $appConfig;
43+
44+
/** @var TrustedServers|\PHPUnit\Framework\MockObject\MockObject */
45+
protected $trustedServers;
46+
3947
/** @var RemotePlugin */
4048
protected $plugin;
4149

@@ -55,6 +63,8 @@ protected function setUp(): void {
5563
$this->createMock(IURLGenerator::class),
5664
$this->createMock(IUserManager::class),
5765
);
66+
$this->appConfig = $this->createMock(IAppConfig::class);
67+
$this->trustedServers = $this->createMock(TrustedServers::class);
5868
$this->searchResult = new SearchResult();
5969
}
6070

@@ -67,7 +77,7 @@ public function instantiatePlugin() {
6777
$userSession->expects($this->any())
6878
->method('getUser')
6979
->willReturn($user);
70-
$this->plugin = new RemotePlugin($this->contactsManager, $this->cloudIdManager, $this->config, $this->userManager, $userSession);
80+
$this->plugin = new RemotePlugin($this->contactsManager, $this->cloudIdManager, $this->config, $this->userManager, $userSession, $this->appConfig, $this->trustedServers);
7181
}
7282

7383
/**
@@ -427,4 +437,87 @@ public static function dataTestSplitUserRemoteError(): array {
427437
['us:erserver'],
428438
];
429439
}
440+
441+
public function testTrustedServerFiltering(): void {
442+
$this->appConfig->expects($this->any())
443+
->method('getValueBool')
444+
->willReturnCallback(function ($app, $key, $default) {
445+
if ($app === 'files_sharing' && $key === 'show_federated_shares_to_trusted_servers_as_internal') {
446+
return true;
447+
}
448+
if ($app === 'files_sharing' && $key === 'show_federated_shares_as_internal') {
449+
return false;
450+
}
451+
return $default;
452+
});
453+
454+
$this->trustedServers->expects($this->any())
455+
->method('isTrustedServer')
456+
->willReturnMap([
457+
['https://mail.example.com', false],
458+
['https://cloud.example.com', true],
459+
]);
460+
461+
$this->config->expects($this->any())
462+
->method('getAppValue')
463+
->willReturn('yes');
464+
465+
$this->contactsManager->expects($this->any())
466+
->method('search')
467+
->willReturn([]);
468+
469+
$this->userManager->expects($this->any())
470+
->method('get')
471+
->willReturn(null);
472+
473+
$this->instantiatePlugin();
474+
475+
$searchResult = new SearchResult();
476+
$this->plugin->search('user@mail.example.com', 10, 0, $searchResult);
477+
$results = $searchResult->asArray();
478+
$this->assertEmpty($results['remotes'], 'Non-trusted server should not appear in results');
479+
480+
$searchResult = new SearchResult();
481+
$this->plugin->search('user@cloud.example.com', 10, 0, $searchResult);
482+
$results = $searchResult->asArray();
483+
$this->assertNotEmpty($results['exact']['remotes'], 'Trusted server should appear in results');
484+
$this->assertEquals('user@cloud.example.com', $results['exact']['remotes'][0]['value']['shareWith']);
485+
}
486+
487+
public function testNoFilteringWhenShowAllFederatedAsInternal(): void {
488+
$this->appConfig->expects($this->any())
489+
->method('getValueBool')
490+
->willReturnCallback(function ($app, $key, $default) {
491+
if ($app === 'files_sharing' && $key === 'show_federated_shares_to_trusted_servers_as_internal') {
492+
return true;
493+
}
494+
if ($app === 'files_sharing' && $key === 'show_federated_shares_as_internal') {
495+
return true;
496+
}
497+
return $default;
498+
});
499+
500+
$this->trustedServers->expects($this->never())
501+
->method('isTrustedServer');
502+
503+
$this->config->expects($this->any())
504+
->method('getAppValue')
505+
->willReturn('yes');
506+
507+
$this->contactsManager->expects($this->any())
508+
->method('search')
509+
->willReturn([]);
510+
511+
$this->userManager->expects($this->any())
512+
->method('get')
513+
->willReturn(null);
514+
515+
$this->instantiatePlugin();
516+
517+
$searchResult = new SearchResult();
518+
$this->plugin->search('user@mail.example.com', 10, 0, $searchResult);
519+
$results = $searchResult->asArray();
520+
$this->assertNotEmpty($results['exact']['remotes'],
521+
'All federated servers should appear when show_federated_shares_as_internal is enabled');
522+
}
430523
}

0 commit comments

Comments
 (0)