Occasionally, developers like to have private lists about who can masquerade as whom. I propose that Masquerade add a simple "hook_masquerade_access" call in the masquerade_switch_user() function that would work something like this:

  $access = true;
  $perms = module_invoke_all('masquerade_access', $uid);
  foreach($perms as $idx => $key) { $access = ($access && $key); }
  
  if ($access == false) { 
  	drupal_set_message('You are not allowed to masquerade as that user', 'error');
  	return;
  }

This would allow module developers to implement "hook_masquerade_access($uid)" in their own modules and return a boolean indicating whether the currently logged in user is allowed to masquerade as the proposed user (specified by uid).

Granted, the appeal for such a feature is probably small, but in my own development I've had need of this and have had to modify contrib modules (which I really don't like doing).

CommentFileSizeAuthor
#5 masquerade.module.patch4.79 KBantgiant

Comments

deekayen’s picture

I'm only a lowly co-maintainer, but I'd be fine with committing such a patch if you wrote it (so it has context of where to insert the new code).

deekayen’s picture

Don't know if it helps your situation at all, but Gurpartap recently added a masquerade function, though it doesn't take a uid parameter, it does check the current logged in user:

function masquerade_access($type) {
  switch ($type) {
    case 'unswitch':
      return $GLOBALS['masquerading'] || arg(2) == 'menu-customize' || arg(2) == 'menu';
    case 'autocomplete':
      return $GLOBALS['masquerading'] || (user_access('masquerade as user') || user_access('masquerade as admin'));
      break;
    case 'switch':
      return empty($GLOBALS['masquerading']) && (user_access('masquerade as user') || user_access('masquerade as admin'));
      break;
  }
}
deviantintegral’s picture

Version: 6.x-1.0 » 6.x-1.x-dev

See my comment over at #290313: Limit to relative users.

IMO a boolean is a bit too limited. Do you see any issues with sets of (uid_from, uid_to)?

The hook should implement an optional $account parameter so it doesn't rely on the logged in user. Relying on the logged in user can cause problems with batch operations.

deviantintegral’s picture

I think masquerade_switch_user() needs to be refactored a bit. As is, it's both a page callback *and* the function to actually switch users. I think we need two functions. The page callback should be a small wrapper function around a public function to switch the user. This will help clean up the code, as well as provide both a useful hook and an API for other modules to use.

antgiant’s picture

Status: Active » Needs review
StatusFileSize
new4.79 KB

Attached patch refactors out the rights checking into masquerade_check_user. It then adds two hooks, masquerade_increase_user_access and masquerade_limit_user_access.
A quick note on the implications on masquerade_increase_user_access. It allows modules to ignore the masquerade as admin permission.

Also, this patch has significant overlap with #723228: Quick change autocomplete should limit usernames

j0nathan’s picture

Subscribing, for request about CiviCRM in #390980: Integration with civicrm which is a duplicate of this issue.

avpaderno’s picture

Issue summary: View changes
Status: Needs review » Closed (outdated)

I am closing this feature request, as Drupal 6 is no longer supported. Please re-open it if you would like the feature implemented in the Drupal 7 or 8 version of the module.