In EntityAccessManager->access(), the current implementation assumes that access should be granted if any requirement is met (OR logic).

Some sites may want to configure it to pass only if all requirements are met (AND logic).

Attached patch provides a lightweight way to do via config, ex:

// in settings.php
$config['dea.settings']['access_check_boolean_operator']   = "AND";
CommentFileSizeAuthor
#2 dea-and_logic_for_access_management-2893901-2.patch2.89 KBrenaudcuny
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

renaudcuny created an issue. See original summary.

renaudcuny’s picture

renaudcuny CreditAttribution: renaudcuny at OSCE: Organization for Security and Co-operation in Europe commented
FileSize
dea-and_logic_for_access_management-2893901-2.patch2.89 KB
gnindl’s picture

gnindl CreditAttribution: gnindl commented

Apart from the global setting (alternatively to be put in a configuration object) I would see the configurable logic operator per entity operation field instance.

pmelab’s picture

pmelab CreditAttribution: pmelab as a volunteer commented

Whats the use case of this? You've got a set of privileges granted to group "A" and a set granted to group "B". And now you want a new set of privileges that is granted to users that are part of group "A" and "B"? This sounds like you need a group "C" with new privileges.

Drupal permissions work based on the "At least one grant and no deny"-rule. I would rather not move away from that.

renaudcuny’s picture

renaudcuny CreditAttribution: renaudcuny at OSCE: Organization for Security and Co-operation in Europe commented

The use case is as follow:

Vocabulary 1 defines a property of the node, ex: Type of Article:
- News article
- Biography
- Interview

Vocabulary 2 defines a subset of users, ex: Department/Unit:
- Marketing team
- Press team
- Legal department
- Web team

You want to use DEA to set limitations in this way:
- Marketing team can edit only News article and Biography
- Press team & Legal only Interview
- Web team can edit them all

pmelab’s picture

pmelab CreditAttribution: pmelab as a volunteer commented

Thats how I thought about this kind of problem when implementing it (yes, this would be a single vocabulary):

- Web team
  - Marketing team
    - News Article
    - Biography
  - Press team
    - Legal department
    - Interview
gnindl’s picture

gnindl CreditAttribution: gnindl commented

If DEA should support multiple access group (field) checks per entity - nothing prevents you to configure that - then the AND operator between fields is the only way to implement the policy "At least one grant and no deny". The current implementation realizes the OR operator "At least one grant, ignore deny". If you only test the entity again one access group, you do not notice that, however, when scaled the difference in logic becomes visible.

Being a bit less philosophical, I alter renaudcuny's example a little bit (you could also use user roles here, but just for illustration):

  1. Both marketing and press team edit their own news articles
  2. Marketing and press team should not be able to update articles (nodes) of the other team

The current implementation (OR operator) violates 2 as permission would have already been granted on 1 as implemented here:

    // If grants and requirements overlap allow, else deny access.
    if (count(array_intersect($requirements, $grants)) > 0) {
      return AccessResult::allowed();
    }
    else {
      return AccessResult::forbidden();
    }

From my perspective, the second check is simply ignored, correct me if I am wrong.

  • renaudcuny authored 6504e37 on 8.x-1.x 
    Issue #2893901 by renaudcuny: Configure operator for Entity access...
  • renaudcuny authored 4f27fd8 on 8.x-1.x
    Issue #2893901 by renaudcuny: Configure operator for Entity access...
  • renaudcuny authored 57792f9 on 8.x-1.x
    Issue #2893901 by renaudcuny: Configure operator for Entity access...
gnindl’s picture

gnindl CreditAttribution: gnindl at OSCE: Organization for Security and Co-operation in Europe commented
Status: Active » Fixed

Merged and committed to development branch.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.