Skip to content

Commit 8152a23

Browse files
authored
Removed Mage_Persistent and improved cookie handling (#36)
1 parent 8c81cdc commit 8152a23

File tree

57 files changed

+329
-2661
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+329
-2661
lines changed

.phpstan.baseline.neon

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2025,11 +2025,6 @@ parameters:
20252025
count: 1
20262026
path: app/code/core/Mage/Core/Model/Config.php
20272027

2028-
-
2029-
message: "#^Method Mage_Core_Model_Cookie\\:\\:getHttponly\\(\\) should return bool but returns null\\.$#"
2030-
count: 1
2031-
path: app/code/core/Mage/Core/Model/Cookie.php
2032-
20332028
-
20342029
message: "#^Binary operation \"\\+\" between string and string results in an error\\.$#"
20352030
count: 1

app/code/core/Mage/Adminhtml/Block/Sales/Order/Create/Items/Grid.php

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,16 +65,6 @@ public function getItems()
6565
return $items;
6666
}
6767

68-
/**
69-
* Returns the session
70-
*
71-
* @return Mage_Persistent_Helper_Session
72-
*/
73-
public function getSession()
74-
{
75-
return $this->getParentBlock()->getSession();
76-
}
77-
7868
/**
7969
* Returns the item's calculation price
8070
*
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
/**
3+
* Maho
4+
*
5+
* @category Mage
6+
* @package Mage_Adminhtml
7+
* @copyright Copyright (c) 2024 Maho (https://mahocommerce.com)
8+
* @license https://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
9+
*/
10+
11+
/**
12+
* Adminhtml system config cookie lifetime field renderer
13+
*
14+
* @category Mage
15+
* @package Mage_Adminhtml
16+
*/
17+
class Mage_Adminhtml_Block_System_Config_Form_Field_Cookie_Lifetime extends Mage_Adminhtml_Block_System_Config_Form_Field
18+
{
19+
#[\Override]
20+
public function render(Varien_Data_Form_Element_Abstract $element): string
21+
{
22+
if ($element->getHtmlId() === 'admin_security_session_cookie_lifetime') {
23+
$min = Mage_Adminhtml_Controller_Action::SESSION_MIN_LIFETIME;
24+
$max = Mage_Adminhtml_Controller_Action::SESSION_MAX_LIFETIME;
25+
} else {
26+
$min = Mage_Core_Controller_Front_Action::SESSION_MIN_LIFETIME;
27+
$max = Mage_Core_Controller_Front_Action::SESSION_MAX_LIFETIME;
28+
}
29+
30+
$element->setComment(Mage::helper('core')->__('Value must be between %d and %d', $min, $max));
31+
$element->addClass("validate-digits validate-digits-range digits-range-$min-$max");
32+
33+
return parent::render($element);
34+
}
35+
}

app/code/core/Mage/Adminhtml/Controller/Action.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,14 @@ class Mage_Adminhtml_Controller_Action extends Mage_Core_Controller_Varien_Actio
2424
public const FLAG_IS_URLS_CHECKED = 'check_url_settings';
2525

2626
/**
27-
* Session namespace to refer in other places
27+
* Session constants to refer in other places
28+
*
29+
* Max lifetime is set to 400 days, as chromium based browsers will not allow anything higher
30+
* https://httpwg.org/http-extensions/draft-ietf-httpbis-rfc6265bis.html#name-cookie-lifetime-limits
2831
*/
29-
public const SESSION_NAMESPACE = 'adminhtml';
32+
public const SESSION_NAMESPACE = 'maho_admin_session';
33+
public const SESSION_MIN_LIFETIME = 60;
34+
public const SESSION_MAX_LIFETIME = 60 * 60 * 24 * 400;
3035

3136
/**
3237
* ACL resource

app/code/core/Mage/Adminhtml/Helper/Help/Mapping.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,6 @@ abstract class Mage_Adminhtml_Helper_Help_Mapping extends Mage_Core_Helper_Abstr
155155
'system_config/edit/section/customer' => 'configuration/customers/customer-configuration.html',
156156
'system_config/edit/section/wishlist' => 'configuration/customers/wishlist.html',
157157
'system_config/edit/section/promo' => 'configuration/customers/promotions.html',
158-
'system_config/edit/section/persistent' => 'configuration/customers/persistent-shopping-cart.html',
159158
'system_config/edit/section/sales' => 'configuration/sales/sales.html',
160159
'system_config/edit/section/sales_email' => 'configuration/sales/sales-emails.html',
161160
'system_config/edit/section/sales_pdf' => 'configuration/sales/pdf-print-outs.html',

app/code/core/Mage/Adminhtml/Model/Observer.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,22 @@ public function clearCacheConfigurationFilesAccessLevelVerification()
6666
Mage::app()->removeCache(Mage_Adminhtml_Block_Notification_Security::VERIFICATION_RESULT_CACHE_KEY);
6767
return $this;
6868
}
69+
70+
/**
71+
* Set the admin's session lifetime based on config
72+
*/
73+
public function setCookieLifetime(Varien_Event_Observer $observer): void
74+
{
75+
/** @var Mage_Core_Model_Session $session */
76+
$session = Mage::getSingleton('adminhtml/session');
77+
if ($session->getSessionName() === Mage_Adminhtml_Controller_Action::SESSION_NAMESPACE) {
78+
$lifetime = Mage::getStoreConfigAsInt('admin/security/session_cookie_lifetime');
79+
$lifetime = min($lifetime, Mage_Adminhtml_Controller_Action::SESSION_MAX_LIFETIME);
80+
$lifetime = max($lifetime, Mage_Adminhtml_Controller_Action::SESSION_MIN_LIFETIME);
81+
82+
/** @var Mage_Core_Model_Cookie $cookie */
83+
$cookie = $observer->getCookie();
84+
$cookie->setLifetime($lifetime);
85+
}
86+
}
6987
}

app/code/core/Mage/Adminhtml/etc/config.xml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,14 @@
7272
</configuration_files_access_level_verification>
7373
</observers>
7474
</admin_user_authenticate_after>
75+
<session_before_renew_cookie>
76+
<observers>
77+
<adminhtml_session_before_renew_cookie>
78+
<class>adminhtml/observer</class>
79+
<method>setCookieLifetime</method>
80+
</adminhtml_session_before_renew_cookie>
81+
</observers>
82+
</session_before_renew_cookie>
7583
</events>
7684
</global>
7785
<admin>

app/code/core/Mage/Checkout/Helper/Data.php

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -349,12 +349,4 @@ public function isCustomerMustBeLogged(): bool
349349
{
350350
return $this->isRedirectRegisterStep();
351351
}
352-
353-
public function isPersistentEnabled(): bool
354-
{
355-
if (Mage::helper('core')->isModuleEnabled('Mage_Persistent')) {
356-
return Mage::helper('persistent')->isEnabled();
357-
}
358-
return false;
359-
}
360352
}

app/code/core/Mage/Checkout/Model/Type/Onepage.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,9 @@ protected function _involveNewCustomer()
757757
);
758758
} else {
759759
$customer->sendNewAccountEmail('registered', '', $this->getQuote()->getStoreId());
760-
$this->getCustomerSession()->loginById($customer->getId());
760+
$this->getCustomerSession()
761+
->setRememberMe($this->_checkoutSession->getRememberMe())
762+
->loginById($customer->getId());
761763
}
762764
return $this;
763765
}

app/code/core/Mage/Checkout/controllers/OnepageController.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,10 @@ public function saveBillingAction()
386386
}
387387
}
388388

389+
/** @var Mage_Checkout_Model_Session */
390+
$session = Mage::getSingleton('checkout/session');
391+
$session->setRememberMe((bool)$this->getRequest()->getPost('remember_me'));
392+
389393
$this->_prepareDataJSON($result);
390394
}
391395
}

app/code/core/Mage/Core/Controller/Front/Action.php

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,15 @@
1919
class Mage_Core_Controller_Front_Action extends Mage_Core_Controller_Varien_Action
2020
{
2121
/**
22-
* Session namespace to refer in other places
22+
* Session constants to refer in other places
23+
*
24+
* Max lifetime is set to 400 days, as chromium based browsers will not allow anything higher
25+
* https://httpwg.org/http-extensions/draft-ietf-httpbis-rfc6265bis.html#name-cookie-lifetime-limits
2326
*/
24-
public const SESSION_NAMESPACE = 'om_frontend';
27+
public const SESSION_NAMESPACE = 'maho_session';
28+
public const SESSION_LEGACY_NAMESPACES = ['om_frontend', 'frontend'];
29+
public const SESSION_MIN_LIFETIME = 60 * 60;
30+
public const SESSION_MAX_LIFETIME = 60 * 60 * 24 * 400;
2531

2632
/**
2733
* Add secret key to url config path

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ class Mage_Core_Model_Config extends Mage_Core_Model_Config_Base
7676
'Mage_ImportExport' => 57,
7777
'Mage_Api2' => 58,
7878
'Mage_PageCache' => 59,
79-
'Mage_Persistent' => 60,
8079
'Mage_Weee' => 61,
8180
'Mage_CurrencySymbol' => 62
8281
];

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

Lines changed: 29 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ class Mage_Core_Model_Cookie
2424
public const XML_PATH_COOKIE_HTTPONLY = 'web/cookie/cookie_httponly';
2525
public const XML_PATH_COOKIE_SAMESITE = 'web/cookie/cookie_samesite';
2626

27+
public const DEFAULT_COOKIE_LIFETIME = 60 * 60 * 24 * 365;
28+
2729
protected $_lifetime;
2830

2931
/**
@@ -123,15 +125,7 @@ public function getPath()
123125
*/
124126
public function getLifetime()
125127
{
126-
if (!is_null($this->_lifetime)) {
127-
$lifetime = $this->_lifetime;
128-
} else {
129-
$lifetime = Mage::getStoreConfig(self::XML_PATH_COOKIE_LIFETIME, $this->getStore());
130-
}
131-
if (!is_numeric($lifetime)) {
132-
$lifetime = 3600;
133-
}
134-
return $lifetime;
128+
return $this->_lifetime ?? self::DEFAULT_COOKIE_LIFETIME;
135129
}
136130

137131
/**
@@ -153,28 +147,27 @@ public function setLifetime($lifetime)
153147
*/
154148
public function getHttponly()
155149
{
156-
$httponly = Mage::getStoreConfig(self::XML_PATH_COOKIE_HTTPONLY, $this->getStore());
157-
if (is_null($httponly)) {
158-
return null;
159-
}
160-
return (bool)$httponly;
150+
return Mage::getStoreConfigFlag(self::XML_PATH_COOKIE_HTTPONLY, $this->getStore());
161151
}
162152

163153
/**
164154
* Retrieve use SameSite
165155
*/
166156
public function getSameSite(): string
167157
{
168-
$sameSite = Mage::getStoreConfig(self::XML_PATH_COOKIE_SAMESITE, $this->getStore());
169-
if (is_null($sameSite)) {
170-
return 'None';
158+
$value = Mage::getStoreConfig(self::XML_PATH_COOKIE_SAMESITE, $this->getStore());
159+
160+
// Do not permit SameSite=None on unsecure pages, upgrade to Lax
161+
// https://developers.google.com/search/blog/2020/01/get-ready-for-new-samesitenone-secure
162+
if ($value === 'None' && $this->isSecure() === false) {
163+
return 'Lax';
171164
}
172-
return (string)$sameSite;
165+
166+
return $value;
173167
}
174168

175169
/**
176-
* Is https secure request
177-
* Use secure on adminhtml only
170+
* Retrieve use secure cookie
178171
*
179172
* @return bool
180173
*/
@@ -212,57 +205,28 @@ public function set($name, $value, $period = null, $path = null, $domain = null,
212205
return $this;
213206
}
214207

208+
$period ??= $this->getLifetime();
215209
if ($period === true) {
216-
$period = 3600 * 24 * 365;
217-
} elseif (is_null($period)) {
218-
$period = $this->getLifetime();
210+
$period = self::DEFAULT_COOKIE_LIFETIME;
219211
}
220-
221212
if ($period == 0) {
222-
$expire = 0;
213+
$expires = 0;
223214
} else {
224-
$expire = time() + $period;
225-
}
226-
if (is_null($path)) {
227-
$path = $this->getPath();
228-
}
229-
if (is_null($domain)) {
230-
$domain = $this->getDomain();
231-
}
232-
if (is_null($secure)) {
233-
$secure = $this->isSecure();
234-
}
235-
if (is_null($httponly)) {
236-
$httponly = $this->getHttponly();
237-
}
238-
if (is_null($sameSite)) {
239-
$sameSite = $this->getSameSite();
215+
$expires = time() + $period;
240216
}
241217

242-
if ($sameSite === 'None') {
243-
// Enforce specification SameSite None requires secure
244-
$secure = true;
245-
}
246-
247-
if (PHP_VERSION_ID >= 70300) {
248-
setcookie(
249-
$name,
250-
(string)$value,
251-
[
252-
'expires' => $expire,
253-
'path' => $path,
254-
'domain' => $domain,
255-
'secure' => $secure,
256-
'httponly' => $httponly,
257-
'samesite' => $sameSite
258-
]
259-
);
260-
} else {
261-
if (!empty($sameSite)) {
262-
$path .= "; samesite={$sameSite}";
263-
}
264-
setcookie($name, (string)$value, $expire, $path, $domain, $secure, $httponly);
265-
}
218+
setcookie(
219+
$name,
220+
(string)$value,
221+
[
222+
'expires' => $expires,
223+
'path' => $path ?? $this->getPath(),
224+
'domain' => $domain ?? $this->getDomain(),
225+
'secure' => $secure ?? $this->isSecure(),
226+
'httponly' => $httponly ?? $this->getHttponly(),
227+
'samesite' => $sameSite ?? $this->getSameSite(),
228+
]
229+
);
266230

267231
return $this;
268232
}
@@ -281,9 +245,6 @@ public function set($name, $value, $period = null, $path = null, $domain = null,
281245
*/
282246
public function renew($name, $period = null, $path = null, $domain = null, $secure = null, $httponly = null, $sameSite = null)
283247
{
284-
if (($period === null) && !$this->getLifetime()) {
285-
return $this;
286-
}
287248
$value = $this->_getRequest()->getCookie($name, false);
288249
if ($value !== false) {
289250
$this->set($name, $value, $period, $path, $domain, $secure, $httponly, $sameSite);
@@ -315,13 +276,6 @@ public function get($name = null)
315276
*/
316277
public function delete($name, $path = null, $domain = null, $secure = null, $httponly = null, $sameSite = null)
317278
{
318-
/**
319-
* Check headers sent
320-
*/
321-
if (!$this->_getResponse()->canSendHeaders(false)) {
322-
return $this;
323-
}
324-
325279
return $this->set($name, '', null, $path, $domain, $secure, $httponly, $sameSite);
326280
}
327281
}

0 commit comments

Comments
 (0)