diff --git a/core/modules/user/src/AccountForm.php b/core/modules/user/src/AccountForm.php index d695968..5c03018 100644 --- a/core/modules/user/src/AccountForm.php +++ b/core/modules/user/src/AccountForm.php @@ -69,7 +69,15 @@ public function form(array $form, FormStateInterface $form_state) { $language_interface = \Drupal::languageManager()->getCurrentLanguage(); $register = $account->isAnonymous(); - $admin = $user->hasPermission('administer users'); + + // This form is used for two cases: a user editing their own account or an + // admin editing another user's account. Check for the admin case by + // comparing the user IDs. If a user has 'administer users' permission + // then they count as an admin even when editing their own account. + // Note that the Entity framework checks access before calling this + // function to ensure that a user can only admin another user if they have + // core or contrib permissions to allow it. + $manage_other_user = $user->hasPermission('administer users') || ($user->id() != $account->id()); // Account information. $form['account'] = [ @@ -78,14 +86,14 @@ public function form(array $form, FormStateInterface $form_state) { ]; // The mail field is NOT required if account originally had no mail set - // and the user performing the edit has 'administer users' permission. + // and the user is managing another user's account. // This allows users without email address to be edited and deleted. // Also see \Drupal\user\Plugin\Validation\Constraint\UserMailRequired. $form['account']['mail'] = [ '#type' => 'email', '#title' => $this->t('Email address'), '#description' => $this->t('A valid email address. All emails from the system will be sent to this address. The email address is not made public and will only be used if you wish to receive a new password or wish to receive certain news or notifications by email.'), - '#required' => !(!$account->getEmail() && $admin), + '#required' => !(!$account->getEmail() && $manage_other_user), '#default_value' => (!$register ? $account->getEmail() : ''), ]; @@ -103,7 +111,7 @@ public function form(array $form, FormStateInterface $form_state) { 'spellcheck' => 'false', ], '#default_value' => (!$register ? $account->getAccountName() : ''), - '#access' => ($register || ($user->id() == $account->id() && $user->hasPermission('change own username')) || $admin), + '#access' => ($register || ($user->id() == $account->id() && $user->hasPermission('change own username')) || $manage_other_user), ]; // Display password field only for existing users or when user is allowed to @@ -150,7 +158,7 @@ public function form(array $form, FormStateInterface $form_state) { } } } - elseif (!$config->get('verify_mail') || $admin) { + elseif (!$config->get('verify_mail') || $manage_other_user) { $form['account']['pass'] = [ '#type' => 'password_confirm', '#size' => 25, @@ -169,7 +177,7 @@ public function form(array $form, FormStateInterface $form_state) { } } - if ($admin || !$register) { + if ($manage_other_user || !$register) { $status = $account->get('status')->value; } else { @@ -181,7 +189,7 @@ public function form(array $form, FormStateInterface $form_state) { '#title' => $this->t('Status'), '#default_value' => $status, '#options' => [$this->t('Blocked'), $this->t('Active')], - '#access' => $admin, + '#access' => $manage_other_user, ]; $roles = array_map(['\Drupal\Component\Utility\Html', 'escape'], user_role_names(TRUE)); @@ -203,7 +211,7 @@ public function form(array $form, FormStateInterface $form_state) { $form['account']['notify'] = [ '#type' => 'checkbox', '#title' => $this->t('Notify user of new account'), - '#access' => $register && $admin, + '#access' => $register && $manage_other_user, ]; $user_preferred_langcode = $register ? $language_interface->getId() : $account->getPreferredLangcode(); @@ -222,7 +230,7 @@ public function form(array $form, FormStateInterface $form_state) { '#open' => TRUE, // Display language selector when either creating a user on the admin // interface or editing a user account. - '#access' => !$register || $admin, + '#access' => !$register || $manage_other_user, ]; $form['language']['preferred_langcode'] = [ diff --git a/core/modules/user/src/Form/UserCancelForm.php b/core/modules/user/src/Form/UserCancelForm.php index da7a5f4..35b96eb 100644 --- a/core/modules/user/src/Form/UserCancelForm.php +++ b/core/modules/user/src/Form/UserCancelForm.php @@ -49,7 +49,8 @@ public function getCancelUrl() { public function getDescription() { $description = ''; $default_method = $this->config('user.settings')->get('cancel_method'); - if ($this->currentUser()->hasPermission('administer users') || $this->currentUser()->hasPermission('select account cancellation method')) { + $manage_other_user = $this->currentUser()->hasPermission('administer users') || ($this->entity->id() != $this->currentUser()->id()); + if ($manage_other_user || $this->currentUser()->hasPermission('select account cancellation method')) { $description = $this->t('Select the method to cancel the account above.'); } // Options supplied via user_cancel_methods() can have a custom @@ -75,18 +76,17 @@ public function buildForm(array $form, FormStateInterface $form_state) { $this->cancelMethods = user_cancel_methods(); // Display account cancellation method selection, if allowed. - $admin_access = $user->hasPermission('administer users'); + $manage_other_user = $user->hasPermission('administer users') || ($this->entity->id() != $user->id()); $form['user_cancel_method'] = [ '#type' => 'radios', '#title' => ($this->entity->id() == $user->id() ? $this->t('When cancelling your account') : $this->t('When cancelling the account')), - '#access' => $admin_access || $user->hasPermission('select account cancellation method'), + '#access' => $manage_other_user || $user->hasPermission('select account cancellation method'), ]; $form['user_cancel_method'] += $this->cancelMethods; - // Allow user administrators to skip the account cancellation confirmation - // mail (by default), as long as they do not attempt to cancel their own - // account. - $override_access = $admin_access && ($this->entity->id() != $user->id()); + // When managing another user, can skip the account cancellation + // confirmation mail (by default). + $override_access = $this->entity->id() != $user->id(); $form['user_cancel_confirm'] = [ '#type' => 'checkbox', '#title' => $this->t('Require email confirmation to cancel account'), @@ -111,7 +111,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { // if desired. $form['access'] = [ '#type' => 'value', - '#value' => $user->hasPermission('administer users'), + '#value' => $manage_other_user, ]; $form = parent::buildForm($form, $form_state); diff --git a/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php b/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php index b500a8d..2773638 100644 --- a/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php +++ b/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php @@ -8,7 +8,7 @@ * Checks if the user's email address is provided if required. * * The user mail field is NOT required if account originally had no mail set - * and the user performing the edit has 'administer users' permission. + * and the user is managing another user's account. * This allows users without email address to be edited and deleted. * * @Constraint( diff --git a/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequiredValidator.php b/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequiredValidator.php index 7e20b7a..030edcc 100644 --- a/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequiredValidator.php +++ b/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequiredValidator.php @@ -9,7 +9,7 @@ * Checks if the user's email address is provided if required. * * The user mail field is NOT required if account originally had no mail set - * and the user performing the edit has 'administer users' permission. + * and the user is managing another user's account. * This allows users without email address to be edited and deleted. */ class UserMailRequiredValidator extends ConstraintValidator { @@ -29,7 +29,8 @@ public function validate($items, Constraint $constraint) { $existing_value = $account_unchanged->getEmail(); } - $required = !(!$existing_value && \Drupal::currentUser()->hasPermission('administer users')); + $manage_other_user = \Drupal::currentUser()->hasPermission('administer users') || ($account->id() != \Drupal::currentUser()->id()); + $required = !(!$existing_value && $manage_other_user); if ($required && (!isset($items) || $items->isEmpty())) { $this->context->addViolation($constraint->message, ['@name' => $account->getFieldDefinition('mail')->getLabel()]); diff --git a/core/modules/user/src/ProfileForm.php b/core/modules/user/src/ProfileForm.php index c856b75..7f35227 100644 --- a/core/modules/user/src/ProfileForm.php +++ b/core/modules/user/src/ProfileForm.php @@ -25,7 +25,7 @@ protected function actions(array $form, FormStateInterface $form_state) { $element['delete']['#type'] = 'submit'; $element['delete']['#value'] = $this->t('Cancel account'); $element['delete']['#submit'] = ['::editCancelSubmit']; - $element['delete']['#access'] = $account->id() > 1 && (($account->id() == $user->id() && $user->hasPermission('cancel account')) || $user->hasPermission('administer users')); + $element['delete']['#access'] = $account->access('delete', $user); return $element; } diff --git a/core/modules/user/src/RegisterForm.php b/core/modules/user/src/RegisterForm.php index 4974e69..df6b398 100644 --- a/core/modules/user/src/RegisterForm.php +++ b/core/modules/user/src/RegisterForm.php @@ -18,28 +18,37 @@ public function form(array $form, FormStateInterface $form_state) { $user = $this->currentUser(); /** @var \Drupal\user\UserInterface $account */ $account = $this->entity; - $admin = $user->hasPermission('administer users'); + + // This form is used for two cases: a new user registering their own + // account or an admin creating another user's account. The registration + // case is only accessible to anonymous users. + // Note that the Entity framework checks access before calling this + // function to ensure that a user can only create another user if they have + // core or contrib permissions to allow it. + $manage_other_user = !$user->isAnonymous(); + // Pass access information to the submit handler. Running an access check // inside the submit function interferes with form processing and breaks // hook_form_alter(). $form['administer_users'] = [ '#type' => 'value', - '#value' => $admin, + '#value' => $manage_other_user, ]; $form['#attached']['library'][] = 'core/drupal.form'; - // For non-admin users, populate the form fields using data from the + // For users editing their own account, populate the form fields using data from the // browser. - if (!$admin) { + if (!$manage_other_user) { $form['#attributes']['data-user-info-from-browser'] = TRUE; } // Because the user status has security implications, users are blocked by // default when created programmatically and need to be actively activated - // if needed. When administrators create users from the user interface, - // however, we assume that they should be created as activated by default. - if ($admin) { + // if needed. When a privileged user is creating another user from the user + // interface, however, we assume that they should be created as activated + // by default. + if ($manage_other_user) { $account->activate(); } @@ -62,9 +71,9 @@ protected function actions(array $form, FormStateInterface $form_state) { * {@inheritdoc} */ public function submitForm(array &$form, FormStateInterface $form_state) { - $admin = $form_state->getValue('administer_users'); + $manage_other_user = $form_state->getValue('administer_users'); - if (!\Drupal::config('user.settings')->get('verify_mail') || $admin) { + if (!\Drupal::config('user.settings')->get('verify_mail') || $manage_other_user) { $pass = $form_state->getValue('pass'); } else { @@ -86,7 +95,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) { public function save(array $form, FormStateInterface $form_state) { $account = $this->entity; $pass = $account->getPassword(); - $admin = $form_state->getValue('administer_users'); + $manage_other_user = $form_state->getValue('administer_users'); $notify = !$form_state->isValueEmpty('notify'); // Save has no return value so this cannot be tested. @@ -102,11 +111,11 @@ public function save(array $form, FormStateInterface $form_state) { $account->password = $pass; // New administrative account without notification. - if ($admin && !$notify) { + if ($manage_other_user && !$notify) { drupal_set_message($this->t('Created a new user account for %name. No email has been sent.', [':url' => $account->url(), '%name' => $account->getUsername()])); } // No email verification required; log in user immediately. - elseif (!$admin && !\Drupal::config('user.settings')->get('verify_mail') && $account->isActive()) { + elseif (!$manage_other_user && !\Drupal::config('user.settings')->get('verify_mail') && $account->isActive()) { _user_mail_notify('register_no_approval_required', $account); user_login_finalize($account); drupal_set_message($this->t('Registration successful. You are now logged in.')); diff --git a/core/modules/user/src/Tests/UserSubAdminTest.php b/core/modules/user/src/Tests/UserSubAdminTest.php new file mode 100644 index 0000000..99e980e --- /dev/null +++ b/core/modules/user/src/Tests/UserSubAdminTest.php @@ -0,0 +1,57 @@ +createUser(['sub-admin']); + $this->drupalLogin($user); + + // Test that the create user page has admin fields but not roles. + $this->drupalGet('admin/people/create'); + $this->assertField("edit-name", "Name field exists."); + $this->assertField("edit-status-0", "Status field exists."); + $this->assertField("edit-notify", "Notify field exists."); + $this->assertNoField("edit-role", "Role field missing."); + + // Test that create user gives an admin style message. + $edit = [ + 'name' => $this->randomMachineName(), + 'mail' => $this->randomMachineName() . '@example.com', + 'pass[pass1]' => $pass = $this->randomString(), + 'pass[pass2]' => $pass, + 'notify' => FALSE, + ]; + $this->drupalPostForm('admin/people/create', $edit, t('Create new account')); + $this->assertText(t('Created a new user account for @name. No email has been sent.', ['@name' => $edit['name']]), 'User created'); + + // Test that the cancel user page has admin fields. + $cancel_user = $this->createUser(); + $this->drupalGet('user/' . $cancel_user->id() . '/cancel'); + $this->assertRaw(t('Are you sure you want to cancel the account %name?', ['%name' => $cancel_user->getUsername()]), 'Confirmation form to cancel account displayed.'); + $this->assertText(t('Select the method to cancel the account above.'), 'Allows to select account cancellation method.'); + + // Test that cancel confirmation gives an admin style message. + $this->drupalPostForm(NULL, NULL, t('Cancel account')); + $this->assertRaw(t('%name has been disabled.', ['%name' => $cancel_user->getUsername()]), "Confirmation message displayed to user."); + } + +} diff --git a/core/modules/user/tests/modules/user_access_test/src/Routing/RouteTestSubscriber.php b/core/modules/user/tests/modules/user_access_test/src/Routing/RouteTestSubscriber.php new file mode 100644 index 0000000..3752b44 --- /dev/null +++ b/core/modules/user/tests/modules/user_access_test/src/Routing/RouteTestSubscriber.php @@ -0,0 +1,24 @@ +get('user.admin_create')) { + $perm = $route->getRequirement('_permission') . '+sub-admin'; + $route->setRequirement('_permission', $perm); + } + } + +} diff --git a/core/modules/user/tests/modules/user_access_test/user_access_test.module b/core/modules/user/tests/modules/user_access_test/user_access_test.module index 470a76a..0f1ef99 100644 --- a/core/modules/user/tests/modules/user_access_test/user_access_test.module +++ b/core/modules/user/tests/modules/user_access_test/user_access_test.module @@ -20,5 +20,11 @@ function user_access_test_user_access(User $entity, $operation, $account) { // Deny delete access. return AccessResult::forbidden(); } + + // Account with role sub-admin can manage users with no roles. + if (count($entity->getRoles()) == 1) { + return AccessResult::allowedIfHasPermission($account, 'sub-admin'); + } + return AccessResult::neutral(); } diff --git a/core/modules/user/tests/modules/user_access_test/user_access_test.permissions.yml b/core/modules/user/tests/modules/user_access_test/user_access_test.permissions.yml new file mode 100644 index 0000000..dadf7ba --- /dev/null +++ b/core/modules/user/tests/modules/user_access_test/user_access_test.permissions.yml @@ -0,0 +1,2 @@ +sub-admin: + title: 'Administer users with no roles' diff --git a/core/modules/user/tests/modules/user_access_test/user_access_test.services.yml b/core/modules/user/tests/modules/user_access_test/user_access_test.services.yml new file mode 100644 index 0000000..1c873bd --- /dev/null +++ b/core/modules/user/tests/modules/user_access_test/user_access_test.services.yml @@ -0,0 +1,5 @@ +services: + user_access_test.route_subscriber: + class: Drupal\user_access_test\Routing\RouteTestSubscriber + tags: + - { name: event_subscriber }