Merge pull request #51422 from nextcloud/backport/51407/stable26

[stable26] fix(lookup-server): disable when not using global scale
2026-05-26 03:04:35 -04:00 · 2025-03-12 21:14:09 +01:00 · 2025-03-12 21:14:09 +01:00 · 8c9bc438d0
commit 8c9bc438d0
parent 5549bacc2c 931668bced
12 changed files with 119 additions and 117 deletions
--- a/apps/federatedfilesharing/lib/FederatedShareProvider.php
+++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php
@ -1071,8 +1071,10 @@ class FederatedShareProvider implements IShareProvider {
 		if ($this->gsConfig->isGlobalScaleEnabled()) {
 			return true;
 		}
-		$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes');
-		return ($result === 'yes');
+		$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
+		// TODO: Reenable if lookup server is used again
+		// return $result;
+		return false;
 	}


@ -1086,8 +1088,10 @@ class FederatedShareProvider implements IShareProvider {
 		if ($this->gsConfig->isGlobalScaleEnabled()) {
 			return false;
 		}
-		$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes');
-		return ($result === 'yes');
+		$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') === 'yes';
+		// TODO: Reenable if lookup server is used again
+		// return $result;
+		return false;
 	}

 	/**
--- a/apps/federatedfilesharing/src/components/AdminSettings.vue
+++ b/apps/federatedfilesharing/src/components/AdminSettings.vue
@ -50,17 +50,22 @@
 			{{ t('federatedfilesharing', 'Allow users on this server to receive group shares from other servers') }}
 		</NcCheckboxRadioSwitch>

-		<NcCheckboxRadioSwitch type="switch"
-			:checked.sync="lookupServerEnabled"
-			@update:checked="update('lookupServerEnabled', lookupServerEnabled)">
-			{{ t('federatedfilesharing', 'Search global and public address book for users') }}
-		</NcCheckboxRadioSwitch>
+		<fieldset>
+			<legend>{{ t('federatedfilesharing', 'The lookup server is only available for global scale.') }}</legend>
+			<NcCheckboxRadioSwitch type="switch"
+				:checked.sync="lookupServerEnabled"
+				disabled
+				@update:checked="update('lookupServerEnabled', lookupServerEnabled)">
+				{{ t('federatedfilesharing', 'Search global and public address book for users') }}
+			</NcCheckboxRadioSwitch>

-		<NcCheckboxRadioSwitch type="switch"
-			:checked.sync="lookupServerUploadEnabled"
-			@update:checked="update('lookupServerUploadEnabled', lookupServerUploadEnabled)">
-			{{ t('federatedfilesharing', 'Allow users to publish their data to a global and public address book') }}
-		</NcCheckboxRadioSwitch>
+			<NcCheckboxRadioSwitch type="switch"
+				:checked.sync="lookupServerUploadEnabled"
+				disabled
+				@update:checked="update('lookupServerUploadEnabled', lookupServerUploadEnabled)">
+				{{ t('federatedfilesharing', 'Allow users to publish their data to a global and public address book') }}
+			</NcCheckboxRadioSwitch>
+		</fieldset>
 	</NcSettingsSection>
 </template>

--- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php
+++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php
@ -849,7 +849,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 		$this->gsConfig->expects($this->once())->method('isGlobalScaleEnabled')
 			->willReturn($gsEnabled);
 		$this->config->expects($this->any())->method('getAppValue')
-			->with('files_sharing', 'lookupServerEnabled', 'yes')
+			->with('files_sharing', 'lookupServerEnabled', 'no')
 			->willReturn($isEnabled);

 		$this->assertSame($expected,
@ -860,10 +860,13 @@ class FederatedShareProviderTest extends \Test\TestCase {

 	public function dataTestIsLookupServerQueriesEnabled() {
 		return [
-			[false, 'yes', true],
-			[false, 'no', false],
 			[true, 'yes', true],
 			[true, 'no', true],
+			// TODO: reenable if we use the lookup server for non-global scale
+			// [false, 'yes', true],
+			// [false, 'no', false],
+			[false, 'no', false],
+			[false, 'yes', false],
 		];
 	}

@ -877,7 +880,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 		$this->gsConfig->expects($this->once())->method('isGlobalScaleEnabled')
 			->willReturn($gsEnabled);
 		$this->config->expects($this->any())->method('getAppValue')
-			->with('files_sharing', 'lookupServerUploadEnabled', 'yes')
+			->with('files_sharing', 'lookupServerUploadEnabled', 'no')
 			->willReturn($isEnabled);

 		$this->assertSame($expected,
@ -887,10 +890,13 @@ class FederatedShareProviderTest extends \Test\TestCase {

 	public function dataTestIsLookupServerUploadEnabled() {
 		return [
-			[false, 'yes', true],
-			[false, 'no', false],
 			[true, 'yes', false],
 			[true, 'no', false],
+			// TODO: reenable if we use the lookup server again
+			// [false, 'yes', true],
+			// [false, 'no', false],
+			[false, 'yes', false],
+			[false, 'no', false],
 		];
 	}

--- a/apps/files_sharing/lib/Controller/ShareesAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareesAPIController.php
@ -38,7 +38,6 @@ declare(strict_types=1);
 */
 namespace OCA\Files_Sharing\Controller;

-use OCP\Constants;
 use function array_slice;
 use function array_values;
 use Generator;
@ -49,6 +48,8 @@ use OCP\AppFramework\OCSController;
 use OCP\Collaboration\Collaborators\ISearch;
 use OCP\Collaboration\Collaborators\ISearchResult;
 use OCP\Collaboration\Collaborators\SearchResultType;
+use OCP\Constants;
+use OCP\GlobalScale\IConfig as GlobalScaleIConfig;
 use OCP\IConfig;
 use OCP\IRequest;
 use OCP\IURLGenerator;
@ -216,15 +217,11 @@ class ShareesAPIController extends OCSController {
 		$this->limit = $perPage;
 		$this->offset = $perPage * ($page - 1);

-		// In global scale mode we always search the loogup server
-		if ($this->config->getSystemValueBool('gs.enabled', false)) {
-			$lookup = true;
-			$this->result['lookupEnabled'] = true;
-		} else {
-			$this->result['lookupEnabled'] = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes') === 'yes';
-		}
+		// In global scale mode we always search the lookup server
+		$this->result['lookupEnabled'] = \OCP\Server::get(GlobalScaleIConfig::class)->isGlobalScaleEnabled();
+		// TODO: Reconsider using lookup server for non-global-scale federation

-		[$result, $hasMoreResults] = $this->collaboratorSearch->search($search, $shareTypes, $lookup, $this->limit, $this->offset);
+		[$result, $hasMoreResults] = $this->collaboratorSearch->search($search, $shareTypes, $this->result['lookupEnabled'], $this->limit, $this->offset);

 		// extra treatment for 'exact' subarray, with a single merge expected keys might be lost
 		if (isset($result['exact'])) {
--- a/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php
+++ b/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php
@ -35,6 +35,7 @@ use OCA\Files_Sharing\Tests\TestCase;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\OCS\OCSBadRequestException;
 use OCP\Collaboration\Collaborators\ISearch;
+use OCP\GlobalScale\IConfig as GlobalScaleIConfig;
 use OCP\IConfig;
 use OCP\IRequest;
 use OCP\IURLGenerator;
@ -240,14 +241,14 @@ class ShareesAPIControllerTest extends TestCase {
 		$perPage = $getData['perPage'] ?? 200;
 		$shareType = $getData['shareType'] ?? null;

+		$globalConfig = $this->createMock(GlobalScaleIConfig::class);
+		$globalConfig->expects(self::once())
+			->method('isGlobalScaleEnabled')
+			->willReturn(true);
+		$this->overwriteService(GlobalScaleIConfig::class, $globalConfig);
+
 		/** @var IConfig|MockObject $config */
 		$config = $this->createMock(IConfig::class);
-		$config->expects($this->exactly(1))
-			->method('getAppValue')
-			->with($this->anything(), $this->anything(), $this->anything())
-			->willReturnMap([
-				['files_sharing', 'lookupServerEnabled', 'yes', 'yes'],
-			]);

 		$this->shareManager->expects($this->once())
 			->method('allowGroupSharing')
--- a/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php
+++ b/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php
@ -115,10 +115,12 @@ class RetryJob extends Job {
 	 * - max retries are reached (set to 5)
 	 */
 	protected function shouldRemoveBackgroundJob(): bool {
-		return $this->config->getSystemValueBool('has_internet_connection', true) === false ||
-			$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === '' ||
-			$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') !== 'yes' ||
-			$this->retries >= 5;
+		// TODO: Remove global scale condition once lookup server is used for non-global scale federation
+		// return $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') !== 'yes'
+		return !$this->config->getSystemValueBool('gs.enabled', false)
+			|| $this->config->getSystemValueBool('has_internet_connection', true) === false
+			|| $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === ''
+			|| $this->retries >= 5;
 	}

 	protected function shouldRun(): bool {
@ -180,7 +182,7 @@ class RetryJob extends Job {
 				$user->getUID(),
 				'lookup_server_connector',
 				'update_retries',
-				$this->retries + 1
+				(string)($this->retries + 1),
 			);
 		}
 	}
--- a/apps/lookup_server_connector/lib/UpdateLookupServer.php
+++ b/apps/lookup_server_connector/lib/UpdateLookupServer.php
@ -82,8 +82,9 @@ class UpdateLookupServer {
 	 * @return bool
 	 */
 	private function shouldUpdateLookupServer(): bool {
-		return $this->config->getSystemValueBool('has_internet_connection', true) === true &&
-			$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') === 'yes' &&
-			$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== '';
+		// TODO: Consider reenable for non-global-scale setups by checking "'files_sharing', 'lookupServerUploadEnabled'" instead of "gs.enabled"
+		return $this->config->getSystemValueBool('gs.enabled', false)
+			&& $this->config->getSystemValueBool('has_internet_connection', true)
+			&& $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== '';
 	}
 }
--- a/apps/settings/lib/BackgroundJobs/VerifyUserData.php
+++ b/apps/settings/lib/BackgroundJobs/VerifyUserData.php
@ -169,9 +169,11 @@ class VerifyUserData extends Job {
 	}

 	protected function verifyViaLookupServer(array $argument, string $dataType): bool {
-		if (empty($this->lookupServerUrl) ||
-			$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') !== 'yes' ||
-			$this->config->getSystemValue('has_internet_connection', true) === false) {
+		// TODO: Consider to enable for non-global-scale setups by checking 'files_sharing', 'lookupServerUploadEnabled'
+		if (!$this->config->getSystemValueBool('gs.enabled', false)
+			|| empty($this->lookupServerUrl)
+			|| $this->config->getSystemValue('has_internet_connection', true) === false
+		) {
 			return true;
 		}

--- a/dist/federatedfilesharing-vue-settings-admin.js
+++ b/dist/federatedfilesharing-vue-settings-admin.js
--- a/dist/federatedfilesharing-vue-settings-admin.js.map
+++ b/dist/federatedfilesharing-vue-settings-admin.js.map
--- a/lib/private/Collaboration/Collaborators/LookupPlugin.php
+++ b/lib/private/Collaboration/Collaborators/LookupPlugin.php
@ -63,12 +63,14 @@ class LookupPlugin implements ISearchPlugin {
 	}

 	public function search($search, $limit, $offset, ISearchResult $searchResult) {
-		$isGlobalScaleEnabled = $this->config->getSystemValue('gs.enabled', false);
-		$isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes') === 'yes';
+		$isGlobalScaleEnabled = $this->config->getSystemValueBool('gs.enabled', false);
+		$isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
 		$hasInternetConnection = $this->config->getSystemValueBool('has_internet_connection', true);

-		// if case of Global Scale we always search the lookup server
-		if (!$isGlobalScaleEnabled && (!$isLookupServerEnabled || !$hasInternetConnection)) {
+		// If case of Global Scale we always search the lookup server
+		// TODO: Reconsider using the lookup server for non-global scale
+		// if (!$isGlobalScaleEnabled && (!$isLookupServerEnabled || !$hasInternetConnection || $disableLookupServer)) {
+		if (!$isGlobalScaleEnabled) {
 			return false;
 		}

--- a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php
+++ b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php
@ -87,24 +87,20 @@ class LookupPluginTest extends TestCase {
 	}

 	public function testSearchNoLookupServerURI() {
-		$this->config->expects($this->once())
+		$this->config->expects($this->atMost(1))
 			->method('getAppValue')
-			->with('files_sharing', 'lookupServerEnabled', 'yes')
+			->with('files_sharing', 'lookupServerEnabled', 'no')
 			->willReturn('yes');
 		$this->config->expects($this->exactly(2))
-			->method('getSystemValue')
-			->withConsecutive(
-				['gs.enabled', false],
-				['lookup_server', 'https://lookup.nextcloud.com'],
-			)->willReturnOnConsecutiveCalls(
-				false,
-				'',
-			);
-
-		$this->config->expects($this->once())
 			->method('getSystemValueBool')
-			->with('has_internet_connection', true)
-			->willReturn(true);
+			->willReturnMap([
+				['gs.enabled', false, true],
+				['has_internet_connection', true, true],
+			]);
+		$this->config->expects($this->once())
+			->method('getSystemValue')
+			->with('lookup_server', 'https://lookup.nextcloud.com')
+			->willReturn('');

 		$this->clientService->expects($this->never())
 			->method('newClient');
@ -118,17 +114,15 @@ class LookupPluginTest extends TestCase {
 	public function testSearchNoInternet() {
 		$this->config->expects($this->once())
 			->method('getAppValue')
-			->with('files_sharing', 'lookupServerEnabled', 'yes')
+			->with('files_sharing', 'lookupServerEnabled', 'no')
 			->willReturn('yes');
-		$this->config->expects($this->once())
-			->method('getSystemValue')
-			->with('gs.enabled', false)
-			->willReturn(false);

-		$this->config->expects($this->once())
+		$this->config->expects($this->exactly(2))
 			->method('getSystemValueBool')
-			->with('has_internet_connection', true)
-			->willReturn(false);
+			->willReturnMap([
+				['has_internet_connection', true, false],
+				['gs.enabled', false, true],
+			]);

 		$this->clientService->expects($this->never())
 			->method('newClient');
@ -154,22 +148,18 @@ class LookupPluginTest extends TestCase {

 		$this->config->expects($this->once())
 			->method('getAppValue')
-			->with('files_sharing', 'lookupServerEnabled', 'yes')
+			->with('files_sharing', 'lookupServerEnabled', 'no')
 			->willReturn('yes');
-		$this->config->expects($this->exactly(2))
-			->method('getSystemValue')
-			->withConsecutive(
-				['gs.enabled', false],
-				['lookup_server', 'https://lookup.nextcloud.com'],
-			)->willReturnOnConsecutiveCalls(
-				false,
-				$searchParams['server'],
-			);
-
 		$this->config->expects($this->once())
+			->method('getSystemValue')
+			->with('lookup_server', 'https://lookup.nextcloud.com')
+			->willReturn($searchParams['server']);
+		$this->config->expects($this->exactly(2))
 			->method('getSystemValueBool')
-			->with('has_internet_connection', true)
-			->willReturn(true);
+			->willReturnMap([
+				['gs.enabled', false, true],
+				['has_internet_connection', true, true],
+			]);

 		$response = $this->createMock(IResponse::class);
 		$response->expects($this->once())
@ -214,26 +204,24 @@ class LookupPluginTest extends TestCase {

 		$this->config->expects($this->once())
 			->method('getAppValue')
-			->with('files_sharing', 'lookupServerEnabled', 'yes')
+			->with('files_sharing', 'lookupServerEnabled', 'no')
 			->willReturn($LookupEnabled ? 'yes' : 'no');
-		if ($GSEnabled || $LookupEnabled) {
+
+		$this->config->expects($this->exactly(2))
+			->method('getSystemValueBool')
+			->willReturnMap([
+				['has_internet_connection', true, true],
+				['gs.enabled', false, $GSEnabled],
+			]);
+		if ($GSEnabled) {
 			$searchResult->expects($this->once())
 				->method('addResultSet')
 				->with($type, $searchParams['expectedResult'], []);

 			$this->config->expects($this->once())
-				->method('getSystemValueBool')
-				->with('has_internet_connection', true)
-				->willReturn(true);
-			$this->config->expects($this->exactly(2))
 				->method('getSystemValue')
-				->withConsecutive(
-					['gs.enabled', false],
-					['lookup_server', 'https://lookup.nextcloud.com'],
-				)->willReturnOnConsecutiveCalls(
-					$GSEnabled,
-					$searchParams['server'],
-				);
+				->with('lookup_server', 'https://lookup.nextcloud.com')
+				->willReturn($searchParams['server']);

 			$response = $this->createMock(IResponse::class);
 			$response->expects($this->once())
@ -254,10 +242,6 @@ class LookupPluginTest extends TestCase {
 				->willReturn($client);
 		} else {
 			$searchResult->expects($this->never())->method('addResultSet');
-			$this->config->expects($this->once())
-				->method('getSystemValue')
-				->with('gs.enabled', false)
-				->willReturn($GSEnabled);
 		}
 		$moreResults = $this->plugin->search(
 			$searchParams['search'],
@ -269,12 +253,13 @@ class LookupPluginTest extends TestCase {
 		$this->assertFalse($moreResults);
 	}

-
-	public function testSearchLookupServerDisabled() {
-		$this->config->expects($this->once())
-			->method('getAppValue')
-			->with('files_sharing', 'lookupServerEnabled', 'yes')
-			->willReturn('no');
+	public function testSearchGSDisabled(): void {
+		$this->config->expects($this->atLeastOnce())
+			->method('getSystemValueBool')
+			->willReturnMap([
+				['has_internet_connection', true, true],
+				['gs.enabled', false, false],
+			]);

 		/** @var ISearchResult|MockObject $searchResult */
 		$searchResult = $this->createMock(ISearchResult::class);
@ -393,7 +378,6 @@ class LookupPluginTest extends TestCase {
 						'label' => $fedIDs[0],
 						'value' => [
 							'shareType' => IShare::TYPE_REMOTE,
-							'globalScale' => false,
 							'shareWith' => $fedIDs[0]
 						],
 						'extra' => ['federationId' => $fedIDs[0]],
@ -402,7 +386,6 @@ class LookupPluginTest extends TestCase {
 						'label' => $fedIDs[1],
 						'value' => [
 							'shareType' => IShare::TYPE_REMOTE,
-							'globalScale' => false,
 							'shareWith' => $fedIDs[1]
 						],
 						'extra' => ['federationId' => $fedIDs[1]],
@ -411,7 +394,6 @@ class LookupPluginTest extends TestCase {
 						'label' => $fedIDs[2],
 						'value' => [
 							'shareType' => IShare::TYPE_REMOTE,
-							'globalScale' => false,
 							'shareWith' => $fedIDs[2]
 						],
 						'extra' => ['federationId' => $fedIDs[2]],
@ -486,7 +468,7 @@ class LookupPluginTest extends TestCase {
 						'label' => $fedIDs[0],
 						'value' => [
 							'shareType' => IShare::TYPE_REMOTE,
-							'globalScale' => false,
+							'globalScale' => true,
 							'shareWith' => $fedIDs[0]
 						],
 						'extra' => ['federationId' => $fedIDs[0]],
@ -495,7 +477,7 @@ class LookupPluginTest extends TestCase {
 						'label' => $fedIDs[1],
 						'value' => [
 							'shareType' => IShare::TYPE_REMOTE,
-							'globalScale' => false,
+							'globalScale' => true,
 							'shareWith' => $fedIDs[1]
 						],
 						'extra' => ['federationId' => $fedIDs[1]],
@ -504,7 +486,7 @@ class LookupPluginTest extends TestCase {
 						'label' => $fedIDs[2],
 						'value' => [
 							'shareType' => IShare::TYPE_REMOTE,
-							'globalScale' => false,
+							'globalScale' => true,
 							'shareWith' => $fedIDs[2]
 						],
 						'extra' => ['federationId' => $fedIDs[2]],