I'd suggest that we need to introduce a new permission access users overview that grants access to admin/people, similar to how node.module has a access content overview permission. This allows for easier management without needing to grant the all powerful administer users permission, or knowing that the create user page is located at /admin/people.

Files: 
CommentFileSizeAuthor
#7 administerusersbyrole-access_users_overview_permission-1776666-7.patch1.93 KBptsimard
PASSED: [[SimpleTest]]: [MySQL] 1,141 pass(es).
[ View ]
#2 1776666.3-administerusersbyrole-access-users-overview-permission.patch1.19 KBmrfelton
PASSED: [[SimpleTest]]: [MySQL] 1,141 pass(es).
[ View ]
#1 1776666-administerusersbyrole-access-users-overview-permission.patch1.28 KBmrfelton
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1776666-administerusersbyrole-access-users-overview-permission.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

mrfelton’s picture

Status:Active» Needs review
StatusFileSize
new1.28 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1776666-administerusersbyrole-access-users-overview-permission.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

This should work, but it's not in mysetup. Well, it works for admin/people/people, but not for admin/people.

mrfelton’s picture

StatusFileSize
new1.19 KB
PASSED: [[SimpleTest]]: [MySQL] 1,141 pass(es).
[ View ]

Oops, here it is against a clean checkout (I had another patch applied when I created that)

mrfelton’s picture

Status:Needs work» Needs review

Actually, this does work perfectly. My issue was because I also had admin_views installed. I resolved by adding the following to my own module:

<?php
/**
 * Implements hook_views_default_view_alter().
 */
function mymodule_core_views_default_views_alter(&$views) {
  if (isset(
$views['admin_views_user'])) {
   
// Adjust the permissions for the people admin screen so that users with
    // administerusersbyrole's 'access users overview' can access.
   
$handler =& $views['admin_views_user']->display['default']->handler;
   
$handler->display->display_options['access']['type'] = 'perm';
   
$handler->display->display_options['access']['perm'] = 'access users overview';
  }
}
?>
kaizerking’s picture

Should we apply this patch #2 or not?

mrfelton’s picture

Status:Needs review» Needs work

I think we might need an upgrade path - one that grants the new permission to all users that hav the administer users permission.

Status:Needs review» Needs work
ptsimard’s picture

Issue summary:View changes
StatusFileSize
new1.93 KB
PASSED: [[SimpleTest]]: [MySQL] 1,141 pass(es).
[ View ]

Here is a a new patch combining #2 and including the hook in comment #3.

I think that hook should be included in administeruserbyroles and I'm using it with it patched in.

Sorry I have nothing to say at this time relating to the upgrade path issue.

sinasalek’s picture

Status:Needs work» Reviewed & tested by the community

I thinks this is really essential patch, giving administer users permission is too much and can causes security issues.
However it does not seem to work for me.
Also there is another patch that has some overlapping with this one #1717876: Remove dependency on 'Administer users' permission

ptsimard’s picture

Oh yes, I forgot, I am using the https://www.drupal.org/project/user_settings_access refered in that issue by https://www.drupal.org/node/1717876#comment-8278033 to make this work. I think we should probably patch in what that tiny module does.

AdamPS’s picture

AdamPS’s picture

Assigned:Unassigned» AdamPS
Category:Feature request» Bug report
Status:Reviewed & tested by the community» Needs review

I intend to fix this as part of #2378869: Meta-issue for Beta 2 release. Please sign up as a follower of that issue and there will shortly be a patch that I would like feedback on.

I have taken the patch from #2, with slight modifications. I test both the new permission and "Administer Users" which reduces issues with losing permission when the module is added. I have also added migration code.

I have not taken the patch from #3 as I don't use that module so can't test it. The code would likely need to be altered a little to work on top of the batch of changes I'm committing. If anyone does have interest in providing the modified patch, please raise as a separate issue as a feature request. I would need a patch based on the code after the beta2 release is published.

AdamPS’s picture

Issue tags:+beta2
AdamPS’s picture

Version:7.x-1.x-dev» 7.x-2.0-beta1
Status:Needs review» Closed (fixed)

Fix now available in latest release