It seems that search_api does not support view_unpublished module...

I'm not a pro of Drupal node access stuff... I'm working on this.

I added to search index node access info.
But I had to change SearchApiAlterNodeAccess alterItems() method to remove the node_access('view', $item, $account) check because it always returned true so it didn't query the node_access table with my view unpublished permissions :

  /**
   * Alter items before indexing.
   *
   * Items which are removed from the array won't be indexed, but will be marked
   * as clean for future indexing. This could for instance be used to implement
   * some sort of access filter for security purposes (e.g., don't index
   * unpublished nodes or comments).
   *
   * @param array $items
   *   An array of items to be altered, keyed by item IDs.
   */
  public function alterItems(array &$items) {
    static $account;

    if (!isset($account)) {
      // Load the anonymous user.
      $account = drupal_anonymous_user();
    }

    foreach ($items as $nid => &$item) {
      // Check whether all users have access to the node.
//      if (!node_access('view', $item, $account)) {
        // Get node access grants.
        $result = db_query('SELECT * FROM {node_access} WHERE (nid = 0 OR nid = :nid) AND grant_view = 1', array(':nid' => $item->nid));

        // Store all grants together with it's realms in the item.
        foreach ($result as $grant) {
          if (!isset($items[$nid]->search_api_access_node)) {
            $items[$nid]->search_api_access_node = array();
          }
          $items[$nid]->search_api_access_node[] = "node_access_$grant->realm:$grant->gid";
        }
//      }
//      else {
//        // Add the generic view grant if we are not using node access or the
//        // node is viewable by anonymous users.
//        $items[$nid]->search_api_access_node = array('node_access__all');
//      }
    }
  }

In search_api.module, _search_api_query_add_node_access() method only checks 'view own unpublished content' permission... but I think it should also check permissions from view_unpublished module:
'view any unpublished content' and 'view any unpublished $content_type content'.

function _search_api_query_add_node_access($account, SearchApiQueryInterface $query) {
  if (!user_access('access content', $account)) {
    // Simple hack for returning no results.
    $query->condition('status', 0);
    $query->condition('status', 1);
    watchdog('search_api', 'User @name tried to execute a search, but cannot access content.', array('@name' => theme('username', array('account' => $account))), WATCHDOG_NOTICE);
    return;
  }

  // Only filter for user which don't have full node access.
  if (!user_access('bypass node access', $account)) {
    // Filter by node "published" status.
    if (user_access('view own unpublished content')) {
      $filter = $query->createFilter('OR');
      $filter->condition('status', NODE_PUBLISHED);
      $filter->condition('author', $account->uid);
      $query->filter($filter);
    }
    else {
      $query->condition('status', NODE_PUBLISHED);
    }
    // Filter by node access grants.
    $filter = $query->createFilter('OR');
    $grants = node_access_grants('view', $account);
    foreach ($grants as $realm => $gids) {
      foreach ($gids as $gid) {
        $filter->condition('search_api_access_node', "node_access_$realm:$gid");
      }
    }
    $filter->condition('search_api_access_node', 'node_access__all');
    $query->filter($filter);
  }
}

I continue my investigations but if someone already used search_api and view_unpublished with success... let me know!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cthiebault’s picture

I've also created an issue for view_unpublished module because I don't know whose responsibility is it...
http://drupal.org/node/1616890

cthiebault’s picture

Here is a patch that checks if view_unpublished module is enabled...

cthiebault’s picture

Status: Active » Needs review
cthiebault’s picture

Version: 7.x-1.1 » 7.x-1.x-dev
cthiebault’s picture

Status: Needs review » Needs work

Sorry, it does not work...

Looking at view_unpublished Views filter, we need to create a new query filter in order to support view_unpublished permissions...

view_unpublished_handler_filter_node_status.inc:

/**
 * Filter by view all unpublished permissions granted by view_unpublished
 * Takes over the Published or Admin filter query.
 */
class view_unpublished_handler_filter_node_status extends views_handler_filter_node_status {
  function query() {
    $table = $this->ensure_my_table();
    $where_per_type = array();
    foreach (node_type_get_types () as $type => $name) {
      $where_per_type[] = "($table.type = '$type' AND ***VIEWUNPUBLISHED_$type*** = 1)";
    }
    $where_per_type = implode(' OR ', $where_per_type);
    $this->query->add_where_expression($this->options['group'], "$table.status = 1 OR ($table.uid = ***CURRENT_USER*** AND ***CURRENT_USER*** <> 0 AND ***VIEW_OWN_UNPUBLISHED_NODES*** = 1) OR ***ADMINISTER_NODES*** = 1 OR $where_per_type");
  }
}
cthiebault’s picture

Status: Needs work » Needs review
FileSize
2.77 KB

OK this one seems to work.
I used view_unpublished_handler_filter_node_status as inspiration...

semiaddict’s picture

Attached is a patch that should make the Search API work with all node access modules.
It makes a slight modification to the _search_api_query_add_node_access function in search_api.module.

To make it work correctly with node access, don't forget to add the 'node access' data alteration in the index's workflow.

Note: only tested with the Search API DB, but should work fine with Search API Solr.

Status: Needs review » Needs work

The last submitted patch, search_api-view_unpublished-support-1617794-7.patch, failed testing.

naxoc’s picture

Status: Needs work » Needs review
FileSize
2.97 KB

I found an error in the patch in #6. If you have the permission "view own unpublished content" you get zero results because the combination of the filters becomes way too narrow.

This is still using the approach with if module_exists(). Not ideal but it works. It would be interesting to explore a more generic way to do it.

Here is a new patch though.

drunken monkey’s picture

Status: Needs review » Needs work

Thanks for reporting this issue!
I can understand that this is a problem for sites using the view_unpublished module, but I don't think we should go and hard-code special cases for any module out there which might conflict with the Search API.
If it's not possible otherwise, then maybe a small integration module would help? Also far from ideal, I know, but at least we wouldn't add bulk to this already highly complex module, and the users of both modules could still just use this to fix things.

The approach in #7, however, would be far better. If we can somehow solve this in a generic way, I'm all for it. However, I'm pretty sure the patch in #7 won't work, as it seems to me it just allows access to all published nodes, disabling any node access mechanism other than for unbublished nodes.
In the end, I just think that node access in D7 (don't know about D8) is just not flexible enough to deal with things like view_unpublished, and therefore we're powerless to generically support both.

If you have any ideas, they are still more than welcome, though!

semiaddict’s picture

The approach in #7, however, would be far better. If we can somehow solve this in a generic way, I'm all for it. However, I'm pretty sure the patch in #7 won't work, as it seems to me it just allows access to all published nodes, disabling any node access mechanism other than for unbublished nodes.
In the end, I just think that node access in D7 (don't know about D8) is just not flexible enough to deal with things like view_unpublished, and therefore we're powerless to generically support both.

I don't understand why this won't work.
I have a custom module that implements hook_node_access, hook_node_access_records and hook_node_grants to allow complex access algorithms. Patch #7 works great for me.

Did you try it out ?

gilsbert’s picture

Hi.

I have "2 cents" about this issue.

I believe "view_unpublished" module exists to give a profile the power to edit and republish a node (without view_unpublished we would need to ask a superuser to republish the node).

For others reasons a not published content should be invisible (except for superusers). Otherwise why turning off the property "published" in the node?

Finally, in my personal opinion search_api's behavior is correct and should not be changed (unless by a custom hook or by a new integration module).

Regards,
Gilsberty

drunken monkey’s picture

Did you try it out ?

No, I didn't. Please create a valid patch file if you want me to do that.

efratsh’s picture

I also need the proposed patch in #7 with the module OG Moderation which gives permissions "View unpublished content" per content type inside an Organic Group.
Any help is welcomed.

drunken monkey’s picture

I'm really not sure how we could solve this. Since it seems to me (though I might be mistaken) that in Drupal core the access of unpublished nodes is hard-coded instead of using the Node Access API, we are more or less powerless to support both use cases in a clean way. Either we add a condition on "published" and other modules are powerless to allow users to view unpublished nodes based on custom rules; or we don't add it and sites without such custom modules suddenly have all their unpublished content exposed.
Both aren't ideal, but I think the latter is far preferable.

But, again, I might be mistaken here. I'm also open for suggestions on how to solve this.

gilsbert’s picture

Hi.

As I wrote at #12 I dont see anything to be solved.

Regards,
Gilsberty

guillaumev’s picture

Status: Needs work » Needs review
FileSize
2.57 KB

Hi,

What about adding an option at the index level that would allow users to chose whether they want this "publish safeguard" or not ?

Attached is a patch that does exactly this, and should work with both view_unpublished and og_moderation (I've only tested with og_moderation though), as long as you deactivate the "Published safe" checkbox in your index.

Note that the wording of the checkbox should probably be improved...

drunken monkey’s picture

Status: Needs review » Needs work

Hm, OK, that solution might be acceptable. However, since the option only takes effect if the "Node access" (or "Comment access") data alteration is used, the option should be located there.
Also, since old indexes won't have the option, you'll have to check for it's presence – and it should definitely default to TRUE.

Other than that, if a few people can then review the patch and verify it works correctly for their setup and use case, I would commit this. I just want to make very sure that we don't break node access on some site.

drunken monkey’s picture

Also, could we maybe merge this patch with the one from #2269163: Node access information is indexed in a wrong way? If we fix Node access to make it more flexible and compatible with more contrib modules, I'd like to do that in one issue/patch so that hopefully more people will test this. This is really something we should make sure doesn't break.

drunken monkey’s picture

Title: view_unpublished module support » Make "Node access" compatible with additional contrib modules
heyyo’s picture

I just tried the patch provided in#17 with og_moderation.
Now my OG Administrators could also see Unpublished nodes inside their group. So it's working great.

But Drupal users even if the Drupal permission "View own unpublished content" don't see their unpublished content inside Views.

Am I missing something ?

drunken monkey’s picture

Am I missing something ?

Probably not and the patch just doesn't work as intended.

There is another node access issue, #1949828: Node access shouldn't add a generic view grant based on the anonymous users, about the same problem as #2269163: Node access information is indexed in a wrong way. We should merge all of these into this one (though the approach in the latter seems more valid to me), come up with a single, combined patch and then make sure this absolutely works for everyone (most importantly, without regressions or false positives) before committing. Node access would be a very bad part to get wrong.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
1.55 KB

Posting the patch here as requested for node access from #1949828: Node access shouldn't add a generic view grant based on the anonymous users for review.
Hopefully it will help and maybe it can be merged in? #17 doesn't look all that similar but maybe we're trying to solve the same problem?

drunken monkey’s picture

Status: Needs review » Needs work

Thanks for posting here. Would be great if we could come up with a unified strategy on how to solve all of these special-case problems with node access.
See #1949828-10: Node access shouldn't add a generic view grant based on the anonymous users for why I want to have all of these problems bundled in one issue.

Also, to be even more certain node access doesn't break in some cases, or in the future, it would be great to add a new test case and test module with tests covering all the different use cases.

arosboro’s picture

Curious to see if this issue resolves any of the problems with using the node access api in search api. #2347981: Node access query alter incorrectly handles filters for view own unpublished content and node access grants

idebr’s picture

I updated the query alter for node access to reflects node grants are added to the OR filter instead of AND. In my use case, the view_unpublished module grants permission on a entity:bundle basis. The access is not applied by search_api however, since it created a query:

Before:
- type =
AND
- status = 1 OR author = [uid]
AND
- search_api_access_node = node_access_$realm:$gid

After:
- type =
AND
- status = 1 OR author = [uid] OR search_api_access_node = node_access_$realm:$gid

joelpittet’s picture

Status: Needs review » Needs work

@idebr thanks for laying it out like that. I think it's wrong with status =1 OR ... because that would bypass the access permissions if it's 1 OR the author. Whouldn't it?

drunken monkey’s picture

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

Thanks for answering this, joelpittet. I'm also pretty sure that, while this does solve the problems with view_unpublished, it will break node access in other scenarios, which is of course unacceptable.
I now took a quick look at the source code of view_unpublished and it seems that we won't be able to make it work generically with that module in any case: the module doesn't work with the Core node access mechanism but even "breaks" it by manually removing existing conditions from the query. If the module can only work with normal node queries this way, I don't think we have to be responsible for supporting it by default in our module. Rather, view_unpublished or some other contrib module should provide a new data alteration/processor for the Search API that implements an access mechanism that works with that module specifically.

So, I think before reporting here that our content access mechanism doesn't work with a certain module, you should review whether the module really does work with the Core node access system, and doesn't alter the query itself, maybe even removing or changing existing conditions. In the latter case, I don't think it's possible that we can support that module's use case in our access system, and we also probably shouldn't try.

So, the question is: is there even such a module that doesn't work with our access system but should? Otherwise, we should probably close here and leave it to other modules to implement this.

lucas.constantino’s picture

Drupal 8

Search API's assumption of access rights based on anonymous using testing is completely mistaken. Drupal's node access system allows for runtime calculated access granting, such as avoiding having a node be retrieved on specific hours of the day, but this assumption by the search_api module simply kills this kind of possibility. Therefore, I suppose the easiest solution is to simply remove any node_access__all related code, for that Drupal's own system might step in as is.

Here goes a Drupal 8 patch.

sebas5384’s picture

I agree with @lucas.constantino.
A content that can be accessed by an anonymous user, shouldn't mean that all users can have access.
The `node_access__all` approach is breaking the expected behavior when using Views for example, because it will remove the entity right after the Search API returns it, so if your search returns 10 items paged by 5 per page, and one of them has the `node_access__all` grant, it will display 4 items at the first page and then 5 items on the second page and so on.

drunken monkey’s picture

Search API's assumption of access rights based on anonymous using testing is completely mistaken.

That's a pretty harsh way to phrase it. I'm of course aware that our current mechanism doesn't completely reflect all possible scenarios, or allow the same flexibility as the entity access system. However, with what we have, I don't think it's really possible to do any better. How could we ever know that a node can only be viewed at certain hours?

But that's why this isn't baked into the Search API framework itself, but just an optional processor. If it doesn't work for your site, simply write a new, custom one that does! As easy as that, and you have almost the same flexibility as with the normal entity access system.

Regarding your proposed patch: Wouldn't this mean that normal, published posts on a normal site would never get returned by searches? Could you modify the test method you're removing instead to ensure that this isn't the case? (I.e., for that node, verify that it would get returned for searches both for an anonymous user and a registered one with some grants.)

recidive’s picture

Re-rolling patch in #29.

drunken monkey’s picture

Issue tags: +Node access

NB: See #2948707: ContentAccess processor fails to account for all grant records for a discussion about the anonymous user access handling in D8. We should stay on topic here, that's tricky enough.

awm’s picture

For sites using view_unpublished and needing a quick patch, what's the best resource on how to implement a custom processor in D8?

Yazzbe’s picture

After upgrading to search_api search_api 7.x-1.23 all content visible for anonymous users is now gone/hidden.

The existing grants were set by the 'content access' module (contrib).
I have field 'node access information' in my index.
I also have filter 'node access' checked on.
Re-indexed, updb, cc all
Using drupal 7.51

Is there something I need to do or patch to keep this version of search_api backwards compatible with the existing grants and access behaviour?

PS: don't want to cross-post so I hope this is the correct thread to report this.

jcnventura’s picture

awm’s picture

for those using d8 version view_unpublished I attempted to write a custom processor for the module at https://www.drupal.org/project/view_unpublished/issues/2958568#comment-1.... Please take a look if you can.

drunken monkey’s picture

Status: Postponed (maintainer needs more info) » Closed (won't fix)

Closed due to long time without response.
Please re-open with the requested information if still relevant.