Merge branch 'sw-26704/change-csrf-token-validation' into '5.7'

Revert "Merge branch 'sw-26689/set-cookie-secure' into '5.7'"

See merge request shopware/5/product/shopware!805

Author: philipreinken

.phpstan.neon

@@ -177,7 +177,3 @@ parameters:
         - # For testing purposes the constructor of the price struct gets strings
             message: '#Parameter .* of class Shopware\\Components\\Cart\\Struct\\Price constructor expects float.*string given#'
             path: tests/Functional/Components/Cart/ProportionalTaxCalculatorTest.php
-
-        - # The Enlight_Response and Enlight_Request interfaces do not extend from Symfony, and therefore we have to find a unified usage pattern
-            message: '#Parameter .* of method Shopware\\Components\\CSRFTokenValidator\:\:regenerateToken\(\) expects Symfony\\Component\\HttpFoundation\\Request.* given#'
-            path: engine/Shopware/Core/sAdmin.php

engine/Shopware/Components/CSRFTokenValidator.php

@@ -33,16 +33,17 @@
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\Cookie;
 use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpFoundation\Response;
 
 class CSRFTokenValidator implements SubscriberInterface
 {
-    public const CSRF_SESSION_KEY = '__csrf_token-';
+    public const CSRF_KEY = '__csrf_token-';
 
     public const CSRF_TOKEN_ARGUMENT = '__csrf_token';
 
     public const CSRF_TOKEN_HEADER = 'X-CSRF-Token';
 
+    private const CSRF_WAS_VALIDATED = 'isValidated';
+
     /**
      * @var ContainerInterface
      */
@@ -140,16 +141,13 @@ public function checkFrontendTokenValidation(Enlight_Event_EventArgs $args)
         /** @var Enlight_Controller_Action $controller */
         $controller = $args->getSubject();
         $request = $controller->Request();
-        $response = $controller->Response();
 
         // do not check internal sub-requests or validated requests
-        if ($request->getAttribute('_isSubrequest') || $request->getAttribute('isValidated')) {
+        if ($request->getAttribute('_isSubrequest') || $request->getAttribute(self::CSRF_WAS_VALIDATED)) {
             return;
         }
 
         if ($request->isGet() && !$this->isProtected($controller)) {
-            $this->updateCsrfTokenIfNotAvailable($request, $response);
-
             return;
         }
 
@@ -162,76 +160,40 @@ public function checkFrontendTokenValidation(Enlight_Event_EventArgs $args)
             return;
         }
 
-        if (!$this->checkRequest($request, $response)) {
-            $this->regenerateToken($request, $response);
+        if (!$this->checkRequest($request)) {
             throw new CSRFTokenValidationException(sprintf('The provided X-CSRF-Token for path "%s" is invalid. Please go back, reload the page and try again.', $request->getRequestUri()));
         }
 
         // mark request as validated to avoid double validation
-        $request->setAttribute('isValidated', true);
+        $request->setAttribute(self::CSRF_WAS_VALIDATED, true);
     }
 
-    public function regenerateToken(Request $request, Response $response): string
+    public function clearExistingCookie(): void
     {
         $shop = $this->contextService->getShopContext()->getShop();
         $name = $this->getCsrfName();
 
-        $token = Random::getAlphanumericString(30);
-        $this->container->get('session')->set($name, $token);
-
-        $response->headers->clearCookie($name);
-
-        /*
-         * Appending a '/' to the $basePath is not strictly necessary, but it is
-         * done to all cookie base paths in the
-         * `themes/Frontend/Bare/frontend/index/index.tpl` template. It's done
-         * here as well for compatibility reasons.
-         */
-        $response->headers->setCookie(new Cookie(
+        $front = $this->container->get('front');
+        $response = $front->Response();
+        $response->headers->clearCookie(
             $name,
-            $token,
-            0,
             sprintf('%s/', $shop->getPath() ?: ''),
-            null,
+            '',
             $shop->getSecure(),
             false
-        ));
-
-        return $token;
-    }
-
-    private function updateCsrfTokenIfNotAvailable(Request $request, Response $response): void
-    {
-        $name = $this->getCsrfName();
-
-        if ($this->container->get('session')->has($name)) {
-            return;
-        }
-
-        $this->regenerateToken($request, $response);
-    }
-
-    private function getCsrfName(): string
-    {
-        $shop = $this->contextService->getShopContext()->getShop();
-
-        $name = self::CSRF_SESSION_KEY . $shop->getId();
-
-        if ($shop->getParentId() && $this->componentsConfig->get('shareSessionBetweenLanguageShops')) {
-            $name = self::CSRF_SESSION_KEY . $shop->getParentId();
-        }
-
-        return $name;
+        );
     }
 
     /**
-     * Check if the submitted CSRF token in cookie or header matches with the token stored in the session
+     * Check if the submitted CSRF token matches with the token stored in the cookie or header
+     *
+     * @return bool
      */
-    private function checkRequest(Request $request, Response $response): bool
+    private function checkRequest(Request $request)
     {
         $name = $this->getCsrfName();
 
-        $token = $this->container->get('session')->get($name);
+        $token = $request->cookies->get($name);
 
         if (!\is_string($token)) {
             return false;
@@ -246,6 +208,19 @@ private function checkRequest(Request $request, Response $response): bool
         return hash_equals($token, $requestToken);
     }
 
+    private function getCsrfName(): string
+    {
+        $shop = $this->contextService->getShopContext()->getShop();
+
+        $name = self::CSRF_KEY . $shop->getId();
+
+        if ($shop->getParentId() && $this->componentsConfig->get('shareSessionBetweenLanguageShops')) {
+            $name = self::CSRF_KEY . $shop->getParentId();
+        }
+
+        return $name;
+    }
+
     /**
      * Check if the controller has opted in for CSRF whitelisting and if the
      * called action is on the whitelist

engine/Shopware/Components/DependencyInjection/Controllers/frontend.xml

@@ -38,7 +38,6 @@
         </service>
 
         <service id="shopware_controllers_frontend_csrftoken" class="Shopware_Controllers_Frontend_Csrftoken">
-            <argument type="service" id="shopware.csrftoken_validator"/>
             <tag name="shopware.controller" module="frontend" controller="csrftoken"/>
         </service>
 

engine/Shopware/Controllers/Frontend/Csrftoken.php

@@ -22,17 +22,9 @@
  * our trademarks remain entirely with us.
  */
 
-use Shopware\Components\CSRFTokenValidator;
-use Symfony\Component\HttpFoundation\Request;
-
 class Shopware_Controllers_Frontend_Csrftoken extends Enlight_Controller_Action
 {
-    private CSRFTokenValidator $CSRFTokenValidator;
-
-    public function __construct(CSRFTokenValidator $CSRFTokenValidator)
-    {
-        $this->CSRFTokenValidator = $CSRFTokenValidator;
-    }
+    private const CSRF_TOKEN_HEADER = 'x-csrf-token';
 
     /**
      * Loads auth and script renderer resource
@@ -45,10 +37,10 @@ public function preDispatch()
     /**
      * Generates a token and fills the cookie and session
      */
-    public function indexAction(Request $request)
+    public function indexAction()
     {
-        $token = $this->CSRFTokenValidator->regenerateToken($request, $this->Response());
+        $token = \Shopware\Components\Random::getAlphanumericString(30);
 
-        $this->Response()->headers->set(CSRFTokenValidator::CSRF_TOKEN_HEADER, $token);
+        $this->Response()->headers->set(self::CSRF_TOKEN_HEADER, $token);
     }
 }

engine/Shopware/Core/sAdmin.php

@@ -718,7 +718,7 @@ public function logout()
 
         $this->contextService->initializeContext();
 
-        $this->csrfTokenValidator->regenerateToken($this->front->Request(), $this->front->Response());
+        $this->csrfTokenValidator->clearExistingCookie();
 
         if (!$this->config->get('clearBasketAfterLogout')) {
             $this->moduleManager->Basket()->sRefreshBasket();
@@ -3277,7 +3277,7 @@ protected function loginUser($getUser, $email, $password, $isPreHashed, $encoder
         $this->session->offsetSet('sUserId', $userId);
         $this->session->offsetSet('sNotesQuantity', $this->moduleManager->Basket()->sCountNotes());
 
-        $this->csrfTokenValidator->regenerateToken($this->front->Request(), $this->front->Response());
+        $this->csrfTokenValidator->clearExistingCookie();
 
         if (!$this->sCheckUser()) {
             return;