Skip to content
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

Session Saved Twice #13026

Closed
austinh opened this issue Dec 18, 2014 · 5 comments
Closed

Session Saved Twice #13026

austinh opened this issue Dec 18, 2014 · 5 comments

Comments

@austinh
Copy link

austinh commented Dec 18, 2014

I am not 100% sure if this is a symfony bug because I am using FoSUserBundle, but from my debugging it seems to be a symfony error.

The Symfony Session caller saves two sessions on authentication. One of them is a blank session with an array that looks like an empty _sf2_attributes array, and the other is the actual session. The PHPSESSID cookie is then changed to the new, different session id.

Technically, everything works. I am able to authenticate my users. but my redis cache is double the size it needs to be and Im starting to get short on memory. I want to get rid of these duplicate keys.

I have also tried memcached and file storage ontop of sncRedisBundle. They both also produce double sessions when authenticated. So I do believe it is a symfony issue.

You can see this in the logs by watching for the REDIS commands being logged by SncRedisBundle (the memcache and file storage do not have debug logs)
Here is some examples

INFO - Matched route "fos_user_security_check" (parameters: "_controller": "FOS\UserBundle\Controller\SecurityController::checkAction", "_route": "fos_user_security_check")
INFO - Executing command "SETNX SESSIONagj232vmqo5ngavqvha016lv1oh00oqe7g22q2eta9skv0lftqr1.lock 1"
INFO - Executing command "EXPIRE SESSIONagj232vmqo5ngavqvha016lv1oh00oqe7g22q2eta9skv0lftqr1.lock 61"
INFO - Executing command "GET SESSION:agj232vmqo5ngavqvha016lv1oh00oqe7g22q2eta9skv0lftqr1"
INFO - User "Austin" has been authenticated successfully
INFO - Executing command "SETEX SESSION:lu735ketuop9dgfh3qitmujge6491274vadfdsdfafadBLAH BLAH SERIALIZED TOKEN CODE HERE;}}i:3;a:0:{}}";}}
INFO - Executing command "DEL SESSIONagj232vmqo5ngavqvha016lv1oqe7g22q2eta9skv0lftqr1.lock" 

As you can see there are two keys being authenticated here:

lu735ketuop9dgfh3qitmujge6491274vadfdsdfafad
VS agj232vmqo5ngavqvha016lv1oh00oqe7g22q2eta9skv0lftqr

I can also show you two session files that have the same thing when using the default session File Handler

sess_bcf61a9jhticgmlec8q32i3vp8hculhojt8e15p20fte8k1  sess_tbtp0p5dsfdl6hu6pelkpr64vlq1sjrlpg4nsuq2fc2l00

Two session files. First one contains empty sf_attributes{} and the second the logged in session.

Something changes after "DEBUG - Write SecurityContext in the session " call. That's how far I have narrowed it down (so...not much)

Anyway this can get fixed? or maybe it's something on my end? I'm not so great with Framework internals, so maybe someone can help me out.

@Tobion
Copy link
Member

Tobion commented Dec 18, 2014

I agree that it should not happen. But it's not a real problem because the empty session will be deleted by garbage collection.

@austinh
Copy link
Author

austinh commented Dec 18, 2014

Hm...But its not technically empty. It contains serialized texts. So of course the data is meaningless but it still takes up bytes, especially for in memory cache. I do not think garbage collection deletes it.

@xelaris
Copy link
Contributor

xelaris commented Dec 20, 2014

It's the default behavior that the session id is regenerated on authentication. The strategy can be configured with the session_fixation_strategy configuration of the SecurityBundle (see http://symfony.com/doc/current/reference/configuration/security.html). You may choose none instead of the default migrate, to avoid regeneration of session ids. But since the regeneration happens to improve security (see 'Renew the Session ID After Any Privilege Level Change' on https://www.owasp.org/index.php/Session_Management_Cheat_Sheet), IMO it isn't a proper solution.

Instead the old session could be deleted. The strategy is evaluated in https://github.com/symfony/symfony/blob/2.3/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategy.php#L50
where migrate could be called with true to delete the old session immediately, rather than leaving it to the garbage collection. I will open a PR to propose it.

fabpot added a commit that referenced this issue Dec 20, 2014
…laris)

This PR was merged into the 2.3 branch.

Discussion
----------

[Security] Delete old session on auth strategy migrate

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #13026
| License       | MIT
| Doc PR        |

As identified by @austinh in #13026 there are two sessions after authentication, since the previous session is migrated to a new one by ``session_regenerate_id``. This PR ensures the old session is been deleted immediately on migration.
I can't see any drawbacks, but if the change would break BC, another approach would be to add a new strategy like ``switch`` to enable instant deletion of the old session.

Commits
-------

5dd11e6 [Security] Delete old session on auth strategy migrate
@fabpot fabpot closed this as completed Dec 20, 2014
@Tobion
Copy link
Member

Tobion commented Dec 20, 2014

@austinh can you confirm that #13048 fixes your case?

@austinh
Copy link
Author

austinh commented Dec 20, 2014

Just tried it. Yes it does! Thank you very much.

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

4 participants