Problem/Motivation
This is approved as a public security hardening in #25.
Access to modify a users role is handled inconsistently. The user access control handler incorrectly allows users with administer users to modify roles. The account form correctly requires administer permissions to edit roles of a user since it is implementing its own form elements.
The permission administer permissions is meant to be used to assign roles to users, CRUD roles, and to edit permissions grants for each role. Not administer users.
Drupal 7
In Drupal 7, when you go to a users page, you can not edit the roles of the user. Nor can you use the bulk "[Assign/Remove] a role from the selected users" operation on admin/people. Unless you have administer permissions.
Drupal 8
In Drupal 8, if you do not have administer permissions, you also cannot edit the roles when you are on the /user/%/edit page.
However if you have administer users, you can go to /admin/people and use the 'add the XXXX role to the selected users' actions.
This is a problem, because the user with administer users can assign himself the built-in 'Administrator' role. Which gives him all permissions, including the ability to edit permissions.
This effectively makes administer permissions useless.
Proposed resolution
- Disallow users without
administer permissions to edit the user roles in the user edit form.
- Add a mechanism to determine if the current user is allowed to use a specific action. Note that the actual action plugin
::access() method is only a run-time check, checking if the action can be done by the current user, on a specific object/entity. But we need more generic check to see if a user is able to read/access an action before the action starts to be executed. The method could be used, mainly, on views with bulk operation form, where the list of actions should be limited to the actions that the current user is allowed to access.
- Use the above check to filter out the add/remove role operations from the bulk operations form on
/admin/people view.
Remaining tasks
None.
User interface changes
Users without administer permissions permission will not see anymore:
- The roles field on user account edit form.
- The add/remove role actions, in the bulk operations selector, on user administrative page, at
/admin/people
API changes
- The action entity defines now its own access control handler in the annotation, pointing to:
\Drupal\system\ActionAccessControlHandler
ActionAccessControlHandler::checkAccess() will grant the access to the action, first, if the current user has administer actions permission. If not, will ask the plugin to make a decision.- New method
::userAccess() in the plugin ActionInterface interface.
ActionBase::userAccess() will return TRUE. In this way we ensure the Backward Compatibility- Implement
ChangeUserRoleBase::userAccess() by checking the user permission.
- In
\Drupal\views\Plugin\views\field\BulkForm, limit the bulk operation action options based on action entity ::access().
Data model changes
None.
Original report by kevin.dutra
Access to the roles field on the User entity is not handled consistently. This gap has been made possible due to the sort of non-standard approach that the AccountForm takes by providing form elements for fields directly in the form builder instead of using the field system to handle form display with normal widgets. Since it's providing the form elements, it's also setting the #access property, but not in a manner that is consistent with the actual field access. As a result, there is a disconnect between what can be done via the UI and via the REST API for the same user.
The AccountForm only allows access to the roles field for a user with administer permissions.
$form['account']['roles'] = array(
'#type' => 'checkboxes',
'#title' => $this->t('Roles'),
'#default_value' => (!$register ? $account->getRoles() : array()),
'#options' => $roles,
'#access' => $roles && $user->hasPermission('administer permissions'),
);
However, the UserAccessControlHandler allows anyone with administer users access to the roles field.
// Administrative users are allowed to edit and view all fields.
if (!in_array($field_definition->getName(), $explicit_check_fields) && $account->hasPermission('administer users')) {
return AccessResult::allowed()->cachePerPermissions();
}
Left to be done
- Decide on who should actually have access to the field
- Create a patch