Not safe to use with "Administration Views" module

Thanks for the report.

I can reproduce the bug you describe. As the maintainer for Administer Users by Role but with little knowledge of Administration Views I am baffled. Administer Users by Role does not alter admin/people/permissions and from a quick search I can't see that Administration Views does either. I would welcome any one with a better Drupal understanding who can explain this one!

For the record, there are some more dangers of using these two modules together.

1) The sub-admin can create a no-roles user for themselves, then use the "Change user roles" action to assign it an administrator role. The solution is to enable actions_permissions configure it to restrict access to the "Change user roles" action (which arguably is a danger anyway - a user with "Administer Users" gets to set roles when they wouldn't normally be able to).

2) Administer Users by Role contains two modifications to /admin/people:

• It allows access to this page with only "Access the users overview page"
• It alters the page to remove users for which the sub-admin/moderator doesn't have permission to edit.

When Administration Views is enabled, the first of these still occurs, but the second doesn't. This means that the sub-admin/moderator can alter administrators. For example, use the "Change value" action on an admin, change their email, request a password reset, use it login (although this would probably send an email to tip the admin off). Or just block all the admins!

=========

Sorry, the best advice I can offer for the moment is don't run these two modules together. I will document this right away. In the longer term if there's no fix available, I will at least ensure a highly visible warning if both modules are enabled.

OK, after considerable head-scratching, I think I now understand the original bug you raised. It seems to be a bug in Administration Views

See file plugins/views_plugin_display_system.inc, in particular the comment

* To achieve a correct replacement of an existing system path,
* execute_hook_menu() performs some advanced processing on menu router
* definitions to account for possible child router items that would normally
* inherit properties of the original system path.

I believe this function ends up copying the "access callback" of 'admin/people' into 'admin/people/permissions' in the mistaken belief that it had previously been inherited. However the documentation for hook_menu (https://api.drupal.org/api/drupal/modules!system!system.api.php/function...) states

only MENU_DEFAULT_LOCAL_TASK items can inherit access callbacks

And this code is failing to make this check before copying. So this ought to be raised as a bug against Administration Views. (However the two supplementary points I raised belong in this module.) Sorry I don't have time to raise it right now, but hopefully this has helped explain the mystery and pointed you in the right direction.

The problem I have with this is that on the core admin/people page (not the admin views version) , the links for editing users appear in the table even if your role does not have access to do so. I've tried to work around this but I can't because since this module hijacks core permissions, there's no way for this page to know whether you truly have access to that operation. It seems very kludgy.

Below is a sample piece of code that could solve the problem I stated above. Probably not exactly patch-worthy, especially due to the explicit reference to the administrator role and edit operation, but it, or something like it could definitely help. (This also may belong in another issue entirely). Sadly, this does not yet solve the issue of being able to perform bulk operations on users:

function administerusersbyrole_form_user_admin_account_alter(&$form, &$form_state) {
  global $user;

  // Loop through each user in the table
  $users = &$form['accounts']['#options'];
  foreach ($users as $uid => &$info) {

    // Hide default admin from table unless logged in as that user
    if ($uid == 1 && $user->uid != 1) {
      unset($users[$uid]);
      continue;
    }

    // Deny access to operations by default
    $access = FALSE;

    // Load user in current row
    $account = user_load($uid);

    // Loop through each role this user belongs to
    foreach ($account->roles as $rid => $name) {

      // Optional code to hide administrator users from other roles
      if ($rid == 3 && !in_array($name, $user->roles) && $user->uid !=1) {
        unset($users[$uid]);
        continue;
      }

      // Get permission string
      $permission = _administerusersbyrole_build_perm_string($rid, 'edit');

      // Check if logged in user has permission to perform operation on user in this row
      $access = user_access($permission, $user);
    }

    // If the logged in user does not have access to edit the user in this row, hide operations
    if (!$access) {
      unset($users[$uid]['operations']['data']);
    }
  }
}

@chrisgross Thanks for taking time to post a patch. If I understand you correctly, the code you are looking for should already be present in administerusersbyrole_form_user_admin_account_alter, and it works for me. If you want to pursue this further, then yes it should be a separate issue. The next step would be to determine a reliable scenario that allows me to reproduce the bug you see.

Sorry for the delayed follow up but I have been away.

@spira, please go ahead and raise the bug in "Administration Views" if you feel able to do so, or else let me know, I'm happy to do it.

I have created an issue for it over at admin_views: #2425121: Security flaw when used with "Administer Users By Role" module. It would be good to get this fixed because I think these two modules are very frequently used together, and this is quite a security risk.

@spira and @AdamPS I've just raised an issue about incorrect access to the permissions page (as mentioned in the original post) here: https://www.drupal.org/node/2429879

Please see that issue to see whether the patch provides a fix for that particular problem.

Thanks,

Barry

I can see there is a lot of interest in having this module compatible with "Administration Views" and will do what I can to get it working, but I'm likely too busy for a few days.

The next step is a patch that addresses item 2 in my comment #2 - assuming that this bug does in fact exist - I'm still speculating as I don't run a site with both modules on. I think it would need a hook_views_default_views_alter hook to alter the view set up by "Administration Views". In this hook, we need to filter target users in/out of the view depending whether the currently logged in user has access to admin the target user. This is potentially tricky.

Then there is also a need for a workaround to the "Administration Views" bug. I think it might work to alter the module weight so that this module comes after Administration Views. This should be fairly easy.

Finally we might wish for a warning as per item 1 in my comment #2. Warn if "Administration Views" is enabled, and "Change user roles" is enabled. I would think perhaps code in hook_requirements raising REQUIREMENT_ERROR. This should be fairly easy.

If anyone has a patch to just one of these, or even some notes on trying but perhaps not entirely succeeding, please contribute.

OK I've done some testing. The main gotcha is with the bulk operations. Results of my testing:

• Sub-admin can run change user roles.
• For all other operations, the operation reports success, but nothing changes.

I don't really understand why. But it's worth noting that Drupal has a sophisticated inbuilt node access system, but there's nothing for users. Hence trying to persuade the Entity/Actions system to allow certain actions on users but not others is likely to be pretty tricky.

I also noticed that "Change Value" is a dangerous operation because one of the things it allows is to change roles. However the permissions of "Administration Views" have "Modify entity values" - so turn that off, and I expect you can't bulk block/unblock users, nor can you even do bulk modification on content.

So my current thinking is to change tack entirely, and go for a very simple solution.

• Create new path, for example admin/usersbyrole.
• This path leads to a copy of the admin/people form (of course modified to restrict users by means of the code already in place).
• Even when "Administration Views" is installed, this URL stays with the form in core.
• Sub-admin gets permission to this new path, but NOT to admin/people

So maybe 5 lines of code, and as far as I can see, it solves all of the problems in this thread. We can even add another few lines of code to redirect from the old URL to the new one for sub-admins.

Thoughts on this one welcome.

Personally I wouldn't want a path like admin/usersbyrole for the general user admin page. One of the things I like about admin_views is that it keeps the same paths, but if that's not possible I'd prefer a more generic path.

I don't really understand why. But it's worth noting that Drupal has a sophisticated inbuilt node access system, but there's nothing for users. Hence trying to persuade the Entity/Actions system to allow certain actions on users but not others is likely to be pretty tricky.

I ran into this problem a while back while still using version 1 of this module and noticed that people couldn't bulk cancel. The problem is in VBO's _views_bulk_operations_entity_access(), which calls entity_access(). This will return the entity's access callback. By default this is entity_metadata_user_access for user entities, which only allows users with the 'administer users' permission to edit or cancel accounts.

I fixed this in a custom module. I used hook_entity_info_alter() to add my own access callback, and in that callback I basically made it do the same thing as _administerusersbyrole_can_cancel_user(), with a few modifications. From a brief glance at the version 2 code you could probably use _administerusersbyrole_check_access(). I also had to use hook_entity_info_alter() to make sure my hook_entity_info_alter() was called after the entity module's.

I also noticed that "Change Value" is a dangerous operation because one of the things it allows is to change roles.

The "Change value" operation doesn't allow you to change roles. There is actually a separate "Change user roles" operation for that. I don't know what access checking VBO does for it. The fact that you said sub-admins were able to change roles suggests it doesn't do any checking, which is concerning and probably needs fixing in VBO if true. I haven't tested it myself.

use workbench module. this module help you top giving permission by node and view to other user. there also permission creator publisher .

@Rob230
Thanks for your useful comments. Some responses:

Personally I wouldn't want a path like admin/usersbyrole for the general user admin page. One of the things I like about admin_views is that it keeps the same paths, but if that's not possible I'd prefer a more generic path.

Sorry what I had meant was that "Administer Users by Role" would use the URL admin/usersbyrole when sub-admins are managing the people they have access too. Therefore sub-admins bypass VBO and the inherent problems in this issue. "Administration Views" would not change.

The "Change value" operation doesn't allow you to change roles.

Try running "Change Value" and selecting "User roles" (under Properties). You can then add or replace roles. I agree with you that there is a separate "Change user roles" operation - both ways work for me as an admin.

Try running "Change Value" and selecting "User roles" (under Properties). You can then add or replace roles.

Oh, I hadn't seen that. In that case it might be impossible to get these modules working together nicely because the access callback will never know whether it's a role being changed or something else.

Sorry what I had meant was that "Administer Users by Role" would use the URL admin/usersbyrole when sub-admins are managing the people they have access too. Therefore sub-admins bypass VBO and the inherent problems in this issue. "Administration Views" would not change.

Sorry I don't think I explained it clearly. The entire reason I use admin_views is for the sub-admins. I myself am fine with Drupal's default "People" page, but it's not very user friendly, whereas a view can be fully customised and made easy to use for whatever site I've made. I also like that everyone goes to the same "admin/people" page, since it's predictable and consistent for documentation, whereas "admin/usersbyrole" has less meaning to non-Drupal people that I hand the site over to. What about 'admin/users' as a more generic path? It's not currently in use but it does have the potential for conflicts if another module uses that path.

Just to clarify are you suggesting bypassing admin_views entirely for sub-admins? Or duplicating what it does?

A possible alternative: you could use hook_views_default_views_alter() to disable the bulk operations from the views provided by admin_views. Only problem is there'd be nothing stopping the admin from adding them back.

I'm starting to think the only way to have a custom user admin page with fine-grained permissions is a separate module dedicated to it. It's difficult to solve with three different modules that have their own agendas. I really hope Drupal 8 won't have this nebulous "administer users" permission.

@Rob230

OK, I understand now - yes I can see that it would be nice to have "Administration Views" working for the sub-admins. Let's hope we can find a way.

But I think it might be possible, or at least I have some hope. The properties that "user" has seem to be defined in entity_metadata_user_entity_property_info. Scroll down and there is a property "roles" with access callback entity_metadata_user_properties_access. And this function does "return ($op == 'view' && $is_own_account);". In other words, never allows write access except for people with "bypass permissions". So fingers crossed, it might actually do the right thing. I'm beginning to realise that some clever people wrote the entity code.

Anyway I'll update once I have the hook_entity_info_alter code working - your suggestions have helped a lot with that.

OK, I reckon I've roughly proved I can make it work on my test system. Thanks to everyone who helped out. It will still be a while before I can make the patch available whilst I get everything properly coded and tested.

It is no longer the case that I need other people to help solving/coding. However I will be very glad of people to test once I do manage to post the patch, so please leave you "follow" turned on.

@spira you are correct.

The recent VBO security update [#2516688] is a patch I contributed and it solves the remaining security problems in this issue. Ensure you have the latest Administration Views release to solve #2425121: Security flaw when used with "Administer Users By Role" module. Provided you have both these fixes you are secure. Note that VBO will often display that bulk operations are successful even if they have actually been blocked due to insufficient permissions.

There is still one more stage before this is fully fixed. The sub-admin can't yet make bulk changes using the view provided by Administration Views. I have just now pushed a commit to solve this by implementing various entity module hooks. The fix should be built into the next dev release and I will be very grateful to anyone who can test it - post here with any feedback.

NB The new release adds a dependency on chain_menu_access which you will need to add when you upgrade. This handy module gives much better co-operation when using multiple access modules together, plus it simplifies the code.

OK, so there have been a few downloads of the dev at least, so hopefully it has got some testing. I'll mark as fixed. Any further feedback please let me know.