Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need to check access when retrieving entities for autocomplete reference field #15

Open
Tracked by #16
herbdool opened this issue Dec 23, 2016 · 7 comments · May be fixed by #66
Open
Tracked by #16

Need to check access when retrieving entities for autocomplete reference field #15

herbdool opened this issue Dec 23, 2016 · 7 comments · May be fixed by #66
Labels
Release blocker Should be done before initial release.

Comments

@herbdool
Copy link

When using an autocomplete reference field, it should check that the user has access to view the entities before allowing them to be available for selecting.

I'm wondering if this should also be available if a select widget is used or if it's better to use a view for that use case.

@mikemccaffrey
Copy link
Member

I'm not sure what level of access control we need to support. To me, it seems like if an admin creates a field to reference a particular entity type (and optionally specify the bundle and whether it is published) that is implicitly stating that anyone that can edit that field should have permission to view the list of applicable entities, or at least their labels.

I know that @quicksketch disagrees with me, and has ideas of how to alter the database query to allow the access of individual entities to be controlled. I know he posted instructions for doing that somewhere, but I don't recall exactly where. Perhaps he can make the case for that course of action again here?

@herbdool
Copy link
Author

I believe that backwards compatibility is important for this module, particularly if it's going to end up in core and replace References and Entity Reference. So I also believe that the permissions should work the same, otherwise it could expose private information that people don't want to expose when they use the upgrade path.

I'm not sure if it would get messy since we're dealing with different entities. The basic approach for nodes is to use node grants such as is done in References https://github.com/backdrop-contrib/references/blob/1.x-1.x/node_reference/node_reference.module. But for users, terms, X entities... I'm not sure.

@herbdool herbdool added the Release blocker Should be done before initial release. label Mar 28, 2018
@mikemccaffrey
Copy link
Member

@quicksketch Is the node grants approach applicable to other entity types in general? What is a way that we can do permissions checks across all entity types?

@mikemccaffrey
Copy link
Member

mikemccaffrey commented Apr 26, 2018

Okay, Nate explained the query tag concept to me again. He says that entityreference just adds these lines in when creating the query:

    $query->addTag($this->field['settings']['target_type'] . '_access');
    $query->addTag('entityreference');

From this full function: https://gist.github.com/quicksketch/7f607798b0078f5486b06cb6c510efb0

And then any entity that implements access control can alter the query to add their own tables and conditions to the query.

@jenlampton
Copy link
Member

jenlampton commented Jul 10, 2021

We were talking about this today in at BackdropLIVE and decided to use the existing access checks for entities rather than running our own database queries. I think this means the access callback for reference_autocomplete becomes entity_access? Hm, I don't think that will work, we'll need to some reconfiguring.

jenlampton pushed a commit to jenlampton/reference that referenced this issue Oct 6, 2021
@jenlampton jenlampton linked a pull request Oct 6, 2021 that will close this issue
@jenlampton
Copy link
Member

I found some changes on my local copy of references switching to entity_access() but I don't remember how much testing I did, and if it worked as expected. I need to focus on something else right now, so I'm pushing this PR so anyone (including myself!) can pick this up later if they want.

@hosef
Copy link
Contributor

hosef commented Oct 11, 2021

@jenlampton All entities have an access() method, so you can do that with $entity->access($op); as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release blocker Should be done before initial release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants