Split off from #955088-50: Provide (additional) access functionality, we could additionally improve access functionality (in cases were this is a very important requirement) in views if we pass all results to entity_access().

CommentFileSizeAuthor
#8 search_api_1278942_8_entity_views_access.patch1.81 KBsimon georges
#6 search_api_1278942_6_entity_views_access.patch2 KBsimon georges
#1 1278942--views-entity_access-1.patch4.18 KBdrunken monkey

Comments

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Active » Needs review
StatusFileSize
new 1278942--views-entity_access-1.patch4.18 KB

The attached would add a corresponding Views option. Please review!

BarisW’s picture

BarisW commented

Hmm, this doesn't seem to work for me. For user 1 it's okay, but with a default user I get a lot of notices and zero results.

Notice: Trying to get property of non-object in node_node_access() (line 2931 of /modules/node/node.module).
Notice: Trying to get property of non-object in og_node_access() (line 431 of /sites/all/modules/contrib/og/og.module).
Notice: Trying to get property of non-object in workbench_moderation_node_access() (line 378 of /sites/all/modules/contrib/workbench_moderation/workbench_moderation.module).

And some more..

BarisW’s picture

BarisW commented
Status: Needs review » Needs work
BarisW’s picture

BarisW commented

For reference, I reported a bug in OG, which turned out to be caused by this bug (?) in Search API Views: #1419538: How to exclude private group content from Search API?

BarisW’s picture

BarisW commented

Thomas, if you have time, please look into this. Currently it is not possible to exclude private content from search results if those results are displayed using Views. All help is welcome!

simon georges’s picture

simon georges commented
Status: Needs work » Needs review
StatusFileSize
new search_api_1278942_6_entity_views_access.patch2 KB

Trying to adapt the patch to the current -dev version.

simon georges’s picture

simon georges commented
Status: Needs review » Needs work

Ok, this patch does not work at all. Let me try to do better.

simon georges’s picture

simon georges commented
Status: Needs work » Needs review
StatusFileSize
new search_api_1278942_8_entity_views_access.patch1.81 KB

This one works. But I'm sure it could be optimized (because it loads every entity one after another). There's also a small fix from a copy-paste of the patch in #1 regarding the #default_value in the Views form options.

BarisW’s picture

BarisW commented
Status: Needs review » Needs work

Thanks, this seems to work. I don't see the nodes any more in the results I'm not allowed to see. However, the views counter still remains the same and the Facets (facet api) still show a filter. So I can filter on the content type I'm not allowed to see, and after filtering, I get a 'no results found'.

Any thoughts on this?

BarisW’s picture

BarisW commented

Plus, are we're doing it on a page level now? I'd expect 10 results per page, where I'm now seeing 9 (one node disappears which I'm not allowed to see).

simon georges’s picture

simon georges commented

I have the same issues. But I only adapted the patch, I'm not an expert in Search/Facet APIs, so I was thinking somebody would just jump on it to help ;-)

balintbrews’s picture

balintbrews
he/him/his
Primary language Hungarian
Location Netherlands 🇪🇺
commented
Status: Needs work » Closed (fixed)

This issue is already fixed, it's implemented in the current recommended stable release (maybe even in earlier releases as well, I'm not sure about that).

olofbokedal’s picture

olofbokedal commented
Status: Closed (fixed) » Needs review

This doesn't exist in the latest dev, so no, it hasn't been implemented yet.

The patch from #8 removes the results, but the pager acts on the original result count. The "removal logic" should probably be executed earlier, but I don't know where and when. I'll continue to investigate this.

olofbokedal’s picture

olofbokedal commented
Status: Needs review » Needs work

After some heavy thinking, I think the only possible way to respect the pager, facets count etc, is to alter the query before it has been executed. This requires that the access information is stored in a table, for instance in the node_access table.

This might be possible to achieve for nodes, but it'll probably be hard. However, I think it's impossible to achieve in a proper way for other entities.

simon georges’s picture

simon georges commented

Yes, I've actually completely dismissed the idea, as I didn't find any proper way to do it...

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

After some heavy thinking, I think the only possible way to respect the pager, facets count etc, is to alter the query before it has been executed. This requires that the access information is stored in a table, for instance in the node_access table.

This might be possible to achieve for nodes, but it'll probably be hard. However, I think it's impossible to achieve in a proper way for other entities.

As balintk said (and as can be read in the issue link in the OP), this is already implemented, at least for nodes. See the "Node access" data alteration under "Workflow". However, it's sadly not possible to do it on a generic level for all entity types, as far as I can see. That and the fact that content that isn't indexed right away might still be shown to unauthorized users, leads to the need for this option, which would be kind of a last ressort.

Yes, the paging will be wrong, as will the facets, the "Current search" block, etc. But it's usually still preferable compared to showing classified content to unauthorized users.

However, as far as you can you should of course use normal filters to do that (or, as said, the "Node access" data alteration). Or implement a corresponding data alteration for the entity type you're using.

Anyways, the patch in #8 looks pretty good to me. Anyone else can confirm it works?
We should probably just change the description to warn users to only use this as a last ressort, because it'll break things. (And re-add the README.txt changes from my original patch in #1.)

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Title: Provide an option to apply entity_access() to Views results » Add an option to apply entity_access() to Views results
Status: Needs work » Needs review
olofbokedal’s picture

olofbokedal commented

Yes, #8 does solve this issue and works as intended.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Fixed

Thanks for testing!
Committed.

Status: Fixed » Closed (fixed)

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