Content is still indexed when private

greg.harvey - March 12, 2008 - 14:28
Project:nodeprofile privacy
Version:5.x-1.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs review
Description

Hi,

I'm in the process of debugging this, but I have the following issue:

When you mark a profile field as private, it is hidden from users who do not have the correct permissions to view it (which is users who do not have 'administrate users' permissions). This works fine. It also works in search listings, BUT I can still search *and get results* on the hidden data.

Let's say I have these CCK fields:

* Field 1: Foo
* Field 2: Bar

If I set Field 1 to be private, and I am not a user administrator, I will only see this at node view and search:

* Field 2: Bar

This is cool, but if I *search* for "Foo", I will still get the search listing. I won't see "Foo" in the result, but the result will be returned nonetheless, so I will know "Foo" exists in that node, even if it is hidden by virtue of the fact the node is listed. I checked the search_index table, and sure enough, my private fields are indexed regardless.

I think the problem is this - the nodeprofile_privacy_remove_private_fields() function unsets the fields which are private like this:

* $node[field_name]
* $node->content[field_name]

But search indexing in node.module uses $node->body which, for some reason, still contains the unset fields when cron runs. Here's the line which defines what is sent to the search.module within the node_update_index() function in node.module:

<?php
$text
= '<h1>'. check_plain($node->title) .'</h1>'. $node->body;
?>

So, in the example, $node['field_field_1'] and $node->content['field_field_1'] will be correctly unset, but $node->body (which is passed to search.module) will still contain the HTML, something like this:

<div class="field_field_1"><span>Field 1:</span><div class="content">Foo</div></div>

So Field 1 will still be indexed, even though the field was unset in the $node object.

Problem is, I can't work out why this works when you view a node yet fails when the search index runs. Any ideas?? =(

#1

greg.harvey - March 12, 2008 - 14:57

Actually, it's worse than I thought ... (and this is with the latest version, btw). Here's a node outputted by print_r($node) in the nodeprofile_privacy_nodeapi() function in the 'view' op when cron is run manually from /admin/logs/status/run-cron:

stdClass Object
(
    [nid] => 26
    [vid] => 26
    [type] => profile_basic
    [status] => 1
    [created] => 1199733007
    [changed] => 1199733007
    [comment] => 0
    [promote] => 0
    [sticky] => 0
    [revision_timestamp] => 1199733007
    [title] => gharvey's basic profile
    [body] =>
    [teaser] =>
    [log] =>
    [format] => 0
    [uid] => 2
    [name] => gharvey
    [picture] =>
    [data] => a:8:{s:16:"privatemsg_allow";i:1;s:28:"privatemsg_setmessage_notify";i:1;s:20:"privatemsg_mailalert";s:1:"0";s:10:"current_id";s:1:"2";s:12:"current_date";s:10:"1199728293";s:14:"picture_delete";s:0:"";s:14:"picture_upload";s:0:"";s:7:"display";s:1:"1";}
    [comment_type] => comment
    [taxonomy] => Array
        (
        )

    [field_first_name] => Array
        (
            [0] => Array
                (
                    [value] => Greg
                    [view] => Greg
                )

        )

    [field_last_name] => Array
        (
            [0] => Array
                (
                    [value] => Harvey
                    [view] => Harvey
                )

        )

    [field_date_of_birth] => Array
        (
            [0] => Array
                (
                    [value] => 1978-02-16T00:00:00
                    [view] => <span class="date-display-single">02/16/1978</span>
                )

        )

    [field_city] => Array
        (
            [0] => Array
                (
                    [value] => London
                    [view] => London
                )

        )

    [field_country] => Array
        (
            [0] => Array
                (
                    [value] => UK
                    [view] => UK
                )

        )

    [last_comment_timestamp] => 1199733007
    [last_comment_name] =>
    [comment_count] => 0
    [readmore] =>
    [content] => Array
        (
            [body] => Array
                (
                    [#value] =>
                    [#weight] => 0
                )

            [field_first_name] => Array
                (
                    [#weight] => -6
                    [#value] => <div class="field field-type-text field-field-first-name"><div class="field-label">First name:&nbsp;</div><div class="field-items"><div class="field-item">Greg</div></div></div>

                    [#access] => 1
                )

            [field_last_name] => Array
                (
                    [#weight] => -5
                    [#value] => <div class="field field-type-text field-field-last-name"><div class="field-label">Last name:&nbsp;</div><div class="field-items"><div class="field-item">Harvey</div></div></div>
                    [#access] => 1
                )

            [field_date_of_birth] => Array
                (
                    [#weight] => -3
                    [#value] => <div class="field field-type-date field-field-date-of-birth"><div class="field-label">Date of birth:&nbsp;</div><div class="field-items"><div class="field-item"><span class="date-display-single">02/16/1978</span></div></div></div>
                    [#access] => 1
                )

            [field_city] => Array
                (
                    [#weight] => 0
                    [#value] => <div class="field field-type-text field-field-city"><div class="field-label">City:&nbsp;</div><div class="field-items"><div class="field-item">London</div></div></div>
                    [#access] => 1
                )

            [field_country] => Array
                (
                    [#weight] => 1
                    [#value] => <div class="field field-type-text field-field-country"><div class="field-label">Country:&nbsp;</div><div class="field-items"><div class="field-item">UK</div></div></div>

                    [#access] => 1
                )

        )

)

field_last_name and field_date_of_birth are both set to private, a setting respected when a node is viewed (in full, a teaser or a search listing) but, as you can see, they are not being unset when cron is run manually. =(

I guess the problem is here, but it looks correct:

<?php
   
case 'view' :

     
/**
       * Remove the hidden fields if not user administrator
       * Also remove hidden fields if file cron.php is run manually
       * as this can inherit permissions of logged in user which will
       * result in hidden fields being indexed
       */

     
if (strpos($_SERVER['PHP_SELF'], 'cron.php') !== false || (!user_access('administer users'))) {
       
$node = nodeprofile_privacy_remove_private_fields($node);
      }
     
print_r($node);

    break;
?>

I'll keep chipping away...

#2

greg.harvey - March 12, 2008 - 15:54

Got it! The module currently assumes you run cron by hitting /cron.php, but I'm not doing that - I'm hitting /admin/logs/status/run-cron. This needs changing in hook_nodeapi from:

<?php
     
if (strpos($_SERVER['PHP_SELF'], 'cron.php') !== false || (!user_access('administer users'))) {
       
$node = nodeprofile_privacy_remove_private_fields($node);
      }
?>

to:

<?php
     
if (strpos($_SERVER['PHP_SELF'], 'cron.php') !== false || strpos($_SERVER['REQUEST_URI'], 'run-cron') !== false || (!user_access('administer users'))) {
       
$node = nodeprofile_privacy_remove_private_fields($node);
      }
?>

Just an additional "or" to catch the additional case of going through the admin panel to run cron manually. =)

#3

greg.harvey - March 12, 2008 - 16:17
Status:active» fixed

Patch attached! Thanks.

AttachmentSize
cron_fix-nodeprofile_privacy.module.patch 890 bytes

#4

Anonymous (not verified) - March 26, 2008 - 16:23
Status:fixed» closed

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

#5

bluecobalt - March 26, 2008 - 17:25
Status:closed» needs review

This got closed by mistake. The patch needs testing and review.

 
 

Drupal is a registered trademark of Dries Buytaert.