diff options
author | Joas Schilling <213943+nickvergessen@users.noreply.github.com> | 2021-11-10 16:12:38 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-11-10 16:12:38 +0300 |
commit | 9673579f2097be9cce225a69b804f4f06f124473 (patch) | |
tree | b43f9f51f89657cb3e97c770b416b4d038078d4d | |
parent | f5954a27613c7ad7e8bedc096e600bd93d697a4f (diff) | |
parent | e8f7a08e5215c3ac73ae447f5f656509694674f6 (diff) |
Merge pull request #29522 from nextcloud/bugfix/26256/make-sure-trusted_proxies-is-an-array
Make sure trusted_proxies is an array
-rw-r--r-- | apps/settings/lib/Controller/CheckSetupController.php | 10 | ||||
-rw-r--r-- | apps/settings/tests/Controller/CheckSetupControllerTest.php | 79 |
2 files changed, 63 insertions, 26 deletions
diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 99e731b594c..f29b585da68 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -330,7 +330,7 @@ class CheckSetupController extends Controller { * * @return bool */ - private function forwardedForHeadersWorking() { + private function forwardedForHeadersWorking(): bool { $trustedProxies = $this->config->getSystemValue('trusted_proxies', []); $remoteAddress = $this->request->getHeader('REMOTE_ADDR'); @@ -338,8 +338,12 @@ class CheckSetupController extends Controller { return false; } - if (\is_array($trustedProxies) && \in_array($remoteAddress, $trustedProxies, true) && $remoteAddress !== '127.0.0.1') { - return $remoteAddress !== $this->request->getRemoteAddress(); + if (\is_array($trustedProxies)) { + if (\in_array($remoteAddress, $trustedProxies, true) && $remoteAddress !== '127.0.0.1') { + return $remoteAddress !== $this->request->getRemoteAddress(); + } + } else { + return false; } // either not enabled or working correctly diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 1924ddda951..612a04d7d62 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -55,6 +55,7 @@ use OCP\IRequest; use OCP\ITempManager; use OCP\IURLGenerator; use OCP\Lock\ILockingProvider; +use OCP\Notification\IManager; use PHPUnit\Framework\MockObject\MockObject; use Psr\Http\Message\ResponseInterface; use Psr\Log\LoggerInterface; @@ -102,6 +103,8 @@ class CheckSetupControllerTest extends TestCase { private $connection; /** @var ITempManager|\PHPUnit\Framework\MockObject\MockObject */ private $tempManager; + /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ + private $notificationManager; /** * Holds a list of directories created during tests. @@ -145,6 +148,7 @@ class CheckSetupControllerTest extends TestCase { $this->connection = $this->getMockBuilder(IDBConnection::class) ->disableOriginalConstructor()->getMock(); $this->tempManager = $this->getMockBuilder(ITempManager::class)->getMock(); + $this->notificationManager = $this->getMockBuilder(IManager::class)->getMock(); $this->checkSetupController = $this->getMockBuilder(CheckSetupController::class) ->setConstructorArgs([ 'settings', @@ -164,6 +168,7 @@ class CheckSetupControllerTest extends TestCase { $this->iniGetWrapper, $this->connection, $this->tempManager, + $this->notificationManager, ]) ->setMethods([ 'isReadOnlyConfig', @@ -186,7 +191,6 @@ class CheckSetupControllerTest extends TestCase { 'hasBigIntConversionPendingColumns', 'isMysqlUsedWithoutUTF8MB4', 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed', - 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed', ])->getMock(); } @@ -342,7 +346,7 @@ class CheckSetupControllerTest extends TestCase { * @param string $remoteAddr * @param bool $result */ - public function testForwardedForHeadersWorking(array $trustedProxies, string $remoteAddrNotForwarded, string $remoteAddr, bool $result) { + public function testForwardedForHeadersWorking(array $trustedProxies, string $remoteAddrNotForwarded, string $remoteAddr, bool $result): void { $this->config->expects($this->once()) ->method('getSystemValue') ->with('trusted_proxies', []) @@ -363,7 +367,7 @@ class CheckSetupControllerTest extends TestCase { ); } - public function dataForwardedForHeadersWorking() { + public function dataForwardedForHeadersWorking(): array { return [ // description => trusted proxies, getHeader('REMOTE_ADDR'), getRemoteAddr, expected result 'no trusted proxies' => [[], '2.2.2.2', '2.2.2.2', true], @@ -373,7 +377,28 @@ class CheckSetupControllerTest extends TestCase { ]; } - public function testForwardedHostPresentButTrustedProxiesEmpty() { + public function testForwardedHostPresentButTrustedProxiesNotAnArray(): void { + $this->config->expects($this->once()) + ->method('getSystemValue') + ->with('trusted_proxies', []) + ->willReturn('1.1.1.1'); + $this->request->expects($this->atLeastOnce()) + ->method('getHeader') + ->willReturnMap([ + ['REMOTE_ADDR', '1.1.1.1'], + ['X-Forwarded-Host', 'nextcloud.test'] + ]); + $this->request->expects($this->any()) + ->method('getRemoteAddress') + ->willReturn('1.1.1.1'); + + $this->assertEquals( + false, + self::invokePrivate($this->checkSetupController, 'forwardedForHeadersWorking') + ); + } + + public function testForwardedHostPresentButTrustedProxiesEmpty(): void { $this->config->expects($this->once()) ->method('getSystemValue') ->with('trusted_proxies', []) @@ -594,7 +619,7 @@ class CheckSetupControllerTest extends TestCase { 'eol' => true, 'version' => PHP_VERSION ], - 'forwardedForHeadersWorking' => true, + 'forwardedForHeadersWorking' => false, 'reverseProxyDocs' => 'reverse-proxy-doc-link', 'isCorrectMemcachedPHPModuleInstalled' => true, 'hasPassedCodeIntegrityCheck' => true, @@ -623,6 +648,8 @@ class CheckSetupControllerTest extends TestCase { 'imageMagickLacksSVGSupport' => false, 'isDefaultPhoneRegionSet' => false, 'OCA\Settings\SetupChecks\SupportedDatabase' => ['pass' => true, 'description' => '', 'severity' => 'info'], + 'isFairUseOfFreePushService' => false, + 'temporaryDirectoryWritable' => false, ] ); $this->assertEquals($expected, $this->checkSetupController->check()); @@ -647,6 +674,8 @@ class CheckSetupControllerTest extends TestCase { $this->secureRandom, $this->iniGetWrapper, $this->connection, + $this->tempManager, + $this->notificationManager, ]) ->setMethods(null)->getMock(); @@ -1401,23 +1430,25 @@ Array }); $checkSetupController = new CheckSetupController( - 'settings', - $this->request, - $this->config, - $this->clientService, - $this->urlGenerator, - $this->l10n, - $this->checker, - $this->logger, - $this->dispatcher, - $this->db, - $this->lockingProvider, - $this->dateTimeFormatter, - $this->memoryInfo, - $this->secureRandom, - $this->iniGetWrapper, - $this->connection - ); + 'settings', + $this->request, + $this->config, + $this->clientService, + $this->urlGenerator, + $this->l10n, + $this->checker, + $this->logger, + $this->dispatcher, + $this->db, + $this->lockingProvider, + $this->dateTimeFormatter, + $this->memoryInfo, + $this->secureRandom, + $this->iniGetWrapper, + $this->connection, + $this->tempManager, + $this->notificationManager + ); $this->assertSame($expected, $this->invokePrivate($checkSetupController, 'isMysqlUsedWithoutUTF8MB4')); } @@ -1466,7 +1497,9 @@ Array $this->memoryInfo, $this->secureRandom, $this->iniGetWrapper, - $this->connection + $this->connection, + $this->tempManager, + $this->notificationManager ); $this->assertSame($expected, $this->invokePrivate($checkSetupController, 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed')); |