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

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
pull/51406/head
Ferdinand Thiessen 2025-03-10 16:24:26 +07:00
parent 7d3047f5cb
commit c0003fbd9c
No known key found for this signature in database
GPG Key ID: 45FAE7268762B400
11 changed files with 79 additions and 62 deletions

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

@ -32,17 +32,23 @@
{{ t('federatedfilesharing', 'Allow people on this server to receive group shares from other servers') }} {{ t('federatedfilesharing', 'Allow people on this server to receive group shares from other servers') }}
</NcCheckboxRadioSwitch> </NcCheckboxRadioSwitch>
<NcCheckboxRadioSwitch type="switch" <fieldset>
:checked.sync="lookupServerEnabled" <legend>{{ t('federatedfilesharing', 'The lookup server is only available for global scale.') }}</legend>
@update:checked="update('lookupServerEnabled', lookupServerEnabled)">
{{ t('federatedfilesharing', 'Search global and public address book for people') }}
</NcCheckboxRadioSwitch>
<NcCheckboxRadioSwitch type="switch" <NcCheckboxRadioSwitch type="switch"
:checked.sync="lookupServerUploadEnabled" :checked.sync="lookupServerEnabled"
@update:checked="update('lookupServerUploadEnabled', lookupServerUploadEnabled)"> disabled
{{ t('federatedfilesharing', 'Allow people to publish their data to a global and public address book') }} @update:checked="update('lookupServerEnabled', lookupServerEnabled)">
</NcCheckboxRadioSwitch> {{ 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>
</NcSettingsSection> </NcSettingsSection>
</template> </template>

@ -834,10 +834,13 @@ class FederatedShareProviderTest extends \Test\TestCase {
public function dataTestIsLookupServerQueriesEnabled() { public function dataTestIsLookupServerQueriesEnabled() {
return [ return [
[false, 'yes', true],
[false, 'no', false],
[true, 'yes', true], [true, 'yes', true],
[true, 'no', 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],
]; ];
} }
@ -861,10 +864,13 @@ class FederatedShareProviderTest extends \Test\TestCase {
public function dataTestIsLookupServerUploadEnabled() { public function dataTestIsLookupServerUploadEnabled() {
return [ return [
[false, 'yes', true],
[false, 'no', false],
[true, 'yes', false], [true, 'yes', false],
[true, 'no', 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],
]; ];
} }

@ -20,6 +20,7 @@ use OCP\Collaboration\Collaborators\ISearch;
use OCP\Collaboration\Collaborators\ISearchResult; use OCP\Collaboration\Collaborators\ISearchResult;
use OCP\Collaboration\Collaborators\SearchResultType; use OCP\Collaboration\Collaborators\SearchResultType;
use OCP\Constants; use OCP\Constants;
use OCP\GlobalScale\IConfig as GlobalScaleIConfig;
use OCP\IConfig; use OCP\IConfig;
use OCP\IRequest; use OCP\IRequest;
use OCP\IURLGenerator; use OCP\IURLGenerator;
@ -181,12 +182,11 @@ class ShareesAPIController extends OCSController {
$this->limit = $perPage; $this->limit = $perPage;
$this->offset = $perPage * ($page - 1); $this->offset = $perPage * ($page - 1);
// In global scale mode we always search the loogup server // In global scale mode we always search the lookup server
$lookup = $this->config->getSystemValueBool('gs.enabled', false) $this->result['lookupEnabled'] = \OCP\Server::get(GlobalScaleIConfig::class)->isGlobalScaleEnabled();
|| $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes'; // TODO: Reconsider using lookup server for non-global-scale federation
$this->result['lookupEnabled'] = $lookup;
[$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 // extra treatment for 'exact' subarray, with a single merge expected keys might be lost
if (isset($result['exact'])) { if (isset($result['exact'])) {

@ -12,6 +12,7 @@ use OCP\AppFramework\Http;
use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSBadRequestException;
use OCP\Collaboration\Collaborators\ISearch; use OCP\Collaboration\Collaborators\ISearch;
use OCP\GlobalScale\IConfig as GlobalScaleIConfig;
use OCP\IConfig; use OCP\IConfig;
use OCP\IRequest; use OCP\IRequest;
use OCP\IURLGenerator; use OCP\IURLGenerator;
@ -230,14 +231,14 @@ class ShareesAPIControllerTest extends TestCase {
$perPage = $getData['perPage'] ?? 200; $perPage = $getData['perPage'] ?? 200;
$shareType = $getData['shareType'] ?? null; $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 */ /** @var IConfig|MockObject $config */
$config = $this->createMock(IConfig::class); $config = $this->createMock(IConfig::class);
$config->expects($this->exactly(1))
->method('getAppValue')
->with($this->anything(), $this->anything(), $this->anything())
->willReturnMap([
['files_sharing', 'lookupServerEnabled', 'no', 'yes'],
]);
$this->shareManager->expects($this->once()) $this->shareManager->expects($this->once())
->method('allowGroupSharing') ->method('allowGroupSharing')

@ -90,10 +90,12 @@ class RetryJob extends Job {
* - max retries are reached (set to 5) * - max retries are reached (set to 5)
*/ */
protected function shouldRemoveBackgroundJob(): bool { protected function shouldRemoveBackgroundJob(): bool {
return $this->config->getSystemValueBool('has_internet_connection', true) === false || // TODO: Remove global scale condition once lookup server is used for non-global scale federation
$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === '' || // return $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') !== 'yes'
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') !== 'yes' || return !$this->config->getSystemValueBool('gs.enabled', false)
$this->retries >= 5; || $this->config->getSystemValueBool('has_internet_connection', true) === false
|| $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === ''
|| $this->retries >= 5;
} }
protected function shouldRun(): bool { protected function shouldRun(): bool {
@ -155,7 +157,7 @@ class RetryJob extends Job {
$user->getUID(), $user->getUID(),
'lookup_server_connector', 'lookup_server_connector',
'update_retries', 'update_retries',
$this->retries + 1 (string)($this->retries + 1),
); );
} }
} }

@ -61,8 +61,9 @@ class UpdateLookupServer {
* @return bool * @return bool
*/ */
private function shouldUpdateLookupServer(): bool { private function shouldUpdateLookupServer(): bool {
return $this->config->getSystemValueBool('has_internet_connection', true) === true && // TODO: Consider reenable for non-global-scale setups by checking "'files_sharing', 'lookupServerUploadEnabled'" instead of "gs.enabled"
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') === 'yes' && return $this->config->getSystemValueBool('gs.enabled', false)
$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== ''; && $this->config->getSystemValueBool('has_internet_connection', true)
&& $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== '';
} }
} }

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

@ -1027,11 +1027,6 @@
<code><![CDATA[$storage1->instanceOfStorage('\OC\Files\ObjectStore\ObjectStoreStorage')]]></code> <code><![CDATA[$storage1->instanceOfStorage('\OC\Files\ObjectStore\ObjectStoreStorage')]]></code>
</RedundantCondition> </RedundantCondition>
</file> </file>
<file src="apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php">
<InvalidArgument>
<code><![CDATA[$this->retries + 1]]></code>
</InvalidArgument>
</file>
<file src="apps/oauth2/lib/Controller/OauthApiController.php"> <file src="apps/oauth2/lib/Controller/OauthApiController.php">
<NoInterfaceProperties> <NoInterfaceProperties>
<code><![CDATA[$this->request->server]]></code> <code><![CDATA[$this->request->server]]></code>

@ -35,8 +35,10 @@ class LookupPlugin implements ISearchPlugin {
$isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes'; $isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
$hasInternetConnection = $this->config->getSystemValueBool('has_internet_connection', true); $hasInternetConnection = $this->config->getSystemValueBool('has_internet_connection', true);
// if case of Global Scale we always search the lookup server // If case of Global Scale we always search the lookup server
if (!$isGlobalScaleEnabled && (!$isLookupServerEnabled || !$hasInternetConnection)) { // TODO: Reconsider using the lookup server for non-global scale
// if (!$isGlobalScaleEnabled && (!$isLookupServerEnabled || !$hasInternetConnection || $disableLookupServer)) {
if (!$isGlobalScaleEnabled) {
return false; return false;
} }

@ -80,7 +80,7 @@ class LookupPluginTest extends TestCase {
['gs.enabled', false], ['gs.enabled', false],
['has_internet_connection', true], ['has_internet_connection', true],
)->willReturnOnConsecutiveCalls( )->willReturnOnConsecutiveCalls(
false, true,
true, true,
); );
@ -145,7 +145,7 @@ class LookupPluginTest extends TestCase {
['gs.enabled', false], ['gs.enabled', false],
['has_internet_connection', true], ['has_internet_connection', true],
)->willReturnOnConsecutiveCalls( )->willReturnOnConsecutiveCalls(
false, true,
true, true,
); );
@ -199,7 +199,7 @@ class LookupPluginTest extends TestCase {
->method('getAppValue') ->method('getAppValue')
->with('files_sharing', 'lookupServerEnabled', 'no') ->with('files_sharing', 'lookupServerEnabled', 'no')
->willReturn($LookupEnabled ? 'yes' : 'no'); ->willReturn($LookupEnabled ? 'yes' : 'no');
if ($GSEnabled || $LookupEnabled) { if ($GSEnabled) {
$searchResult->expects($this->once()) $searchResult->expects($this->once())
->method('addResultSet') ->method('addResultSet')
->with($type, $searchParams['expectedResult'], []); ->with($type, $searchParams['expectedResult'], []);
@ -257,12 +257,13 @@ class LookupPluginTest extends TestCase {
$this->assertFalse($moreResults); $this->assertFalse($moreResults);
} }
public function testSearchGSDisabled(): void {
public function testSearchLookupServerDisabled() { $this->config->expects($this->atLeastOnce())
$this->config->expects($this->once()) ->method('getSystemValueBool')
->method('getAppValue') ->willReturnMap([
->with('files_sharing', 'lookupServerEnabled', 'yes') ['has_internet_connection', true, true],
->willReturn('no'); ['gs.enabled', false, false],
]);
/** @var ISearchResult|MockObject $searchResult */ /** @var ISearchResult|MockObject $searchResult */
$searchResult = $this->createMock(ISearchResult::class); $searchResult = $this->createMock(ISearchResult::class);
@ -381,7 +382,6 @@ class LookupPluginTest extends TestCase {
'label' => $fedIDs[0], 'label' => $fedIDs[0],
'value' => [ 'value' => [
'shareType' => IShare::TYPE_REMOTE, 'shareType' => IShare::TYPE_REMOTE,
'globalScale' => false,
'shareWith' => $fedIDs[0] 'shareWith' => $fedIDs[0]
], ],
'extra' => ['federationId' => $fedIDs[0]], 'extra' => ['federationId' => $fedIDs[0]],
@ -390,7 +390,6 @@ class LookupPluginTest extends TestCase {
'label' => $fedIDs[1], 'label' => $fedIDs[1],
'value' => [ 'value' => [
'shareType' => IShare::TYPE_REMOTE, 'shareType' => IShare::TYPE_REMOTE,
'globalScale' => false,
'shareWith' => $fedIDs[1] 'shareWith' => $fedIDs[1]
], ],
'extra' => ['federationId' => $fedIDs[1]], 'extra' => ['federationId' => $fedIDs[1]],
@ -399,7 +398,6 @@ class LookupPluginTest extends TestCase {
'label' => $fedIDs[2], 'label' => $fedIDs[2],
'value' => [ 'value' => [
'shareType' => IShare::TYPE_REMOTE, 'shareType' => IShare::TYPE_REMOTE,
'globalScale' => false,
'shareWith' => $fedIDs[2] 'shareWith' => $fedIDs[2]
], ],
'extra' => ['federationId' => $fedIDs[2]], 'extra' => ['federationId' => $fedIDs[2]],
@ -474,7 +472,7 @@ class LookupPluginTest extends TestCase {
'label' => $fedIDs[0], 'label' => $fedIDs[0],
'value' => [ 'value' => [
'shareType' => IShare::TYPE_REMOTE, 'shareType' => IShare::TYPE_REMOTE,
'globalScale' => false, 'globalScale' => true,
'shareWith' => $fedIDs[0] 'shareWith' => $fedIDs[0]
], ],
'extra' => ['federationId' => $fedIDs[0]], 'extra' => ['federationId' => $fedIDs[0]],
@ -483,7 +481,7 @@ class LookupPluginTest extends TestCase {
'label' => $fedIDs[1], 'label' => $fedIDs[1],
'value' => [ 'value' => [
'shareType' => IShare::TYPE_REMOTE, 'shareType' => IShare::TYPE_REMOTE,
'globalScale' => false, 'globalScale' => true,
'shareWith' => $fedIDs[1] 'shareWith' => $fedIDs[1]
], ],
'extra' => ['federationId' => $fedIDs[1]], 'extra' => ['federationId' => $fedIDs[1]],
@ -492,7 +490,7 @@ class LookupPluginTest extends TestCase {
'label' => $fedIDs[2], 'label' => $fedIDs[2],
'value' => [ 'value' => [
'shareType' => IShare::TYPE_REMOTE, 'shareType' => IShare::TYPE_REMOTE,
'globalScale' => false, 'globalScale' => true,
'shareWith' => $fedIDs[2] 'shareWith' => $fedIDs[2]
], ],
'extra' => ['federationId' => $fedIDs[2]], 'extra' => ['federationId' => $fedIDs[2]],