Clean up the way we check the viewall argument so that we have one
central place to maintain that logic.
Patch by kareila.
diffs (213 lines):
diff -r edf86d263b7e -r aa45028acb2a cgi-bin/LJ/S2/DayPage.pm
--- a/cgi-bin/LJ/S2/DayPage.pm Fri Oct 09 03:30:13 2009 +0000
+++ b/cgi-bin/LJ/S2/DayPage.pm Fri Oct 09 04:11:27 2009 +0000
@@ -49,14 +49,10 @@ sub DayPage
if ($remote) {
# do they have the viewall priv?
- if ( $get->{viewall} && $remote->has_priv( "canview", "suspended" ) ) {
- LJ::statushistory_add($u->{'userid'}, $remote->{'userid'},
- "viewall", "day: $user, statusvis:
$u->{'statusvis'}");
- $viewall = $remote->has_priv( 'canview', '*' );
- $viewsome = $viewall || $remote->has_priv( 'canview', 'suspended'
);
- }
+ ( $viewall, $viewsome ) =
+ $remote->view_priv_check( $u, $get->{viewall}, 'day' );
- if ($remote->{'userid'} == $u->{'userid'} || $viewall) {
+ if ( $viewall || $remote->equals( $u ) ) {
$secwhere = ""; # see everything
} elsif ( $remote->is_individual ) {
my $gmask = $u->is_community ? $remote->member_of( $u ) :
$u->trustmask( $remote );
diff -r edf86d263b7e -r aa45028acb2a cgi-bin/LJ/S2/EntryPage.pm
--- a/cgi-bin/LJ/S2/EntryPage.pm Fri Oct 09 03:30:13 2009 +0000
+++ b/cgi-bin/LJ/S2/EntryPage.pm Fri Oct 09 04:11:27 2009 +0000
@@ -19,11 +19,11 @@ sub EntryPage
# setup viewall options
my ($viewall, $viewsome) = (0, 0);
- if ( $get->{viewall} && $remote && $remote->has_priv( 'canview',
'suspended' ) ) {
- # we don't log here, as we don't know what entry we're viewing yet.
the logging
- # is done when we call EntryPage_entry below.
- $viewall = $remote->has_priv( 'canview', '*' );
- $viewsome = $viewall || $remote->has_priv( 'canview', 'suspended' );
+ if ( $remote ) {
+ # we don't log here, as we don't know what entry we're viewing yet.
+ # the logging is done when we call EntryPage_entry below.
+ ( $viewall, $viewsome ) =
+ $remote->view_priv_check( $u, $get->{viewall} );
}
my ($entry, $s2entry) = EntryPage_entry($u, $remote, $opts);
@@ -407,10 +407,8 @@ sub EntryPage_entry
my $canview = $get->{viewall} && $remote && $remote->has_priv( "canview" );
my ($viewall, $viewsome) = (0, 0);
if ($canview) {
- LJ::statushistory_add($u->{'userid'}, $remote->{'userid'},
- "viewall", "entry: $u->{'user'}, itemid:
$itemid, statusvis: $u->{'statusvis'}");
- $viewall = $remote->has_priv( 'canview', '*' );
- $viewsome = $viewall || $remote->has_priv( 'canview', 'suspended' );
+ ( $viewall, $viewsome ) =
+ $remote->view_priv_check( $u, $get->{viewall}, 'entry', $itemid );
}
# check using normal rules
diff -r edf86d263b7e -r aa45028acb2a cgi-bin/LJ/S2/MonthPage.pm
--- a/cgi-bin/LJ/S2/MonthPage.pm Fri Oct 09 03:30:13 2009 +0000
+++ b/cgi-bin/LJ/S2/MonthPage.pm Fri Oct 09 04:11:27 2009 +0000
@@ -49,14 +49,10 @@ sub MonthPage
if ($remote) {
# do they have the viewall priv?
- if ( $get->{viewall} && $remote && $remote->has_priv( "canview",
"suspended" ) ) {
- LJ::statushistory_add($u->{'userid'}, $remote->{'userid'},
- "viewall", "month: $user, statusvis:
$u->{'statusvis'}");
- $viewall = $remote->has_priv( 'canview', '*' );
- $viewsome = $viewall || $remote->has_priv( 'canview', 'suspended'
);
- }
+ ( $viewall, $viewsome ) =
+ $remote->view_priv_check( $u, $get->{viewall}, 'month' );
- if ($remote->{'userid'} == $u->{'userid'} || $viewall) {
+ if ( $viewall || $remote->equals( $u ) ) {
$secwhere = ""; # see everything
} elsif ( $remote->is_individual ) {
my $gmask = $u->is_community ? $remote->member_of( $u ) :
$u->trustmask( $remote );
diff -r edf86d263b7e -r aa45028acb2a cgi-bin/LJ/S2/RecentPage.pm
--- a/cgi-bin/LJ/S2/RecentPage.pm Fri Oct 09 03:30:13 2009 +0000
+++ b/cgi-bin/LJ/S2/RecentPage.pm Fri Oct 09 04:11:27 2009 +0000
@@ -76,11 +76,9 @@ sub RecentPage
# do they want to view all entries, regardless of security?
my $viewall = 0;
my $viewsome = 0;
- if ( $get->{viewall} && $remote && $remote->has_priv( "canview",
"suspended" ) ) {
- LJ::statushistory_add($u->{'userid'}, $remote->{'userid'},
- "viewall", "lastn: $user, statusvis:
$u->{'statusvis'}");
- $viewall = $remote->has_priv( 'canview', '*' );
- $viewsome = $viewall || $remote->has_priv( 'canview', 'suspended' );
+ if ( $remote ) {
+ ( $viewall, $viewsome ) =
+ $remote->view_priv_check( $u, $get->{viewall}, 'lastn' );
}
## load the itemids
diff -r edf86d263b7e -r aa45028acb2a cgi-bin/LJ/User.pm
--- a/cgi-bin/LJ/User.pm Fri Oct 09 03:30:13 2009 +0000
+++ b/cgi-bin/LJ/User.pm Fri Oct 09 04:11:27 2009 +0000
@@ -4408,6 +4408,42 @@ sub view_control_strip {
}
+# BE VERY CAREFUL about the return values and arguments you pass to this
+# method. please understand the security implications of this, and how to
+# properly and safely use it.
+#
+sub view_priv_check {
+ my ( $remote, $u, $requested, $page, $itemid ) = @_;
+
+ # $requested is set to the 'viewall' GET argument. this should ONLY be on
if the
+ # user is EXPLICITLY requesting to view something they can't see normally.
most
+ # of the time this is off, so we can bail now.
+ return unless $requested;
+
+ # now check the rest of our arguments for validity
+ return unless LJ::isu( $remote ) && LJ::isu( $u );
+ return if defined $page && $page !~ /^\w+$/;
+ return if defined $itemid && $itemid !~ /^\d+$/;
+
+ # viewsome = "this user can view suspended content"
+ my $viewsome = $remote->has_priv( canview => 'suspended' );
+
+ # viewall = "this user can view all content, even private"
+ my $viewall = $viewsome && $remote->has_priv( canview => '*' );
+
+ # make sure we log the content being viewed
+ if ( $viewsome && $page ) {
+ my $user = $u->user;
+ $user .= ", itemid: $itemid" if defined $itemid;
+ my $sv = $u->statusvis;
+ LJ::statushistory_add( $u->userid, $remote->userid, 'viewall',
+ "$page: $user, statusvis: $sv");
+ }
+
+ return wantarray ? ( $viewall, $viewsome ) : $viewsome;
+}
+
+
########################################################################
### 25. Subscription, Notifiction, and Messaging Functions
diff -r edf86d263b7e -r aa45028acb2a htdocs/allpics.bml
--- a/htdocs/allpics.bml Fri Oct 09 03:30:13 2009 +0000
+++ b/htdocs/allpics.bml Fri Oct 09 04:11:27 2009 +0000
@@ -50,10 +50,8 @@
# no need for viewsome, due to the fact that none of this is private
anyway. just
# allow anybody with any version of viewall to see userpics for non-V
statusvis users
- if ( $GET{viewall} && $remote && $remote->has_priv( 'canview', 'suspended'
) ) {
- LJ::statushistory_add($u->{'userid'}, $remote->{'userid'},
- "viewall", "allpics: $u->{'user'}, statusvis: "
. $u->statusvis);
- } else {
+
+ unless ( $remote && $remote->view_priv_check( $u, $GET{viewall}, 'allpics'
) ) {
if ($u->is_suspended) {
$title = $ML{'error.suspended.title'};
$body = "<?h1 $ML{'error.suspended.name'} h1?><?p " .
diff -r edf86d263b7e -r aa45028acb2a htdocs/talkread.bml
--- a/htdocs/talkread.bml Fri Oct 09 03:30:13 2009 +0000
+++ b/htdocs/talkread.bml Fri Oct 09 04:11:27 2009 +0000
@@ -155,11 +155,9 @@ body<=
# set viewall/viewsome
my $viewall = 0;
my $viewsome = 0;
- if ( $GET{viewall} && $remote && $remote->has_priv( 'canview', 'suspended'
) ) {
- LJ::statushistory_add($u->{'userid'}, $remote->{'userid'},
- "viewall", "entry: $u->{'user'}, itemid:
$item->{'itemid'}, statusvis: " . $u->statusvis);
- $viewall = $remote && $remote->has_priv( 'canview', '*' );
- $viewsome = $viewall || ( $remote && $remote->has_priv( 'canview',
'suspended' ) );
+ if ( $remote ) {
+ ( $viewall, $viewsome ) =
+ $remote->view_priv_check( $u, $GET{viewall}, 'entry',
$item->{itemid} );
}
# check for deleted/suspended/security
diff -r edf86d263b7e -r aa45028acb2a htdocs/userinfo.bml
--- a/htdocs/userinfo.bml Fri Oct 09 03:30:13 2009 +0000
+++ b/htdocs/userinfo.bml Fri Oct 09 04:11:27 2009 +0000
@@ -127,10 +127,9 @@ body<=
# can't view suspended/deleted profiles unless you have viewall
my $viewall = 0;
- if ( $GET{viewall} && $remote && $remote->has_priv( "canview", "suspended"
) ) {
- $viewall = $remote->has_priv( 'canview', '*' ) ? 1 : 0;
- LJ::statushistory_add( $u->id, $remote->id, "viewall", "userinfo: " .
$u->user . ", statusvis: " . $u->statusvis );
- } else {
+ ( $viewall ) = $remote->view_priv_check( $u, $GET{viewall}, 'userinfo' )
+ if $remote;
+ unless ( $viewall ) {
if ( $u->is_suspended ) {
BML::set_status(403); # 403 Forbidden
$title = $ML{'error.suspended.title'};
diff -r edf86d263b7e -r aa45028acb2a htdocs/view/index.bml
--- a/htdocs/view/index.bml Fri Oct 09 03:30:13 2009 +0000
+++ b/htdocs/view/index.bml Fri Oct 09 04:11:27 2009 +0000
@@ -56,14 +56,11 @@
# do they have the viewall priv?
my $viewall = 0;
my $viewsome = 0;
- if ( $GET{'viewall'} && $remote && $remote->has_priv( "canview" ) ) {
- LJ::statushistory_add($u->{'userid'}, $remote->{'userid'},
- "viewall", "month: $u->{'user'}, statusvis: " .
$u->statusvis);
- $viewall = $remote->has_priv( 'canview', '*' );
- $viewsome = $viewall || $remote->has_priv( 'canview', 'suspended' );
-
- # check for deleted/suspended
- } else {
+ if ( $remote ) {
+ ( $viewall, $viewsome ) =
+ $remote->view_priv_check( $u, $GET{viewall}, 'month' );
+ }
+ unless ( $viewsome ) {
# check suspended
if ($u->is_suspended) {
_______________________________________________
dw-changelog mailing list
[email protected]
http://lists.dwscoalition.org/mailman/listinfo/dw-changelog
