From 790ffbc15197bb4ad5969b5b927a81c5d5838838 Mon Sep 17 00:00:00 2001
From: Lukas Bestle <mail@lukasbestle.com>
Date: Fri, 24 Apr 2020 13:43:17 +0200
Subject: [PATCH] Fix install check when a reverse proxy is used

---
 src/Cms/System.php              | 28 ++++++++++++++++++++-----
 src/Http/Visitor.php            |  2 +-
 tests/Cms/System/SystemTest.php | 37 +++++++++++++++++----------------
 3 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/src/Cms/System.php b/src/Cms/System.php
index 4d6c47ceea..8cc4981477 100644
--- a/src/Cms/System.php
+++ b/src/Cms/System.php
@@ -189,22 +189,40 @@ public function isInstalled(): bool
      */
     public function isLocal(): bool
     {
-        $server = $this->app->server();
-        $host   = $server->host();
+        $server  = $this->app->server();
+        $visitor = $this->app->visitor();
+        $host    = $server->host();
 
         if ($host === 'localhost') {
             return true;
         }
 
-        if (in_array($server->address(), ['::1', '127.0.0.1', '0.0.0.0']) === true) {
+        if (Str::endsWith($host, '.local') === true) {
             return true;
         }
 
-        if (Str::endsWith($host, '.local') === true) {
+        if (Str::endsWith($host, '.test') === true) {
             return true;
         }
 
-        if (Str::endsWith($host, '.test') === true) {
+        if (in_array($visitor->ip(), ['::1', '127.0.0.1']) === true) {
+            // ensure that there is no reverse proxy in between
+
+            if (
+                isset($_SERVER['HTTP_X_FORWARDED_FOR']) === true &&
+                in_array($_SERVER['HTTP_X_FORWARDED_FOR'], ['::1', '127.0.0.1']) === false
+            ) {
+                return false;
+            }
+
+            if (
+                isset($_SERVER['HTTP_CLIENT_IP']) === true &&
+                in_array($_SERVER['HTTP_CLIENT_IP'], ['::1', '127.0.0.1']) === false
+            ) {
+                return false;
+            }
+
+            // no reverse proxy or the real client also comes from localhost
             return true;
         }
 
diff --git a/src/Http/Visitor.php b/src/Http/Visitor.php
index 4ae7102db9..0a7e8f3d3d 100644
--- a/src/Http/Visitor.php
+++ b/src/Http/Visitor.php
@@ -55,7 +55,7 @@ class Visitor
      */
     public function __construct(array $arguments = [])
     {
-        $this->ip($arguments['ip'] ?? getenv('REMOTE_ADDR'));
+        $this->ip($arguments['ip'] ?? $_SERVER['REMOTE_ADDR'] ?? '');
         $this->userAgent($arguments['userAgent'] ?? $_SERVER['HTTP_USER_AGENT'] ?? '');
         $this->acceptedLanguage($arguments['acceptedLanguage'] ?? $_SERVER['HTTP_ACCEPT_LANGUAGE'] ?? '');
         $this->acceptedMimeType($arguments['acceptedMimeType'] ?? $_SERVER['HTTP_ACCEPT'] ?? '');
diff --git a/tests/Cms/System/SystemTest.php b/tests/Cms/System/SystemTest.php
index 8cb21f7d8e..7bbd6172dd 100644
--- a/tests/Cms/System/SystemTest.php
+++ b/tests/Cms/System/SystemTest.php
@@ -76,33 +76,34 @@ public function testIsLocalWithServerNames($name, $expected)
 
         $system = new System($this->app);
         $this->assertEquals($expected, $system->isLocal());
-
-        // reset SERVER_NAME
-        $_SERVER['SERVER_NAME'] = null;
     }
 
-    public function serverAddressProvider()
+    public function clientAddressProvider()
     {
         return [
-            ['127.0.0.1', true],
-            ['::1', true],
-            ['0.0.0.0', true],
-            ['1.2.3.4', false],
+            ['127.0.0.1', '127.0.0.1', true],
+            ['::1', '::1', true],
+            ['127.0.0.1', '::1', true],
+            ['::1', '127.0.0.1', true],
+            ['1.2.3.4', '127.0.0.1', false],
+            ['127.0.0.1', '1.2.3.4', false],
         ];
     }
 
     /**
-     * @dataProvider serverAddressProvider
+     * @dataProvider clientAddressProvider
      */
-    public function testIsLocalWithServerAddresses($address, $expected)
+    public function testIsLocalWithClientAddresses(string $address, string $forwardedAddress, bool $expected)
     {
-        $_SERVER['SERVER_ADDR'] = $address;
-
         $system = new System($this->app);
-        $this->assertEquals($expected, $system->isLocal());
 
-        // reset SERVER_ADDR
-        $_SERVER['SERVER_ADDR'] = null;
+        $_SERVER['REMOTE_ADDR'] = $address;
+        $_SERVER['HTTP_X_FORWARDED_FOR'] = $forwardedAddress;
+        $this->assertSame($expected, $system->isLocal());
+
+        unset($_SERVER['HTTP_X_FORWARDED_FOR']);
+        $_SERVER['HTTP_CLIENT_IP'] = $forwardedAddress;
+        $this->assertSame($expected, $system->isLocal());
     }
 
     public function indexUrlProvider()
@@ -172,7 +173,7 @@ public function testLicenseUrl($url, $expected)
 
     public function testIsInstallableOnLocalhost()
     {
-        $_SERVER['SERVER_NAME'] = 'localhost';
+        $_SERVER['REMOTE_ADDR'] = '127.0.0.1';
 
         $system = new System($this->app);
 
@@ -181,7 +182,7 @@ public function testIsInstallableOnLocalhost()
 
     public function testIsInstallableOnPublicServer()
     {
-        $_SERVER['SERVER_NAME'] = 'getkirby.com';
+        $_SERVER['REMOTE_ADDR'] = '1.2.3.4';
 
         $system = new System($this->app);
 
@@ -190,7 +191,7 @@ public function testIsInstallableOnPublicServer()
 
     public function testIsInstallableOnPublicServerWithOverride()
     {
-        $_SERVER['SERVER_NAME'] = 'getkirby.com';
+        $_SERVER['REMOTE_ADDR'] = '1.2.3.4';
 
         $app = $this->app->clone([
             'options' => [