Merge pull request #25608 from nextcloud/trusted-proxy-check-correct-header
use the configured forwarded headers for the setup check
This commit is contained in:
commit
ae4e47cbe7
|
@ -309,7 +309,14 @@ class CheckSetupController extends Controller {
|
||||||
$trustedProxies = $this->config->getSystemValue('trusted_proxies', []);
|
$trustedProxies = $this->config->getSystemValue('trusted_proxies', []);
|
||||||
$remoteAddress = $this->request->getHeader('REMOTE_ADDR');
|
$remoteAddress = $this->request->getHeader('REMOTE_ADDR');
|
||||||
|
|
||||||
if (empty($trustedProxies) && $this->request->getHeader('X-Forwarded-Host') !== '') {
|
$forwardedForHeaders = $this->config->getSystemValue('forwarded_for_headers', [
|
||||||
|
'HTTP_X_FORWARDED_FOR'
|
||||||
|
]);
|
||||||
|
$hasForwardedHeaderSet = array_reduce($forwardedForHeaders, function($set, $header) {
|
||||||
|
return $set || ($this->request->getHeader($header) !== '');
|
||||||
|
}, false);
|
||||||
|
|
||||||
|
if (empty($trustedProxies) && $hasForwardedHeaderSet) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -329,26 +329,37 @@ class CheckSetupControllerTest extends TestCase {
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @dataProvider dataForwardedForHeadersWorking
|
* @dataProvider dataForwardedForHeaders
|
||||||
*
|
*
|
||||||
* @param array $trustedProxies
|
* @param string[] $trustedProxies
|
||||||
* @param string $remoteAddrNotForwarded
|
* @param string $remoteAddrNotForwarded
|
||||||
* @param string $remoteAddr
|
* @param string $remoteAddr
|
||||||
|
* @param string[] $forwardedForHeaders
|
||||||
|
* @param array $requestHeaders
|
||||||
* @param bool $result
|
* @param bool $result
|
||||||
*/
|
*/
|
||||||
public function testForwardedForHeadersWorking(array $trustedProxies, string $remoteAddrNotForwarded, string $remoteAddr, bool $result) {
|
public function testForwardedForHeaders(array $trustedProxies, string $remoteAddrNotForwarded, string $remoteAddr, array $forwardedForHeaders, array $requestHeaders, bool $result) {
|
||||||
$this->config->expects($this->once())
|
$this->config->method('getSystemValue')
|
||||||
->method('getSystemValue')
|
->willReturnCallback(function($key, $default) use ($forwardedForHeaders, $trustedProxies) {
|
||||||
->with('trusted_proxies', [])
|
switch ($key) {
|
||||||
->willReturn($trustedProxies);
|
case 'forwarded_for_headers':
|
||||||
|
return $forwardedForHeaders;
|
||||||
|
case 'trusted_proxies':
|
||||||
|
return $trustedProxies;
|
||||||
|
default:
|
||||||
|
return $default;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
$headers = array_merge(
|
||||||
|
['REMOTE_ADDR' => $remoteAddrNotForwarded],
|
||||||
|
$requestHeaders
|
||||||
|
);
|
||||||
$this->request->expects($this->atLeastOnce())
|
$this->request->expects($this->atLeastOnce())
|
||||||
->method('getHeader')
|
->method('getHeader')
|
||||||
->willReturnMap([
|
->willReturnCallback(function($header) use ($headers) {
|
||||||
['REMOTE_ADDR', $remoteAddrNotForwarded],
|
return isset($headers[$header]) ? $headers[$header] : '';
|
||||||
['X-Forwarded-Host', '']
|
});
|
||||||
]);
|
$this->request->method('getRemoteAddress')
|
||||||
$this->request->expects($this->any())
|
|
||||||
->method('getRemoteAddress')
|
|
||||||
->willReturn($remoteAddr);
|
->willReturn($remoteAddr);
|
||||||
|
|
||||||
$this->assertEquals(
|
$this->assertEquals(
|
||||||
|
@ -357,37 +368,18 @@ class CheckSetupControllerTest extends TestCase {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function dataForwardedForHeadersWorking() {
|
public function dataForwardedForHeaders() {
|
||||||
return [
|
return [
|
||||||
// description => trusted proxies, getHeader('REMOTE_ADDR'), getRemoteAddr, expected result
|
// description => trusted proxies, getHeader('REMOTE_ADDR'), getRemoteAddr, expected result
|
||||||
'no trusted proxies' => [[], '2.2.2.2', '2.2.2.2', true],
|
'no trusted proxies' => [[], '2.2.2.2', '2.2.2.2', ['HTTP_X_FORWARDED_FOR'], [], 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 not trusted proxy' => [['1.1.1.1'], '2.2.2.2', '2.2.2.2', ['HTTP_X_FORWARDED_FOR'], [], 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, forwarded header working' => [['1.1.1.1'], '1.1.1.1', '2.2.2.2', ['HTTP_X_FORWARDED_FOR'], [], 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],
|
'trusted proxy, remote addr is trusted proxy, forwarded header not set' => [['1.1.1.1'], '1.1.1.1', '1.1.1.1', ['HTTP_X_FORWARDED_FOR'], [], false],
|
||||||
|
'no trusted proxies, but header present' => [[], '2.2.2.2', '2.2.2.2', ['HTTP_X_FORWARDED_FOR'], ['HTTP_X_FORWARDED_FOR' => '1.1.1.1'], false],
|
||||||
|
'no trusted proxies, different header present' => [[], '2.2.2.2', '2.2.2.2', ['HTTP_X_FORWARDED_FOR'], ['FORWARDED' => '1.1.1.1'], true],
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testForwardedHostPresentButTrustedProxiesEmpty() {
|
|
||||||
$this->config->expects($this->once())
|
|
||||||
->method('getSystemValue')
|
|
||||||
->with('trusted_proxies', [])
|
|
||||||
->willReturn([]);
|
|
||||||
$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 testCheck() {
|
public function testCheck() {
|
||||||
$this->config->expects($this->at(0))
|
$this->config->expects($this->at(0))
|
||||||
->method('getAppValue')
|
->method('getAppValue')
|
||||||
|
|
Loading…
Reference in New Issue