diff options
| -rw-r--r-- | lib/private/AppFramework/Http/Request.php | 8 | ||||
| -rw-r--r-- | settings/Controller/CheckSetupController.php | 7 | ||||
| -rw-r--r-- | tests/Settings/Controller/CheckSetupControllerTest.php | 52 |
3 files changed, 37 insertions, 30 deletions
diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index 0485d178b49..1d5a29f74f9 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -320,14 +320,18 @@ class Request implements \ArrayAccess, \Countable, IRequest { // There's a few headers that seem to end up in the top-level // server array. - switch($name) { + switch ($name) { case 'CONTENT_TYPE' : case 'CONTENT_LENGTH' : if (isset($this->server[$name])) { return $this->server[$name]; } break; - + case 'REMOTE_ADDR' : + if (isset($this->server[$name])) { + return $this->server[$name]; + } + break; } return ''; diff --git a/settings/Controller/CheckSetupController.php b/settings/Controller/CheckSetupController.php index 42bb4739870..fa4ed57ab95 100644 --- a/settings/Controller/CheckSetupController.php +++ b/settings/Controller/CheckSetupController.php @@ -287,12 +287,11 @@ class CheckSetupController extends Controller { */ private function forwardedForHeadersWorking() { $trustedProxies = $this->config->getSystemValue('trusted_proxies', []); - $remoteAddress = $this->request->getRemoteAddress(); + $remoteAddress = $this->request->getHeader('REMOTE_ADDR'); - if (is_array($trustedProxies) && in_array($remoteAddress, $trustedProxies)) { - return false; + if (\is_array($trustedProxies) && \in_array($remoteAddress, $trustedProxies)) { + return $remoteAddress !== $this->request->getRemoteAddress(); } - // either not enabled or working correctly return true; } diff --git a/tests/Settings/Controller/CheckSetupControllerTest.php b/tests/Settings/Controller/CheckSetupControllerTest.php index 6706573a1ad..34c7d19bd8d 100644 --- a/tests/Settings/Controller/CheckSetupControllerTest.php +++ b/tests/Settings/Controller/CheckSetupControllerTest.php @@ -295,38 +295,41 @@ class CheckSetupControllerTest extends TestCase { ); } - public function testForwardedForHeadersWorkingFalse() { + /** + * @dataProvider dataForwardedForHeadersWorking + * + * @param array $trustedProxies + * @param string $remoteAddrNoForwarded + * @param string $remoteAddr + * @param bool $result + */ + public function testForwardedForHeadersWorking(array $trustedProxies, string $remoteAddrNoForwarded, string $remoteAddr, bool $result) { $this->config->expects($this->once()) ->method('getSystemValue') ->with('trusted_proxies', []) - ->willReturn(['1.2.3.4']); + ->willReturn($trustedProxies); $this->request->expects($this->once()) + ->method('getHeader') + ->with('REMOTE_ADDR') + ->willReturn($remoteAddrNoForwarded); + $this->request->expects($this->any()) ->method('getRemoteAddress') - ->willReturn('1.2.3.4'); + ->willReturn($remoteAddr); - $this->assertFalse( - self::invokePrivate( - $this->checkSetupController, - 'forwardedForHeadersWorking' - ) + $this->assertEquals( + $result, + self::invokePrivate($this->checkSetupController, 'forwardedForHeadersWorking') ); } - public function testForwardedForHeadersWorkingTrue() { - $this->config->expects($this->once()) - ->method('getSystemValue') - ->with('trusted_proxies', []) - ->willReturn(['1.2.3.4']); - $this->request->expects($this->once()) - ->method('getRemoteAddress') - ->willReturn('4.3.2.1'); - - $this->assertTrue( - self::invokePrivate( - $this->checkSetupController, - 'forwardedForHeadersWorking' - ) - ); + public function dataForwardedForHeadersWorking() { + return [ + // description => trusted proxies, getHeader('REMOTE_ADDR'), getRemoteAddr, expected result + 'no trusted proxies' => [[], '2.2.2.2', '2.2.2.2', true], + 'trusted proxy, remote addr not trusted proxy' => [['1.1.1.1'], '2.2.2.2', '2.2.2.2', true], + 'trusted proxy, remote addr is trusted proxy, x-forwarded-for working' => [['1.1.1.1'], '1.1.1.1', '2.2.2.2', true], + 'trusted proxy, remote addr is trusted proxy, x-forwarded-for not set' => [['1.1.1.1'], '1.1.1.1', '1.1.1.1', false], + ]; } public function testCheck() { @@ -348,7 +351,8 @@ class CheckSetupControllerTest extends TestCase { ->will($this->returnValue(false)); $this->request->expects($this->once()) - ->method('getRemoteAddress') + ->method('getHeader') + ->with('REMOTE_ADDR') ->willReturn('4.3.2.1'); $client = $this->getMockBuilder('\OCP\Http\Client\IClient') |