Index: modules/user/user.module =================================================================== RCS file: /cvs/drupal/drupal/modules/user/user.module,v retrieving revision 1.1021 diff -u -r1.1021 user.module --- modules/user/user.module 12 Aug 2009 12:36:05 -0000 1.1021 +++ modules/user/user.module 16 Aug 2009 20:24:41 -0000 @@ -1659,7 +1659,7 @@ /** * The final validation handler on the login form. - * + * * Sets a form error if user has not been authenticated, or if too many * logins have been attempted. This validation function should always * be the last one. @@ -2333,46 +2333,66 @@ function user_multiple_cancel_confirm(&$form_state) { $edit = $form_state['input']; - $form['accounts'] = array('#prefix' => '', '#tree' => TRUE); - // array_filter() returns only elements with TRUE values. - foreach (array_filter($edit['accounts']) as $uid => $value) { - $user = db_query('SELECT name FROM {users} WHERE uid = :uid', array(':uid' => $uid))->fetchField(); - $form['accounts'][$uid] = array('#type' => 'hidden', '#value' => $uid, '#prefix' => '
  • ', '#suffix' => check_plain($user) . "
  • \n"); - } - - $form['operation'] = array('#type' => 'hidden', '#value' => 'cancel'); - - module_load_include('inc', 'user', 'user.pages'); - $form['user_cancel_method'] = array( - '#type' => 'item', - '#title' => t('When cancelling these accounts'), - ); - $form['user_cancel_method'] += user_cancel_methods(); - // Remove method descriptions. - foreach (element_children($form['user_cancel_method']) as $element) { - unset($form['user_cancel_method'][$element]['#description']); - } + // Remove uid1 from the $accounts array so it can't be accidentally deleted. + if ($edit['accounts'] && array_key_exists(1, $edit['accounts'])) { + $user1 = user_load(1); + unset($edit['accounts'][1]); + drupal_set_message(t('You attempted to cancel %user1 which is the superuser account (ID #1). This might have broken your site, so that part of the request has been disallowed.', array('%user1' => check_plain($user1->name))), 'warning'); + } + + if (count($edit['accounts']) > 0) { + $form['accounts'] = array('#prefix' => '', '#tree' => TRUE); + // array_filter() returns only elements with TRUE values. + foreach (array_filter($edit['accounts']) as $uid => $value) { + $user = db_query('SELECT name FROM {users} WHERE uid = :uid', array(':uid' => $uid))->fetchField(); + $form['accounts'][$uid] = array('#type' => 'hidden', '#value' => $uid, '#prefix' => '
  • ', '#suffix' => check_plain($user) . "
  • \n"); + } + + $form['operation'] = array('#type' => 'hidden', '#value' => 'cancel'); + + module_load_include('inc', 'user', 'user.pages'); + $form['user_cancel_method'] = array( + '#type' => 'item', + '#title' => t('When cancelling these accounts'), + ); + $form['user_cancel_method'] += user_cancel_methods(); + // Remove method descriptions. + foreach (element_children($form['user_cancel_method']) as $element) { + unset($form['user_cancel_method'][$element]['#description']); + } - // Allow to send the account cancellation confirmation mail. - $form['user_cancel_confirm'] = array( - '#type' => 'checkbox', - '#title' => t('Require e-mail confirmation to cancel account.'), - '#default_value' => FALSE, - '#description' => t('When enabled, the user must confirm the account cancellation via e-mail.'), - ); - // Also allow to send account canceled notification mail, if enabled. - $form['user_cancel_notify'] = array( - '#type' => 'checkbox', - '#title' => t('Notify user when account is canceled.'), - '#default_value' => FALSE, - '#access' => variable_get('user_mail_status_canceled_notify', FALSE), - '#description' => t('When enabled, the user will receive an e-mail notification after the account has been cancelled.'), - ); + // Allow to send the account cancellation confirmation mail. + $form['user_cancel_confirm'] = array( + '#type' => 'checkbox', + '#title' => t('Require e-mail confirmation to cancel account.'), + '#default_value' => FALSE, + '#description' => t('When enabled, the user must confirm the account cancellation via e-mail.'), + ); + // Also allow to send account canceled notification mail, if enabled. + $form['user_cancel_notify'] = array( + '#type' => 'checkbox', + '#title' => t('Notify user when account is canceled.'), + '#default_value' => FALSE, + '#access' => variable_get('user_mail_status_canceled_notify', FALSE), + '#description' => t('When enabled, the user will receive an e-mail notification after the account has been cancelled.'), + ); - return confirm_form($form, - t('Are you sure you want to cancel these user accounts?'), - 'admin/people', t('This action cannot be undone.'), - t('Cancel accounts'), t('Cancel')); + return confirm_form($form, + t('Are you sure you want to cancel these user accounts?'), + 'admin/user/user', t('This action cannot be undone.'), + t('Cancel accounts'), t('Cancel')); + } + else { + $form = ''; + return confirm_form($form, + t('No valid user accounts to cancel'), + 'admin/user/user', + t('You have not selected any valid user accounts to cancel. Go back and try again.'), + t('Cancel'), + NULL, + t('Cancel') + ); + } } /** @@ -2386,6 +2406,10 @@ if ($form_state['values']['confirm']) { foreach ($form_state['values']['accounts'] as $uid => $value) { + // Prevent uid 1 from being deleted. + if ($uid <= 1) { + continue; + } // Prevent user administrators from deleting themselves without confirmation. if ($uid == $user->uid) { $admin_form_state = $form_state; Index: modules/user/user.pages.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/user/user.pages.inc,v retrieving revision 1.48 diff -u -r1.48 user.pages.inc --- modules/user/user.pages.inc 12 Aug 2009 12:36:05 -0000 1.48 +++ modules/user/user.pages.inc 16 Aug 2009 20:24:41 -0000 @@ -257,7 +257,7 @@ $form['_category'] = array('#type' => 'value', '#value' => $category); $form['_account'] = array('#type' => 'value', '#value' => $account); $form['submit'] = array('#type' => 'submit', '#value' => t('Save'), '#weight' => 30); - if (($account->uid == $user->uid && user_access('cancel account')) || user_access('administer users')) { + if ((($account->uid == $user->uid && user_access('cancel account')) || user_access('administer users')) && $account->uid > 1) { $form['cancel'] = array( '#type' => 'submit', '#value' => t('Cancel account'), Index: modules/user/user.test =================================================================== RCS file: /cvs/drupal/drupal/modules/user/user.test,v retrieving revision 1.54 diff -u -r1.54 user.test --- modules/user/user.test 12 Aug 2009 23:51:19 -0000 1.54 +++ modules/user/user.test 16 Aug 2009 20:24:41 -0000 @@ -107,7 +107,6 @@ } } - class UserValidationTestCase extends DrupalWebTestCase { public static function getInfo() { return array( @@ -286,38 +285,113 @@ } /** - * Attempt to cancel account without permission. + * Test for protection of user account #1 while logged as uid1. This should + * never be possible, or the site's owner could become unable to administer + * the site. + */ + function testUserCancelUser1() { + // Load user #1 from the database. + $user1 = user_load(1, TRUE); + // Make sure we know what is in user1's object. + $password = user_password(); + $account = array(); + $account['name'] = 'user1'; + $account['pass'] = $password; + require_once DRUPAL_ROOT . '/' . variable_get('password_inc', 'includes/password.inc'); + $account['pass'] = user_hash_password(trim($account['pass'])); + // Do not use user_save() here. + db_update('users') + ->fields($account) + ->condition('uid', 1) + ->execute(); + $user1 = user_load(1, TRUE); + $user1->pass_raw = $password; + $this->drupalLogin($user1); + // Prefill some form fields. + $edit = array(); + $edit['operation'] = 'cancel'; + $edit['accounts[1]'] = TRUE; + $this->drupalPost('admin/people', $edit, t('Update')); + $user1 = user_load(1, TRUE); + $this->assertRaw(t('You attempted to cancel %name which is the superuser account (ID #1)', array('%name' => $user1->name)), t('User #1 attempted to cancel user #1 account, but was successfully thwarted.')); + $this->assertTrue($user1->status == 1, t('User #1 still exists and is enabled.')); + } + + /** + * Use an administrative user to try to cancel a regular user. */ - function testUserCancelWithoutPermission() { + function testUserCancelByAdmin() { variable_set('user_cancel_method', 'user_cancel_reassign'); - // Create a user. + // Create a regular user. $account = $this->drupalCreateUser(array()); - $this->drupalLogin($account); - // Load real user object. - $account = user_load($account->uid, TRUE); - // Create a node. - $node = $this->drupalCreateNode(array('uid' => $account->uid)); + // Create administrative user. + $admin_user = $this->drupalCreateUser(array('administer users')); + $this->drupalLogin($admin_user); - // Attempt to cancel account. + // Delete regular user. $this->drupalGet('user/' . $account->uid . '/edit'); - $this->assertNoRaw(t('Cancel account'), t('No cancel account button displayed.')); + $this->drupalPost(NULL, NULL, t('Cancel account')); + $this->assertRaw(t('Are you sure you want to cancel the account %name?', array('%name' => $account->name)), t('Confirmation form to cancel account displayed.')); + $this->assertText(t('Select the method to cancel the account above.'), t('Allows to select account cancellation method.')); - // Attempt bogus account cancellation request confirmation. - $timestamp = $account->login; - $this->drupalGet("user/$account->uid/cancel/confirm/$timestamp/" . user_pass_rehash($account->pass, $timestamp, $account->login)); - $this->assertResponse(403, t('Bogus cancelling request rejected.')); - $account = user_load($account->uid); - $this->assertTrue($account->status == 1, t('User account was not canceled.')); + // Confirm deletion. + $this->drupalPost(NULL, NULL, t('Cancel account')); + $this->assertRaw(t('%name has been deleted.', array('%name' => $account->name)), t('User deleted.')); + $this->assertFalse(user_load($account->uid), t('User is not found in the database.')); + } - // Confirm user's content has not been altered. - $test_node = node_load($node->nid, NULL, TRUE); - $this->assertTrue(($test_node->uid == $account->uid && $test_node->status == 1), t('Node of the user has not been altered.')); + /** + * Use an administrative user to try and bulk-cancel regular users. + */ + function testBulkUserCancelByAdmin() { + variable_set('user_cancel_method', 'user_cancel_reassign'); + // Enable account cancellation notification. + variable_set('user_mail_status_canceled_notify', TRUE); + + // Create administrative user. + $admin_user = $this->drupalCreateUser(array('administer users')); + $this->drupalLogin($admin_user); + + // Create three regular users. + $users = array(); + for ($i = 0; $i < 3; $i++) { + $account = $this->drupalCreateUser(array()); + $users[$account->uid] = $account; + } + // Attempt to cancel newly created user accounts. + $edit = array(); + $edit['operation'] = 'cancel'; + foreach ($users as $uid => $account) { + if ($uid > 1) { + $edit['accounts[' . $uid . ']'] = TRUE; + } + } + $this->drupalPost('admin/people', $edit, t('Update')); + $this->assertText(t('Are you sure you want to cancel these user accounts?'), t('Confirmation form to cancel accounts displayed.')); + $this->assertText(t('When cancelling these accounts'), t('Allows to select account cancellation method.')); + $this->assertText(t('Require e-mail confirmation to cancel account.'), t('Allows to send confirmation mail.')); + + // Confirm deletion. + $this->drupalPost(NULL, NULL, t('Cancel accounts')); + $status = TRUE; + foreach ($users as $account) { + $status = $status && (strpos($this->content, t('%name has been deleted.', array('%name' => $account->name))) !== FALSE); + $status = $status && !user_load($account->uid, TRUE); + } + $this->assertTrue($status, t('Users deleted and not found in the database.')); + // $this->assertText(t('A confirmation request to cancel your account has been sent to your e-mail address.'), t('Account cancellation request mailed message displayed.')); + + // Ensure that neither the administrative account nor uid1 was cancelled. + $admin_user = user_load($admin_user->uid); + $this->assertTrue($admin_user->status == 1, t('The administrative user still exists and is enabled.')); + $uid1 = user_load(1); + $this->assertTrue($uid1->status == 1, t('User #1 still exists and is enabled.')); } /** - * Attempt invalid account cancellations. + * Attempt some invalid or bogus user account cancellations. */ function testUserCancelInvalid() { variable_set('user_cancel_method', 'user_cancel_reassign'); @@ -358,9 +432,40 @@ } /** - * Disable account and keep all content. + * Attempt a single user account cancellation without permission. + */ + function testUserCancelWithoutPermission() { + variable_set('user_cancel_method', 'user_cancel_reassign'); + + // Create a user. + $account = $this->drupalCreateUser(array()); + $this->drupalLogin($account); + // Load real user object. + $account = user_load($account->uid, TRUE); + + // Create a node. + $node = $this->drupalCreateNode(array('uid' => $account->uid)); + + // Attempt to cancel account. + $this->drupalGet('user/' . $account->uid . '/edit'); + $this->assertNoRaw(t('Cancel account'), t('Cancel account button is hidden if user does not have sufficient permission.')); + + // Attempt bogus account cancellation request confirmation. + $timestamp = $account->login; + $this->drupalGet("user/$account->uid/cancel/confirm/$timestamp/" . user_pass_rehash($account->pass, $timestamp, $account->login)); + $this->assertResponse(403, t('Bogus account cancellation request was rejected.')); + $account = user_load($account->uid); + $this->assertTrue($account->status == 1, t('User account was not canceled.')); + + // Confirm user's content has not been altered. + $test_node = node_load($node->nid, NULL, TRUE); + $this->assertTrue(($test_node->uid == $account->uid && $test_node->status == 1), t('Node of the user has not been altered.')); + } + + /** + * Attempt a single user account cancellation and keep all their content. */ - function testUserBlock() { + function testUserCancel() { variable_set('user_cancel_method', 'user_cancel_block'); // Create a user. @@ -393,9 +498,9 @@ } /** - * Disable account and unpublish all content. + * Attempt a single user account cancellation and unpublish all their content. */ - function testUserBlockUnpublish() { + function testUserCancelAndUnpublish() { variable_set('user_cancel_method', 'user_cancel_block_unpublish'); // Create a user. @@ -437,9 +542,9 @@ } /** - * Delete account and anonymize all content. + * Attempt a single user account cancellation and anonymize all their content. */ - function testUserAnonymize() { + function testUserCancelAndAnonymize() { variable_set('user_cancel_method', 'user_cancel_reassign'); // Create a user. @@ -488,9 +593,9 @@ } /** - * Delete account and remove all content. + * Attempt a single user account cancellation and delete all their content. */ - function testUserDelete() { + function testUserCancelAndDelete() { variable_set('user_cancel_method', 'user_cancel_delete'); // Create a user. @@ -540,78 +645,6 @@ // Confirm that user is logged out. $this->assertNoText($account->name, t('Logged out.')); } - - /** - * Create an administrative user and delete another user. - */ - function testUserCancelByAdmin() { - variable_set('user_cancel_method', 'user_cancel_reassign'); - - // Create a regular user. - $account = $this->drupalCreateUser(array()); - - // Create administrative user. - $admin_user = $this->drupalCreateUser(array('administer users')); - $this->drupalLogin($admin_user); - - // Delete regular user. - $this->drupalGet('user/' . $account->uid . '/edit'); - $this->drupalPost(NULL, NULL, t('Cancel account')); - $this->assertRaw(t('Are you sure you want to cancel the account %name?', array('%name' => $account->name)), t('Confirmation form to cancel account displayed.')); - $this->assertText(t('Select the method to cancel the account above.'), t('Allows to select account cancellation method.')); - - // Confirm deletion. - $this->drupalPost(NULL, NULL, t('Cancel account')); - $this->assertRaw(t('%name has been deleted.', array('%name' => $account->name)), t('User deleted.')); - $this->assertFalse(user_load($account->uid), t('User is not found in the database.')); - } - - /** - * Create an administrative user and mass-delete other users. - */ - function testMassUserCancelByAdmin() { - variable_set('user_cancel_method', 'user_cancel_reassign'); - // Enable account cancellation notification. - variable_set('user_mail_status_canceled_notify', TRUE); - - // Create administrative user. - $admin_user = $this->drupalCreateUser(array('administer users')); - $this->drupalLogin($admin_user); - - // Create some users. - $users = array(); - for ($i = 0; $i < 3; $i++) { - $account = $this->drupalCreateUser(array()); - $users[$account->uid] = $account; - } - - // Cancel user accounts, including own one. - $edit = array(); - $edit['operation'] = 'cancel'; - foreach ($users as $uid => $account) { - $edit['accounts[' . $uid . ']'] = TRUE; - } - $edit['accounts[' . $admin_user->uid . ']'] = TRUE; - $this->drupalPost('admin/people', $edit, t('Update')); - $this->assertText(t('Are you sure you want to cancel these user accounts?'), t('Confirmation form to cancel accounts displayed.')); - $this->assertText(t('When cancelling these accounts'), t('Allows to select account cancellation method.')); - $this->assertText(t('Require e-mail confirmation to cancel account.'), t('Allows to send confirmation mail.')); - $this->assertText(t('Notify user when account is canceled.'), t('Allows to send notification mail.')); - - // Confirm deletion. - $this->drupalPost(NULL, NULL, t('Cancel accounts')); - $status = TRUE; - foreach ($users as $account) { - $status = $status && (strpos($this->content, t('%name has been deleted.', array('%name' => $account->name))) !== FALSE); - $status = $status && !user_load($account->uid, TRUE); - } - $this->assertTrue($status, t('Users deleted and not found in the database.')); - - // Ensure that admin account was not cancelled. - $this->assertText(t('A confirmation request to cancel your account has been sent to your e-mail address.'), t('Account cancellation request mailed message displayed.')); - $admin_user = user_load($admin_user->uid); - $this->assertTrue($admin_user->status == 1, t('Administrative user is found in the database and enabled.')); - } } class UserPictureTestCase extends DrupalWebTestCase { Index: modules/user/user.admin.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/user/user.admin.inc,v retrieving revision 1.67 diff -u -r1.67 user.admin.inc --- modules/user/user.admin.inc 11 Aug 2009 11:47:58 -0000 1.67 +++ modules/user/user.admin.inc 16 Aug 2009 20:24:41 -0000 @@ -126,7 +126,6 @@ * @see user_admin_account_submit() */ function user_admin_account() { - $header = array( array(), array('data' => t('Username'), 'field' => 'u.name'), @@ -195,7 +194,7 @@ } $form['accounts'] = array( '#type' => 'checkboxes', - '#options' => $accounts + '#options' => $accounts, ); $form['pager'] = array('#markup' => theme('pager', NULL));