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.

Files: 
CommentFileSizeAuthor
#1 masquerade-6.x-allow_SSO_boot_to_identify_masquerading_user-1813696-1.patch1.28 KBcgmonroe
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch masquerade-6.x-allow_SSO_boot_to_identify_masquerading_user-1813696-1.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#1 masquerade-7.x-allow_SSO_boot_to_identify_masquerading_user-1813696-1.patch1 KBcgmonroe
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch masquerade-7.x-allow_SSO_boot_to_identify_masquerading_user-1813696-1.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

StatusFileSize
new1 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch masquerade-7.x-allow_SSO_boot_to_identify_masquerading_user-1813696-1.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new1.28 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch masquerade-6.x-allow_SSO_boot_to_identify_masquerading_user-1813696-1.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Here 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.

Status:Active» Needs review

+++ b/masquerade.module
@@ -846,6 +847,10 @@ function masquerade_switch_back() {
+  if ( isset($_SESSION) ) {

extra space within if()

Title:Allow SSO boot functions to identify a masquerading userSet/unset Session flag when masquerade status changes.
Version:6.x-1.x-dev» 7.x-1.x-dev

Re: 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.

Is 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...

Hmm, 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:

  • User selects one of the "Masquerade as options"
  • All options call masquerade_switch_user()
  • $_SESSION['masquerading'] used to prevent recursive masquerading
  • This sets up the masquerading stuff and inserts a record in the masquerade table.
  • Drupal go to called to send to a new page.
  • hook_exit called (Session not set yet)

All "Masquerading as" Page loads:

  • hook_boot called (PROBLEM: $_Session['masquerading'] not set on initial masquerading as page load)
  • masquerade_init() called on page load
  • does a DB query to masquerade table to get the original id
  • sets the $_SESSION['masquerade'] flag
  • Other page logic uses $_SESSION['masquerading'] to modify links.
  • hook_exit called (Session set)

Stop Masquerading page

  • User selects to switch back
  • masquerade_switch_back() called
  • Masquerading session info cleared from the masquerade DB table
  • user info swapped back
  • drupal go to called
  • hook_exit called ( PROBLEM: $_SESSION['masquerading'] is still set )

Normal User Page load

  • hook_boot called (PROBLEM: $_Session['masquerading'] is set on initial post masquerading swich page load)
  • masquerade_init() called on page load
  • does a DB query to masquerade table to get the original id - Not found
  • unsets the $_SESSION['masquerade'] flag if needed.
  • Other page logic uses the fact that $_SESSION['masquerading'] is not set to modify links.
  • hook_exit called

The modified logic

Start Masquerading Page:

  • User selects one of the "Masquerade as options"
  • All options call masquerade_switch_user()
  • $_SESSION['masquerading'] used to prevent recursive masquerading
  • This sets up the masquerading stuff and inserts a record in the masquerade table.
  • NEW: sets the $_SESSION['masquerading'] flag
  • Drupal go to called to send to a new page.
  • hook_exit called (Session not set yet)

All "Masquerading as" Page loads:

  • hook_boot called (CORRECT BEHAVIOR: $_Session['masquerading'] IS SET on initial masquerading as page load)
  • masquerade_init() called on page load
  • does a DB query to masquerade table to get the original id
  • sets the $_SESSION['masquerading'] flag
  • Other page logic uses $_SESSION['masquerading'] to modify links.
  • hook_exit called (Session set)

Stop Masquerading page

  • User selects to switch back
  • masquerade_switch_back() called
  • Masquerading session info cleared from the masquerade DB table
  • user info swapped back
  • NEW: unsets the $_SESSION['masquerading'] flag if needed.
  • drupal go to called
  • hook_exit called ( CORRECT BEHAVIOR: $_SESSION['masquerading'] IS NOT SET )

Normal User Page load

  • hook_boot called (CORRECT BEHAVIOR: $_Session['masquerading'] IS NOT SET on initial post masquerading switch page load)
  • masquerade_init() called on page load
  • does a DB query to masquerade table to get the original id - Not found
  • unsets the $_SESSION['masquerading'] flag if needed.
  • Other page logic uses the fact that $_SESSION['masquerading'] is not set to modify links.
  • hook_exit called

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.

Priority:Normal» Major

@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

There'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

Status:Needs review» Needs work