Login hooks can't detect masqueraded user with $_SESSION['masquerading']

This might be a dup (or is at least related to) #1813696: Set/unset Session flag when masquerade status changes.

Note that in the masquerade_switch_user($uid) function

The following has been done

  $user->masquerading = $new_user->uid;
  $user = $new_user;

  // Call all login hooks when switching to masquerading user.
  $edit = array(); // Passed by reference.
  user_module_invoke('login', $edit, $user);

This makes the property $user->masquerading useless since you overwrite the variable
If the property $user->masquerading was set before overwriting it you could use it as flag

It would also be better if the property contains the uid of the user that is masquerading instead of the user that has been masqueraded

There's no such code now http://cgit.drupalcode.org/masquerade/tree/masquerade.module#n811

Session holds state that initialized in hook_init() only

@andypost see line 858-868 in masquerade.module

<?php

  watchdog('masquerade', 'User %user now masquerading as %masq_as.', array('%user' => $user->name, '%masq_as' => $new_user->name ? $new_user->name : variable_get('anonymous', t('Anonymous'))), WATCHDOG_INFO);
  drupal_set_message(t('You are now masquerading as !masq_as.', array('!masq_as' => theme('username', array('account' => $new_user)))));
  $user->masquerading = $new_user->uid;
  $user = $new_user;

  // Call all login hooks when switching to masquerading user.
  $edit = array(); // Passed by reference.
  user_module_invoke('login', $edit, $user);

  return TRUE;
?>

If you implement hook_user_login() you can't access the $user->masquerading property, and that would be very usefull.

-xpost-

looks we need to revert this to $new_user->masquerading = $user->uid so new user will get this property initialized

PS: also if get rid of this property (+1 to that) we need to update hook_help() that mention about that

Chiming in here, bc this appears relevant. I'm looking over the whole issue queue for mention of this and looks like a couple places have. This doesn't make sense to me:

  $user->masquerading = $new_user->uid;
  $user = $new_user;

It appears than the flag ->masquerading is getting set needlessly, because the entire $user object is immediately overwritten. If the intent is to mark the masquerading $user object with the flag ->masquerading, then let's put a boolean in there and keep it on the overwritten object:

  $user = $new_user;
  $user->masquerading = TRUE;

No change in functionality to implement this except maybe if over the years someone else has gone and implemented their own $user->masquerading implementation like this or not.

In other modules, the $_SESSION['masquerading'] (set elsewhere by Masquerade) is a suitable check, but in the case of contrib module contrib module Legal, the hook_user_login action invoked by Masquerade without this $_SESSION marking doesn't allow Legal to detect Masquerade, but (what looks like the intent of) these two lines of code allows the Masquerade detection to occur.

Here's a patch based on #9.

Cheers to texas-bronius.

Meanwhile makes sense to add test for that

While the flag indicating if the user is masquerading is useful. I think it would likely be helpful to include something like the following just before overwriting the $user variable with $new_user.

$new_user->masquerading_user = $user->uid;

In my testing, this made the masquerading user available to the hook_user_login function for my purposes. Thoughts?