Merge remote-tracking branch '30699/ip-range-for-maintenance-mode' into community_prs_march

Author: Indrani Sonawane

app/code/Magento/Backend/Model/Validator/IpValidator.php

@@ -1,10 +1,14 @@
 <?php
+
 /**
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
 namespace Magento\Backend\Model\Validator;
 
+use Magento\Framework\App\Utility\IPAddress;
+
 /**
  * Class to validate list of IPs for maintenance commands
  */
@@ -25,12 +29,22 @@ class IpValidator
      */
     private $invalidIps;
 
+    /**
+     * @param IPAddress $ipAddress
+     */
+    public function __construct(
+        private readonly IPAddress $ipAddress,
+    ) {
+    }
+
     /**
      * Validates list of ips
      *
      * @param string[] $ips
      * @param bool $noneAllowed
+     *
      * @return string[]
+     *
      * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      */
     public function validateIps(array $ips, $noneAllowed)
@@ -55,22 +69,26 @@ public function validateIps(array $ips, $noneAllowed)
                 $messages[] = "Invalid IP $invalidIp";
             }
         }
+
         return $messages;
     }
 
     /**
      * Filter ips into 'none', valid and invalid ips
      *
      * @param string[] $ips
+     *
      * @return void
      */
     private function filterIps(array $ips)
     {
         foreach ($ips as $ip) {
-            if (filter_var($ip, FILTER_VALIDATE_IP)) {
-                $this->validIps[] = $ip;
-            } elseif ($ip == 'none') {
+            if ($ip === 'none') {
                 $this->none[] = $ip;
+            } elseif ($this->ipAddress->isValidAddress($ip)) {
+                $this->validIps[] = $ip;
+            } elseif ($this->ipAddress->isValidRange($ip)) {
+                $this->validIps[] = $ip;
             } else {
                 $this->invalidIps[] = $ip;
             }

app/code/Magento/Backend/Test/Unit/Model/Validator/IpValidatorTest.php

@@ -8,6 +8,7 @@
 namespace Magento\Backend\Test\Unit\Model\Validator;
 
 use Magento\Backend\Model\Validator\IpValidator;
+use Magento\Framework\App\Utility\IPAddress;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -25,7 +26,9 @@ class IpValidatorTest extends TestCase
      */
     protected function setUp(): void
     {
-        $this->ipValidator = new IpValidator();
+        $this->ipValidator = new IpValidator(
+            new IPAddress()
+        );
     }
 
     /**
@@ -45,6 +48,7 @@ public function validateIpsNoneAllowedDataProvider(): array
     {
         return [
             [['127.0.0.1', '127.0.0.2'], []],
+            [['127.0.0.0/24'], []],
             [['none'], []],
             [['none', '127.0.0.1'], ["Multiple values are not allowed when 'none' is used"]],
             [['127.0.0.1', 'none'], ["Multiple values are not allowed when 'none' is used"]],
@@ -72,6 +76,7 @@ public function validateIpsNoneNotAllowedDataProvider()
     {
         return [
             [['127.0.0.1', '127.0.0.2'], []],
+            [['127.0.0.0/24'], []],
             [['none'], ["'none' is not allowed"]],
             [['none', '127.0.0.1'], ["'none' is not allowed"]],
             [['127.0.0.1', 'none'], ["'none' is not allowed"]],

lib/internal/Magento/Framework/App/MaintenanceMode.php

@@ -1,13 +1,15 @@
 <?php
+
 /**
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
 namespace Magento\Framework\App;
 
 use Magento\Framework\App\Filesystem\DirectoryList;
-use Magento\Framework\Filesystem;
 use Magento\Framework\Event\Manager;
+use Magento\Framework\Filesystem;
 
 /**
  * Application Maintenance Mode
@@ -17,20 +19,20 @@ class MaintenanceMode
     /**
      * Maintenance flag file name
      *
-     * DO NOT consolidate this file and the IP white list into one.
+     * DO NOT consolidate this file and the IP allow list into one.
      * It is going to work much faster in 99% of cases: the isOn() will return false whenever file doesn't exist.
      */
-    const FLAG_FILENAME = '.maintenance.flag';
+    public const FLAG_FILENAME = '.maintenance.flag';
 
     /**
      * IP-addresses file name
      */
-    const IP_FILENAME = '.maintenance.ip';
+    public const IP_FILENAME = '.maintenance.ip';
 
     /**
      * Maintenance flag dir
      */
-    const FLAG_DIR = DirectoryList::VAR_DIR;
+    public const FLAG_DIR = DirectoryList::VAR_DIR;
 
     /**
      * Path to store files
@@ -45,36 +47,59 @@ class MaintenanceMode
     private $eventManager;
 
     /**
-     * @param \Magento\Framework\Filesystem $filesystem
+     * @param Filesystem $filesystem
+     * @param Utility\IPAddress $ipAddress
      * @param Manager|null $eventManager
      */
-    public function __construct(Filesystem $filesystem, ?Manager $eventManager = null)
-    {
+    public function __construct(
+        Filesystem $filesystem,
+        private readonly Utility\IPAddress $ipAddress,
+        Manager $eventManager = null,
+    ) {
         $this->flagDir = $filesystem->getDirectoryWrite(self::FLAG_DIR);
         $this->eventManager = $eventManager ?: ObjectManager::getInstance()->get(Manager::class);
     }
 
     /**
      * Checks whether mode is on
      *
-     * Optionally specify an IP-address to compare against the white list
+     * Optionally specify an IP-address to compare against the allow list
      *
      * @param string $remoteAddr
+     *
      * @return bool
      */
     public function isOn($remoteAddr = '')
     {
         if (!$this->flagDir->isExist(self::FLAG_FILENAME)) {
             return false;
         }
-        $info = $this->getAddressInfo();
-        return !in_array($remoteAddr, $info);
+
+        if ($remoteAddr) {
+            $allowedAddresses = $this->getAddressInfo();
+            foreach ($allowedAddresses as $allowed) {
+                if ($allowed === $remoteAddr) {
+                    return false;
+                }
+
+                if (!$this->ipAddress->isValidRange($allowed)) {
+                    continue;
+                }
+
+                if ($this->ipAddress->rangeContainsAddress($allowed, $remoteAddr)) {
+                    return false;
+                }
+            }
+        }
+
+        return true;
     }
 
     /**
      * Sets maintenance mode "on" or "off"
      *
      * @param bool $isOn
+     *
      * @return bool
      */
     public function set($isOn)
@@ -84,17 +109,21 @@ public function set($isOn)
         if ($isOn) {
             return $this->flagDir->touch(self::FLAG_FILENAME);
         }
+
         if ($this->flagDir->isExist(self::FLAG_FILENAME)) {
             return $this->flagDir->delete(self::FLAG_FILENAME);
         }
+
         return true;
     }
 
     /**
      * Sets list of allowed IP addresses
      *
      * @param string $addresses
+     *
      * @return bool
+     *
      * @throws \InvalidArgumentException
      */
     public function setAddresses($addresses)
@@ -104,11 +133,14 @@ public function setAddresses($addresses)
             if ($this->flagDir->isExist(self::IP_FILENAME)) {
                 return $this->flagDir->delete(self::IP_FILENAME);
             }
+
             return true;
         }
+
         if (!preg_match('/^[^\s,]+(,[^\s,]+)*$/', $addresses)) {
             throw new \InvalidArgumentException("One or more IP-addresses is expected (comma-separated)\n");
         }
+
         $result = $this->flagDir->writeFile(self::IP_FILENAME, $addresses);
         return false !== $result;
     }