If you are supporting SSO with Bakery (or similar code), masquerade will not work. The masqueraded user will be denied access because the persistent id info (Cookie in Bakery's case) will not match.
These actions generally take place when hook_boot() is called.
It is hard to add code to a hook_boot() implementation that identifies a masquerade user switch because the session flag is only set in the masquerade_init() process... after the boot() process. In other words:
- Masquerade user is selected
- Next page loaded is loaded: SSO Boot process checks for $_SESSION[masquerade], doesn't find it and denies user
- Masquerade_init() not called to set session flag
Currently, IFAIK, the only work around for this is to modify your SSO code to duplicate the masquerade init code and double check for masquerading users. Very inefficient since the DB calls are made twice... and you may need to query the system table to see if masquerade is installed.
If masquerade_switch_user0 where to set the session flag and masquerade_switch_back() unset it, the the session flag could be used by boot() implementations to manage masqueraded users. This would be much more efficient since no DB calls would be needed... e.g. just do something like:
if ( isset($_SESSION['masquerade']) )
// don't check persistent data
An alternative to this would be to add a couple of masquerade API hooks that get called with users are switched. This would allow implementations to do any additional user setup / teardown needed by other modules.
Comments
Comment #1
cgmonroe CreditAttribution: cgmonroe commentedHere are patches for the current git 6.x and 7.x branches that add setting/unsetting the session flag when a user change is made.
Comment #2
cgmonroe CreditAttribution: cgmonroe commentedComment #3
andypostLooks like duplicate of commited #1364574: Invoke logout & login hooks for compatibility with bakery
Comment #4
andypostextra space within if()
Comment #5
cgmonroe CreditAttribution: cgmonroe commentedRe: Duplicate issue
Yes, the use case illustrating these issues are duplicate but the underlying issues are different. This is because the problem that lead me to creating this issue was caused by a similar use case with some custom SSO code. So my use case used to describe the need / issue was a duplicate of the other issue's use case.
However, IMHO, delaying setting the session flag until the next page load init call is a separate bug. Downstream developers should be able to depend on the session flag being set at all times during the masquerade process. SSO is a single use case that caught this, but I'm sure there are others.
I have changed the title to more clearly reflect the larger issue... and upped the version to 7.x... since it effects both 6 and 7.
To restate the problem:
The $_SESSION['masquerading'] property (or lack of it) is missing during the start and end masquerade periods because the flag is not set until the next page's init() call. This means that it is not available to some hook implementation that might need it. E.g., hook_boot() on the page after masquerading starts and hook_exit() on page ends.
Re: space?
Not sure of the problem here... the patched module passes Coder with report minor problems set (well some other parts of the masquerade code fails, but not my patched code). AFAIK the Drupal coding standard only requires a space between if and the starting paren of the logicial. It does not define the layout of the logical except to keep it readable and under 80 chars if possible.
I'll rebuild the patch if this is a masquerade coding requirement.
Comment #6
andypostIs there any other way to fill $_SESSION['masquerading'] to make it accessible at hook_boot/init()
We lack of tests so this patch needs deeper review and probably refactoring the logic of using the setting
PS: np, about spaces...
Comment #7
cgmonroe CreditAttribution: cgmonroe commentedHmm, the logic doesn't seem to complex...
The session flag is used to determine what link to show the user. E.g., non-masquerading users do not have the session flag and see "Switch to User" text. Masquerading users have the flag and this is used to determine that the "Switch back to..." text. is displayed.
The current logic
Start Masquerading Page:
All "Masquerading as" Page loads:
Stop Masquerading page
Normal User Page load
The modified logic
Start Masquerading Page:
All "Masquerading as" Page loads:
Stop Masquerading page
Normal User Page load
This patch does not change the default behavior, just covers the entry/exit points better. Note that the additional session flag changes are occurring right before a drupal_goto() which ends the request processing with an exit call. So there should not be any conflict with other code.
Comment #8
andypost@cgmonroe, Thanx a lot for write-up. I suppose you mostly right but I need to test more the module.
I think we need to implement hook_boot() anyway to allow currently fragile masquerading as anonymous user.
It would be a great clean-up before stable release
Comment #9
andypostThere's a D8 version that relies only on $_SESSION #1926074: Remove {masquerade} table and rely on session flag only
After the test coverage it will be commited and should backported to 7 branch
Comment #11
rjacobs CreditAttribution: rjacobs commentedAnother potentially related issue: #2226531: Login hooks can't detect masqueraded user with $_SESSION['masquerading']
Comment #12
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedI'm linking this to an existing issue for that solution.