From d2bf30b9ec11d5a2159244433be61de689f4bea8 Mon Sep 17 00:00:00 2001 From: niv Date: Thu, 11 Jun 2026 17:40:08 +0200 Subject: [PATCH] feat(sharing): allow email-based labels for lookup results Signed-off-by: niv --- .../Collaborators/LookupPlugin.php | 15 ++- .../Collaborators/LookupPluginTest.php | 119 ++++++++++++++++-- 2 files changed, 121 insertions(+), 13 deletions(-) diff --git a/lib/private/Collaboration/Collaborators/LookupPlugin.php b/lib/private/Collaboration/Collaborators/LookupPlugin.php index 9c48670daaaa4..d6aa7a0391521 100644 --- a/lib/private/Collaboration/Collaborators/LookupPlugin.php +++ b/lib/private/Collaboration/Collaborators/LookupPlugin.php @@ -39,6 +39,11 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b $isGlobalScaleEnabled = $this->config->getSystemValueBool('gs.enabled', false); $isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes'; $hasInternetConnection = $this->config->getSystemValueBool('has_internet_connection', true); + // App config flag (core app config): shareapi_lookup_label_show_email + // Controls whether email addresses from federated lookup results are shown in share dialog labels. + // Defaults to 'no' to avoid exposing email addresses unnecessarily. Enabling this may reveal + // users' email addresses to people using the share dialog and should therefore be considered carefully. + $showEmailInLabel = $this->config->getAppValue('core', 'shareapi_lookup_label_show_email', 'no') === 'yes'; // If case of Global Scale we always search the lookup server // TODO: Reconsider using the lookup server for non-global scale @@ -79,7 +84,15 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b continue; } $name = $lookup['name']['value'] ?? ''; - $label = empty($name) ? $lookup['federationId'] : $name . ' (' . $lookup['federationId'] . ')'; + $email = $lookup['email']['value'] ?? ''; + + if ($showEmailInLabel && $email !== '') { + $id = $email; + } else { + $id = $lookup['federationId']; + } + $label = $name === '' ? $id : $name . ' (' . $id . ')'; + $result[] = [ 'label' => $label, 'value' => [ diff --git a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php index 7f70c3ba3d734..ee5daa3777641 100644 --- a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php @@ -76,10 +76,12 @@ protected function setUp(): void { } public function testSearchNoLookupServerURI(): void { - $this->config->expects($this->once()) + $this->config->expects($this->exactly(2)) ->method('getAppValue') - ->with('files_sharing', 'lookupServerEnabled', 'no') - ->willReturn('yes'); + ->willReturnMap([ + ['files_sharing', 'lookupServerEnabled', 'no', 'yes'], + ['core', 'shareapi_lookup_label_show_email', 'no', 'no'], + ]); $this->config->expects($this->exactly(2)) ->method('getSystemValueBool') ->willReturnMap([ @@ -102,10 +104,12 @@ public function testSearchNoLookupServerURI(): void { } public function testSearchNoInternet(): void { - $this->config->expects($this->once()) + $this->config->expects($this->exactly(2)) ->method('getAppValue') - ->with('files_sharing', 'lookupServerEnabled', 'no') - ->willReturn('yes'); + ->willReturnMap([ + ['files_sharing', 'lookupServerEnabled', 'no', 'yes'], + ['core', 'shareapi_lookup_label_show_email', 'no', 'no'], + ]); $this->config->expects($this->exactly(2)) ->method('getSystemValueBool') ->willReturnMap([ @@ -135,10 +139,12 @@ public function testSearch(array $searchParams): void { ->method('addResultSet') ->with($type, $searchParams['expectedResult'], []); - $this->config->expects($this->once()) + $this->config->expects($this->exactly(2)) ->method('getAppValue') - ->with('files_sharing', 'lookupServerEnabled', 'no') - ->willReturn('yes'); + ->willReturnMap([ + ['files_sharing', 'lookupServerEnabled', 'no', 'yes'], + ['core', 'shareapi_lookup_label_show_email', 'no', 'no'], + ]); $this->config->expects($this->exactly(2)) ->method('getSystemValueBool') ->willReturnMap([ @@ -179,6 +185,93 @@ public function testSearch(array $searchParams): void { $this->assertFalse($moreResults); } + #[\PHPUnit\Framework\Attributes\DataProvider('dataSearchEmailLabel')] + public function testSearchEmailLabel(string $showEmailFlag, array $lookupEntry, string $expectedLabel): void { + $type = new SearchResultType('lookup'); + $expectedResult = [ + [ + 'label' => $expectedLabel, + 'value' => [ + 'shareType' => IShare::TYPE_REMOTE, + 'globalScale' => true, + 'shareWith' => $lookupEntry['federationId'], + 'server' => 'enceladus.moon', + 'isTrustedServer' => false, + ], + 'extra' => $lookupEntry, + ], + ]; + + /** @var ISearchResult|MockObject $searchResult */ + $searchResult = $this->createMock(ISearchResult::class); + $searchResult->expects($this->once()) + ->method('addResultSet') + ->with($type, $expectedResult, []); + + $this->config->expects($this->exactly(2)) + ->method('getAppValue') + ->willReturnMap([ + ['files_sharing', 'lookupServerEnabled', 'no', 'yes'], + ['core', 'shareapi_lookup_label_show_email', 'no', $showEmailFlag], + ]); + $this->config->expects($this->exactly(2)) + ->method('getSystemValueBool') + ->willReturnMap([ + ['gs.enabled', false, true], + ['has_internet_connection', true, true], + ]); + $this->config->expects($this->once()) + ->method('getSystemValueString') + ->with('lookup_server', 'https://lookup.nextcloud.com') + ->willReturn('https://lookup.example.io'); + + $response = $this->createMock(IResponse::class); + $response->expects($this->once()) + ->method('getBody') + ->willReturn(json_encode([$lookupEntry])); + + $client = $this->createMock(IClient::class); + $client->expects($this->once()) + ->method('get') + ->willReturnCallback(function ($url) use ($response) { + $this->assertSame(0, strpos($url, 'https://lookup.example.io/users?search=')); + $this->assertNotFalse(strpos($url, urlencode('foo'))); + return $response; + }); + $this->clientService->expects($this->once()) + ->method('newClient') + ->willReturn($client); + + $this->assertFalse($this->plugin->search('foo', 10, 0, $searchResult)); + } + + public static function dataSearchEmailLabel(): array { + $fedId = 'foo@enceladus.moon'; + $email = 'foo.bar@example.org'; + return [ + 'flag disabled, email present: federation id label' => [ + 'no', + ['federationId' => $fedId, 'name' => ['value' => 'Foo Bar'], 'email' => ['value' => $email]], + 'Foo Bar (foo@enceladus.moon)', + ], + 'flag enabled, email present: email label' => [ + 'yes', + ['federationId' => $fedId, 'name' => ['value' => 'Foo Bar'], 'email' => ['value' => $email]], + 'Foo Bar (foo.bar@example.org)', + ], + 'flag enabled, email missing: fallback to federation id' => [ + 'yes', + ['federationId' => $fedId, 'name' => ['value' => 'Foo Bar']], + 'Foo Bar (foo@enceladus.moon)', + ], + 'flag enabled, no name: bare email' => [ + 'yes', + ['federationId' => $fedId, 'email' => ['value' => $email]], + 'foo.bar@example.org', + ], + ]; + } + /** * @param array $searchParams * @param bool $GSEnabled @@ -191,10 +284,12 @@ public function testSearchEnableDisableLookupServer(array $searchParams, $GSEnab /** @var ISearchResult|MockObject $searchResult */ $searchResult = $this->createMock(ISearchResult::class); - $this->config->expects($this->once()) + $this->config->expects($this->exactly(2)) ->method('getAppValue') - ->with('files_sharing', 'lookupServerEnabled', 'no') - ->willReturn($LookupEnabled ? 'yes' : 'no'); + ->willReturnMap([ + ['files_sharing', 'lookupServerEnabled', 'no', $LookupEnabled ? 'yes' : 'no'], + ['core', 'shareapi_lookup_label_show_email', 'no', 'no'], + ]); if ($GSEnabled) { $searchResult->expects($this->once()) ->method('addResultSet')