In the instance where one user is viewing another user's profile (by clicking on the submitted by name of a post, let's say), they are given the ability to create a subuser under that other person's account. I can't imagine the situation where this would be acceptable.

To fix the problem, I needed to add two things.

1) to not present the user the opportunity to add the subaccount:
In hook_user under the 'view' case, change the code following the first line below to the new code

      // The parent user should either have access to create subusers, or have
      // existing subusers.
      //GMM: logged in user is not viewing someone else's profile (unless is admin)
      $access = user_access('create subuser');
      $view = views_get_view('subusers');
      if (($access && $GLOBALS["user"]->uid == $account->uid)|| $GLOBALS["user"]->uid == 1 || (isset($view->results) && $view->results)) {
        $view = views_embed_view('subusers');
        $account->content['subusers'] = array(
          '#type' => 'user_profile_item',
          '#title' => t(SUBUSER_LIST),
          '#value' => ($access ? l(t(SUBUSER_CREATE), 'user/' . $account->uid . '/subuser/create') : '') . '<br />' . $view,
          '#weight' => 11,
        );
      }
      break;

2) So a user shouldn't just change the URL to some other number, add a permission check:

Add the following routine:

//GMM: 
/**
 * Check if the user has permission to create the specified user.
 *
 * Pass cases:
 *   - Super user.
 *   - The user is a parent of the user being created
 *
 * @param array 
 * @return boolean Access granted.
 */
function subuser_create_user_access($info) {
  global $user;

  if ($user->uid == 1 || $info->uid==$user->uid) {
    return TRUE;
  }
  return FALSE;
}

AND

add the following to subuser_menu() for $items subuser/create add:

    'access callback' => 'subuser_create_user_access',
    'access arguments' => array(1),

Would the maintainer consider adding this to the code? Or, perhaps if there's a reason to let users create a subuser under some other parent, then add another variable setting to disallow it.

--glen

CommentFileSizeAuthor
#4 600390-4-eojthebrave-subuser_create_permissions.patch2.39 KBeojthebrave

Comments

Rosamunda’s picture

Rosamunda commented

subscribing

glen201’s picture

glen201 commented

I added a new component to the patch in step 1) above, namely a new view for users who are not the parent of the subuser.

Replace the patch in 1) above with:

      // The parent user should either have access to create subusers, or have
      // existing subusers.
      //GMM: logged in user is not viewing someone else's profile (unless is admin)
      $access = user_access('create subuser');
      $view = views_get_view('subusers');
      if (($access && $GLOBALS["user"]->uid == $account->uid)|| $GLOBALS["user"]->uid == 1 || (isset($view->results) && $view->results)) {
        $view = views_embed_view('subusers');
        $account->content['subusers'] = array(
          '#type' => 'user_profile_item',
          '#title' => t(SUBUSER_LIST),
          '#value' => ($access ? l(t(SUBUSER_CREATE), 'user/' . $account->uid . '/subuser/create') : '') . '<br />' . $view,
          '#weight' => 11,
        );
      } else if(!$parent) { //GMM: this is not a subuser, add the no-switch view to not list the switch user option
        $view = views_embed_view('subusers_noswitch');
        $account->content['subusers'] = array(
          '#type' => 'user_profile_item',
          '#title' => t(SUBUSER_LIST),
          '#value' => $view,
          '#weight' => 11,
        );
      }
      break;

and add the following view called "subusers_noswitch"

$view = new view;
$view->name = 'subusers_noswitch';
$view->description = 'Display the subuser account without a switch option';
$view->tag = 'users_noswitch';
$view->view_php = '';
$view->base_table = 'users';
$view->is_cacheable = FALSE;
$view->api_version = 2;
$view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */
$handler = $view->new_display('default', 'Defaults', 'default');
$handler->override_option('fields', array(
  'uid' => array(
    'label' => 'Uid',
    'alter' => array(
      'alter_text' => 0,
      'text' => '',
      'make_link' => 0,
      'path' => '',
      'alt' => '',
      'prefix' => '',
      'suffix' => '',
      'help' => '',
      'trim' => 0,
      'max_length' => '',
      'word_boundary' => 1,
      'ellipsis' => 1,
      'html' => 0,
    ),
    'link_to_user' => 0,
    'exclude' => 1,
    'id' => 'uid',
    'table' => 'users',
    'field' => 'uid',
    'relationship' => 'none',
  ),
  'name' => array(
    'label' => 'Username',
    'alter' => array(
      'alter_text' => 0,
      'text' => '',
      'make_link' => 0,
      'path' => '',
      'link_class' => '',
      'alt' => '',
      'prefix' => '',
      'suffix' => '',
      'target' => '',
      'help' => '',
      'trim' => 0,
      'max_length' => '',
      'word_boundary' => 1,
      'ellipsis' => 1,
      'strip_tags' => 0,
      'html' => 0,
    ),
    'empty' => '',
    'hide_empty' => 0,
    'empty_zero' => 0,
    'link_to_user' => 1,
    'overwrite_anonymous' => 0,
    'anonymous_text' => '',
    'exclude' => 0,
    'id' => 'name',
    'table' => 'users',
    'field' => 'name',
    'relationship' => 'none',
  ),
  'value' => array(
    'label' => 'First Name',
    'alter' => array(
      'alter_text' => 0,
      'text' => '',
      'make_link' => 0,
      'path' => '',
      'link_class' => '',
      'alt' => '',
      'prefix' => '',
      'suffix' => '',
      'target' => '',
      'help' => '',
      'trim' => 0,
      'max_length' => '',
      'word_boundary' => 1,
      'ellipsis' => 1,
      'strip_tags' => 0,
      'html' => 0,
    ),
    'empty' => '',
    'hide_empty' => 0,
    'empty_zero' => 0,
    'exclude' => 0,
    'id' => 'value',
    'table' => 'profile_values_profile_fname',
    'field' => 'value',
    'relationship' => 'none',
  ),
  'picture' => array(
    'label' => 'Picture',
    'alter' => array(
      'alter_text' => 0,
      'text' => '',
      'make_link' => 0,
      'path' => '',
      'link_class' => '',
      'alt' => '',
      'prefix' => '',
      'suffix' => '',
      'target' => '',
      'help' => '',
      'trim' => 0,
      'max_length' => '',
      'word_boundary' => 1,
      'ellipsis' => 1,
      'strip_tags' => 0,
      'html' => 0,
    ),
    'empty' => '',
    'hide_empty' => 0,
    'empty_zero' => 0,
    'exclude' => 0,
    'id' => 'picture',
    'table' => 'users',
    'field' => 'picture',
    'relationship' => 'none',
  ),
));
$handler->override_option('sorts', array(
  'name' => array(
    'order' => 'ASC',
    'id' => 'name',
    'table' => 'users',
    'field' => 'name',
    'relationship' => 'none',
  ),
));
$handler->override_option('arguments', array(
  'parent_id' => array(
    'default_action' => 'default',
    'style_plugin' => 'default_summary',
    'style_options' => array(),
    'wildcard' => 'all',
    'wildcard_substitution' => 'All',
    'title' => '',
    'default_argument_type' => 'user',
    'default_argument' => '',
    'validate_type' => 'none',
    'validate_fail' => 'not found',
    'break_phrase' => 0,
    'not' => 0,
    'id' => 'parent_id',
    'table' => 'user_relationship',
    'field' => 'parent_id',
    'relationship' => 'none',
    'default_options_div_prefix' => '',
    'default_argument_user' => 0,
    'default_argument_fixed' => '',
    'default_argument_php' => '',
    'validate_argument_node_type' => array(
      'webform' => 0,
      'blog' => 0,
      'client' => 0,
      'event' => 0,
      'image' => 0,
      'images' => 0,
      'page' => 0,
      'revent' => 0,
      'story' => 0,
      'venue' => 0,
    ),
    'validate_argument_node_access' => 0,
    'validate_argument_nid_type' => 'nid',
    'validate_argument_vocabulary' => array(
      '1' => 0,
      '4' => 0,
      '2' => 0,
    ),
    'validate_argument_type' => 'tid',
    'validate_argument_php' => '',
  ),
));
$handler->override_option('access', array(
  'type' => 'none',
));
$handler->override_option('cache', array(
  'type' => 'none',
));
$handler->override_option('use_ajax', TRUE);
$handler->override_option('use_pager', 'mini');
$handler->override_option('style_plugin', 'table');

This will create a listing of all the subaccounts of a particular member whose user profile page you are viewing without the edit/switch links, but with some nice features such as first name and picture (if you have those fields).

-- glen

john.money’s picture

john.money commented

Confirmed logic error...

A has create subuser perm which I assume means create subuser under only their own account

A views B

A creates subuser C under B

A views C

A creates subuser D under C

etc...

Patch form would be welcome.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented
Version: 6.x-1.x-dev »
Status: Active » Needs review
StatusFileSize
new 600390-4-eojthebrave-subuser_create_permissions.patch2.39 KB

Rolled a patch based on solutions proposed above against the 6.x-2.x-dev version.

fonant’s picture

fonant commented

I think the "subuser/create" URL should be just that, and not "user/%user/subuser/create". We should only be creating sub-users for the currently-logged in user, and that user ID doesn't need to come from the URL.

Will think further on this.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented

What if an administrator wants to create a sub-user for another account? I can come up with a handful of use cases where you would want someone with proper permissions to be able to create sub-users of an account that is not there own.

fonant’s picture

fonant commented

@eojthebrave: good point. Perhaps we need both methods:

"subuser/create" => creates subuser of current user, if they have permission "create subuser"
"user/%user/subuser/create" => creates subuser of %user, if the current user has permission "administer subusers" or %user = current user (as per your patch #4)

I could do with the first URL to avoid having to construct menu link paths from the current user's ID.

I wonder if two permissions "create any subuser" and "create own subuser" might be useful, to follow the node permissions method? Only needed if "create any subuser" is usefully different to "administer subusers", I suppose.

fonant’s picture

fonant commented

Have made my own "subuser/create" menu item using a custom module:

<?php

/**
 * Add new menu item to simply create subuser of the current logged-in user.
 */
function mymodule_menu_alter(&$items) {
  // Copy the existing URL to a new one that doesn't have the %user in it.
  $items['subuser/create'] = $items['user/%user/subuser/create'];
  // Change the callback function and arguments
  $items['subuser/create']['page callback'] = 'mymodule_new_subuser_form';
  $items['subuser/create']['page arguments'] = array();

  return $items;
}

/**
 * Create subuser for the current logged-in user
 */
function mymodule_new_subuser_form() {
  global $user;
  // Return the subuser creation form with the current user as the parent user
  return drupal_get_form('subuser_create_form', $user);
}

?>

Something similar could be added to the subuser module.

fonant’s picture

fonant commented

Patch in #4 works for me to fix the "create sub-user for another user when I don't have administer subusers permission" problem. Haven't fully analysed the boolean logic though.

Without patch:

User 14 can go to user/14/subuser/create and user/13/subuser/create as well, even if they don't have admin permissions.

With patch:

User 14 can go to user/14/subuser/create, but user/13/subuser/create gives "permission denied".

LiamRo’s picture

LiamRo commented

Thanks for the module and the patch in #4. Subusers up and running with parents able to add and monitor their kids on our site.

blakehall’s picture

blakehall commented
Version: » 6.x-1.x-dev
Status: Needs review » Fixed

Committed a slightly modified version of #4 to the 6.x-1.x-dev branch. Thanks @glen201 and @eojthebrave

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.