Fix admin login for non-secure cookies

app/code/core/Mage/Adminhtml/Model/System/Config/Source/Cookie/Samesite.php

@@ -12,15 +12,21 @@
  */
 class Mage_Adminhtml_Model_System_Config_Source_Cookie_Samesite
 {
+    public const NONE = 'None';
+
+    public const STRICT = 'Strict';
+
+    public const LAX = 'Lax';
+
     /**
      * @return array[]
      */
     public function toOptionArray(): array
     {
         return [
-            ['value' => 'None', 'label' => Mage::helper('adminhtml')->__('None')],
-            ['value' => 'Strict', 'label' => Mage::helper('adminhtml')->__('Strict')],
-            ['value' => 'Lax', 'label' => Mage::helper('adminhtml')->__('Lax')],
+            ['value' => self::NONE, 'label' => Mage::helper('adminhtml')->__('None')],
+            ['value' => self::STRICT, 'label' => Mage::helper('adminhtml')->__('Strict')],
+            ['value' => self::LAX, 'label' => Mage::helper('adminhtml')->__('Lax')],
         ];
     }
 }

app/code/core/Mage/Core/Model/Cookie.php

@@ -7,6 +7,8 @@
  * @package    Mage_Core
  */
 
+use Mage_Adminhtml_Model_System_Config_Source_Cookie_Samesite as CookieSameSite;
+
 /**
  * Core cookie model
  *
@@ -115,7 +117,7 @@ public function getPath()
     /**
      * Retrieve cookie lifetime
      *
-     * @return int
+     * @return int|string
      */
     public function getLifetime()
     {
@@ -163,7 +165,7 @@ public function getSameSite(): string
     {
         $sameSite = Mage::getStoreConfig(self::XML_PATH_COOKIE_SAMESITE, $this->getStore());
         if (is_null($sameSite)) {
-            return 'None';
+            return CookieSameSite::NONE;
         }
         return (string) $sameSite;
     }
@@ -173,14 +175,15 @@ public function getSameSite(): string
      * Use secure on adminhtml only
      *
      * @return bool
+     * @throws Mage_Core_Exception
      */
     public function isSecure()
     {
         if ($this->getStore()->isAdmin()) {
             return $this->_getRequest()->isSecure();
         }
         // Use secure cookie if unsecure base url is actually secure
-        if (preg_match('/^https:/', $this->getStore()->getBaseUrl(Mage_Core_Model_Store::URL_TYPE_LINK, false))) {
+        if (str_starts_with($this->getStore()->getBaseUrl(Mage_Core_Model_Store::URL_TYPE_LINK, false), 'https:')) {
             return true;
         }
         return false;
@@ -198,6 +201,8 @@ public function isSecure()
      * @param bool $httponly
      * @param string $sameSite
      * @return $this
+     * @throws Zend_Controller_Response_Exception
+     * @throws Mage_Core_Exception
      */
     public function set($name, $value, $period = null, $path = null, $domain = null, $secure = null, $httponly = null, $sameSite = null)
     {
@@ -235,7 +240,7 @@ public function set($name, $value, $period = null, $path = null, $domain = null,
             $sameSite = $this->getSameSite();
         }
 
-        if ($sameSite === 'None') {
+        if ($sameSite === CookieSameSite::NONE) {
             // Enforce specification SameSite None requires secure
             $secure = true;
         }
@@ -267,6 +272,8 @@ public function set($name, $value, $period = null, $path = null, $domain = null,
      * @param bool $httponly
      * @param string $sameSite
      * @return $this
+     * @throws Zend_Controller_Response_Exception
+     * @throws Mage_Core_Exception
      */
     public function renew($name, $period = null, $path = null, $domain = null, $secure = null, $httponly = null, $sameSite = null)
     {
@@ -284,7 +291,7 @@ public function renew($name, $period = null, $path = null, $domain = null, $secu
      * Retrieve cookie or false if not exists
      *
      * @param string $name The cookie name
-     * @return mixed
+     * @return mixed|false
      */
     public function get($name = null)
     {
@@ -301,6 +308,8 @@ public function get($name = null)
      * @param int|bool $httponly
      * @param string $sameSite
      * @return $this
+     * @throws Zend_Controller_Response_Exception
+     * @throws Mage_Core_Exception
      */
     public function delete($name, $path = null, $domain = null, $secure = null, $httponly = null, $sameSite = null)
     {

app/code/core/Mage/Core/Model/Session/Abstract/Varien.php

@@ -7,6 +7,8 @@
  * @package    Mage_Core
  */
 
+use Mage_Adminhtml_Model_System_Config_Source_Cookie_Samesite as CookieSamesite;
+
 /**
  * @package    Mage_Core
  *
@@ -109,6 +111,11 @@ public function start($sessionName = null)
             'samesite' => $cookie->getSameSite(),
         ];
 
+        if (!$cookie->isSecure() && $cookieParams['samesite'] === CookieSamesite::NONE) {
+            // PHP doesn't allow to set SameSite=None for non-secure cookies
+            unset($cookieParams['samesite']);
+        }
+
         if (!$cookieParams['httponly']) {
             unset($cookieParams['httponly']);
             if (!$cookieParams['secure']) {
@@ -129,12 +136,12 @@ public function start($sessionName = null)
             $this->setSessionName($sessionName);
 
             // Migrate old cookie from 'frontend'
-            if ($sessionName === \Mage_Core_Controller_Front_Action::SESSION_NAMESPACE
+            if ($sessionName === Mage_Core_Controller_Front_Action::SESSION_NAMESPACE
                 && $cookie->get('frontend')
-                && ! $cookie->get(\Mage_Core_Controller_Front_Action::SESSION_NAMESPACE)
+                && ! $cookie->get(Mage_Core_Controller_Front_Action::SESSION_NAMESPACE)
             ) {
                 $frontendValue = $cookie->get('frontend');
-                $_COOKIE[\Mage_Core_Controller_Front_Action::SESSION_NAMESPACE] = $frontendValue;
+                $_COOKIE[Mage_Core_Controller_Front_Action::SESSION_NAMESPACE] = $frontendValue;
                 $cookie->set(Mage_Core_Controller_Front_Action::SESSION_NAMESPACE, $frontendValue);
                 $cookie->delete('frontend');
             }
@@ -172,7 +179,7 @@ public function start($sessionName = null)
 
             // Migrate old cookie from 'frontend'
             if (!$cookieValue
-                && $sessionName === \Mage_Core_Controller_Front_Action::SESSION_NAMESPACE
+                && $sessionName === Mage_Core_Controller_Front_Action::SESSION_NAMESPACE
                 && $cookie->get('frontend_cid')
             ) {
                 $cookieValue = $cookie->get('frontend_cid');

tests/unit/Mage/Core/Model/CookieTest.php

@@ -0,0 +1,142 @@
+<?php
+
+/**
+ * @copyright  For copyright and license information, read the COPYING.txt file.
+ * @link       /COPYING.txt
+ * @license    Open Software License (OSL 3.0)
+ * @package    OpenMage_Tests
+ */
+
+declare(strict_types=1);
+
+namespace unit\Mage\Core\Model;
+
+use Mage;
+use Mage_Core_Model_Cookie as Subject;
+use Mage_Core_Model_Store;
+use OpenMage\Tests\Unit\OpenMageTest;
+
+final class CookieTest extends OpenMageTest
+{
+    private static Subject $subject;
+
+    public static function setUpBeforeClass(): void
+    {
+        parent::setUpBeforeClass();
+        self::$subject = Mage::getModel('core/cookie');
+    }
+
+    /**
+     * @group Model
+     */
+    public function testSetStore(): void
+    {
+        self::assertInstanceOf(Subject::class, self::$subject->setStore(null));
+    }
+
+    /**
+     * @group Model
+     */
+    public function testGetStore(): void
+    {
+        self::assertInstanceOf(Mage_Core_Model_Store::class, self::$subject->getStore());
+    }
+
+    /**
+     * @group Model
+     */
+    public function testGetDomain(): void
+    {
+        self::assertFalse(self::$subject->getDomain());
+    }
+
+    /**
+     * @group Model
+     */
+    public function testGetConfigDomain(): void
+    {
+        self::assertIsString(self::$subject->getConfigDomain());
+    }
+
+    /**
+     * @group Model
+     */
+    public function testGetPath(): void
+    {
+        self::assertIsString(self::$subject->getPath());
+    }
+
+    /**
+     * @group Model
+     */
+    public function testGetLifetime(): void
+    {
+        self::assertIsNumeric(self::$subject->getLifetime());
+    }
+
+    /**
+     * @group Model
+     */
+    public function testSetLifetime(): void
+    {
+        self::assertInstanceOf(Subject::class, self::$subject->setLifetime(0));
+    }
+
+    /**
+     * @group Model
+     */
+    public function testGetHttponly(): void
+    {
+        self::assertIsBool(self::$subject->getHttponly());
+    }
+
+    /**
+     * @group Model
+     */
+    public function testGetSameSite(): void
+    {
+        self::assertIsString(self::$subject->getSameSite());
+    }
+
+    /**
+     * @group Model
+     */
+    public function testIsSecure(): void
+    {
+        self::assertIsBool(self::$subject->isSecure());
+    }
+
+    /**
+     * @group Model
+     * @runInSeparateProcess
+     */
+    public function testSet(): void
+    {
+        self::assertInstanceOf(Subject::class, self::$subject->set('test', 'value'));
+    }
+
+    /**
+     * @group Model
+     */
+    public function testRenew(): void
+    {
+        self::assertInstanceOf(Subject::class, self::$subject->renew('test'));
+    }
+
+    /**
+     * @group Model
+     */
+    public function testGet(): void
+    {
+        self::assertIsArray(self::$subject->get());
+    }
+
+    /**
+     * @group Model
+     * @runInSeparateProcess
+     */
+    public function testDelete(): void
+    {
+        self::assertInstanceOf(Subject::class, self::$subject->delete('test'));
+    }
+}