From 8e4ae23c487b29bd8ea1a6d7d81e6996957e86fb Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 17 Oct 2018 13:09:11 +0200 Subject: [PATCH] Do not try to contact lookup server without internet connection or URL Signed-off-by: Arthur Schiwon --- .../lib/BackgroundJobs/RetryJob.php | 12 ++- .../lib/UpdateLookupServer.php | 14 ++- .../Collaborators/LookupPlugin.php | 7 +- settings/BackgroundJobs/VerifyUserData.php | 3 + .../Collaborators/LookupPluginTest.php | 99 +++++++++++++++---- 5 files changed, 108 insertions(+), 27 deletions(-) diff --git a/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php b/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php index c0fba230a2..5a7212a652 100644 --- a/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php +++ b/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php @@ -50,9 +50,15 @@ class RetryJob extends Job { $this->clientService = $clientService; $this->jobList = $jobList; + if ($config->getSystemValue('has_internet_connection', true) === false) { + return; + } + $this->lookupServer = $config->getSystemValue('lookup_server', 'https://lookup.nextcloud.com'); - $this->lookupServer = rtrim($this->lookupServer, '/'); - $this->lookupServer .= '/users'; + if (!empty($this->lookupServer)) { + $this->lookupServer = rtrim($this->lookupServer, '/'); + $this->lookupServer .= '/users'; + } } /** @@ -69,7 +75,7 @@ class RetryJob extends Job { } protected function run($argument) { - if($argument['retryNo'] === 5) { + if ($argument['retryNo'] === 5 || empty($this->lookupServer)) { return; } diff --git a/apps/lookup_server_connector/lib/UpdateLookupServer.php b/apps/lookup_server_connector/lib/UpdateLookupServer.php index ae8fcbd67c..7902ede881 100644 --- a/apps/lookup_server_connector/lib/UpdateLookupServer.php +++ b/apps/lookup_server_connector/lib/UpdateLookupServer.php @@ -64,15 +64,25 @@ class UpdateLookupServer { $this->signer = $signer; $this->jobList = $jobList; + if($config->getSystemValue('has_internet_connection', true) === false) { + return; + } + $this->lookupServer = $config->getSystemValue('lookup_server', 'https://lookup.nextcloud.com'); - $this->lookupServer = rtrim($this->lookupServer, '/'); - $this->lookupServer .= '/users'; + if(!empty($this->lookupServer)) { + $this->lookupServer = rtrim($this->lookupServer, '/'); + $this->lookupServer .= '/users'; + } } /** * @param IUser $user */ public function userUpdated(IUser $user) { + if(empty($this->lookupServer)) { + return; + } + $userData = $this->accountManager->getUser($user); $publicData = []; diff --git a/lib/private/Collaboration/Collaborators/LookupPlugin.php b/lib/private/Collaboration/Collaborators/LookupPlugin.php index ae5f7fd0cb..fb7c872c30 100644 --- a/lib/private/Collaboration/Collaborators/LookupPlugin.php +++ b/lib/private/Collaboration/Collaborators/LookupPlugin.php @@ -63,12 +63,17 @@ 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', 'no') === 'yes'; + $hasInternetConnection = (bool)$this->config->getSystemValue('has_internet_connection', true); + // if case of Global Scale we always search the lookup server - if (!$isLookupServerEnabled && !$isGlobalScaleEnabled) { + if ((!$isLookupServerEnabled && !$isGlobalScaleEnabled) || !$hasInternetConnection) { return false; } $lookupServerUrl = $this->config->getSystemValue('lookup_server', 'https://lookup.nextcloud.com'); + if(empty($lookupServerUrl)) { + return false; + } $lookupServerUrl = rtrim($lookupServerUrl, '/'); $result = []; diff --git a/settings/BackgroundJobs/VerifyUserData.php b/settings/BackgroundJobs/VerifyUserData.php index 8d54cde9dd..b4a60ec840 100644 --- a/settings/BackgroundJobs/VerifyUserData.php +++ b/settings/BackgroundJobs/VerifyUserData.php @@ -182,6 +182,9 @@ class VerifyUserData extends Job { * @return bool true if we could check the verification code, otherwise false */ protected function verifyViaLookupServer(array $argument, $dataType) { + if(empty($this->lookupServerUrl) || $this->config->getSystemValue('has_internet_connection', true) === false) { + return false; + } $user = $this->userManager->get($argument['uid']); diff --git a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php index 9019176d24..98e4adf2a7 100644 --- a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php @@ -70,7 +70,7 @@ class LookupPluginTest extends TestCase { $this->userSession->expects($this->any())->method('getUser') ->willReturn($user); $this->cloudIdManager->expects($this->any())->method('resolveCloudId') - ->willReturnCallback(function($cloudId) { + ->willReturnCallback(function ($cloudId) { if ($cloudId === 'user@myNextcloud.net') { return new CloudId('user@myNextcloud.net', 'user', 'myNextcloud.net'); } @@ -87,6 +87,58 @@ class LookupPluginTest extends TestCase { ); } + public function testSearchNoLookupServerURI() { + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('files_sharing', 'lookupServerEnabled', 'no') + ->willReturn('yes'); + $this->config->expects($this->at(0)) + ->method('getSystemValue') + ->with('gs.enabled', false) + ->willReturn(false); + + $this->config->expects($this->at(2)) + ->method('getSystemValue') + ->with('has_internet_connection', true) + ->willReturn(true); + $this->config->expects($this->at(3)) + ->method('getSystemValue') + ->with('lookup_server', 'https://lookup.nextcloud.com') + ->willReturn(''); + + $this->clientService->expects($this->never()) + ->method('newClient'); + + /** @var ISearchResult|\PHPUnit_Framework_MockObject_MockObject $searchResult */ + $searchResult = $this->createMock(ISearchResult::class); + + $this->plugin->search('foobar', 10, 0, $searchResult); + } + + public function testSearchNoInternet() { + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('files_sharing', 'lookupServerEnabled', 'no') + ->willReturn('yes'); + $this->config->expects($this->at(0)) + ->method('getSystemValue') + ->with('gs.enabled', false) + ->willReturn(false); + + $this->config->expects($this->at(2)) + ->method('getSystemValue') + ->with('has_internet_connection', true) + ->willReturn(false); + + $this->clientService->expects($this->never()) + ->method('newClient'); + + /** @var ISearchResult|\PHPUnit_Framework_MockObject_MockObject $searchResult */ + $searchResult = $this->createMock(ISearchResult::class); + + $this->plugin->search('foobar', 10, 0, $searchResult); + } + /** * @dataProvider searchDataProvider * @param array $searchParams @@ -108,7 +160,12 @@ class LookupPluginTest extends TestCase { ->method('getSystemValue') ->with('gs.enabled', false) ->willReturn(false); + $this->config->expects($this->at(2)) + ->method('getSystemValue') + ->with('has_internet_connection', true) + ->willReturn(true); + $this->config->expects($this->at(3)) ->method('getSystemValue') ->with('lookup_server', 'https://lookup.nextcloud.com') ->willReturn($searchParams['server']); @@ -121,7 +178,7 @@ class LookupPluginTest extends TestCase { $client = $this->createMock(IClient::class); $client->expects($this->once()) ->method('get') - ->willReturnCallback(function($url) use ($searchParams, $response) { + ->willReturnCallback(function ($url) use ($searchParams, $response) { $this->assertSame(strpos($url, $searchParams['server'] . '/users?search='), 0); $this->assertNotFalse(strpos($url, urlencode($searchParams['search']))); return $response; @@ -138,8 +195,6 @@ class LookupPluginTest extends TestCase { $searchResult ); - - $this->assertFalse($moreResults); } @@ -170,6 +225,10 @@ class LookupPluginTest extends TestCase { ->with($type, $searchParams['expectedResult'], []); $this->config->expects($this->at(2)) + ->method('getSystemValue') + ->with('has_internet_connection', true) + ->willReturn(true); + $this->config->expects($this->at(3)) ->method('getSystemValue') ->with('lookup_server', 'https://lookup.nextcloud.com') ->willReturn($searchParams['server']); @@ -201,8 +260,6 @@ class LookupPluginTest extends TestCase { $searchResult ); - - $this->assertFalse($moreResults); } @@ -237,9 +294,9 @@ class LookupPluginTest extends TestCase { 'offset' => 0, 'server' => 'https://lookup.example.io', 'resultBody' => [ - [ 'federationId' => $fedIDs[0] ], - [ 'federationId' => $fedIDs[1] ], - [ 'federationId' => $fedIDs[2] ], + ['federationId' => $fedIDs[0]], + ['federationId' => $fedIDs[1]], + ['federationId' => $fedIDs[2]], ], 'expectedResult' => [ [ @@ -276,9 +333,9 @@ class LookupPluginTest extends TestCase { 'offset' => 0, 'server' => 'https://lookup.example.io', 'resultBody' => [ - [ 'federationId' => $fedIDs[0] ], - [ 'federationId' => $fedIDs[1] ], - [ 'federationId' => $fedIDs[2] ], + ['federationId' => $fedIDs[0]], + ['federationId' => $fedIDs[1]], + ['federationId' => $fedIDs[2]], ], 'expectedResult' => [ [ @@ -315,9 +372,9 @@ class LookupPluginTest extends TestCase { 'offset' => 0, 'server' => 'https://lookup.example.io', 'resultBody' => [ - [ 'federationId' => $fedIDs[0] ], - [ 'federationId' => $fedIDs[1] ], - [ 'federationId' => $fedIDs[2] ], + ['federationId' => $fedIDs[0]], + ['federationId' => $fedIDs[1]], + ['federationId' => $fedIDs[2]], ], 'expectedResult' => [ [ @@ -354,9 +411,9 @@ class LookupPluginTest extends TestCase { 'offset' => 0, 'server' => 'https://lookup.example.io', 'resultBody' => [ - [ 'federationId' => $fedIDs[0] ], - [ 'federationId' => $fedIDs[1] ], - [ 'federationId' => $fedIDs[2] ], + ['federationId' => $fedIDs[0]], + ['federationId' => $fedIDs[1]], + ['federationId' => $fedIDs[2]], ], 'expectedResult' => [ [ @@ -405,9 +462,9 @@ class LookupPluginTest extends TestCase { 'offset' => 0, 'server' => 'https://lookup.example.io', 'resultBody' => [ - [ 'federationId' => $fedIDs[0] ], - [ 'federationId' => $fedIDs[1] ], - [ 'federationId' => $fedIDs[2] ], + ['federationId' => $fedIDs[0]], + ['federationId' => $fedIDs[1]], + ['federationId' => $fedIDs[2]], ], 'expectedResult' => [ [