Allow entity access to accept multiple operations

Actually what would be more flexible would be for the 'access callback' to accept an array of callbacks. All callbacks would have to return TRUE for access to be granted. This would probably turn the access arguments item into an array of arrays. Or we combine access callback and access arguments into one associative array.

Anyway, the idea proposed here is useful too.

I think this patch would be very handy. Plus it doesn't break compatibility with any previous code.

Moving to Drupal 8.

+1. I have a utility function that I often use as a menu access callback that does exactly this. Having it as the default user_access() behavior would be great.

I disagree with #1; a menu entry would not always need "all" permissions callbacks, maybe sometimes "any" is enough.

An option would be adding a new parameter to function:
function user_access($permission, $account = NULL, $reset = FALSE, $all = TRUE) {...}

Patch ported to D8

+++ b/modules/user/user.module
@@ -809,7 +813,14 @@ function user_access($string, $account = NULL) {
+      $access = !isset($access) ? user_access($string, $account, $reset) : $access && user_access($string, $account, $reset);

why do you call user_access() recursively? The access information should be in $perm already, no?

And we need to clearly document the policy on multiple permissions given: is one of them enough (OR) or must all permissions be satisfied (AND)?

8 days to next Drupal core point release.

You're totally right, I could have used the $perm array, I've modified a little the patch to avoid the recursive call.

I think that AND makes more sense from a code perspective but if you search in the core code, OR would be more used (user_access() || user_access()).

+++ b/modules/user/user.module
@@ -767,10 +767,14 @@ function user_role_permissions($roles = array()) {
+ * @param $reset
+ *   Reset user permissions static cache.

you introduce a new parameter that is never used in the function?

+++ b/modules/user/user.module
@@ -809,7 +813,14 @@ function user_access($string, $account = NULL) {
+  if (is_array($permission) ) {
+    foreach ($permission as $string) {
+      $access = !isset($access) ? isset($perm[$account->uid][$string]) : $access && isset($perm[$account->uid][$string]);
+    }
+    return $access;
+  }

so this is the AND use case I guess? why do you continue to check permissions if a particular permission isn't satisfied then?

Still no documentation that says that all permissions have to be met (AND).

I would do it like this, patch attached.

I have removed the change in menu.inc, as I think people may want to pass in the $account object there.

I'm not sure if the current AND solution is the best, what is typically use more often? Is OR more common?

Yeah, my idea was to find a solution that we could quickly switch from AND to OR (or even pass a parameter to change it).

AND makes more sense b/c we can exit quicker if one of the permissions is false, but I don't think it is very commonly used in core, I've found a couple of use cases for OR, though.

We could change the $permissions array for something like this:

array(
'permissions' => array('Permission 1', 'Permission 2'),
'operator' => 'AND',
);

We'd also need to create a test case.

Without the modification of menu.inc for the access arguments, the call to user_access is done with $argument[0] and $argument[1], which both are permissions and not the account as expected.

Example of hook_menu.

/**
 * Implements hook_menu().
 */
function test_menu() {
  $items['test_path'] = array(
    'title' => 'Test',
    'type' => MENU_CALLBACK,
    'page callback' => 'test_callback',
    'access arguments' => array('Permission 1', 'Permission 2'),
  );
  return $items;
}

I get a bunch of these errors:

Notice: Trying to get property of non-object in user_access() (line 794 of /webs/contrib/drupal8/modules/user/user.module).

You have to wrap the permissions in an additional array:

'access arguments' => array(array('Permission 1', 'Permission 2')),

The access arguments are taken form the outer array and are passed one by one to the access callback function (user_access() in the default case). ==> we need an additional array to wrap multiple permissions.

Wouldn't it make sense then to have the structure above to define the access arguments? I feel like defining array(array('Permission 1', 'Permission 2')) is not straight away since we need to use 0 and 1 keys for permissions & accounts.

Modifying menu.inc to get $argument['permission'], etc(instead of 0 and 1) and defining hook_menu like this:

/**
 * Implements hook_menu().
 */
function test_menu() {
  $items['test_path'] = array(
    'title' => 'Test',
    'type' => MENU_CALLBACK,
    'page callback' => 'test_callback',
    'access arguments' => array(
      'permissions' => array('Permission 1', 'Permission 2'),
      'operator' => 'AND',
      'acccount' => $account,
    ),
  );
  return $items;
}

Then we could implement the OR way for defining permissions also.

Here's the patch modified to get sensitive keys

+++ b/includes/menu.inc
@@ -616,7 +616,7 @@ function _menu_check_access(&$item, $map) {
+      $item['access'] = (count($arguments) == 1) ? user_access(reset($arguments)) : user_access($arguments['permission'], $arguments['account']);

This appears to change the way 'access arguments' works, but doesn't change its documentation.

26 days to next Drupal core point release.

It is no longer such an issue since we do not use a user_access in routing anymore. The _permission requirement also accepts AND,OR conjunctions with + and ,. See \Drupal\user\Access\PermissionAccessCheck::access

The issue of allowing multiple operations is mitigated by:

// Object combiners (for any entity type):
$entity->access('operation')->andIf($entity->access('operation2');
// For current user permission:
AccessResult::allowedIfHasPermissions();

And lastly, changing the user_access equivalent, $entity->access, to accept an array of operations would be an API change.

Changed title to Drupal 8 terminology