[drupal-devel] [bug] Statistic information about node reads is shown to unprivileged users

Fri Jul 1 08:10:30 UTC 2005

Issue status update for http://drupal.org/node/22565

 Project:      Drupal
 Version:      cvs
 Component:    statistics.module
 Category:     bug reports
 Priority:     normal
 Assigned to:  Robin Monks
 Reported by:  massabob
 Updated by:   mfb
 Status:       patch

+1

mfb

Previous comments:
------------------------------------------------------------------------

May 11, 2005 - 00:51 : massabob

Information about '%count reads' in node's footer is shown to
unprivileged users. I suggest that function statistics_link in
statistics.module should be corrected in this way:

// Original

function statistics_link($type, $node = 0, $main = 0) {
  global $id;

  $links = array();

  if ($type != 'comment' && variable_get('statistics_display_counter',
0)) {
    $statistics = statistics_get($node->nid);
    if ($statistics) {
      $links[] = format_plural($statistics['totalcount'], '1 read',
'%count reads');
    }
  }
  return $links;
}

// Fixed

function statistics_link($type, $node = 0, $main = 0) {
  global $id;

  $links = array();

  if ($type != 'comment' && variable_get('statistics_display_counter',
0) && user_access('display statistics')) {
    $statistics = statistics_get($node->nid);
    if ($statistics) {
      $links[] = format_plural($statistics['totalcount'], '1 read',
'%count reads');
    }
  }
  return $links;
}

The only change is in "&& user_access('display statistics')" on line
98.

------------------------------------------------------------------------

May 26, 2005 - 18:44 : RobRoy

Should read user_access('access statistics') not user_access('display
statistics').

------------------------------------------------------------------------

May 27, 2005 - 04:19 : Robin Monks

Attachment: http://drupal.org/files/issues/user.access.stats.patch (730 bytes)

And here that is in patch form.

Robin

------------------------------------------------------------------------

May 31, 2005 - 21:09 : Steven

I'm not sure about this patch: often, read counts are shown directly on
the site. But if the permission for viewing the counts is the same as
the permission for accessing the administrator's detailed logs, then
you wouldn't give that to everyone.

There is already an option to choose whether counts are displayed.
Perhaps we could change that to "No" "For priviledged users" "For
everyone". In last case it acts like it is now, it the second case it
requires "access statistics" permission.

What do you think?

------------------------------------------------------------------------

June 1, 2005 - 07:13 : Robin Monks

Sounds good to me.  I'll try to code something up for this.

Robin

------------------------------------------------------------------------

June 9, 2005 - 06:55 : Robin Monks

Attachment: http://drupal.org/files/issues/authstats.patch (2.26 KB)

Here is the patch.  Uses a switch to choose between signed in users, all
users, users with permissions or noone.

Robin

------------------------------------------------------------------------

June 9, 2005 - 07:08 : Robin Monks

I tested this patch with various settings on my local install and it
worked fine.

Robin

------------------------------------------------------------------------

June 9, 2005 - 09:30 : Bèr Kessels

Is there a reason why you check for $user->uid?
Whaen someone has "access statistics" set to anonymous users, your
check for $user->uid will override taht settings. Not good IMO.

<?php
$group .= form_radios(t('Display counter values'),
'statistics_display_counter',
variable_get('statistics_display_counter', 0), array('1' => t('For all
users'), '2' => t('For authenticated users'), '3' => t('For priviledged
users'), '0' => t('Disabled')), t('Display how many times given content
has been viewed.'));
?>

is very inconsistent. please use *only* the permissions page to set
permissions, and do not create new permissions-alike settings in any
configuration pages. 

I would say a simple check for user_access('access statistics') will do
the trick

------------------------------------------------------------------------

June 9, 2005 - 09:32 : Bèr Kessels

sorry, i meant to say user_access('access statistics counter'), not
user_access('access statistics').

We already have "access statistics'" an additional "access statistics
counter" for showing users the counter should work

Ber

------------------------------------------------------------------------

June 9, 2005 - 09:49 : Robin Monks

Attachment: http://drupal.org/files/issues/statistics.module (23.1 KB)

OK, here is a patch to that end...

Robin

------------------------------------------------------------------------

June 9, 2005 - 10:00 : Robin Monks

Attachment: http://drupal.org/files/issues/authstats_0.patch (1 KB)

Um, let's just pretend I didn't just upload the entire stats module. 
OK?  OK!

Robin

------------------------------------------------------------------------

June 9, 2005 - 10:05 : Robin Monks

Attachment: http://drupal.org/files/issues/authstats_1.patch (1.06 KB)

Hotfix.

Robin

------------------------------------------------------------------------

June 9, 2005 - 10:11 : Robin Monks

Attachment: http://drupal.org/files/issues/authstats_2.patch (1.11 KB)

Hopefuly the final version.  Thanks to chx and berkes for pulling it
apart ;-)

Robin

------------------------------------------------------------------------

June 9, 2005 - 10:21 : Bèr Kessels

+1 for this patch. it adds functionality, but does not add clutter nor
any config options.

------------------------------------------------------------------------

June 9, 2005 - 19:39 : frjo

+1 I have implemented the patch authstats_2.patch on my site running
Drupal 4.6.1 and it works well.