refactor: Replace manual IP extraction with IpAddressRetriever

poweradmin · Dec 27, 2023 · 6d84801 · 6d84801
1 parent a5533ea
commit 6d84801
Show file tree

Hide file tree

Showing 6 changed files with 158 additions and 9 deletions.
diff --git a/addons/clientip.php b/addons/clientip.php
@@ -1,11 +1,28 @@
 <?php
 
-$client_ip = '';
+/*  Poweradmin, a friendly web-based admin tool for PowerDNS.
+ *  See <https://www.poweradmin.org> for more details.
+ *
+ *  Copyright 2007-2010 Rejo Zenger <rejo@zenger.nl>
+ *  Copyright 2010-2023 Poweradmin Development Team
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
 
-if (isset($_SERVER['HTTP_X_FORWARDED_FOR'])) {
-    $client_ip = filter_var($_SERVER['HTTP_X_FORWARDED_FOR'], FILTER_VALIDATE_IP);
-} else if (isset($_SERVER['REMOTE_ADDR'])) {
-    $client_ip = filter_var($_SERVER['REMOTE_ADDR'], FILTER_VALIDATE_IP);
-}
+require dirname(__DIR__) . '/vendor/autoload.php';
 
-echo htmlspecialchars($client_ip);
+use Poweradmin\Infrastructure\Web\IpAddressRetriever;
+
+$ipAddressRetriever = new IpAddressRetriever($_SERVER);
+echo htmlspecialchars($ipAddressRetriever->getClientIp());
diff --git a/lib/Infrastructure/Web/IpAddressRetriever.php b/lib/Infrastructure/Web/IpAddressRetriever.php
@@ -0,0 +1,58 @@
+<?php
+
+namespace Poweradmin\Infrastructure\Web;
+
+/*  Poweradmin, a friendly web-based admin tool for PowerDNS.
+ *  See <https://www.poweradmin.org> for more details.
+ *
+ *  Copyright 2007-2010 Rejo Zenger <rejo@zenger.nl>
+ *  Copyright 2010-2023 Poweradmin Development Team
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+class IpAddressRetriever
+{
+    private array $server;
+
+    public function __construct(array $server) {
+        $this->server = $server;
+    }
+
+    /**
+     * Get the client IP address.
+     *
+     * @return string
+     */
+    function getClientIp(): string
+    {
+        $clientIpHeaders = [
+            'HTTP_CLIENT_IP',
+            'HTTP_X_FORWARDED_FOR',
+            'REMOTE_ADDR'
+        ];
+
+        foreach ($clientIpHeaders as $header) {
+            if (!empty($this->server[$header])) {
+                $ips = array_filter(explode(',', $this->server[$header]), function($ip) {
+                    return filter_var($ip, FILTER_VALIDATE_IP);
+                });
+
+                return $ips[0] ?? '';
+            }
+        }
+
+        return '';
+    }
+}
diff --git a/tests/Infrastructure/Web/IpAddressRetrieverTest.php b/tests/Infrastructure/Web/IpAddressRetrieverTest.php
@@ -0,0 +1,72 @@
+<?php
+
+namespace Infrastructure\Web;
+
+use Poweradmin\Infrastructure\Web\IpAddressRetriever;
+use PHPUnit\Framework\TestCase;
+
+class IpAddressRetrieverTest extends TestCase
+{
+    public function testGetClientIpWithHttpClientIp()
+    {
+        $server = ['HTTP_CLIENT_IP' => '192.168.1.1'];
+        $ipRetriever = new IpAddressRetriever($server);
+        $this->assertEquals('192.168.1.1', $ipRetriever->getClientIp());
+    }
+
+    public function testGetClientIpWithHttpClientIp6()
+    {
+        $server = ['HTTP_CLIENT_IP' => '2001:db8:85a3:8d3:1319:8a2e:370:7348'];
+        $ipRetriever = new IpAddressRetriever($server);
+        $this->assertEquals('2001:db8:85a3:8d3:1319:8a2e:370:7348', $ipRetriever->getClientIp());
+    }
+
+    public function testGetClientIpWithMultipleIpsFirstValid()
+    {
+        $server = ['HTTP_X_FORWARDED_FOR' => '192.168.1.2, 10.0.0.1'];
+        $ipRetriever = new IpAddressRetriever($server);
+        $this->assertEquals('192.168.1.2', $ipRetriever->getClientIp());
+    }
+
+    public function testGetClientIpWithMultipleIpsFirstInvalid()
+    {
+        $server = ['HTTP_X_FORWARDED_FOR' => 'invalid_ip, 192.168.1.3'];
+        $ipRetriever = new IpAddressRetriever($server);
+        $this->assertEquals('', $ipRetriever->getClientIp());
+    }
+
+    public function testGetClientIpWithAllInvalidIps()
+    {
+        $server = ['HTTP_X_FORWARDED_FOR' => 'invalid_ip1, invalid_ip2'];
+        $ipRetriever = new IpAddressRetriever($server);
+        $this->assertEquals('', $ipRetriever->getClientIp());
+    }
+
+    public function testGetClientIpWithHttpXForwardedFor()
+    {
+        $server = ['HTTP_X_FORWARDED_FOR' => '192.168.1.2'];
+        $ipRetriever = new IpAddressRetriever($server);
+        $this->assertEquals('192.168.1.2', $ipRetriever->getClientIp());
+    }
+
+    public function testGetClientIpWithRemoteAddr()
+    {
+        $server = ['REMOTE_ADDR' => '192.168.1.3'];
+        $ipRetriever = new IpAddressRetriever($server);
+        $this->assertEquals('192.168.1.3', $ipRetriever->getClientIp());
+    }
+
+    public function testGetClientIpWithInvalidIp()
+    {
+        $server = ['HTTP_CLIENT_IP' => 'invalid_ip'];
+        $ipRetriever = new IpAddressRetriever($server);
+        $this->assertEquals('', $ipRetriever->getClientIp());
+    }
+
+    public function testGetClientIpWithNoIp()
+    {
+        $server = [];
+        $ipRetriever = new IpAddressRetriever($server);
+        $this->assertEquals('', $ipRetriever->getClientIp());
+    }
+}
diff --git a/vendor/composer/autoload_classmap.php b/vendor/composer/autoload_classmap.php
@@ -77,6 +77,7 @@
     'Poweradmin\\Infrastructure\\Service\\RedirectService' => $baseDir . '/lib/Infrastructure/Service/RedirectService.php',
     'Poweradmin\\Infrastructure\\SystemMonitoring\\SimpleSizeFormatter' => $baseDir . '/lib/Infrastructure/SystemMonitoring/SimpleSizeFormatter.php',
     'Poweradmin\\Infrastructure\\Web\\HttpPaginationParameters' => $baseDir . '/lib/Infrastructure/Web/HttpPaginationParameters.php',
+    'Poweradmin\\Infrastructure\\Web\\IpAddressRetriever' => $baseDir . '/lib/Infrastructure/Web/IpAddressRetriever.php',
     'Poweradmin\\Infrastructure\\Web\\LanguageCode' => $baseDir . '/lib/Infrastructure/Web/LanguageCode.php',
     'Poweradmin\\Infrastructure\\Web\\ThemeManager' => $baseDir . '/lib/Infrastructure/Web/ThemeManager.php',
     'Poweradmin\\LdapUserEventLogger' => $baseDir . '/lib/LdapUserEventLogger.php',

diff --git a/vendor/composer/autoload_static.php b/vendor/composer/autoload_static.php
@@ -154,6 +154,7 @@ class ComposerStaticInit02fd231869a48df50cb631d6580008da
         'Poweradmin\\Infrastructure\\Service\\RedirectService' => __DIR__ . '/../..' . '/lib/Infrastructure/Service/RedirectService.php',
         'Poweradmin\\Infrastructure\\SystemMonitoring\\SimpleSizeFormatter' => __DIR__ . '/../..' . '/lib/Infrastructure/SystemMonitoring/SimpleSizeFormatter.php',
         'Poweradmin\\Infrastructure\\Web\\HttpPaginationParameters' => __DIR__ . '/../..' . '/lib/Infrastructure/Web/HttpPaginationParameters.php',
+        'Poweradmin\\Infrastructure\\Web\\IpAddressRetriever' => __DIR__ . '/../..' . '/lib/Infrastructure/Web/IpAddressRetriever.php',
         'Poweradmin\\Infrastructure\\Web\\LanguageCode' => __DIR__ . '/../..' . '/lib/Infrastructure/Web/LanguageCode.php',
         'Poweradmin\\Infrastructure\\Web\\ThemeManager' => __DIR__ . '/../..' . '/lib/Infrastructure/Web/ThemeManager.php',
         'Poweradmin\\LdapUserEventLogger' => __DIR__ . '/../..' . '/lib/LdapUserEventLogger.php',

diff --git a/vendor/composer/installed.php b/vendor/composer/installed.php
@@ -3,7 +3,7 @@
         'name' => 'poweradmin/poweradmin',
         'pretty_version' => 'dev-master',
         'version' => 'dev-master',
-        'reference' => '9cd23eae7571ea9757ffdf33cf7a880312436c7f',
+        'reference' => 'a5533eafe8cf25b20ec4e3b34a6767c8e71f373b',
         'type' => 'project',
         'install_path' => __DIR__ . '/../../',
         'aliases' => array(),
@@ -13,7 +13,7 @@
         'poweradmin/poweradmin' => array(
             'pretty_version' => 'dev-master',
             'version' => 'dev-master',
-            'reference' => '9cd23eae7571ea9757ffdf33cf7a880312436c7f',
+            'reference' => 'a5533eafe8cf25b20ec4e3b34a6767c8e71f373b',
             'type' => 'project',
             'install_path' => __DIR__ . '/../../',
             'aliases' => array(),