changelog | [dw-free] tag editing fails silently in comms with restricted permissions (Reply)

[commit: http://hg.dwscoalition.org/dw-free/rev/dbaae7e52705]

http://bugs.dwscoalition.org/show_bug.cgi?id=1623

If you are editing tags on a post in a community and don't have access, show
an error message indicating what went wrong. Prior to this patch, the tag
system failed silently, and you didn't know your edits didn't save.

Patch by
afuna.

Files modified:
bin/upgrading/en.dat
cgi-bin/LJ/Tags.pm
cgi-bin/ljlib-local.pl
cgi-bin/ljprotocol.pl
--------------------------------------------------------------------------------
diff -r 14f4ca2b8912 -r dbaae7e52705 bin/upgrading/en.dat
--- a/bin/upgrading/en.dat	Mon Nov 23 11:33:59 2009 -0600
+++ b/bin/upgrading/en.dat	Tue Nov 24 19:51:11 2009 +0000
@@ -3467,9 +3467,13 @@ subscribe_interface.save=Save
 
 subscribe_interface.special_subs.note=These notification options are only available by email and will not show in your [[sitenameabbrev]] Inbox.
 
+subscribe_interface.subs.total=You are using [[active]] out of [[max_active]] active subscriptions.
+
 taglib.error.access=You are not allowed to tag entries in this journal.
 
-subscribe_interface.subs.total=You are using [[active]] out of [[max_active]] active subscriptions.
+taglib.error.add=You are not allowed to create new tags for this journal; the entry was not tagged with [[tags]].
+
+taglib.error.delete=You are not allowed to delete tags from entries for this journal; the entry is still tagged with [[tags]].
 
 taglib.error.invalid=The following tag name is invalid: [[tagname]]
 
diff -r 14f4ca2b8912 -r dbaae7e52705 cgi-bin/LJ/Tags.pm
--- a/cgi-bin/LJ/Tags.pm	Mon Nov 23 11:33:59 2009 -0600
+++ b/cgi-bin/LJ/Tags.pm	Tue Nov 24 19:51:11 2009 +0000
@@ -708,6 +708,10 @@ sub update_logtags {
     my $utags = LJ::Tags::get_usertags($u);
     return undef unless $utags;
 
+    # for errors that we want to skip over silently instead of failing, but still report at the end
+    my @skippable_errors;
+    my @unauthorized_add;
+
     # take arrayrefs of tag strings and stringify them for validation
     my @to_create;
     foreach my $verb (qw(add set delete)) {
@@ -734,7 +738,10 @@ sub update_logtags {
             } else {
                 # if we're not creating, who cares, just skip; also skip if the keyword
                 # is not really a tag (don't promote it)
-                next unless $kwid && $utags->{$kwid};
+                unless ( $kwid && $utags->{$kwid} ) {
+                    push @unauthorized_add, $kw;
+                    next;
+                }
             }
 
             # add the ids to the list, and save to create later if needed
@@ -770,6 +777,11 @@ sub update_logtags {
 
     # now don't readd things we already have
     delete $add{$_} foreach keys %{$tags};
+
+    # populate the errref, but don't actually return.
+    push @skippable_errors, LJ::Lang::ml( "taglib.error.add", { tags => join( ", ", @unauthorized_add ) } ) if @unauthorized_add;
+    push @skippable_errors, LJ::Lang::ml( "taglib.error.delete", { tags => join( ", ", map { $utags->{$_}->{name} } keys %delete ) } ) if %delete && ! $can_control ;
+    $err->( join " ", @skippable_errors ) if @skippable_errors;
 
     # but delete nothing if we're not a controller
     %delete = () unless $can_control || $opts->{force};
diff -r 14f4ca2b8912 -r dbaae7e52705 cgi-bin/ljlib-local.pl
--- a/cgi-bin/ljlib-local.pl	Mon Nov 23 11:33:59 2009 -0600
+++ b/cgi-bin/ljlib-local.pl	Tue Nov 24 19:51:11 2009 +0000
@@ -1,8 +1,6 @@ package JF;
-package JF;
+# This file is loaded early on.  If you want to add your own methods or load
+# modules or other things and you don't want to create a hook module you can
+# stick them here.
 
-use strict;
-
-use DW::Pay;
-use DW::Hooks;
 
 1;
diff -r 14f4ca2b8912 -r dbaae7e52705 cgi-bin/ljprotocol.pl
--- a/cgi-bin/ljprotocol.pl	Mon Nov 23 11:33:59 2009 -0600
+++ b/cgi-bin/ljprotocol.pl	Tue Nov 24 19:51:11 2009 +0000
@@ -1801,22 +1801,28 @@ sub editevent
            $req->{security} =~ /^(?:public|private|usemask)$/;
 
     my $do_tags = $req->{props} && defined $req->{props}->{taglist};
+    my $do_tags_security;
+    my $entry_tags;
+
     if ($oldevent->{security} ne $security || $qallowmask != $oldevent->{allowmask}) {
         # FIXME: this is a hopefully temporary hack which deletes tags from the entry
         # when the security has changed.  the real fix is to make update_logtags aware
         # of security changes so it can update logkwsum appropriately.
 
-        unless ($do_tags) {
-            # we need to fix security on this entry's tags, but the user didn't give us a tag list
-            # to work with, so we have to go get the tags on the entry, and construct a tag list,
-            # in order to pass to update_logtags down at the bottom of this whole update
-            my $tags = LJ::Tags::get_logtags($uowner, $itemid);
-            $tags = $tags->{$itemid};
-            $req->{props}->{taglist} = join(',', sort values %{$tags || {}});
-            $do_tags = 1; # bleh, force the update later
+        # we need to fix security on this entry's tags; if the user didn't give us a
+        # tag list to work with, we use the existing tags on this entry
+        unless ( $do_tags ) {
+            $entry_tags  = LJ::Tags::get_logtags($uowner, $itemid);
+            $entry_tags = $entry_tags->{$itemid};
+            $entry_tags = join(',', sort values %{$entry_tags || {}});
+            $req->{props}->{taglist} = $entry_tags;
         }
 
-        LJ::Tags::delete_logtags($uowner, $itemid);
+        # FIXME: temporary hack until we can make update_logtags recognize entry security edits
+        if ( LJ::Tags::can_control_tags( $uowner, $u ) || LJ::Tags::can_add_tags( $uowner, $u ) ) {
+            my $delete = LJ::Tags::delete_logtags( $uowner, $itemid );
+            $do_tags_security = 1;
+        }
     }
 
     my $qyear = $req->{'year'}+0;
@@ -1898,14 +1904,32 @@ sub editevent
     $req->{'props'}->{'revnum'} = ($curprops{$itemid}->{'revnum'} || 0) + 1;
     $req->{'props'}->{'revtime'} = time();
 
+    if ( $do_tags ) {
+        # we only want to update the tags if they've been modified
+        # so load the original entry tags
+        unless ( $entry_tags ) {
+            $entry_tags  = LJ::Tags::get_logtags($uowner, $itemid);
+            $entry_tags = $entry_tags->{$itemid};
+            $entry_tags = join(',', sort values %{$entry_tags || {}});
+        }
+
+        my $request_tags = [];
+        LJ::Tags::is_valid_tagstring( $req->{props}->{taglist}, $request_tags );
+        $request_tags = join( ",", sort @{ $request_tags || [] } );
+        $do_tags = ( $request_tags ne $entry_tags );
+    }
+
     # handle tags if they're defined
-    if ($do_tags) {
+    if ( $do_tags || $do_tags_security ) {
         my $tagerr = "";
         my $rv = LJ::Tags::update_logtags($uowner, $itemid, {
                 set_string => $req->{props}->{taglist},
                 remote => $u,
                 err_ref => \$tagerr,
             });
+
+        # we only want to fail if we tried to edit the tags, not if we just tried to edit the security
+        return fail( $err, 157, $tagerr ) if $tagerr && $do_tags;
     }
 
     # handle the props
--------------------------------------------------------------------------------
[dw-free] tag editing fails silently in comms with restricted permissions

Post a comment in response:
