Drupal Version

10.5.4

Domain module version

2.0@beta7

Expected Behavior

  • Users should be able to edit or delete content on the content overview page with the permission to "update {$type_id} content on assigned domains".

Actual Behavior

Users can’t edit or delete content the content overview page even if they have the update / delete content on assigned domains permissions

Issue fork domain-3554767

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

lincoln-batsirayi created an issue. See original summary.

mably’s picture

Status: Active » Needs work

MR's current code gives a global update or delete grant to the user a soon as it has a permission to update or delete a particular content type, as our node_access hook will default to node grants if no specific permission is found (AccessResult::neutral()). That's probably not something we want.

This will require a significant change to the domain_access_node_access hook to handle that properly.

That's a change we could probably introduce in 3.x but it should be properly described, implemented and validated.

codebymikey’s picture

Attached a static copy of the latest MR.

lincoln-batsirayi’s picture

@mably yeah i see what you mean, i guess the first part is we need to only allow the permission for the single specific content type and not all of them. Do you have a rough idea on the sorta change that will need to be made to the domain_access_node_access hook? Maybe if i had a better idea of the plan it might be something i can get started on.

mably’s picture

A solution could be to use per bundle grant realms.

A new node node_access record will be have to be created for each content.

It could be interesting to have something before the first 3.x beta release eventually if we need to introduce some BC breaking changes.

mably’s picture

Assigned: lincoln-batsirayi » Unassigned
Category: Bug report » Feature request
Status: Needs work » Needs review

A solution based on bundle-specific grant realms has been implemented for Domain 3.x in MR 268.

@lincoln-batsirayi Could you give it a try and tell me if it solves the problem?

FWIW @agentrickard does not seem to be favorable to adding new node_access records:

The reason I'm against storing per-node-type grants in {node_access} is the problem of storage. The JOIN to {node_access} is already slow because its a varchar index. Adding granular records per node type would explode the storage needs and make that query even slower.

Don't know what to think about it.

I guess those extra joins would only be added to the queries when the user has some per-bundle permissions.

We might also need to add realms for each language for translated contents as some translations might only be available on specific domains...

But still not sure it's worth it.

Any feedback will be appreciated.

mably’s picture

@lincoln-batsirayi could you provide a fully reproducible scenario for the issue?

I'm not sure to be able to reproduce the problem on my side.

mably’s picture

Status: Needs review » Needs work
Related issues: +#2904111: domain_access_node_access ignores node access records
lincoln-batsirayi’s picture

@mably thanks a lot for looking into this, so I’ve tested various scenarios between your implementations for the 2.x and 3.x branch and this is what i found.

Tests conducted
2.x MR (261)

  • Only edit specific content type permission is enabled = Edit access is granted to all content types including one specified
  • Only “Edit any content on assigned domains” = Edit access is granted to all content types

3.x MR (268)

  • Only edit specific content type permission is enabled = No edit access to any content type including one specified
  • Only “Edit any content on assigned domains” = Edit access is granted to all content types

So across both MR's, the edit any content on assigned domains permission is functioning as expected. But the individual particular permissions on a per content type basis aren’t functioning as expected in both MR's.

In terms of the reproducible scenario, i was able to also test this on a sandbox site that I’ve got which has minimal modules so less chance of conflicts and i got the same results as my main site.

Steps to reproduce:

  1. Create a user with a specific role, lets say "content editor" and give them all the node access permissions possible (create, edit, delete)
  2. Set this user up to have domain access for the specific site you want on the edit user page (At this point this user no longer has the ability to do anything to any content type)
  3. Give the content editor role the "Edit any content on assigned domains" permission, on both MR's what happened in my tests will occur, they'll be able to edit ANY content types on the site
  4. Remove the edit any content on assigned domains permission and give the role one of the "{content type}: Edit any content on assigned domains" permissions and depending on which MR you’re in, then they might be able to edit all content (which is wrong) this is for the 2.x MR and if you’re in the 3.x MR then they won’t have the ability to edit any content at all which is also wrong

Imagine this would be the same for the delete permissions so i didn’t test those specifically because the code is the same. Let me know if you have any other questions

mably’s picture

Status: Needs work » Postponed (maintainer needs more info)

This appears to be a rather complex problem to solve — if it’s even possible without causing performance degradation.

Let's see if someone comes up with a clever solution.

@lincoln-batsirayi you might want to check the domain_access_node_access() hook and the DomainAccessManager checkEntityAccess() method.

And check what is present in the node_access table of your Drupal installation. Access authorization will default to the user node_access rights when the content is not assigned to any domains.

And if the content has no assigned domains, a record will still be added to the node_access table using the current domain (check the domain_access_node_access_records hook). That explains your point 3.

You can also check that related issue: #3047514: Logic flaw in grants?.

Remember to rebuild the node permissions when switching between MRs.

Unless new, valuable information is provided, this issue will be closed in 3 months.

mably’s picture

lincoln-batsirayi’s picture

@mably i believe I've managed to resolve the issue of the {content type}: Edit/delete any content on assigned domains permission on a single content type giving users the same permissions to ALL content types. I've added an explicit check which forbids access if the current user doesn’t have the specific update or delete permission for that content type. I’ve done several tests on my sites and this is working as expected.

I’ve only made this change on the 2.x MR for the moment.

lincoln-batsirayi’s picture

@mably ignore my previous comment here, the permission issue i was seeing on my own set up was related to another permission on the content moderation module which I’ve removed and has seemingly resolved things.

So what this current patch is doing, is working for me. I'm gonna do some additional tests on a sandbox site now that i have more knowledge of the issue and then I’ll report back here :)

lincoln-batsirayi’s picture

Okay @mably I’ve done a lot of testing today and i can confirm that the MR for 3.x is working on both my main site and the sandbox site so that's working as expected due to the grants being added based on the content types `$grants['domain_id:' . $type_id][] = $active_domain_id;` so good job on getting that sorted.

Obviously because this doesn’t exist on the 2.x you can guess that my tests haven’t proved fruitful there, but i think should be okay if the main fix will be available in 3.x? I'm guessing we can’t backport that change that's in the 3.x MR into 2.x because of the addition of the new type based grant which would make it a major version change? If we can backport the type based grant change then that's good news!

mably’s picture

Thanks @lincoln-batsirayi for your feedback.

Let's see if we can enable this in 3.x while keeping it optional.

mably’s picture

Version: 2.0.0-beta7 » 3.x-dev
Status: Postponed (maintainer needs more info) » Needs review
mably’s picture

The new per-bundle granting feature can be enabled through a new Domain Access setting:

New configuration option

This new feature is disabled by default.

Could you give it a try @lincoln-batsirayi?

mably’s picture

@lincoln-batsirayi could you please review latest MR?

I'm waiting for your RTBC to merge this.

Thanks.

mably changed the visibility of the branch 3554767-node-access-records-per-bundle to hidden.

mably changed the visibility of the branch 3554767-node-access-records-per-bundle to active.

mably changed the visibility of the branch 3554767-domain-access-further to hidden.

lincoln-batsirayi’s picture

Status: Needs review » Reviewed & tested by the community

I’ve reviewed and tested this and can confirm it's working as expected. The edit and delete permissions on the content overview page and generally on content entities are functioning as requested on this issue.

  • mably committed 29c47d9d on 3.x
    Issue #3554767 by lincoln-batsirayi, mably, codebymikey: Domain Access:...
mably’s picture

Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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