Entity API Change record: https://www.drupal.org/node/3002038

We need a way to secure all entity lists (not just nodes) for entities that can be grouped. So node grants is not the best option and Entity
API has recently come up with a way that allows us to alter queries more easily. Let's investigate that route or write our lightweight version of it until core supports something similar.

CommentFileSizeAuthor
#20 group-3013678-20.patch28.7 KBkristiaanvandeneynde
#18 group-3013678-18.patch26.21 KBkristiaanvandeneynde
#16 group-3013678-16-WIP.patch17.21 KBkristiaanvandeneynde

Comments

kristiaanvandeneynde created an issue. See original summary.

arosboro’s picture

arosboro commented

@kristiaanvandeneynde do you have any WIP for this effort? I'd be happy to review.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

Tentatively scheduled for this month (Jan 2019).

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

See: https://cgit.drupalcode.org/group?h=feature%2Fquery_access

chertzog’s picture

chertzog
he/him
Primary language English
Location Harrisburg, PA, USA
commented

Lending my voice to help as needed.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

Simple group query access has landed in a child issue (and as a result the dev branch). Note: requires Entity API.

bdurbin’s picture

bdurbin commented

Hi @kristiaanvandeneynde Is this something that's already in the works, or are you looking for patches? Our team would be happy to help move this one forward, especially if it's the last remaining feature-level issue blocking a full release.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

Hi, I currently have a feature branch open for this. I did get sidetracked on the permission layer (again), but am hoping to complete it soon.

What we still need:

  1. A GroupContent query access handler akin to the Group one
  2. A query alter event that applies group permissions to any groupable entity type

See #3035067: Add query access to Group entity lists for the work that has landed so far.

bdurbin’s picture

bdurbin commented

Thanks for the context. The GroupContent query access handler based on the Group one makes sense. Would you mind guiding us a bit more on what you mean by a "query alter event"? It looks like hook for altering entity queries isn't yet back in D8. Is there some alternative within events that you had in mind?

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

It's the event that allows you to alter the results generated by query access handlers. It's part of Entity API.

mikran’s picture

mikran commented

Hi kristiaanvandeneynde,

Is that feature branch publicly available somewhere? I couldn't see it on https://git.drupalcode.org/project/group/branches at least.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

It got merged into RC3 already for Group entities. We need one for GroupContent and any generic content entity managed by a group now. See https://git.drupalcode.org/project/group/blob/8.x-1.x/src/Entity/Access/...

mikran’s picture

mikran commented
Issue summary: View changes
moshe weitzman’s picture

moshe weitzman commented
Status: Active » Closed (duplicate)
Related issues: +#2775119: Document making custom entity types available as content for group types

Lets consolidate discussion on this to one issue - #2775119: Document making custom entity types available as content for group types.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Closed (duplicate) » Active

Reopening as this issue is specifically about finishing the query access handlers and nothing else.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Active » Needs review
StatusFileSize
new group-3013678-16-WIP.patch17.21 KB

Here's a work-in-progress. It adds a handler for the GroupContent entities, but no tests yet. Posting here so people can review early and perhaps spot any errors while I write the tests.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented 
+++ b/src/Entity/Access/GroupContentQueryAccessHandler.php
@@ -0,0 +1,210 @@
+      $sub_condition->addCondition('gid', $allowed_any_ids[CGPII::SCOPE_GROUP]);

That's a typo, should be own ids. Will fix as I add tests.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
StatusFileSize
new group-3013678-18.patch26.21 KB

Adds tests but still needs an any-vs-own permission test.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Needs review » Needs work

Will reroll this one with the latest permission provider work.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Needs work » Needs review
StatusFileSize
new group-3013678-20.patch28.7 KB

This takes care of the GroupContent query access. Will see whether I can make any improvements for the memberships and group nodes (if need be).

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Needs review » Reviewed & tested by the community

Seems like the membership and group node queries should work just fine as the former will check for admin permissions and the latter basically extends the default permission provider 100%. I'll add an admin check to the test and call it a day.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Reviewed & tested by the community » Fixed

And fixed.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Related issues: +#3134072: Implement the query access handling for grouped entities

Follow-up here: #3134072: Implement the query access handling for grouped entities

Status: Fixed » Closed (fixed)

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

ajayg’s picture

ajayg commented

Now this is fixed a while ago, what is preventing the official release of 8.1?

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

2 patches in Entity API are blocking us. See the related issue: https://www.drupal.org/project/group/issues/3134072

ajayg’s picture

ajayg commented

perhaps you should add on the roadmap link or main project page it is blocking the release of 8.1.