zendframework / zend-expressive-session

Session middleware for PSR-7 applications
BSD 3-Clause "New" or "Revised" License
27 stars 11 forks source link

Session destroy #29

Open nepster-web opened 5 years ago

nepster-web commented 5 years ago

I see that SessionInterface does not provide the ability to session destroy .

I am writing my session handler that works with the database.

I created my middleware, which is called before zend-expressive-session-ext and zend-expressive-session:

    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
    {
        $sessionHandler = new DatabaseSessionHandler($this->pdo);
        session_set_save_handler($sessionHandler, true);

        return $handler->handle($request);
    }

It all works, the difference is not noticeable. But I had a problem with logout.

    public function handle(ServerRequestInterface $request): ResponseInterface
    {
        /** @var LazySession $session */
        $session = $request->getAttribute(SessionMiddleware::SESSION_ATTRIBUTE);

        $session->clear(); // does not destroy the session (only clearing data)

        return new RedirectResponse($this->urlHelper->generate('home'), ResponseInfo::HTTP_FOUND);
    }

Solution to the problem:

    public function handle(ServerRequestInterface $request): ResponseInterface
    {
        /** @var LazySession $session */
        $session = $request->getAttribute(SessionMiddleware::SESSION_ATTRIBUTE);

        $session->clear(); // does not destroy the session (only clearing data)
        session_destroy(); // everything works however we don't use SessionInterface

        return new RedirectResponse($this->urlHelper->generate('home'), ResponseInfo::HTTP_FOUND);
    }

So, why the SessionInterface doesn't have a destroy method? I think this can be a problem in some cases.

froschdesign commented 5 years ago

@nepster-web

I think this can be a problem in some cases.

The other way around:

Warning: Immediate session deletion may cause unwanted results.

Note: You do not have to call session_destroy() from usual code. Cleanup $_SESSION array rather than destroying session data.

http://php.net/manual/en/function.session-destroy.php

nepster-web commented 5 years ago

@froschdesign thank you for quickly responding.

I thought about it, but then there is another problem.

Suppose a user visits the site: default

Then authenticated: default

And after doing logout.

Code exmple:

    public function handle(ServerRequestInterface $request): ResponseInterface
    {
        /** @var LazySession $session */
        $session = $request->getAttribute(SessionMiddleware::SESSION_ATTRIBUTE);
        $session->clear(); 
        return new RedirectResponse($this->urlHelper->generate('home'), ResponseInfo::HTTP_FOUND);
    }

Result: default

Please note that a new session is not created. The user continues to visit the site with the same session after the logout. I think it is incorrect.

So:

    public function handle(ServerRequestInterface $request): ResponseInterface
    {
        /** @var LazySession $session */
        $session = $request->getAttribute(SessionMiddleware::SESSION_ATTRIBUTE);
        $session->clear(); 
        $session->regenerate();
        return new RedirectResponse($this->urlHelper->generate('home'), ResponseInfo::HTTP_FOUND);
    }

And result: default

A new session has been created, but method clear() did not work for the old session.

pine3ree commented 5 years ago

Hello @nepster-web, I believe I have found the cause of this issue in in the https://github.com/zendframework/zend-expressive-session-ext persistence package. I'll try to spare some time to add a test case and a fix. kind regards. (updated) I mean the 2nd issue, the first is normal behaviour as clear() just clears the data (It should be sufficient though for logout functionality)

pine3ree commented 5 years ago

Hello again @nepster-web, @froschdesign I can fix this behaviour (added tests and fix locally in my pc) but this may involve side effects. Calling $lazySession->regenerate() or $session = $session->regenerate() does not trigger instant regeneration, it marks the session instance (clone or the proxied one in lazy session, thus the lazy instance itself) as to be regenerated on persistSession() calls. I we call clear() and regenerate() (in whichever order), I can make persist as empty the old session as well, BUT, if we add more data after the clear() call, the new data will be stored into the old session as well. I believe there is no workaround for this, as regeneration happens lazily too, during the persistance phase. A quirky workaround could be clearing the old session if the new session is found empty (just cleared) on persist. But this may lead to confusion as well, because in that case a clear() + regenerate() calls combination would empty the old session while a clear() + regenerate() + set('foo', 'bar') calls combination will not.

weierophinney commented 4 years ago

This repository has been closed and moved to mezzio/mezzio-session; a new issue has been opened at https://github.com/mezzio/mezzio-session/issues/2.