From 26b9eee1275b882dac0a84adcf8e22ca643afb61 Mon Sep 17 00:00:00 2001 From: Kenny Deckers Date: Tue, 29 Dec 2020 18:11:05 +0100 Subject: [PATCH] Adds support for "SameSite" cookie property (#1246) * Support SameSite cookie property (fixes issue #414) * Support SameSite cookie property (fixes issue #414) * Open in new tab * Default fallback to None just in case someone forgets to clear cache * * Force secure if SameSite None * SameSite consistency for renew and delete functions * Code formatting #OCD :) * PHP 7.0 compatibility * Fix pipeline errors * Removed incorrect copyright docs --- .../System/Config/Source/Cookie/SameSite.php | 16 +++++ app/code/core/Mage/Core/Model/Cookie.php | 70 +++++++++++++------ app/code/core/Mage/Core/etc/config.xml | 1 + app/code/core/Mage/Core/etc/system.xml | 10 +++ app/locale/en_US/Mage_Core.csv | 4 ++ 5 files changed, 81 insertions(+), 20 deletions(-) create mode 100644 app/code/core/Mage/Adminhtml/Model/System/Config/Source/Cookie/SameSite.php diff --git a/app/code/core/Mage/Adminhtml/Model/System/Config/Source/Cookie/SameSite.php b/app/code/core/Mage/Adminhtml/Model/System/Config/Source/Cookie/SameSite.php new file mode 100644 index 00000000000..1e02db6c2a7 --- /dev/null +++ b/app/code/core/Mage/Adminhtml/Model/System/Config/Source/Cookie/SameSite.php @@ -0,0 +1,16 @@ + 'None', 'label' => Mage::helper('adminhtml')->__('None')], + ['value' => 'Strict', 'label' => Mage::helper('adminhtml')->__('Strict')], + ['value' => 'Lax', 'label' => Mage::helper('adminhtml')->__('Lax')] + ]; + } +} diff --git a/app/code/core/Mage/Core/Model/Cookie.php b/app/code/core/Mage/Core/Model/Cookie.php index c723c03b19f..7e2981487c4 100644 --- a/app/code/core/Mage/Core/Model/Cookie.php +++ b/app/code/core/Mage/Core/Model/Cookie.php @@ -37,6 +37,7 @@ class Mage_Core_Model_Cookie const XML_PATH_COOKIE_PATH = 'web/cookie/cookie_path'; const XML_PATH_COOKIE_LIFETIME = 'web/cookie/cookie_lifetime'; const XML_PATH_COOKIE_HTTPONLY = 'web/cookie/cookie_httponly'; + const XML_PATH_COOKIE_SAMESITE = 'web/cookie/cookie_samesite'; protected $_lifetime; @@ -174,6 +175,20 @@ public function getHttponly() return (bool)$httponly; } + /** + * Retrieve use SameSite + * + * @return string + */ + public function getSameSite(): string + { + $sameSite = Mage::getStoreConfig(self::XML_PATH_COOKIE_SAMESITE, $this->getStore()); + if (is_null($sameSite)) { + return 'None'; + } + return (string)$sameSite; + } + /** * Is https secure request * Use secure on adminhtml only @@ -202,9 +217,10 @@ public function isSecure() * @param string $domain * @param int|bool $secure * @param bool $httponly + * @param string $sameSite * @return $this */ - public function set($name, $value, $period = null, $path = null, $domain = null, $secure = null, $httponly = null) + public function set($name, $value, $period = null, $path = null, $domain = null, $secure = null, $httponly = null, $sameSite = null) { /** * Check headers sent @@ -236,8 +252,34 @@ public function set($name, $value, $period = null, $path = null, $domain = null, if (is_null($httponly)) { $httponly = $this->getHttponly(); } + if (is_null($sameSite)) { + $sameSite = $this->getSameSite(); + } - setcookie($name, $value, $expire, $path, $domain, $secure, $httponly); + if ($sameSite === 'None') { + // Enforce specification SameSite None requires secure + $secure = true; + } + + if (PHP_VERSION_ID >= 70300) { + setcookie( + $name, + $value, + [ + 'expires' => $expire, + 'path' => $path, + 'domain' => $domain, + 'secure' => $secure, + 'httponly' => $httponly, + 'samesite' => $sameSite + ] + ); + } else { + if (!empty($sameSite)) { + $path.= "; samesite=${sameSite}"; + } + setcookie($name, $value, $expire, $path, $domain, $secure, $httponly); + } return $this; } @@ -251,16 +293,17 @@ public function set($name, $value, $period = null, $path = null, $domain = null, * @param string $domain * @param int|bool $secure * @param bool $httponly + * @param string $sameSite * @return $this */ - public function renew($name, $period = null, $path = null, $domain = null, $secure = null, $httponly = null) + public function renew($name, $period = null, $path = null, $domain = null, $secure = null, $httponly = null, $sameSite = null) { if (($period === null) && !$this->getLifetime()) { return $this; } $value = $this->_getRequest()->getCookie($name, false); if ($value !== false) { - $this->set($name, $value, $period, $path, $domain, $secure, $httponly); + $this->set($name, $value, $period, $path, $domain, $secure, $httponly, $sameSite); } return $this; } @@ -284,9 +327,10 @@ public function get($name = null) * @param string $domain * @param int|bool $secure * @param int|bool $httponly + * @param string $sameSite * @return $this */ - public function delete($name, $path = null, $domain = null, $secure = null, $httponly = null) + public function delete($name, $path = null, $domain = null, $secure = null, $httponly = null, $sameSite = null) { /** * Check headers sent @@ -295,20 +339,6 @@ public function delete($name, $path = null, $domain = null, $secure = null, $htt return $this; } - if (is_null($path)) { - $path = $this->getPath(); - } - if (is_null($domain)) { - $domain = $this->getDomain(); - } - if (is_null($secure)) { - $secure = $this->isSecure(); - } - if (is_null($httponly)) { - $httponly = $this->getHttponly(); - } - - setcookie($name, null, null, $path, $domain, $secure, $httponly); - return $this; + return $this->set($name, null, null, $path, $domain, $secure, $httponly, $sameSite); } } diff --git a/app/code/core/Mage/Core/etc/config.xml b/app/code/core/Mage/Core/etc/config.xml index 9e04f2e8062..0ad9e8bd77d 100644 --- a/app/code/core/Mage/Core/etc/config.xml +++ b/app/code/core/Mage/Core/etc/config.xml @@ -407,6 +407,7 @@ 3600 1 + None 0 31536000 diff --git a/app/code/core/Mage/Core/etc/system.xml b/app/code/core/Mage/Core/etc/system.xml index 579eae33e3d..1ce29a5d517 100644 --- a/app/code/core/Mage/Core/etc/system.xml +++ b/app/code/core/Mage/Core/etc/system.xml @@ -1574,6 +1574,16 @@ 1 1 + + + select + adminhtml/system_config_source_cookie_samesite + https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00]]> + 45 + 1 + 1 + 1 + select diff --git a/app/locale/en_US/Mage_Core.csv b/app/locale/en_US/Mage_Core.csv index 9bcf68fb841..0f0bfc66135 100644 --- a/app/locale/en_US/Mage_Core.csv +++ b/app/locale/en_US/Mage_Core.csv @@ -397,6 +397,10 @@ "Use Custom Admin Path","Use Custom Admin Path" "Use Custom Admin URL","Use Custom Admin URL" "Use HTTP Only","Use HTTP Only" +"Same-Site","Same-Site" +"None","None" +"Strict","Strict" +"Lax","Lax" "Use SID on Frontend","Use SID on Frontend" "Use Secure URLs in Admin","Use Secure URLs in Admin" "Use Secure URLs in Frontend","Use Secure URLs in Frontend"