Update logic for forwardedForHeadersWorking

As discussed in https://github.com/nextcloud/server/issues/11594 when discovering if
x-forwarded-for is working properly its not possible to use getRemoteAddr because
the "client ip" is returned. For this check the ip of the last hop would be required.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>

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;
 	}

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')