Author Topic: PHP Session and changing users (Read 502 times)

EranSch · January 01, 2015, 09:09:14 am

I've been working on a user masquerading plugin for WordPress, in order to test a migration into CiviCRM. As an option in this plugin, I've been using the PHP session global to store user data when a user leaves their context... this way the original user can be restored later. I've found this works find in vanilla WordPress however it seems as though Civi ~~overwrites~~ resets the session array when the new user session becomes active.

~~Searching the Civi codebase, I don't see anything that would clear the entire session array but yet it seems to occur anyway... Any thoughts on why?~~

Here's the source for the masquerade plugin if anyone cares to take a look: https://github.com/Swingline0/masquerade

EDIT: Upon deeper inspection, it appears that this a deliberate (and downright reasonable) action of Civi's: https://github.com/civicrm/civicrm-core/blob/b2ac9e9ca74be7f852e37e0dfbf0e77627471c4d/CRM/Core/BAO/UFMatch.php#L107

So my question would be this. Is it super un-practical, crazy, or stupid to be doing what I'm doing with this plugin? Essentially persisting session data between active users?

EDIT2: Assuming the question above is that this isn't a particularly horrible thing to do, would there be anything wrong with modifying the UFMatch synchronize method to only reset the CiviCRM index of the session array rather than the entire thing? Something along the lines of:

Code: [Select]

$session->reset('CiviCRM');

totten · January 01, 2015, 12:49:42 pm

General note: I don't fully understand the design of Civi's session-management and wasn't around when it was prepared, so take my comments with the necessary grain of salt. These are in no particular order.

There's also a user-masquerade feature included with the Drupal "devel" module. I wonder if it has the same problem?
To my mind, there should only be one component who has authority to trigger a general session-reset -- i.e. the same component which manages logins generally -- i.e. the CMS. I have trouble imagining a situation where Civi should be resetting the full session where the CMS should not.
It's true that Civi puts a lot of data in $_SESSION["CiviCRM"]. It also puts data in $_SESSION["_{$className}_{$key}_container"] and $_SESSION["queueRunners"] (e.g. used by the upgrade UI). These probably need to be reset as well.
Third-party dependencies *may* put data in other $_SESSION variables. Use "grep -r _SESSION packages" to see what I mean. There are definitely some matches, although I can't tell if that code is active or inert.
Other modules (besides Civi) may put data in $_SESSION. As a general matter, this seems like a tricky topic for masquerading. There's likely to be a mix of data in $_SESSION -- some data which must be reset (such as user IDs, permission-caches, menu-caches), and some data which could be kept (such as language preferences?). I can't think of anything which *needs* to be kept (except the masquerading metadata), so my gut says to stash away the entire _SESSION and provide a fresh/empty session for the masqueraded user. This could be something like https://gist.github.com/totten/9b766b482251eaccfd41 or something clever in the session-management layer.
If you take the approach of clearing out $_SESSION for the masqueraded user, then the conditional (above https://github.com/civicrm/civicrm-core/blob/b2ac9e9ca74be7f852e37e0dfbf0e77627471c4d/CRM/Core/BAO/UFMatch.php#L107) ought to return FALSE because $ufId would be empty.

EranSch · January 01, 2015, 01:06:38 pm

Hey totten,

This is interesting stuff. Good things to consider...

I suppose you're right that, in my implementation, the $_SESSION array should technically be backed up. As WordPress doesn't use sessions, I wasn't really thinking about doing so but it sounds like a sound practice regardless. Considering the wide array of plugins and modules out there that do utilize sessions. Regardless, login or logout events in WordPress don't seem to interfere with $_SESSION as authentication is handled by other means.

I see that I misread the Session object's reset method. By providing false, I see now that it only resets the index identified by class variable $_key. Either way, this certainly works for my needs.

I suppose the greater question then, is if the UFMatch really needs to be resetting the entire $_SESSION variable as it does now. I really hate making modifications to Civi core but is this really necessary? I mean... should Civi be so strong handed with $_SESSION by default?

Thanks for the great input. It's all very much appreciated!

CiviCRM Community Forums (archive)

News:

Author Topic: PHP Session and changing users (Read 502 times)

EranSch

PHP Session and changing users

totten

Re: PHP Session and changing users

EranSch

Re: PHP Session and changing users