From bc32a951d9341bb404c5ecab1fbf98f5a65cfa29 Mon Sep 17 00:00:00 2001 From: niv Date: Thu, 14 May 2026 12:02:55 +0200 Subject: [PATCH 1/3] feat(sharing): allow email-based labels for lookup results When the new system config 'shareapi_lookup_label_show_email' is set to true, the LookupPlugin composes suggestion labels with the user's email (from the lookup server response) instead of the federation ID. Default behavior is strictly preserved when the flag is absent or false. Motivation: in Global Scale deployments, federation IDs carry sharded identity that is not human-readable (e.g. 'user@node05.example.org'). Operators can opt-in to email-based labels to improve UX without affecting other deployments. Signed-off-by: niv --- .../Collaborators/LookupPlugin.php | 9 +- .../Collaborators/LookupPluginTest.php | 82 +++++++++++++++++-- 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/lib/private/Collaboration/Collaborators/LookupPlugin.php b/lib/private/Collaboration/Collaborators/LookupPlugin.php index de1bfa9edb932..63347ea1076cb 100644 --- a/lib/private/Collaboration/Collaborators/LookupPlugin.php +++ b/lib/private/Collaboration/Collaborators/LookupPlugin.php @@ -38,6 +38,7 @@ 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); + $showFederatedEmail = $this->config->getSystemValueBool('shareapi_lookup_label_show_email', false); // If case of Global Scale we always search the lookup server // TODO: Reconsider using the lookup server for non-global scale @@ -78,7 +79,13 @@ 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 ($showFederatedEmail && !empty($email)) { + $label = empty($name) ? $email : $name . ' (' . $email . ')'; + } else { + $label = empty($name) ? $lookup['federationId'] : $name . ' (' . $lookup['federationId'] . ')'; + } $result[] = [ 'label' => $label, 'value' => [ diff --git a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php index 5fc218dc6d80e..2ae2fcd2d4c7d 100644 --- a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php @@ -81,11 +81,12 @@ public function testSearchNoLookupServerURI(): void { ->method('getAppValue') ->with('files_sharing', 'lookupServerEnabled', 'no') ->willReturn('yes'); - $this->config->expects($this->exactly(2)) + $this->config->expects($this->exactly(3)) ->method('getSystemValueBool') ->willReturnMap([ ['gs.enabled', false, true], ['has_internet_connection', true, true], + ['shareapi_lookup_label_show_email', false, false], ]); $this->config->expects($this->once()) @@ -107,11 +108,12 @@ public function testSearchNoInternet(): void { ->method('getAppValue') ->with('files_sharing', 'lookupServerEnabled', 'no') ->willReturn('yes'); - $this->config->expects($this->exactly(2)) + $this->config->expects($this->exactly(3)) ->method('getSystemValueBool') ->willReturnMap([ ['gs.enabled', false, false], ['has_internet_connection', true, false], + ['shareapi_lookup_label_show_email', false, false], ]); $this->clientService->expects($this->never()) @@ -140,11 +142,12 @@ public function testSearch(array $searchParams): void { ->method('getAppValue') ->with('files_sharing', 'lookupServerEnabled', 'no') ->willReturn('yes'); - $this->config->expects($this->exactly(2)) + $this->config->expects($this->exactly(3)) ->method('getSystemValueBool') ->willReturnMap([ ['gs.enabled', false, true], ['has_internet_connection', true, true], + ['shareapi_lookup_label_show_email', false, false], ]); $this->config->expects($this->once()) @@ -180,6 +183,73 @@ public function testSearch(array $searchParams): void { $this->assertFalse($moreResults); } + public function testSearchWithEmailLabel(): void { + $type = new SearchResultType('lookup'); + $fedId = 'foo@enceladus.moon'; + $email = 'foo@bar.example'; + $server = 'https://lookup.example.io'; + $resultBody = [ + [ + 'federationId' => $fedId, + 'name' => ['value' => 'Foo Bar', 'verified' => 0], + 'email' => ['value' => $email, 'verified' => 0], + ], + ]; + $expectedResult = [ + [ + 'label' => 'Foo Bar (' . $email . ')', + 'value' => [ + 'shareType' => IShare::TYPE_REMOTE, + 'globalScale' => true, + 'shareWith' => $fedId, + 'server' => 'enceladus.moon', + 'isTrustedServer' => false, + ], + 'extra' => [ + 'federationId' => $fedId, + 'name' => ['value' => 'Foo Bar', 'verified' => 0], + 'email' => ['value' => $email, 'verified' => 0], + ], + ], + ]; + + /** @var ISearchResult|MockObject $searchResult */ + $searchResult = $this->createMock(ISearchResult::class); + $searchResult->expects($this->once()) + ->method('addResultSet') + ->with($type, $expectedResult, []); + + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('files_sharing', 'lookupServerEnabled', 'no') + ->willReturn('yes'); + $this->config->expects($this->exactly(3)) + ->method('getSystemValueBool') + ->willReturnMap([ + ['gs.enabled', false, true], + ['has_internet_connection', true, true], + ['shareapi_lookup_label_show_email', false, true], + ]); + $this->config->expects($this->once()) + ->method('getSystemValueString') + ->with('lookup_server', 'https://lookup.nextcloud.com') + ->willReturn($server); + + $response = $this->createMock(IResponse::class); + $response->expects($this->once()) + ->method('getBody') + ->willReturn(json_encode($resultBody)); + $client = $this->createMock(IClient::class); + $client->expects($this->once()) + ->method('get') + ->willReturn($response); + $this->clientService->expects($this->once()) + ->method('newClient') + ->willReturn($client); + + $moreResults = $this->plugin->search('foo', 10, 0, $searchResult); + $this->assertFalse($moreResults); + } /** * @param array $searchParams @@ -202,11 +272,12 @@ public function testSearchEnableDisableLookupServer(array $searchParams, $GSEnab ->method('addResultSet') ->with($type, $searchParams['expectedResult'], []); - $this->config->expects($this->exactly(2)) + $this->config->expects($this->exactly(3)) ->method('getSystemValueBool') ->willReturnMap([ ['gs.enabled', false, $GSEnabled], ['has_internet_connection', true, true], + ['shareapi_lookup_label_show_email', false, false], ]); $this->config->expects($this->once()) ->method('getSystemValueString') @@ -232,11 +303,12 @@ public function testSearchEnableDisableLookupServer(array $searchParams, $GSEnab ->willReturn($client); } else { $searchResult->expects($this->never())->method('addResultSet'); - $this->config->expects($this->exactly(2)) + $this->config->expects($this->exactly(3)) ->method('getSystemValueBool') ->willReturnMap([ ['gs.enabled', false, $GSEnabled], ['has_internet_connection', true, true], + ['shareapi_lookup_label_show_email', false, false], ]); } $moreResults = $this->plugin->search( From ebe93ddec97a50621c4aea926879e1319d2ad993 Mon Sep 17 00:00:00 2001 From: Nicolas Varlot Date: Thu, 14 May 2026 12:26:14 +0200 Subject: [PATCH 2/3] added comments Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Nicolas Varlot --- lib/private/Collaboration/Collaborators/LookupPlugin.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/private/Collaboration/Collaborators/LookupPlugin.php b/lib/private/Collaboration/Collaborators/LookupPlugin.php index 63347ea1076cb..6a65751aaa323 100644 --- a/lib/private/Collaboration/Collaborators/LookupPlugin.php +++ b/lib/private/Collaboration/Collaborators/LookupPlugin.php @@ -38,6 +38,10 @@ 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); + // System config flag: shareapi_lookup_label_show_email + // Controls whether email addresses from federated lookup results are shown in share dialog labels. + // Defaults to false 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. $showFederatedEmail = $this->config->getSystemValueBool('shareapi_lookup_label_show_email', false); // If case of Global Scale we always search the lookup server From 58b293dc6568bdab369a2b72a923e6184990816a Mon Sep 17 00:00:00 2001 From: niv Date: Thu, 14 May 2026 13:02:04 +0200 Subject: [PATCH 3/3] test(sharing): address review comments on lookup label - rename variable $showFederatedEmail to $showEmailInLabel for clarity - add testSearchWithEmailLabelFallbackWhenEmailMissing covering the fallback path when flag is enabled but lookup response has no email - tighten HTTP expectation in testSearchWithEmailLabel to validate the requested URL Signed-off-by: niv --- .../Collaborators/LookupPlugin.php | 4 +- .../Collaborators/LookupPluginTest.php | 69 +++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/lib/private/Collaboration/Collaborators/LookupPlugin.php b/lib/private/Collaboration/Collaborators/LookupPlugin.php index 6a65751aaa323..aa38c209e40df 100644 --- a/lib/private/Collaboration/Collaborators/LookupPlugin.php +++ b/lib/private/Collaboration/Collaborators/LookupPlugin.php @@ -42,7 +42,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b // Controls whether email addresses from federated lookup results are shown in share dialog labels. // Defaults to false 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. - $showFederatedEmail = $this->config->getSystemValueBool('shareapi_lookup_label_show_email', false); + $showEmailInLabel = $this->config->getSystemValueBool('shareapi_lookup_label_show_email', false); // If case of Global Scale we always search the lookup server // TODO: Reconsider using the lookup server for non-global scale @@ -85,7 +85,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b $name = $lookup['name']['value'] ?? ''; $email = $lookup['email']['value'] ?? ''; - if ($showFederatedEmail && !empty($email)) { + if ($showEmailInLabel && !empty($email)) { $label = empty($name) ? $email : $name . ' (' . $email . ')'; } else { $label = empty($name) ? $lookup['federationId'] : $name . ' (' . $lookup['federationId'] . ')'; diff --git a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php index 2ae2fcd2d4c7d..62923161ecb17 100644 --- a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php @@ -213,6 +213,75 @@ public function testSearchWithEmailLabel(): void { ], ]; + /** @var ISearchResult|MockObject $searchResult */ + $searchResult = $this->createMock(ISearchResult::class); + $searchResult->expects($this->once()) + ->method('addResultSet') + ->with($type, $expectedResult, []); + + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('files_sharing', 'lookupServerEnabled', 'no') + ->willReturn('yes'); + $this->config->expects($this->exactly(3)) + ->method('getSystemValueBool') + ->willReturnMap([ + ['gs.enabled', false, true], + ['has_internet_connection', true, true], + ['shareapi_lookup_label_show_email', false, true], + ]); + $this->config->expects($this->once()) + ->method('getSystemValueString') + ->with('lookup_server', 'https://lookup.nextcloud.com') + ->willReturn($server); + + $response = $this->createMock(IResponse::class); + $response->expects($this->once()) + ->method('getBody') + ->willReturn(json_encode($resultBody)); + $client = $this->createMock(IClient::class); + $client->expects($this->once()) + ->method('get') + ->willReturnCallback(function ($url) use ($server, $response) { + $this->assertSame(strpos($url, $server . '/users?search='), 0); + $this->assertNotFalse(strpos($url, urlencode('foo'))); + return $response; + }); + $this->clientService->expects($this->once()) + ->method('newClient') + ->willReturn($client); + + $moreResults = $this->plugin->search('foo', 10, 0, $searchResult); + $this->assertFalse($moreResults); + } + + public function testSearchWithEmailLabelFallbackWhenEmailMissing(): void { + $type = new SearchResultType('lookup'); + $fedId = 'foo@enceladus.moon'; + $server = 'https://lookup.example.io'; + $resultBody = [ + [ + 'federationId' => $fedId, + 'name' => ['value' => 'Foo Bar', 'verified' => 0], + ], + ]; + $expectedResult = [ + [ + 'label' => 'Foo Bar (' . $fedId . ')', + 'value' => [ + 'shareType' => IShare::TYPE_REMOTE, + 'globalScale' => true, + 'shareWith' => $fedId, + 'server' => 'enceladus.moon', + 'isTrustedServer' => false, + ], + 'extra' => [ + 'federationId' => $fedId, + 'name' => ['value' => 'Foo Bar', 'verified' => 0], + ], + ], + ]; + /** @var ISearchResult|MockObject $searchResult */ $searchResult = $this->createMock(ISearchResult::class); $searchResult->expects($this->once())