Search no longer returns unpublished nodes

Search is still supposed to return unpublished nodes to people who have access to see unpublished nodes in D7. If this is not happening, it is a bug. I guess we need to investigate... I guess I would try it first with User 1 and verify that it's the case...

Hey, same for me.
Search does not give any results from unpublised nodes for user 1. Also I think nodes were indexed for search, cause I have about 500 unpublised nodes and only 10 published... and it took a while to index it.

I've made small research on it. It seems node_search_execute it the problem. On line 1642 there is

$query
    ->condition('n.status', 1)
    ->addTag('node_access')
    ->searchExpression($keys, 'node');

as you see there is no checking if you have rights to search unpublished node (i coudn't even find such role). it just searches published ones.

So quick hack to enable admin to search unpublished nodes.

modules/node/node.module line 1642
replace:

  $query
    ->condition('n.status', 1)
    ->addTag('node_access')
    ->searchExpression($keys, 'node');

with:

  global $user;
  if ($user->uid != 1)
    $query ->condition('n.status', 1);

  $query
    ->addTag('node_access')
    ->searchExpression($keys, 'node');

Sounds like a major regression to me. We need a patch. The patch should actually just remove the condition that says (n.status, 1), because the node_access tag should take care of restricting to nodes the searching user can see.

And we need to do it in d8 first, then backport to d7.

You also need to remove the -D# extension on the patch file to get the test bot to run.

I think this patch also needs a test so we can avoid having this problem regress in the future.

tagging

sub

Just re-uploading the patch from #5 to get the testbot to run it. But this needs tests.

So, I should probably re-upload eh?

Tests pass, but this needs a test to make sure that search behaves the way that it is expected to.

The condition on status = 1 is definitely needed. The node access tag doesn't add that by itself.

@Damien Tournoud I think the point of the OP was that search results should contain unpublished nodes if you have permissions to view them, adding the condition would surely mean that you could never view unpublished nodes?

Are we saying that the search functionality has changed from D6 -> D7 to not return unpublished nodes? Does anyone have an issue where that change was discussed?

I don't think it was intentional or discussed, but yes it is a reversion from d6 to d7. My guess is that when all the db queries were redone for d7, someone decided to put that condition in, although it wasn't a good idea for search behavior.

This is a very minor hiccup, but definitely not a major bug.

Looks like we need @moshe or @agentrickard on this issue.

See below

Bug happens when we try to search with 'LIKE' condition. These are lines for this bug.

<?php
protected function parseSearchExpression() {
...
$queryor->condition('d.data', "% $or %", 'LIKE');
...
$this->conditions->condition('d.data', "% $key % ", 'LIKE');
...
$this->conditions->condition('d.data', "% $key %", 'NOT LIKE');
}
?>

And

public function executeFirstPass() {
...
$or->condition('i.word', $word);
...
}

=> fixed

<?php
protected function parseSearchExpression() {
...
$queryor->condition('d.data', "%$or%", 'LIKE');  // "% $or %" => "%$or%"
...
$this->conditions->condition('d.data', "%$key%", 'LIKE'); // "% $key %" => "%$key%"
...
$this->conditions->condition('d.data', "%$key%", 'NOT LIKE'); // "% $key %" => "%$key%"
}
?>

And

public function executeFirstPass() {
...
$or->condition('i.word', '%'. $word .'%', 'LIKE'); // condition('i.word', $word) => condition('i.word', '%'. $word .'%', 'LIKE')
...
}

Please investigate this issue and let me know your suggestions.
Thanks
Truong Vo Quang

Please leave this as a Drupal 8 issue. The policy in Drupal is that any issue gets fixed in the current dev version first, then backported to other versions if needed.

Also, please don't add random tags. Or remove tags that should have been there. Please read the tagging guidelines.

And #17-18 - that looks like a different bug. Please file a different issue for it.

From what I can determine, Drupal 6 and 7 are behaving the same way (and always have). Unpublished nodes are indexed, but not returned in node_search.

D6 node_search code:

function node_search($op = 'search', $keys = NULL) {
  switch ($op) {
...
    case 'search':
...
      $conditions1 = 'n.status = 1';

And I can confirm Damien's comment in #13 that if you remove either this $conditions1 or the ->condition('n.status', 1) in D7 that you can return unpublished nodes in the search, but that everyone who can view search results gets those results even though they're denied on actually trying to view it.

So we need a different mechanism to return the results according to whether you're allowed to view the node.

We should probably revive this issue.

a) It needs a test. The test should verify that a user without permissions does not get unpublished nodes in search results, and that a user with permissions does. This should probably be added as a method in the existing SearchNodeAccessTest class.

b) Once that is written, we need a patch that just uploads the test, and we need to verify that the test fails at the appropriate spot.

c) Once that is working, we can add the patch to remove the status condition. Search has been refactored into plugins; the line of code for the patch is now in the NodeSearch plugin class ( \Drupal\node\Plugin\Search\NodeSearch that is). Upload a patch containing both the test and the line of code change, and verify that the test now passes.

I think now that 7.x has been out for so long, it would be dangerous to backport this patch to 7, as it might cause security issues on existing 7 sites.

Oh and by the way I just tested this in the latest 8.x and verified the bug still exists. The line adding the status condition to search queries is still in the code.

I have confirmed comment #20 - in Drupal 6.x and even in 5.x, there is indeed a condition that the node must be published. So this is not a change in behavior in 8.x or 7.x. It's been this way for a very long time.

If you want to make a query to return unpublished nodes in a search, you can use Views... I don't see an extremely compelling reason to change that behavior (changing this into a feature request, for instance), since Views integration for search keywords is definitely available in Drupal 7 and should soon be available in Drupal 8 (see #1853536: Reintroduce Views integration for search.module (not supporting backlinks view)).