fix(lookup-server): disable lookup server for non-global scale setups

Signed-off-by: Ferdinand Thiessen <[email protected]>

apps/federatedfilesharing/lib/FederatedShareProvider.php

@@ -998,7 +998,10 @@ public function isLookupServerQueriesEnabled(): bool {
 		if ($this->gsConfig->isGlobalScaleEnabled()) {
 			return true;
 		}
-		$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no');
+		$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
+		// TODO: Reenable if lookup server is used again
+		// return $result;
+		return false;
 	}
 
 
@@ -1010,8 +1013,10 @@ public function isLookupServerUploadEnabled(): bool {
 		if ($this->gsConfig->isGlobalScaleEnabled()) {
 			return false;
 		}
-		$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no');
-		return $result === 'yes';
+		$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') === 'yes';
+		// TODO: Reenable if lookup server is used again
+		// return $result;
+		return false;
 	}
 
 	/**

apps/federatedfilesharing/src/components/AdminSettings.vue

@@ -32,17 +32,23 @@
 			{{ t('federatedfilesharing', 'Allow people 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 people') }}
-		</NcCheckboxRadioSwitch>
+		<fieldset>
+			<legend>{{ t('federatedfilesharing', 'The lookup server is only available for global scale.') }}</legend>
 
-		<NcCheckboxRadioSwitch type="switch"
-			:checked.sync="lookupServerUploadEnabled"
-			@update:checked="update('lookupServerUploadEnabled', lookupServerUploadEnabled)">
-			{{ t('federatedfilesharing', 'Allow people to publish their data to a global and public address book') }}
-		</NcCheckboxRadioSwitch>
+			<NcCheckboxRadioSwitch type="switch"
+				:checked.sync="lookupServerEnabled"
+				disabled
+				@update:checked="update('lookupServerEnabled', lookupServerEnabled)">
+				{{ t('federatedfilesharing', 'Search global and public address book for people') }}
+			</NcCheckboxRadioSwitch>
+
+			<NcCheckboxRadioSwitch type="switch"
+				:checked.sync="lookupServerUploadEnabled"
+				disabled
+				@update:checked="update('lookupServerUploadEnabled', lookupServerUploadEnabled)">
+				{{ t('federatedfilesharing', 'Allow people to publish their data to a global and public address book') }}
+			</NcCheckboxRadioSwitch>
+		</fieldset>
 
 		<!-- Trusted server handling -->
 		<div class="settings-subsection">

apps/federatedfilesharing/tests/FederatedShareProviderTest.php

@@ -836,10 +836,13 @@ public function testIsLookupServerQueriesEnabled($gsEnabled, $isEnabled, $expect
 
 	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],
 		];
 	}
 
@@ -863,10 +866,13 @@ public function testIsLookupServerUploadEnabled($gsEnabled, $isEnabled, $expecte
 
 	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],
 		];
 	}
 

apps/files_sharing/lib/Controller/ShareesAPIController.php

@@ -22,6 +22,7 @@
 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;
@@ -175,12 +176,11 @@ public function search(string $search = '', ?string $itemType = null, int $page
 		$this->limit = $perPage;
 		$this->offset = $perPage * ($page - 1);
 
-		// In global scale mode we always search the loogup server
-		$lookup = $this->config->getSystemValueBool('gs.enabled', false)
-			|| $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
-		$this->result['lookupEnabled'] = $lookup;
+		// In global scale mode we always search the lookup server
+		$this->result['lookupEnabled'] = 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'])) {

apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php

@@ -84,10 +84,11 @@ public function start(IJobList $jobList): void {
 	 * - 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', 'no') !== 'yes' ||
-			$this->retries >= 5;
+		// TODO: Remove global scale condition once lookup server is used for non-global scale federation
+		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 {
@@ -149,7 +150,7 @@ protected function run($argument): void {
 				$user->getUID(),
 				'lookup_server_connector',
 				'update_retries',
-				$this->retries + 1
+				(string)($this->retries + 1),
 			);
 		}
 	}

apps/lookup_server_connector/lib/UpdateLookupServer.php

@@ -56,8 +56,9 @@ public function userUpdated(IUser $user): void {
 	 * @return bool
 	 */
 	private function shouldUpdateLookupServer(): bool {
-		return $this->config->getSystemValueBool('has_internet_connection', true) === true &&
-			$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') === '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') !== '';
 	}
 }

apps/settings/lib/BackgroundJobs/VerifyUserData.php

@@ -120,9 +120,11 @@ protected function verifyWebsite(array $argument) {
 	}
 
 	protected function verifyViaLookupServer(array $argument, string $dataType): bool {
-		if (empty($this->lookupServerUrl) ||
-			$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') !== '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;
 		}
 

lib/private/Collaboration/Collaborators/LookupPlugin.php

@@ -35,8 +35,10 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
 		$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;
 		}
 

tests/lib/Collaboration/Collaborators/LookupPluginTest.php

@@ -72,15 +72,15 @@ protected function setUp(): void {
 	public function testSearchNoLookupServerURI(): void {
 		$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('getSystemValueBool')
 			->withConsecutive(
 				['gs.enabled', false],
 				['has_internet_connection', true],
 			)->willReturnOnConsecutiveCalls(
-				false,
+				true,
 				true,
 			);
 
@@ -101,7 +101,7 @@ public function testSearchNoLookupServerURI(): void {
 	public function testSearchNoInternet(): void {
 		$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('getSystemValueBool')
@@ -137,15 +137,15 @@ public function testSearch(array $searchParams): void {
 
 		$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('getSystemValueBool')
 			->withConsecutive(
 				['gs.enabled', false],
 				['has_internet_connection', true],
 			)->willReturnOnConsecutiveCalls(
-				false,
+				true,
 				true,
 			);
 
@@ -197,9 +197,9 @@ public function testSearchEnableDisableLookupServer(array $searchParams, $GSEnab
 
 		$this->config->expects($this->once())
 			->method('getAppValue')
-			->with('files_sharing', 'lookupServerEnabled', 'yes')
+			->with('files_sharing', 'lookupServerEnabled', 'no')
 			->willReturn($LookupEnabled ? 'yes' : 'no');
-		if ($GSEnabled || $LookupEnabled) {
+		if ($GSEnabled) {
 			$searchResult->expects($this->once())
 				->method('addResultSet')
 				->with($type, $searchParams['expectedResult'], []);
@@ -258,11 +258,13 @@ public function testSearchEnableDisableLookupServer(array $searchParams, $GSEnab
 	}
 
 
-	public function testSearchLookupServerDisabled(): void {
-		$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);
@@ -381,7 +383,6 @@ public function dataSearchEnableDisableLookupServer() {
 						'label' => $fedIDs[0],
 						'value' => [
 							'shareType' => IShare::TYPE_REMOTE,
-							'globalScale' => false,
 							'shareWith' => $fedIDs[0]
 						],
 						'extra' => ['federationId' => $fedIDs[0]],
@@ -390,7 +391,6 @@ public function dataSearchEnableDisableLookupServer() {
 						'label' => $fedIDs[1],
 						'value' => [
 							'shareType' => IShare::TYPE_REMOTE,
-							'globalScale' => false,
 							'shareWith' => $fedIDs[1]
 						],
 						'extra' => ['federationId' => $fedIDs[1]],
@@ -399,7 +399,6 @@ public function dataSearchEnableDisableLookupServer() {
 						'label' => $fedIDs[2],
 						'value' => [
 							'shareType' => IShare::TYPE_REMOTE,
-							'globalScale' => false,
 							'shareWith' => $fedIDs[2]
 						],
 						'extra' => ['federationId' => $fedIDs[2]],
@@ -474,7 +473,7 @@ public function searchDataProvider() {
 						'label' => $fedIDs[0],
 						'value' => [
 							'shareType' => IShare::TYPE_REMOTE,
-							'globalScale' => false,
+							'globalScale' => true,
 							'shareWith' => $fedIDs[0]
 						],
 						'extra' => ['federationId' => $fedIDs[0]],
@@ -483,7 +482,7 @@ public function searchDataProvider() {
 						'label' => $fedIDs[1],
 						'value' => [
 							'shareType' => IShare::TYPE_REMOTE,
-							'globalScale' => false,
+							'globalScale' => true,
 							'shareWith' => $fedIDs[1]
 						],
 						'extra' => ['federationId' => $fedIDs[1]],
@@ -492,7 +491,7 @@ public function searchDataProvider() {
 						'label' => $fedIDs[2],
 						'value' => [
 							'shareType' => IShare::TYPE_REMOTE,
-							'globalScale' => false,
+							'globalScale' => true,
 							'shareWith' => $fedIDs[2]
 						],
 						'extra' => ['federationId' => $fedIDs[2]],