Users should not be able to easily to block their own account (i.e., without email confirmation)

Attached patch disables the status options for the active user for his/her own account.

I think that expression ? TRUE : FALSE is a little redundant. Is there some code standard for this? If not, it can be simplified to '#disabled' => $user->uid == $account->uid.

I did a grep search for $user->uid == $account->uid, its commonly used throughout drupal.

New patch implements suggestion #1.

Here's the right patch.

Great! +1 for RTBC in #4.

Related issue from the days before one could cancel own account - #119683: User administrators should not be able to delete their own accounts

I can't think of any reason a user would need/want to block their own account, and this patch applies cleanly and works as intended.

I think this makes sense, but should come with tests.

This patch makes the change in #4 and also adds a test to make sure the Block account radio button is disabled. This is my first attempt at writing a test so please bear with me if I've made some horrible mistakes (that and the crazy hour that I'm awake trying to do this)!

:)

From http://api.drupal.org/api/drupal/developer--topics--forms_api_reference....

Note that disabling a form field doesn't necessarily prevent someone from submitting a value through DOM manipulation.

#disabled is not correct here. We should merge the condition into #access.

New patch attached

Tagging for D7 backport

That looks perfectly reasonable, and bot is happy.

+++ b/modules/user/user.test
@@ -1268,6 +1268,12 @@ class UserAdminTestCase extends DrupalWebTestCase {
+    // Test that a user cannot block their own account (See #920250)

We don't usually include issue numbers, unless the issue contains in-depth discussions and background information which is too much to include in-code. In this code, we can just drop it, I think. Also there should be a period at the end of the sentence.

Leaving at RTBC and marking as "Security improvement".

-3 days to next Drupal core point release.

Darn you Dreditor! :)
Anyway, quick reroll for the comment fix I mentioned above.

And yes, I'm marking my own patch RTBC, because I only touched one line of comments and in a very trivial way. Feel free to mark "needs review", I won't fight it.

Oops, this was pre-/core...

Still RTBC if it's green.

Sorry for the noise, this one should really work.

Thanks for reroll. Looks good.

I'd argue that the Test is not needed. The #access system is tested elsewhere. Browser based tests like these slow our suite down a little bit so there should be tangible benefit from each. I think the committer can choose to omit that hunk if they agree. I'm OK either way.

+++ b/core/modules/user/user.test
@@ -1268,6 +1268,12 @@ class UserAdminTestCase extends DrupalWebTestCase {
+    $element = $this->xpath('//input[@id="edit-status-0"]');
+    $this->assertEqual(count($element), 0, t('Radio button to block own account was not found.'));

Usage of assertNoFieldByName() would be more appropriate here.

Thanks @moshe weitzman, @sun, @tstoeckler. Improving the assertion as sun suggests in #23 would make a good novice task, so marking needs work and tagging for that. Thanks!

new patch using sun's suggestion above.

+++ b/core/modules/user/user.test
@@ -1268,6 +1268,11 @@ class UserAdminTestCase extends DrupalWebTestCase {
+    ¶

There is trailing whitespace here.

-10 days to next Drupal core point release.

Removed trailing whitespace.

Thanks @benjf and @Devin Carlson!

Committed/pushed to 8.x. Moving back to 7.x for backport.

D7 backport.

Yeah! Thanks!

This issue title is a little misleading... Users should definitely be able to block their own accounts, and they can still do it both before and after this patch. That's one of the main use cases of the "Cancel account" button.

So, I'm changing the issue title to what I think was the actual intent here. I don't see how this is a security improvement, actually. It's more of a usability issue.

Given that, we should discuss whether this patch was complete? It addressed the user edit form, but not the bulk edit form at admin/people. I think that's inconsistent; it's just as easy (and maybe even easier) to accidentally block yourself there. If we stop it in one place, we should stop it in the other too. That's consistent with what we do for cancelling the user 1 account also (it's prevented in both places).

And:

-    '#access' => $admin,
+    '#access' => $admin && ($user->uid != $account->uid),

It's pretty minor, but we should add a !empty($account->uid) check there too. Yes, the use case is close to zero.... but there are setups where granting anonymous users "administer users" permission actually does make sense, so we shouldn't prevent them from creating new accounts with whatever status they want in that case.

It's pretty minor, but we should add a !empty($account->uid) check there too.

To clarify what I meant above, I guess what I'm actually saying is that it should be this:

'#access' => $admin && (empty($account->uid) || $user->uid != $account->uid),

***

By the way, a somewhat related issue to this one is here: #472074: Don't let the all administrator accounts get blocked

Hmm, that makes sense. I've reverted the commit to 8.x so we can discuss this more.

This seems like a pretty trivial thing that's been bugging me since D6, but there's no movement (not here neither in #472074: Don't let the all administrator accounts get blocked). Can we come to an agreement and get on with this?

It's mildly humorous that this is still an issue.

For reference, since the commits did not contain issue ID's:

Patch was originally committed to 8.x in #920250-29: Users should not be able to easily to block their own account (i.e., without email confirmation), Ref #1ab05706

Then the patch was reverted in #920250-34: Users should not be able to easily to block their own account (i.e., without email confirmation), Ref #e3a6372c

Patch #27 does not apply. Here is a new patch if we need this.

This came up as a Bug Smash Initiative daily triage target.

Discussed this in the #bugsmash channel with @catch and we felt this should be seen as a feature. There is a clear benefit to user 1 not being able to block user 1, the wider scope might not be something that is wanted on every install. Judging by the lack of activity, this is not something that is relevant for a lot of sites.

As always, feel free to move this back if you believe this does feel like a bug.

I've added reroll of the patch #39 on Drupal 9.5.x., This issue still exists.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.