pimcore / pimcore

Core Framework for the Open Source Data & Experience Management Platform (PIM, MDM, CDP, DAM, DXP/CMS & Digital Commerce)
http://www.pimcore.com
Other
3.41k stars 1.44k forks source link

[SESSION] Frontend session/admin session overwrite each other #1671

Closed maff closed 7 years ago

maff commented 7 years ago

If a frontend session is opened in the admin interface, the cookie for the frontend session will be set to the admin session ID, resulting in pimcore_admin_sid and PHPSESSID being the same and overwriting each other.

dpfaffenbauer commented 7 years ago

I do have the same problem with CoreShop.

I tried to break it down even more. As soon as something starts a session on Kernel Startup, sessions get kinda screwed up. CoreShop starts a session cause of a Profiler DataCollector (it checks for an active cart to determine the users country). Therefore PHPSESSID and pimcore_admin_sid are the same and then things screw up.

Basically as soon as you call $session->getBag (which starts the session then), sessions get screwed up.

I guess you can already reproduce the issue, but here is another case:

Add a Kernel Request Listener (or a Profiler DataCollector, doesn't matter)

class KernelListener
{
    private $session;

    /**
     * @param $session
     */
    public function __construct($session)
    {
        $this->session = $session;
    }

    public function onKernelRequest(GetResponseEvent $event) {
        $this->session->getBag('bag_name');
    }
}
dpfaffenbauer commented 7 years ago

PS: this fixes the issue for CoreShop. Maybe you can use that as hint, or to fix it in Pimcore or to fix it in the framework. Took me a few hours to figure out whats going wrong here.

https://github.com/coreshop/CoreShop/commit/9ac04e2dc6c87aa1ea7caaa4feacca78ea097f70

brusch commented 7 years ago

@maff proposed solution:

A dedicated session for the admin ui (namely pimcore_admin_sid) shouldn't be necessary. So the admin UI should use the default session, without any context switches. To keep the compatibility with existing setups (caching exceptions on load balancers, ...) I'd suggest to set the cookie pimcore_admin_sid manually (eg. to true) after successful login and expire it on logout.

maff commented 7 years ago

The admin session is now handled in the same session as the frontend session (session service definition on the container) and handles its data in custom admin session bags. The pimcore_admin_sid cookie is set/unset on login/logout ("true" instead of session ID) for BC.

Short lived sessions still work as the admin makes use of Pimcore\Tool\Session::useSession() to run session specific code while closing the session as soon as possible.

As the session changes during the update process - you'll be logged out during the update (a warning is raised by the updater script).

dpfaffenbauer commented 7 years ago

Just tested, can confirm its working now 👍

cheers maff 🍻

maff commented 7 years ago

@dpfaffenbauer thanks for the feedback! Did the update work without issues?

dpfaffenbauer commented 7 years ago

@maff actually not, I did the first update manually by replacing the pimcore folder (only in my local dev env). After your comment, I tried it on my demo site and it fails with following error message:

Status: 500 | Internal Server Error
URL: /admin/update/index/job-procedural
Message: Call to a member function getId() on null
Trace: [
  {
    "function": "onKernelRequest",
    "class": "Pimcore\\Bundle\\CoreBundle\\EventListener\\MaintenancePageListener",
    "type": "->",
    "args": [
      {},
      "kernel.request",
      {}
    ]
  },
  {
    "file": "/var/www/vendor/symfony/symfony/src/Symfony/Component/EventDispatcher/Debug/WrappedListener.php",
    "line": 104,
    "function": "call_user_func",
    "args": [
      [
        {},
        "onKernelRequest"
      ],
      {},
      "kernel.request",
      {}
    ]
  },
  {
    "function": "__invoke",
    "class": "Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener",
    "type": "->",
    "args": [
      {},
      "kernel.request",
      {}
    ]
  },
  {
    "file": "/var/www/vendor/symfony/symfony/src/Symfony/Component/EventDispatcher/EventDispatcher.php",
    "line": 212,
    "function": "call_user_func",
    "args": [
      {},
      {},
      "kernel.request",
      {}
    ]
  },
  {
    "file": "/var/www/vendor/symfony/symfony/src/Symfony/Component/EventDispatcher/EventDispatcher.php",
    "line": 44,
    "function": "doDispatch",
    "class": "Symfony\\Component\\EventDispatcher\\EventDispatcher",
    "type": "->",
    "args": [
      [
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {},
        {}
      ],
      "kernel.request",
      {}
    ]
  },
  {
    "file": "/var/www/vendor/symfony/symfony/src/Symfony/Component/EventDispatcher/Debug/TraceableEventDispatcher.php",
    "line": 146,
    "function": "dispatch",
    "class": "Symfony\\Component\\EventDispatcher\\EventDispatcher",
    "type": "->",
    "args": [
      "kernel.request",
      {}
    ]
  },
  {
    "file": "/var/www/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/HttpKernel.php",
    "line": 129,
    "function": "dispatch",
    "class": "Symfony\\Component\\EventDispatcher\\Debug\\TraceableEventDispatcher",
    "type": "->",
    "args": [
      "kernel.request",
      {}
    ]
  },
  {
    "file": "/var/www/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/HttpKernel.php",
    "line": 68,
    "function": "handleRaw",
    "class": "Symfony\\Component\\HttpKernel\\HttpKernel",
    "type": "->",
    "args": [
      {
        "attributes": {},
        "request": {},
        "query": {},
        "server": {},
        "files": {},
        "cookies": {},
        "headers": {}
      },
      1
    ]
  },
  {
    "file": "/var/www/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/Kernel.php",
    "line": 171,
    "function": "handle",
    "class": "Symfony\\Component\\HttpKernel\\HttpKernel",
    "type": "->",
    "args": [
      {
        "attributes": {},
        "request": {},
        "query": {},
        "server": {},
        "files": {},
        "cookies": {},
        "headers": {}
      },
      1,
      true
    ]
  },
  {
    "file": "/var/www/web/app.php",
    "line": 32,
    "function": "handle",
    "class": "Symfony\\Component\\HttpKernel\\Kernel",
    "type": "->",
    "args": [
      {
        "attributes": {},
        "request": {},
        "query": {},
        "server": {},
        "files": {},
        "cookies": {},
        "headers": {}
      }
    ]
  }
]

Pimcore is now failing with following error:

Symfony\Component\Debug\Exception\FatalThrowableError:
Call to a member function getId() on null

  at pimcore/lib/Pimcore/Bundle/CoreBundle/EventListener/MaintenancePageListener.php:92
  at Pimcore\Bundle\CoreBundle\EventListener\MaintenancePageListener->onKernelRequest(object(GetResponseEvent), 'kernel.request', object(TraceableEventDispatcher))
  at call_user_func(array(object(MaintenancePageListener), 'onKernelRequest'), object(GetResponseEvent), 'kernel.request', object(TraceableEventDispatcher))
     (vendor/symfony/symfony/src/Symfony/Component/EventDispatcher/Debug/WrappedListener.php:104)
  at Symfony\Component\EventDispatcher\Debug\WrappedListener->__invoke(object(GetResponseEvent), 'kernel.request', object(ContainerAwareEventDispatcher))
  at call_user_func(object(WrappedListener), object(GetResponseEvent), 'kernel.request', object(ContainerAwareEventDispatcher))
     (vendor/symfony/symfony/src/Symfony/Component/EventDispatcher/EventDispatcher.php:212)
  at Symfony\Component\EventDispatcher\EventDispatcher->doDispatch(array(object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener)), 'kernel.request', object(GetResponseEvent))
     (vendor/symfony/symfony/src/Symfony/Component/EventDispatcher/EventDispatcher.php:44)
  at Symfony\Component\EventDispatcher\EventDispatcher->dispatch('kernel.request', object(GetResponseEvent))
     (vendor/symfony/symfony/src/Symfony/Component/EventDispatcher/Debug/TraceableEventDispatcher.php:146)
  at Symfony\Component\EventDispatcher\Debug\TraceableEventDispatcher->dispatch('kernel.request', object(GetResponseEvent))
     (vendor/symfony/symfony/src/Symfony/Component/HttpKernel/HttpKernel.php:129)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/symfony/src/Symfony/Component/HttpKernel/HttpKernel.php:68)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/symfony/src/Symfony/Component/HttpKernel/Kernel.php:171)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (web/app.php:32)

It is resolvable by renaming/removing var/config/maintenance.php, but after enabling maintenance mode again, it fails with the same message. Even with a new Session it fails (eg. inkognito mode)

dpfaffenbauer commented 7 years ago

PS: also happens in my local dev environment.

maff commented 7 years ago

@dpfaffenbauer thx, we're on it

maff commented 7 years ago

@dpfaffenbauer until we release a new build, you can apply the following patch to fix the maintenance page listener issue: https://github.com/pimcore/pimcore/commit/838f69d1df045925cf8ec1e5859ed17738e87563

dkuss commented 6 years ago

Can we expect the session fix to be applied to the Pimcore 4 branch too? Because this effects the latest build 4.6.3 offered by the integrated updater. This could cause unexpected behavior in some applications when users want to login in backend and frontend at the same time after updating to the latest version of Pimcore 4.

brusch commented 6 years ago

In Pimcore 4 it's handled a bit different and not related to this fix here. For Pimcore 4 you should avoid opening your own session either by using PHPs session_* functions or Zend_Session. Instead you should use Pimcore\Tool\Session::useSession().

dkuss commented 6 years ago

Until now i used Zend_Namespace and since version 4.6.3 the Pimcore backend session and Zend_namespace sessions / PHP Sessions have some kind of conflict. I first have to logout from backend to be able to login in the frontend that is handled by Zend_Session.

However. thanks for the tip, i'll check this out. It would be nice to see a chapter about correct session handling in the docs. :)