Any admin who can admin user accounts can delete the account with uid == 1. That seems like a very bad idea.

If there is a good reason for allowing this, let me know. Otherwise, I will generate a patch to disallow it.

CommentFileSizeAuthor
#4 user_admin_account_delete_0.patch1.41 KBBèr Kessels
#2 user_admin_account_delete.patch1.08 KBBèr Kessels
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JonBob’s picture

JonBob CreditAttribution: JonBob commented

I'm not sure; someone with database access can always recreate the user, so it wouldn't be tragic if uid 1 were accidentally deleted. Nevertheless, I think I'm leaning toward disallowing it.

Bèr Kessels’s picture

Bèr Kessels CreditAttribution: Bèr Kessels commented
FileSize
user_admin_account_delete.patch1.08 KB

Because the patch was marked "critical" I modify that.
the idea that UID 1 can also access the database is not per se correct. I, for example, deliver my clients a drupal installation where one person in the corp has uid 1, not me, but they might not know anything about SQL queries and the like.

The patch does not hide the submit button, because of consistance of the UI. It does check after posting, and presents the user (UID1) with an error message, if trying to delete.

This patch is also important because not only UID 1 can remove accounts, but also other users with the permission administer users. he/she can play a nasty trick by removing the root admin :)

Dries’s picture

Dries CreditAttribution: Dries commented

I would change the order of the if statements. That simplifies the code as you don't need to check against user ID 1 twice. Also, the spacing is incorrect.

Bèr Kessels’s picture

Bèr Kessels CreditAttribution: Bèr Kessels commented
FileSize
user_admin_account_delete_0.patch1.41 KB

Sorry about that, the spacing is fixed and the logic for chechking modified the way you suggest.
Its good that we have people around who do pay attention :) Thanks, one day i might even become a proper coder.

Dries’s picture

Dries CreditAttribution: Dries commented

I'm not sure this is a bug. It could be a feature. Besides, people who can administer users, can also execute PHP code, inject XSS exploits, etc.

Uwe Hermann’s picture

Uwe Hermann CreditAttribution: Uwe Hermann commented

I suggest to at least display a warning like "You're about to delete the admin user." or something.
If you want to leave the "feature" intact, you can allow deletion of the admin account, but IMHO users should be warned about it.