Hi,
I have a problem with the Views module integration.

The pagination does not display correctly, probably because the access check is done in hook_views_post_execute() instead of the "pre_execute".

Someone else has encountered the problem and solved it?

Ty
I.

Comments

Yoris00 created an issue. See original summary.

Peter Majmesku’s picture

Status: Active » Needs work

I have fixed this in 8.x-1.12. I suggest to use the 8 version, by any chance. If you want, you can backport the code from there. It is done by hook_node_grants(), node_access_rebuild(), hook_node_access() and the NodeAccess() class.

It would be highly appreciated if you could share a patch with the backport. Then I would push it into the 7.x branch.

jnicola’s picture

I just opened a far more detailed and accurate ticket on this, but here's the info I had there which is important.

I unfortunately know a fair bit about this issue, because before trying your code, I had a custom solution that had the same result.

ISSUE:
Implementing access check after the query has occurred in hook_views_post_execute() does infact allow you to, in a round about way, get your views results to then check for permissions. This though causes issues, as it adjusts the results and does so with no regard to pagination, or views limited to just one. For example, a featured news item returning just one view will return said view, and if the user doesn't have "permission" via this module, the result will be removed. The view will then be empty and display the empty text, even if more results were technically available.

Reproducing:

  • Setup basic configuration as required for module.
  • Create a view showing just 1 item, latest of a given content type of your choosing. Add text for empty results.
  • Add required field to content type, make sure you have a taxonomy vocab and a term that you can restrict access on a test user.
  • Add two pieces of content. One will be unable to be accessed by said test user, but will have a more current created date (so it will show first). Second will be older content, but accessible.
  • Correct behavior would be that in the absence of a piece of content with permission, the available piece of content would be used. However hooking into views as done here cannot be made to work. You will get an empty view.

Currently, this is more than just pagination. The title should reflect integration as the issue reaches the entirety of integration with views.

Peter Majmesku’s picture

Implementing access check after the query has occurred in hook_views_post_execute() does infact allow you to, in a round about way, get your views results to then check for permissions. This though causes issues, as it adjusts the results and does so with no regard to pagination, or views limited to just one.

Yes, for this I have implemented the usage of the node grants records into the Drupal 8 version. That's the best solution I have found so far. However it is performance costly and I am looking for a better option.

Since the entire node grants database table must be rebuilt after a taxonomy term permission change (currently all node, term and user save actions). If you have more then let's say 1.000 nodes, then you must care about the max_execution_time in your php settings and this update process can take some time and some nodes could be not accessible during this time. See https://www.drupal.org/node/2845735 regarding this issue.

Feel free to provide a patch for the update of the 7.x branch in PbT project. I am working on this module in my sparetime and I have not enough time to work on the 7.x version beside the 8.x version. So it would be much appreciated, if you could provide more into the project then just this feature request.

jnicola’s picture

In regards to D8 and max execution time, have you considering breaking it out into a batch process?

Peter Majmesku’s picture

Yes, I am considering it. I am thinking about moving the node access grants rebuild process into a cron task. Since there are big chances that this process takes too long and just annoys users after they hit node, user, term save. What do you think? Shall I keep this process in node/user/term save process or in a cron task?

Peter Majmesku’s picture

Status: Needs work » Postponed (maintainer needs more info)
jnicola’s picture

Cron tasks also face max execution time IIRC. Batch process is really the way to go in my experience.

I saw you changed this to postponed, needs more information. I'm not sure that's the appropriate thing for it. It's definitely an open/needs work issue, you're just not able to do the work right now. Leaving it open is best so others with the D7 issue may get involved, and who knows, some company may need this fixed bad enough that a paid for patch comes through!

Peter Majmesku’s picture

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

Yes, I have changed the status.

I will definitely check the batch process in the D8 branch.

Peter Majmesku’s picture

Peter Majmesku’s picture

Status: Needs work » Closed (won't fix)

7.x version is not maintained anymore.