Provide a permission or setting to prevent users from canceling their own account

Protection rules for a specific user don't count for the user itself. If you want to prevent an user from canceling its own account there is the permission "Cancel own user account" (provided by Drupal core's user module).

Does the user in question have the permission "administer users"? That permission inherits "Cancel own user account". If so, then this is not an User protect problem. The configured protection rules only protect operations on other users. For operations on the user itself, User protect only controls mail address, password and openid via the provided permissions. Drupal core is responsible for controlling access for changing the own username or for cancel the own user account.

If the user does not have the "administer users" permission and still can cancel its own account, check what happens if you (temporary) disable the User protect module. If the user then can no longer cancel its own account, then it would be a bug in User protect, because then there is a clear difference in behaviour.

Reroll.

Patch still applies and tests pass. University of Waterloo has used these successfully for several years.

Is it possible to address the question raised in comment #8 ?

Is this a "bug" in the userprotect module or is it a new feature? Not to belittle it's potential value.

My concern is that settings that are not designed for it to apply on the logged in user, are applied to the logged in user. You wouldn’t use the permission “access content” either for allowing access to user profiles, even if in your business case user profiles are part of the content.

I think instead we would need to add a new permission: “Cancel own admin user account” or something similar. That is, after #8 is evaluated.

Reviewing this issue and the question in #8 - Is this a bug or not?

As I understand this patch is to allow a role to be able to administer users but to only allow the role to be able to ADD users and not REMOVE users as described in comment #9.

The problem described in comment #5 is that using the drupal permissions a user with "Administer users" enabled and "Cancel own user account" disabled is still able to remove their own account. This might be seen as a drupal core bug. However, the patch provided in this thread allows the userprotect module to provide further granular permissions for this configuration.

Test 1.
With userprotect module disabled to test drupal permissions a user with "Administer users" enabled and "Cancel own user account" disabled is able to cancel their own account. An email is sent to them with a link to confirm cancellation.

Test 2.
With userprotect module enabled user has a role with drupal "Administer users" enabled and "Cancel own user account" disabled. The role is protected by userprotect module with "Cancel" protected. However, the user is able to cancel their own account, an email confirmation link is sent to them. Meanwhile, and in contrast, when the privileged user edits another user account with protected role(s) with "Cancel" protected the privileged user is unable to cancel/remove that account, the option is greyed out.

Test 3.
A user with a role that has "Administer users" disabled and "Cancel own user account" enabled and the userprotect protects the role against cancel, the user can still cancel their own account. They are able to cancel their own account whether the userprotect module is enabled or not.

Conclusions:
1. While Drupal includes separate permissions to "Administer users" and "Cancel own user account" the "Administer users" permissions overrides the "Cancel own user account" when it is disabled, allowing users to cancel their own account if they can "Administer users".

2. The userprotect module does prevent cancelling the accounts of users in protected role(s) by privileged users with "Administer users" enabled. However, the privileged user is able to cancel/remove their own account even if they are in a protected role, making that an exception to the protection. There is no way to prevent this using the drupal permission settings in conjunction with the userprotect module without this patch.

3. The drupal permissions "Administer users" and "Cancel own user account" do not behave exclusive of each other. This poses a difficulty for the userprotect module. The patch in this issue is designed to address this.

Further test to consider is: if this patch is applied what happens when the "Cancel own user account" permission is enabled, whether or not "Administer users" is enabled? There may be circumstances, such as legislative requirements in some jurisdictions that require a user to be able to cancel/remove their account. This should not be unintentionally the result of the "Administer users" permission but be specific to the "Cancel own user account" permission. This may again bring us back to the root issue of a flaw in the drupal core implementation of these two permissions. I have not yet tested the patch against this scenario.

Following up on tests described in #15 with the patch applied:

Test 4.
User in role that is protected against cancel with drupal permission "Cancel own user account" enabled, on editing their account sees the "Cancel" button but it is greyed out and cannot be used. If drupal "Cancel own user account" is disabled they do not see the cancel button, unless they have permission to "Administer users" in which case the button is visible but greyed out.

Only users identified in userprotect "Administrator bypass" have the ability to override and Cancel an account.

If legislation requires a user be able to delete an account that can still be achieved although they will need to make a request to the site administrator and will not be able to do it themselves.

The way drupal core permissions work there is no other way because of the "Administer users" permission trumps the "Cancel own user account" permission.

This patch provides consistency in the expectations of how the userprotect module is expected to behave. If a role is protected against cancelling an account it is consistent and applies to everyone, including the individual themselves.

I have looked at the patch. It is a simple patch and looks good to me. I am changing status to RTBC. Hope this helps to move it towards commit. Thanks.

@izmeez
Thanks for testing, but there is one thing I disagree with:

This patch provides consistency in the expectations of how the userprotect module is expected to behave. If a role is protected against cancelling an account it is consistent and applies to everyone, including the individual themselves.

This contradicts with (documented on the project page):

Protection rules don't count for the user itself. Instead, there are permissions available to prevent an user from editing its own account, username, e-mail address or password.

I agree there's a missing feature, namely there's no setting to prevent an user that may administer users from canceling their own account. But I think the patch mangles with User protect's behavior, namely that protection rules have effect on what an user can do on other users and don't have effect on theirselves. I believe changing this creates an inconsistency with the other protection types - that don't have effect on the user itself.

Retitling.

See also the following comment from the original creator of User protect:

a user editing their own account is a special case. userprotect was designed to provide limited access control to user admins, _not_ to take away all editing privileges from a user editing their own account.

Taken from #282793-5: Can't get it to work.

Here is a related feature request for editing own user account that was eventually fixed: #1172518: Users should NOT always be able to access their own edit page

Thanks for the comments and links as more food for thought :-/

However, in terms of

I think the patch mangles with User protect's behavior, namely that protection rules have effect on what an user can do on other users and don't have effect on theirselves. I believe changing this creates an inconsistency with the other protection types - that don't have effect on the user itself.

Unless i am mis-reading it the patch is only changing the behaviour when cancel is involved. If this is true, is it bringing the module behaviour back in line although it may be using something akin to a "hack" to overcome drupal's default handling?

-  if (isset($form['actions']['cancel']) && ($GLOBALS['user']->uid != $account->uid)) {
+  // "At this point, we only need the userprotect-specific validation if the
+  // current user and the edited user are not the same."
+  // But that doesn't seem to be the case.
+  if (isset($form['actions']['cancel'])) {

A more elegant solution might be to also check the Drupal "Cancel own user account" permission and base action based on that but this will introduce further complexity as will adding another permission or setting. I am not averse to these or other additions if they are thought to be important improvements to the patch.

@MegaChriz To pursue the suggestion of providing a "Cancel own account" permission or setting I've looked at the patch in #1172518: Users should NOT always be able to access their own edit page.

If this is added as a permission under Userprotect permissions it appears as a redundancy to drupal's existing User permissions. Maybe we can just use the existing drupal permission setting instead? This may simplify the patch. I'm looking at that now.

@izmeez
Reusing the core permission is an option. The only downside to that solution is the following: if site admins are used to the fact that user admins can always cancel their own account, then that behavior is changed as soon as Userprotect is enabled. And when Userprotect is disabled again, user admins can cancel their own account again. So this means a change in access rights takes immediate effect upon enabling the module, without explicitly configuring it first. This could be confusing to some users seeing that on some sites user admins can cancel their own account (userprotect not enabled) and on other sites they can't (userprotect enabled).

But having two similar looking permissions could be confusing too.

Attached is a patch for review as described in comment #21 that checks the core permission "Cancel own user account". If the core permission is false then userprotect will extend protection to the users account without the overriding effect of the "Administer users" permission.

To address the question raised in comment #22, I rather doubt that most people or admins realize that if the core permission to "Cancel own user account" for roles is false that the "Administer users" permission will override it. I think this patch with the userprotect module brings about a sensible and expected behaviour with control being left with use of core permissions.

Furthermore, if a site is configured to allow admins or other privileged users with the permission to cancel own user account, this will still behave as expected. The patch will not override the core permission.

@izmeez
Ok. It still feels like we're fixing a flaw in Drupal Core then. The fact that we don't like that "Administer users" inherits "Cancel own user account". As a last thing, I think we need to document that User Protect adjusts the meaning of the "Administer users" permission. Something like this:

Canceling own account

Drupal Core allows users with the "Administer users" permission to cancel their own account, even when they don't have the "Cancel own user account" permission. User Protect alters that behavior: access for canceling an user's own account is denied when that user does not have the "Cancel own user account" permission.

Could be put on the (very long) help page somewhere.

@MegaChriz Yes, we are fixing a flaw in Drupal 7 ("administer users" doesn't inherit "cancel own user account" it overrides it!) and it makes sense to fix this flaw in this module as it is the "userprotect" module. Your suggestion of adding something to the help is great.

Does it need to include

"User Protect alters that behavior: when protecting accounts from being cancelled access for canceling an user's own account

Maybe it should also be added to the Readme file which is short and because this is addressing something to do with core. Thanks.

Attached is a patch for the help and readme.

7.x-1.3 has been released, moving this one to 7.x-1.4.

Patch #23 and #27 does not protect direct url access.
It works by userprotect_user_cancel_access function.

This patch adds url protection and simplify userprotect_user_cancel_access function as userprotect_user_edit_access function.

This fix mistyping on comment from previous patch.

Attached is an interdiff comparing patch in comment#23 with patch in comment #30. It might help when reviewing the changes.

@barami Can you elaborate on your your comment

Patch #23 and #27 does not protect direct url access.
It works by userprotect_user_cancel_access function.

I'm not clear on what the problem is that you are encountering.

There are several questions that have been raised:

1. Need to fix spelling errors in patches. Namely, "cancelling" should be spelled with 2-L's not one. Unfortunately, this is spelled wrong in each of the patches in comment #27, #29, and #30.

2. The previous patch in comment #23 can be improved according to comment #29

This patch adds url protection and simplify userprotect_user_cancel_access function as userprotect_user_edit_access function.

In an attempt to help facilitate the discussion I have first combined the original patches from comments #23 and #27 into one and fixed the spelling and changed "an user's own" to "user's own" for readability. This is attached as a new patch.

I have also attached an interdiff from this new patch to the patch in #30. This may be confusing as it is the reverse of what may be expected with #33 coming after #30 although it combines patches from earlier and because the spelling error is present in #30 showing the reverse of fixing it in the new patch.

As to the change in the approach taken in #29/30 I am still not clear on the "url protection" and if the change in #30 is a simpler approach.

@barami, @izmeez
It would be cool if you could add automated tests to ensure that the feature is working as intended. The test methods could be added to UserProtectPermissionsWebTest.

@kpaxman Thanks for clarity, back to American English here's the combined patch as it was before changing the spelling.

Not experienced with adding tests will have to look into that.

@izmeez Sorry. I'm not familiar with english. Please understand some weired statements in my comment..

@barami Please don't be shy, we can all learn from you. Sorry if my attempts to add an interdiff to understand the differences seem too strong. You came to this issue with a different experience and came up with a different solution. If you can try again to explain what the problem you saw looked like it will help us understand the different perspective. Thank you.

@izmeez Thank you.

Patch #23 modify userprotect_form_user_profile_form_alter function only.

It will set cancel button to disabled on user profile form (user edit page).
But when user tries to access a cancel page directly (user/xxx/cancel), drupal will permit to access.

Because, cancel page (user/xxx/cancel) has access callback of userprotect_user_cancel_access function.

function userprotect_user_cancel_access($account) {
  // Perform core's access check.
  if (((($GLOBALS['user']->uid == $account->uid) && user_access('cancel account')) || user_access('administer users')) && $account->uid > 0) {
    // At this point, we only need the userprotect-specific validation if:
    // 1. The current user and the edited user are not the same.
    // 2. The current user is a user administrator.
    if (($GLOBALS['user']->uid != $account->uid) && user_access('administer users')) {
      // Check to see if the user's roles are protecting cancellation, or the
      // user account itself is protected.
      if (!userprotect_check_bypass('up_cancel') && userprotect_get_user_protection($account, 'up_cancel')) {
        // If so, and we're at /user/X/cancel, set a message.
        if (arg(0) == 'user' && is_numeric(arg(1)) && arg(2) == 'cancel') {
          drupal_set_message(t('%user is currently being protected from cancellation.', array('%user' => $account->name)), 'error', FALSE);
        }
        return FALSE;
      }
      else {
        return TRUE;
      }
    }
    else {
      return TRUE;
    }
  }
  else {
    return FALSE;
  }
}

This will return TRUE when user trying to cancel own account with 'administer users' permission. (act like core).
On the other hand, edit page (user/xxx/edit) is denied for protected account when user trying to access with 'administer users' permission.

I want to simplify cancel process as edit process.

userprotect_user_edit_access function (access callback of user edit page) delegates access checking to userprotect_get_user_protection function and it tries to check a permission ('edit own account') defined in userprotect module.

So, i copied conditions from userprotect_user_edit_access function and modified the userprotect_get_user_protection function to handle an access checking for cancel process.
Also moved uid comparison from userprotect_form_user_profile_form_alter function to userprotect_get_user_protection fuction (uid comparison aleady doing in this function).

@barami Thank you for the explanation, it is very helpful and the patch in comment #30 makes more sense.

The new input to this discussion prompted me to take a closer look at the use case described in comment #9

We have a role "site manager" which we want to be able to ADD users, but not REMOVE them. ... So the "site manager" role would get "administer users" and we take away their ability to delete by using the userprotect module.

This requires a full prevention to deleting accounts. The original patches including patch in comment #10 achieved this by removing the test if the current user and edited user are not the same.

The rather lengthy testing in comment #15 provides a conclusion that

2. The userprotect module does prevent cancelling the accounts of users in protected role(s) by privileged users with "Administer users" enabled. However, the privileged user is able to cancel/remove their own account even if they are in a protected role, making that an exception to the protection.

The subsequent patch in comment #23 was to address this exception using the drupal permission settings.

Now, the patch in comment #30 provides a different approach though similar to the original approach in patch #10 removing the test if current user and edited user are not the same and checking if the user role or the user account is protected and if so disables deleting the account. It also extends coverage to the direct url as described in comment #39. The patch in #30 appears to be well structured and in keeping with the module code structure.

Both the patches in #10 and #30 ignore whether the user is deleting their own account or that of others and therefore bypass the issue of drupal core permissions where "Administer users" overrides "Cancel own user account" and provide consistency to the purpose of the userprotect module. It may raise some GDPR issues but helps large organizations ensure accounts are not deleted inadvertently requiring a secondary step to remove accounts.

The patch in #30 appears to be the best approach although the changes to the README and help may need to be reviewed in light of the changes.