Skip to content

Symfony 2.2.1 - session lost after success login rediraction #7885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
zd-dalibor opened this issue Apr 30, 2013 · 33 comments
Closed

Symfony 2.2.1 - session lost after success login rediraction #7885

zd-dalibor opened this issue Apr 30, 2013 · 33 comments

Comments

@zd-dalibor
Copy link

After login success

Symfony\Component\HttpFoundation\Session\Storage\Handler\PdoSessionHandler::read($id)

always get new session id and cannot load login user from previous session

@sstok
Copy link
Contributor

sstok commented Apr 30, 2013

This is because the session id is regenerated after a successful login.
Its done for security to prevent session hijacking as such.

@zd-dalibor
Copy link
Author

@sstok session is regenerated after user credentials is checked and if check success, next, user object is stored in this session and after that symfony doing redirect to "default_target_path". After that redirect I got new session id that does't exist in database so I don't get loged-in user

@denkiryokuhatsuden
Copy link
Contributor

@dalibor983 Check whether Set-Cookie response header is sent to your browser properly and next request is with Cookie request header.

As Symfony2 is designed to save session after redirect is sent,
the next request can possibly reach and begin reading session before
previous session has saved.(In your case, SELECT is made before INSERT, or with locking, before COMMIT)

@zd-dalibor
Copy link
Author

@denkiryokuhatsuden I think you are correct because this happens on my development server where my database is very slow so SELECT is made before INSERT is finished. Is there some workaround for this? I don't understand why symfony2 send redirect header and than performs INSERT for session data in database.

@stof
Copy link
Member

stof commented May 2, 2013

@dalibor983 PHP saves the session at the end of the process. And when you use PHP-FPM, symfony does the dispatching of the kernel.terminate event after sending the response to improve the response time (it does not when you use Apache as it is not possible)

@zd-dalibor
Copy link
Author

@stof It seems that SELECT is made before INSERT from previous request finished. On my production server everything works like a charm but on my development server doesn't (database is full of dirty data from other projects and tests). I need to slow down process execution by putting break point in

Symfony\Component\HttpFoundation\Session\Storage\Handler\PdoSessionHandler::read($id)

and turn on debugging in my ide to be able to login. I don't know what will happen on production when my database start to grow and slow down.

@denkiryokuhatsuden
Copy link
Contributor

@dalibor983 To write session before sending request, I overrode AppKernel#handle like below:

    public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQUEST, $catch = true)
    {
        $response = parent::handle($request, $type, $catch);

        if ($type == HttpKernelInterface::MASTER_REQUEST) {
            if ($this->getContainer()->get('session')->isStarted()) {
                $this->getContainer()->get('session')->save();
            }
        }

        return $response;
    }

I use memcache (without trailing "d") for session handler and faced similar problem.
after this change, it's working almost fine, works when requests are serial.

But when requests are done parallel (i.e. using ajax), it doesn't work because PdoSessionHandler
does not have lock mechanism like one that php-native file session has.

Probably php-native file session uses flock(LOCK_EX). So next request waits until lock is released.

To do like that, you have to introduce row level locking (begin transaction when open and SELECT FOR UPDATE when read)

@zd-dalibor
Copy link
Author

@denkiryokuhatsuden "SELECT ... FOR UPDATE" lock rows for reading but only rows that SELECT returns. In symfony in first request you perform INSERT new row in session table and then redirect. Redirect request try to read new session data by session id but that session id is not in session table until INSERT from previous request is finished.

I think that something is wrong with my development database. It says that INSERT is finished but some time need to pass until SELECT can return that value from database.

@josecelano
Copy link

I had the same problem in dev env but not in staging server. But now I have update in staging with the last source code version and the problem has also appeared there.

Is there a solution without overrinding AppKernel#handle?

@zd-dalibor
Copy link
Author

I use REDIS(http://redis.io) for storing session instead of mysql. Problem with mysql is race condition, return session before new value is written to session table.

@zd-dalibor
Copy link
Author

The problem is not with symfony, problem is related to slow RDBMS. If you have slow mysql server new session record with session id is not yet written in mysql server after redirection and when symfony performs SELECT from session table with session id from previous request mysql returns no records so symfony create new php session with new session id and all data from previous request is lost.

@zdebol
Copy link

zdebol commented Apr 10, 2014

Is there any solutions for this situation without overrinding AppKernel#handle or not using pdo handler?

@elyotechgit
Copy link

I'm working on Symfony 2.4.3 and I have the same problems, the solution of override the AppKernel handle function resolve it for me.
Thanks

@collinkrawll
Copy link

I just ran into this issue on a new Vagrant VM running Symfony 2.3. Every time I logged in I was immediately logged back out. I was able to fix it by moving the session cache directory outside of the shared vagrant folder. See http://symfony.com/doc/current/cookbook/session/sessions_directory.html

@leviferreira
Copy link

@collinkrawll Thanks for the help, i got the same issue!!! []s

@ghost
Copy link

ghost commented Jun 3, 2014

@collinkrawll Thanks! I've had the same issue.

@antonydb
Copy link

antonydb commented Jun 4, 2014

@collinkrawll Thanks for the advice.

@jrdnrc
Copy link

jrdnrc commented Jun 4, 2014

Thankyou @collinkrawll you have solved my problem! Been having problems logging into a local copy for ages. Do you know why the folder has to be changed?

@collinkrawll
Copy link

@jrdnhannah The shared vagrant folder has to do a lot of syncing and is much slower than the rest of the folders on the VM. I think what happens is Symfony logs the user in, writes a session file then goes to read that session file and it's not there yet (because the folder is slow). At this point Symfony loses the session and the user is effectively logged back out. Moving the session cache to a faster folder fixes the issue since the file is there when Symfony expects it. For me it was actually working on a computer with a SSD (faster IO) and failing on a computer with a traditional HDD. The SSD was fast enough to be able to support caching to the shared folder but the HDD couldn't keep up. Moving the cache outside of the shared folder fixes it. This article has some more good tips on speeding up a Symfony/Vagrant environment : http://www.whitewashing.de/2013/08/19/speedup_symfony2_on_vagrant_boxes.html

@jrdnrc
Copy link

jrdnrc commented Jun 4, 2014

Thanks @collinkrawll. That makes sense. One thing I am unsure of though - I have been using auth with the sessions stored in the shared folder before (I have a 2013 MBP so the SSD is pretty speedy) and it worked fine. It was just this one symfony app that wasn't playing ball...

@stof
Copy link
Member

stof commented Jun 8, 2014

@jrdnhannah it is a race condition between the session reading and the shared folder write. The actual racing may depend on the load of your HDD or other things like that

@Tobion
Copy link
Contributor

Tobion commented Jun 9, 2014

@collinkrawll which save handler did you use? The native files or one provided by symfony? With natives I should actually not happen because it locks the files and should only release the lock when the files is written.

@sudent
Copy link

sudent commented Jul 19, 2014

Just to throw in some more pointers in case anyone (mainly developers) stumbles across this problem (using vagrant with symfony2). Yesterday I destroy my 4 months-old vagrant box so that I can develop in a fresh environment. I got a few symfony2 project which works beautifully in this box since last I provision it (complete with chef provisioning).

Here's the thing, before I re-provision the box, I decided to upgrade my virtualbox and vagrant first. Upon first provisioning, I tried accessing my current Symfony project and I came into issue like @collinkrawll experiencing. Confused, I rechecked if I have touched anything in my project, but nothing is changed. Everything is the same except that I re-provision new box (using the same chef provisioning as before) except this time with the latest VirtbualBox and Vagrant software.

Embarrassingly, I spent quite a few hours debugging before stumbling on this issue and specifically @collinkrawll message. Since my sf2 project is based on 2.3 LTS, my config.yml used the default sf2-standard session configuration session: ~ which writes the session in %kernel.cache_dir%/sessions. My project is located under the vagrant shared folder mounted as NFS. Changing it to session: handler_id: ~ fixes the issue, which uses my default php.ini setting to /var/lib/php5/session (not under vagrant shared folder). Do note that this isn't an issue before. But now I need to change the session config and point it to somewhere which is not under vagrant shared folder.

With this I guess that somehow, newer version of virtualbox/vagrant made the session write somehow slower which resulted in the next request started first before the session file getting created/picked up. For some who develops in vagrant, it might confuse them when stumbling across this kind of issue.. so I thought to share it here.

kongr45gpen added a commit to allejo/bzion that referenced this issue Sep 22, 2014
Do not lose the session data when the client's browser redirects too fast, without allowing the session handler to store the session.
See symfony/symfony#7885 for more information
@josecelano
Copy link

It seems I have fixed the problem using kernel events. I have implemented a KERNEL::RESPONSE event listener and I save the session data on that event only if request has been a login_check POST. This way redirection is sent after session is saved in database. I use MySQL to store session data and I encrytp session data before storing it so it takes even more than a simple sql INSERT.

This way I avoid patching kernel.

use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
use Symfony\Component\HttpKernel\Log\LoggerInterface;

class LastLoginListener implements EventSubscriberInterface
{
    public static function getSubscribedEvents()
    {
        return array(
            KernelEvents::RESPONSE => 'onKernelResponse',               
        );
    }

    public function onKernelResponse(FilterResponseEvent $event)
    {
        if ($event->isMasterRequest() && $event->getRequest()->get('_route') == 'fos_user_security_check') {

            $event->getRequest()->getSession()->save();

            $this->logger->info("Session saved {session_id} {route}", array(
                'session_id'    =>$event->getRequest()->getSession()->getId(),
                'route'         =>$event->getRequest()->get('_route'),
            ));

            return;
        }
    }    
}

Service configuration using yml

    fos_user.security.interactive_login_listener: 
        class: "%acme_user.security.interactive_login_listener.class%"
        arguments:
            - "@logger"
        tags:
          - { name: kernel.event_listener, event: kernel.response, method: onKernelResponse }

Extra: It is supposed Race Conditions with PHP (http://thwartedefforts.org/2006/11/11/race-conditions-with-ajax-and-php-sessions/) is supported by Symfony default PDOSessionHandler:

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php

@Tobion
Copy link
Contributor

Tobion commented Oct 9, 2014

The PdoSessionHandler in the master branch now implements locking by default. So if you use that, the problem should not occur anymore. And your listener is not required.

@josecelano
Copy link

Thanks @Tobion but the problem is not a Race Condition between two read/writes of the same session but between the first write and a read before the session had been created as @dalibor983 explained. If I disable the listener then the problem appears again.

@Tobion
Copy link
Contributor

Tobion commented Oct 9, 2014

@josecelano oh yeah it happens with a regenerated session id that is not written the moment the redirection is done. So this is definitely a problem in the implementation of the security part. It should save the session when doing a regeneration.

@Tobion
Copy link
Contributor

Tobion commented Oct 28, 2014

@josecelano should be fixed with #12341

@josecelano
Copy link

@Tobion thanks, that´s fine. In fact I have a lot of $session->save(); in my code becuase it´s a very common problem when using AJAX and flashbag messages. But really I thought you would not change it in the core becuase it can may increase response time when it´s not required saving the session before sending the response.

@Tobion
Copy link
Contributor

Tobion commented Oct 28, 2014

The only time where the session save would not be required, and thus would hurt performance when using fastcgi_finish_request(), is when the session data did not change. For this case there is https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/WriteCheckSessionHandler.php which would just no do anything on save.

@josecelano
Copy link

@Tobion I want to completely clarify the isuue. Imagine this scenario:
1.- I upload a bit file with POST and I want to store it in session (using MySQL for session storage on a very remote and slow MySQL server :-))
2.- I want to respond to user with HTTP 200: "OK we have received your file and we are storing it, please wait".
3.- User can wait to make new request or not.

If user does not wait and he makes a new request then the "session write locking system" has to delay the session read until the previous write has finished (only implemented in master sysmfony version?).

In this case may be useful to send the response before the session write had finished.

This scenario will not possible with the new patch.

What is more, user could make a lot of read-only requests while the session is being written.
(requests which do not use session data or do not require fresh session data). In this case I supposed there must be a way to use a locked session in a request.

@sstok
Copy link
Contributor

sstok commented Oct 28, 2014

For that scenario you are better-of with:

  1. Upload file (session closed for now)
  2. After uploading generate a unique id and store it in the session and some queue table.
    1.1 Close session if needed.
  3. move the uploaded file to shared temporary storage (with the generated id) and update the queue status to uploaded, and restart the session when needed.

Storing a file-content upload in a session is really troubling because the entire time the session is locked and it also takes a big amount of space in the database.

@josecelano
Copy link

Thanks @sstok for your response. I was only trying to find a case where not to write session data before sending the response were needed or desiderable. Of course my sample it is not a good solution. Probably if I can not find a simpler case then probably there is not a such case.

fabpot added a commit that referenced this issue Nov 2, 2014
…Tobion)

This PR was merged into the 2.3 branch.

Discussion
----------

[Kernel] ensure session is saved before sending response

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #6417, #7885
| License       | MIT
| Doc PR        | n/a

Saves the session, in case it is still open, before sending the response.

This ensures several things in case the developer did not save the session explicitly:

- If a session save handler without locking is used, it ensures the data is available
on the next request, e.g. after a redirect. PHPs auto-save at script end via
session_register_shutdown is executed after fastcgi_finish_request. So in this case
the data could be missing the next request because it might not be saved the moment
the new request is processed.

- A locking save handler (e.g. the native 'files') circumvents concurrency problems like
the one above. By saving the session before long-running things in the terminate event,
we ensure the session is not blocked longer than needed.

- When regenerating the session ID no locking is involved in PHPs session design. See
https://bugs.php.net/bug.php?id=61470 for a discussion. So in this case, the session must
be saved anyway before sending the headers with the new session ID. Otherwise session
data could get lost again for concurrent requests with the new ID. One result could be
that you get logged out after just logging in.

This listener should be executed as one of the last listeners, so that previous listeners
can still operate on the open session. This prevents the overhead of restarting it.
Listeners after closing the session can still work with the session as usual because
 Symfonys session implementation starts the session on demand. So writing to it after
it is saved will just restart it.

Commits
-------

b7bfef0 [Kernel] ensure session is saved before sending response
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests