I've tested on a non-English Drupal installation. Using bulk operation, a user who doesn't have the edit permission can still block/unblock any user including the administrator. After submission, I see the error message AND successful message. The selected user(s) are in fact blocked (or unblocked).

This may be due to the fact that I'm using non-English UI, but nonetheless, a security concern.

BTW: This module is a lifesaver for me. Thank you so much to compile the patches and whatnot into one place!

CommentFileSizeAuthor
#15 administerusersbyrole-hide_access_denied_links-1670954-15.patch807 bytesptsimard
#12 administerusersbyrole.module-1670954.patch1.25 KBsnuyt
#3 administerusersbyrole-load-name-role-1670954.patch526 bytesdenison
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

NobuT’s picture

NobuT CreditAttribution: NobuT commented

To mitigate the issue, I've added form alter hook so that users that the current user doesn't have the edit permission don't show up.

/**
 * Implements hook_form_user_admin_account_alter().
 */
function administerusersbyrole_form_user_admin_account_alter(&$form, &$form_state, $form_id) {
    // Remove rows if I don't have access to them. 
  foreach ($form['accounts']['#options'] as $uid => $fields) {
    $account = user_load($uid);
    if (!_administerusersbyrole_can_edit_user($account)) {
      unset($form['accounts']['#options'][$uid]);
    }
  }
}
mrfelton’s picture

mrfelton CreditAttribution: mrfelton commented
Project: Administer Users by Role D7 » Administer Users by Role
Version: » 7.x-1.0-beta1

Moving over to the Administer Users by Role queue.

denison’s picture

denison CreditAttribution: denison commented
FileSize
administerusersbyrole-load-name-role-1670954.patch526 bytes

Sorry, I did not understand where the topic has been moved. I had a similar problem, with the same message. But in my case I was using UI in English.

What was happening: In place of the string with the name of the role, was coming from the id eg id => id.

 Solved with this patch. I'm using version 7.x-1.0-beta1

NobuT’s picture

NobuT CreditAttribution: NobuT commented

Thanks for the patch. It seems working.

edb’s picture

edb CreditAttribution: edb commented
Priority: Normal » Major
Status: Active » Reviewed & tested by the community

Works fine here too, moving to RTBC.

Also marking as major as this bug prevents any user apart from user 1 from editing a users status.

dmegatool’s picture

dmegatool CreditAttribution: dmegatool commented

#1 solution works but when I create a new user, it's back to the old behavior. When I clear the cache, it works again and only the appropriate users are shown...

Any idea what's causing this and how to fix ?

EDIT: Found out that's when I create a user when using the overlay/seven theme. If I disable the admin theme and create the user, everything's fine. I've put the code in my theme template.tlp.php... Is that where it goes ? I would like to reenable the admin theme overlay.

EDIT 2: I copied over the code to the Seven's template.tlp.php which I find kinda hacky. Works now but if the theme gets updated... Anyway maybe someone could answer to help others.

Dimetry’s picture

Dimetry CreditAttribution: Dimetry commented

I have same issue.
Using bulk operation the user can block/unblock any other user, includes Administator.
I'm waiting for update the module.
And thanks for module.

HaloFX’s picture

HaloFX CreditAttribution: HaloFX commented

Applied #3 to beta1 and dev, both applied OK, but didn't resolve issue.

gaele’s picture

gaele CreditAttribution: gaele commented
Issue summary: View changes
Priority: Major » Critical
Status: Reviewed & tested by the community » Closed (duplicate)

#1717876: Remove dependency on 'Administer users' permission

gaele’s picture

gaele CreditAttribution: gaele commented
Status: Closed (duplicate) » Needs work

Sorry, 1717876 appears to be another issue.

snuyt’s picture

snuyt CreditAttribution: snuyt commented

using

function administerusersbyrole_user_presave(&$edit, $account, $category) {

instead of

function administerusersbyrole_user_update(&$edit, $account, $category) {

is probably a good idea ?

snuyt’s picture

snuyt CreditAttribution: snuyt commented
FileSize
administerusersbyrole.module-1670954.patch1.25 KB
gaele’s picture

gaele CreditAttribution: gaele commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 12: administerusersbyrole.module-1670954.patch, failed testing.

ptsimard’s picture

ptsimard CreditAttribution: ptsimard commented
FileSize
administerusersbyrole-hide_access_denied_links-1670954-15.patch807 bytes

Hello,

while not dealing with the other patches right now, I just thought that NobuT's hook at comment #1 needed its own patch as it is very useful.

ptsimard’s picture

ptsimard CreditAttribution: ptsimard commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 15: administerusersbyrole-hide_access_denied_links-1670954-15.patch, failed testing.

AdamPS’s picture

AdamPS CreditAttribution: AdamPS commented
Assigned: Unassigned » AdamPS
Status: Needs work » Needs review
Issue tags: +beta1

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.

Roughly speaking I took the patch in #15 but tweaked the permissions to guard permissions to users own account more carefully.

Should now be fixed on that issue, please let me know.

AdamPS’s picture

AdamPS CreditAttribution: AdamPS commented
Version: 7.x-1.0-beta1 » 7.x-2.0-beta1
Status: Needs review » Closed (fixed)

Fix now available in latest release