NEXT-13664 - Fix session handling

changelog/_unreleased/2021-02-23-fix-session-handling-on-logouts.md

@@ -0,0 +1,9 @@
+---
+title: Fix session handling on logouts
+issue: NEXT-13664
+author: Timo Altholtmann
+---
+# Storefront
+*  Changed behaviour of the setting `invalidateSessionOnLogOut` in `loginRegistration.xml` which can be found under settings -> login / registration.
+* The session of the user now gets invalidated on every logout, regardless of the value of this setting. This settings controls only, if the cart gets saved on logout.
+___

src/Core/System/Resources/config/loginRegistration.xml

@@ -111,8 +111,8 @@
 
         <input-field type="bool">
             <name>invalidateSessionOnLogOut</name>
-            <label>Clear session data on log out</label>
-            <label lang="de-DE">Sitzungsdaten bei Abmeldung löschen</label>
+            <label>Clear and delete cart on log out</label>
+            <label lang="de-DE">Warenkorb bei Abmeldung löschen</label>
             <helpText>When the setting is activated, the cart won't be saved and can't be restored after login.</helpText>
             <helpText lang="de-DE">Wenn die Einstellung aktiviert ist, wird der Warenkorb nicht gespeichert und kann nach der Anmeldung nicht wiederhergestellt werden.</helpText>
         </input-field>

src/Storefront/Controller/AuthController.php

@@ -24,7 +24,6 @@
 use Shopware\Core\Framework\Validation\DataBag\RequestDataBag;
 use Shopware\Core\Framework\Validation\Exception\ConstraintViolationException;
 use Shopware\Core\System\SalesChannel\SalesChannelContext;
-use Shopware\Core\System\SystemConfig\SystemConfigService;
 use Shopware\Storefront\Framework\Routing\RequestTransformer;
 use Shopware\Storefront\Page\Account\Login\AccountLoginPageLoader;
 use Symfony\Component\HttpFoundation\Request;
@@ -67,11 +66,6 @@ class AuthController extends StorefrontController
      */
     private $logoutRoute;
 
-    /**
-     * @var SystemConfigService
-     */
-    private $systemConfig;
-
     /**
      * @var CartService
      */
@@ -83,7 +77,6 @@ public function __construct(
         AbstractSendPasswordRecoveryMailRoute $sendPasswordRecoveryMailRoute,
         AbstractResetPasswordRoute $resetPasswordRoute,
         AbstractLoginRoute $loginRoute,
-        SystemConfigService $systemConfig,
         AbstractLogoutRoute $logoutRoute,
         CartService $cartService
     ) {
@@ -93,7 +86,6 @@ public function __construct(
         $this->resetPasswordRoute = $resetPasswordRoute;
         $this->loginRoute = $loginRoute;
         $this->logoutRoute = $logoutRoute;
-        $this->systemConfig = $systemConfig;
         $this->cartService = $cartService;
     }
 
@@ -168,11 +160,6 @@ public function logout(Request $request, SalesChannelContext $context): Response
 
         try {
             $this->logoutRoute->logout($context);
-            $salesChannelId = $context->getSalesChannel()->getId();
-            if ($request->hasSession() && $this->systemConfig->get('core.loginRegistration.invalidateSessionOnLogOut', $salesChannelId)) {
-                $request->getSession()->invalidate();
-            }
-
             $this->addFlash('success', $this->trans('account.logoutSucceeded'));
 
             $parameters = [];

src/Storefront/DependencyInjection/controller.xml

@@ -57,7 +57,6 @@
             <argument type="service" id="Shopware\Core\Checkout\Customer\SalesChannel\SendPasswordRecoveryMailRoute"/>
             <argument type="service" id="Shopware\Core\Checkout\Customer\SalesChannel\ResetPasswordRoute"/>
             <argument type="service" id="Shopware\Core\Checkout\Customer\SalesChannel\LoginRoute"/>
-            <argument type="service" id="Shopware\Core\System\SystemConfig\SystemConfigService"/>
             <argument type="service" id="Shopware\Core\Checkout\Customer\SalesChannel\LogoutRoute"/>
             <argument type="service" id="Shopware\Core\Checkout\Cart\SalesChannel\CartService"/>
             <call method="setContainer">

src/Storefront/Framework/Routing/StorefrontSubscriber.php

@@ -200,14 +200,14 @@ public function updateSessionAfterLogin(CustomerLoginEvent $event): void
         $this->updateSession($token);
     }
 
-    public function updateSessionAfterLogout(CustomerLogoutEvent $event): void
+    public function updateSessionAfterLogout(): void
     {
-        $newToken = $event->getSalesChannelContext()->getToken();
+        $newToken = Random::getAlphanumericString(32);
 
-        $this->updateSession($newToken);
+        $this->updateSession($newToken, true);
     }
 
-    public function updateSession(string $token): void
+    public function updateSession(string $token, bool $destroyOldSession = false): void
     {
         $master = $this->requestStack->getMasterRequest();
         if (!$master) {
@@ -222,7 +222,7 @@ public function updateSession(string $token): void
         }
 
         $session = $master->getSession();
-        $session->migrate();
+        $session->migrate($destroyOldSession);
         $session->set('sessionId', $session->getId());
 
         $session->set(PlatformRequest::HEADER_CONTEXT_TOKEN, $token);

src/Storefront/Resources/config/packages/test/framework.yaml

@@ -0,0 +1,6 @@
+framework:
+    test: true
+    session:
+        storage_id: session.storage.mock_file
+    profiler:
+        collect: false

src/Storefront/Test/Controller/AuthControllerTest.php

@@ -118,16 +118,14 @@ public function testDoNotLogoutWhenSalesChannelIdChangedIfCustomerScopeIsOff():
         static::assertEquals($contextToken, $browser->getRequest()->getSession()->get('sw-context-token'));
     }
 
-    public function testSessionIsInvalidatedOnLogOutIsDeactivated(): void
+    public function testSessionIsInvalidatedOnLogoutAndInvalidateSettingFalse(): void
     {
         $systemConfig = $this->getContainer()->get(SystemConfigService::class);
         $systemConfig->set('core.loginRegistration.invalidateSessionOnLogOut', false);
 
         $browser = $this->login();
 
-        $session = $browser->getRequest()->getSession();
-        $contextToken = $session->get('sw-context-token');
-        $sessionId = $session->getId();
+        $sessionCookie = $browser->getCookieJar()->get('session-');
 
         $browser->request('GET', '/account/logout', []);
         $response = $browser->getResponse();
@@ -136,14 +134,25 @@ public function testSessionIsInvalidatedOnLogOutIsDeactivated(): void
         $browser->request('GET', '/', []);
         $response = $browser->getResponse();
         static::assertSame(200, $response->getStatusCode(), $response->getContent());
+        $session = $browser->getRequest()->getSession();
+
+        // Close the old session
+        $session->save();
+        // Set previous session id
+        $session->setId($sessionCookie->getValue());
+        // Set previous session cookie
+        $browser->getCookieJar()->set($sessionCookie);
 
+        // Try opening account page
+        $browser->request('GET', $_SERVER['APP_URL'] . '/account', []);
+        $response = $browser->getResponse();
         $session = $browser->getRequest()->getSession();
 
-        $newContextToken = $session->get('sw-context-token');
-        static::assertNotEquals($contextToken, $newContextToken);
+        // Expect the session to have the same value as the initial session
+        static::assertSame($session->getId(), $sessionCookie->getValue());
 
-        $newSessionId = $session->getId();
-        static::assertNotEquals($sessionId, $newSessionId);
+        // Expect a redirect response, since the old session should be destroyed
+        static::assertSame(302, $response->getStatusCode(), $response->getContent());
     }
 
     public function testRedirectToAccountPageAfterLogin(): void