[dw-free] fix logic for editing and deleting community entries
[commit: http://hg.dwscoalition.org/dw-free/rev/188993a48636]
http://bugs.dwscoalition.org/show_bug.cgi?id=2080
This is a logic change/update for accessing (viewing, editing, and deleting
posts). This will mostly impact OpenID users as this will give them a lot
more access than they had before.
Notably: when we do a community import, the OpenID accounts that now own
those posts will be able to manage the comments on them, as well as edit and
delete the posts in question.
This should have no impact on regular users, but as always with a change to
such core logic, there may be some unintended consequences. Please test.
Patch by
mark.
Files modified:
http://bugs.dwscoalition.org/show_bug.cgi?id=2080
This is a logic change/update for accessing (viewing, editing, and deleting
posts). This will mostly impact OpenID users as this will give them a lot
more access than they had before.
Notably: when we do a community import, the OpenID accounts that now own
those posts will be able to manage the comments on them, as well as edit and
delete the posts in question.
This should have no impact on regular users, but as always with a change to
such core logic, there may be some unintended consequences. Please test.
Patch by
![[staff profile]](https://www.dreamwidth.org/img/silk/identity/user_staff.png)
Files modified:
- cgi-bin/LJ/S2.pm
- cgi-bin/LJ/S2/EntryPage.pm
- cgi-bin/LJ/S2/ReplyPage.pm
- cgi-bin/LJ/Talk.pm
- cgi-bin/LJ/User.pm
- cgi-bin/ljlib.pl
- cgi-bin/ljprotocol.pl
- cgi-bin/weblib.pl
- htdocs/editjournal.bml
-------------------------------------------------------------------------------- diff -r 3414d4ea320e -r 188993a48636 cgi-bin/LJ/S2.pm --- a/cgi-bin/LJ/S2.pm Mon Jan 04 19:40:25 2010 +0000 +++ b/cgi-bin/LJ/S2.pm Tue Jan 05 01:00:54 2010 +0000 @@ -2371,8 +2371,8 @@ use strict; use strict; sub UserLite { - my ($ctx,$username) = @_; - my $u = LJ::load_user($username); + my ($ctx,$user) = @_; + my $u = LJ::load_user($user); return LJ::S2::UserLite($u); } @@ -2502,7 +2502,7 @@ sub get_url # now get data from one of two paths, depending on if we were given a UserLite # object or a string for the username, so make sure we have the username. if (ref $obj eq 'HASH') { - $user = $obj->{username}; + $user = $obj->{user}; } else { $user = $obj; } @@ -2979,7 +2979,7 @@ sub _Comment__get_link my ($ctx, $this, $key) = @_; my $page = get_page(); my $u = $page->{'_u'}; - my $post_user = $page->{'entry'} ? $page->{'entry'}->{'poster'}->{'username'} : undef; + my $post_user = $page->{'entry'} ? $page->{'entry'}->{'poster'}->{'user'} : undef; my $com_user = $this->{'poster'} ? $this->{'poster'}->{'user'} : undef; my $remote = LJ::get_remote(); my $null_link = { '_type' => 'Link', '_isnull' => 1 }; @@ -3336,6 +3336,8 @@ sub Page__print_trusted { my ($ctx, $this, $key) = @_; + # use 'username' so that we can put 'foo.site.com' in the hash instead of + # having to look up their 'ext_nnnn' name my $username = $this->{journal}->{username}; my $fullkey = "$username-$key"; @@ -3588,8 +3590,8 @@ sub _Entry__get_link sub _Entry__get_link { my ($ctx, $this, $key) = @_; - my $journal = $this->{'journal'}->{'username'}; - my $poster = $this->{'poster'}->{'username'}; + my $journal = $this->{'journal'}->{'user'}; + my $poster = $this->{'poster'}->{'user'}; my $remote = LJ::get_remote(); my $null_link = { '_type' => 'Link', '_isnull' => 1 }; my $journalu = LJ::load_user($journal); @@ -3764,7 +3766,7 @@ sub EntryPage__print_multiform_start return unless $this->{'multiform_on'}; $S2::pout->("<form style='display: inline' method='post' action='$LJ::SITEROOT/talkmulti' name='multiform'>\n" . LJ::html_hidden("ditemid", $this->{'entry'}->{'itemid'}, - "journal", $this->{'entry'}->{'journal'}->{'username'}) . "\n"); + "journal", $this->{'entry'}->{'journal'}->{'user'}) . "\n"); } sub Page__print_control_strip diff -r 3414d4ea320e -r 188993a48636 cgi-bin/LJ/S2/EntryPage.pm --- a/cgi-bin/LJ/S2/EntryPage.pm Mon Jan 04 19:40:25 2010 +0000 +++ b/cgi-bin/LJ/S2/EntryPage.pm Tue Jan 05 01:00:54 2010 +0000 @@ -16,6 +16,8 @@ use strict; package LJ::S2; + +use Carp; sub EntryPage { @@ -112,7 +114,7 @@ sub EntryPage 'userpicref' => \%userpic, 'userref' => \%user, # user object is cached from call just made in EntryPage_entry - 'up' => LJ::load_user($s2entry->{'poster'}->{'username'}), + 'up' => LJ::load_user($s2entry->{'poster'}->{'user'}), 'viewall' => $viewall, 'expand_all' => $opts->{expand_all}, }; @@ -214,9 +216,16 @@ sub EntryPage if ($pu) { $poster = UserLite($pu); } else { + # I can't determine where this code is called, if it ever is? so for now, + # let's spit out a backtrace so we can figure out how this case happens. we need + # to fix it since the journal_type is wrong in some cases. + # FIXME: watch logs + Carp::cluck "LJ::S2::EntryPage faked a UserLite; userpost=$com->{userpost}:"; + $poster = { '_type' => 'UserLite', 'username' => $com->{'userpost'}, + 'user' => $com->{'userpost'}, 'name' => $com->{'userpost'}, # we don't have this, so fake it 'journal_type' => 'P', # fake too, but only people can post, so correct }; @@ -335,7 +344,7 @@ sub EntryPage my $cmt = LJ::Comment->new($u, dtalkid => $i->{talkid}); my $has_threads = scalar @{$i->{'replies'}}; - my $poster = $i->{'poster'} ? $i->{'poster'}{'username'} : ""; + my $poster = $i->{'poster'} ? $i->{'poster'}{'user'} : ""; my @child_ids = map { $_->{'talkid'} } @{$i->{'replies'}}; $cmtinfo->{$i->{talkid}} = { rc => \@child_ids, diff -r 3414d4ea320e -r 188993a48636 cgi-bin/LJ/S2/ReplyPage.pm --- a/cgi-bin/LJ/S2/ReplyPage.pm Mon Jan 04 19:40:25 2010 +0000 +++ b/cgi-bin/LJ/S2/ReplyPage.pm Tue Jan 05 01:00:54 2010 +0000 @@ -135,7 +135,7 @@ sub ReplyPage $opts->{status} = "404 Not Found"; return "<p>This comment has been deleted; you cannot reply to it.</p>"; } - if ($parpost->{'state'} eq 'S' && !LJ::Talk::can_unscreen($remote, $u, $s2entry->{'poster'}->{'username'}, undef)) { + if ($parpost->{'state'} eq 'S' && !LJ::Talk::can_unscreen($remote, $u, $s2entry->{'poster'}->{'user'}, undef)) { $opts->{'handler_return'} = 403; return; } diff -r 3414d4ea320e -r 188993a48636 cgi-bin/LJ/Talk.pm --- a/cgi-bin/LJ/Talk.pm Mon Jan 04 19:40:25 2010 +0000 +++ b/cgi-bin/LJ/Talk.pm Tue Jan 05 01:00:54 2010 +0000 @@ -109,9 +109,9 @@ sub link_bar my $opts = shift; my ($u, $up, $remote, $headref, $itemid) = map { $opts->{$_} } qw(u up remote headref itemid); - my $ret; - my @linkele; + # we want user objects, so make sure they are + ( $u, $up, $remote ) = map { LJ::want_user( $_ ) } ( $u, $up, $remote ); my $mlink = sub { my ($url, $piccode) = @_; @@ -126,6 +126,7 @@ sub link_bar my $entry = LJ::Entry->new($u, ditemid => $itemid); # << Previous + my @linkele; push @linkele, $mlink->("$LJ::SITEROOT/go?${jargent}itemid=$itemid&dir=prev", "prev_entry"); $$headref .= "<link href='$LJ::SITEROOT/go?${jargent}itemid=$itemid&dir=prev' rel='Previous' />\n"; @@ -137,8 +138,8 @@ sub link_bar # edit entry - if we have a remote, and that person can manage # the account in question, OR, they posted the entry, and have # access to the community in question - if (defined $remote && (LJ::can_manage($remote, $u) || - (LJ::u_equals($remote, $up) && LJ::can_use_journal($up->{userid}, $u->{user}, {})))) + if ( defined $remote && ( $remote->can_manage( $u ) || + ( $remote->equals( $up ) && $up->can_post_to( $u ) ) ) ) { push @linkele, $mlink->("$LJ::SITEROOT/editjournal?${jargent}itemid=$itemid", "editentry"); } @@ -166,14 +167,14 @@ sub link_bar push @linkele, $mlink->("$LJ::SITEROOT/go?${jargent}itemid=$itemid&dir=next", "next_entry"); $$headref .= "<link href='$LJ::SITEROOT/go?${jargent}itemid=$itemid&dir=next' rel='Next' />\n"; - if (@linkele) { - $ret .= BML::fill_template("standout", { + my $ret; + if ( @linkele ) { + $ret = BML::fill_template("standout", { 'DATA' => "<table><tr><td valign='middle'>" . join(" ", @linkele) . "</td></tr></table>", }); } - return $ret; } diff -r 3414d4ea320e -r 188993a48636 cgi-bin/LJ/User.pm --- a/cgi-bin/LJ/User.pm Mon Jan 04 19:40:25 2010 +0000 +++ b/cgi-bin/LJ/User.pm Tue Jan 05 01:00:54 2010 +0000 @@ -3416,9 +3416,29 @@ sub can_manage { # can $u post to $targetu? sub can_post_to { - my ($u, $targetu) = @_; - - return LJ::can_use_journal($u->id, $targetu->user); + my ( $u, $targetu, %opts ) = @_; + croak "Invalid users passed to LJ::User->can_post_to." + unless LJ::isu( $u ) && LJ::isu( $targetu ); + + # if it's you, and you're a person, you can post to it + return 1 if $u->is_person && $u->equals( $targetu ); + + # else, you have to be an individual and the target has to be a comm + return 0 unless $u->is_individual && $targetu->is_community; + + # check if user has access explicit posting access + return 1 if LJ::check_rel( $targetu, $u, 'P' ); + + # let's check if this community is allowing post access to non-members + if ( $targetu->prop( 'nonmember_posting' ) ) { + my ( $ml, $pl ) = LJ::get_comm_settings( $targetu ); + return 1 if $pl eq 'members'; + } + + # is the poster an admin for this community? admins can always post + return 1 if $u->can_manage( $targetu ); + + return 0; } diff -r 3414d4ea320e -r 188993a48636 cgi-bin/ljlib.pl --- a/cgi-bin/ljlib.pl Mon Jan 04 19:40:25 2010 +0000 +++ b/cgi-bin/ljlib.pl Tue Jan 05 01:00:54 2010 +0000 @@ -1594,55 +1594,6 @@ sub get_sitekeyword_id { # <LJFUNC> -# name: LJ::can_use_journal -# class: -# des: -# info: -# args: -# des-: -# returns: -# </LJFUNC> -sub can_use_journal -{ - &nodb; - my ($posterid, $reqownername, $res) = @_; - - ## find the journal owner's info - my $uowner = LJ::load_user($reqownername); - unless ($uowner) { - $res->{'errmsg'} = "Journal \"$reqownername\" does not exist."; - return 0; - } - my $ownerid = $uowner->{'userid'}; - - # the 'ownerid' necessity came first, way back when. but then - # with clusters, everything needed to know more, like the - # journal's dversion and clusterid, so now it also returns the - # user row. - $res->{'ownerid'} = $ownerid; - $res->{'u_owner'} = $uowner; - - ## check if user has access - return 1 if LJ::check_rel($ownerid, $posterid, 'P'); - - # let's check if this community is allowing post access to non-members - LJ::load_user_props($uowner, "nonmember_posting"); - if ($uowner->{'nonmember_posting'}) { - my $dbr = LJ::get_db_reader() or die "nodb"; - my $postlevel = $dbr->selectrow_array("SELECT postlevel FROM ". - "community WHERE userid=$ownerid"); - return 1 if $postlevel eq 'members'; - } - - # is the poster an admin for this community? - return 1 if LJ::can_manage($posterid, $uowner); - - $res->{'errmsg'} = "You do not have access to post to this journal."; - return 0; -} - - -# <LJFUNC> # name: LJ::load_talk_props2 # class: # des: diff -r 3414d4ea320e -r 188993a48636 cgi-bin/ljprotocol.pl --- a/cgi-bin/ljprotocol.pl Mon Jan 04 19:40:25 2010 +0000 +++ b/cgi-bin/ljprotocol.pl Tue Jan 05 01:00:54 2010 +0000 @@ -2763,9 +2763,6 @@ sub check_altusage { my ($req, $err, $flags) = @_; - # see note in ljlib.pl::can_use_journal about why we return - # both 'ownerid' and 'u_owner' in $flags - my $alt = $req->{'usejournal'}; my $u = $flags->{'u'}; unless ($u) { @@ -2786,30 +2783,27 @@ sub check_altusage # complain if the username is invalid return fail($err,206) unless LJ::canonical_username($alt); + # we are going to load the alt user + $flags->{u_owner} = LJ::load_user( $alt ); + $flags->{ownerid} = $flags->{u_owner} ? $flags->{u_owner}->id : undef; my $r = eval { BML::get_request() }; + $r->notes->{journalid} = $flags->{ownerid} + if $r && !$r->notes->{journalid}; # allow usage if we're told explicitly that it's okay - if ($flags->{'usejournal_okay'}) { - $flags->{'u_owner'} = LJ::load_user($alt); - $flags->{'ownerid'} = $flags->{'u_owner'}->{'userid'}; - $r->notes->{journalid} = $flags->{'ownerid'} - if $r && !$r->notes->{journalid}; - return 1 if $flags->{'ownerid'}; - return fail($err,206); + if ( $flags->{usejournal_okay} ) { + return 1 if $flags->{ownerid}; + return fail( $err, 206 ); } - # otherwise, check for access: - my $info = {}; - my $canuse = LJ::can_use_journal($u->{'userid'}, $alt, $info); - $flags->{'ownerid'} = $info->{'ownerid'}; - $flags->{'u_owner'} = $info->{'u_owner'}; - $r->notes->{journalid} = $flags->{'ownerid'} - if $r && !$r->notes->{journalid}; + # or, if they have explicitly said to ignore canuse + return 1 if $flags->{ignorecanuse}; - return 1 if $canuse || $flags->{'ignorecanuse'}; + # otherwise, check for access + return 1 if $u->can_post_to( $flags->{u_owner} ); - # not allowed to access it - return fail($err,300); + # not allowed to access it, bad user, no post + return fail( $err, 300 ); } sub authenticate diff -r 3414d4ea320e -r 188993a48636 cgi-bin/weblib.pl --- a/cgi-bin/weblib.pl Mon Jan 04 19:40:25 2010 +0000 +++ b/cgi-bin/weblib.pl Tue Jan 05 01:00:54 2010 +0000 @@ -1841,10 +1841,8 @@ PREVIEW { $out .= "<div id='submitbar' class='pkg'>\n\n"; - $out .= "<div id='security_container'>\n"; - $out .= "<label for='security'>" . BML::ml('entryform.security2') . " </label>\n"; - # Security + my $secbar = 0; if ($opts->{'mode'} eq "update" || !$opts->{'disabled_save'}) { my $usejournalu = LJ::load_user($opts->{usejournal}); my $is_comm = $usejournalu && $usejournalu->is_comm ? 1 : 0; @@ -1877,6 +1875,12 @@ PREVIEW if ( scalar @trust_groups && !$is_comm ) { push @secs, ("custom", $string_custom); push @secopts, ("onchange" => "customboxes()"); + } + + if ( @secs ) { + $secbar = 1; + $out .= "<div id='security_container'>\n"; + $out .= "<label for='security'>" . BML::ml('entryform.security2') . " </label>\n"; } $out .= LJ::html_select({ 'id' => "security", 'name' => 'security', 'include_ids' => 1, @@ -1924,7 +1928,8 @@ PREVIEW $out .= LJ::html_submit('action:update', BML::ml('entryform.update4'), { 'onclick' => $onclick, 'class' => 'submit', 'id' => 'formsubmit', - 'tabindex' => $tabindex->() }) . " \n"; } + 'tabindex' => $tabindex->() }) . " \n"; + } if ($opts->{'mode'} eq "edit") { my $onclick = ""; @@ -1934,7 +1939,7 @@ PREVIEW $out .= LJ::html_submit('action:save', BML::ml('entryform.save'), { 'onclick' => $onclick, 'disabled' => $opts->{'disabled_save'}, 'tabindex' => $tabindex->() }) . " \n"; - } else { + } elsif ( $opts->{maintainer_mode} ) { $out .= LJ::html_submit('action:savemaintainer', BML::ml('entryform.save.maintainer'), { 'onclick' => $onclick, 'disabled' => !$opts->{'maintainer_mode'}, 'tabindex' => $tabindex->() }) . " \n"; @@ -1961,7 +1966,7 @@ PREVIEW } } - $out .= "</div><!-- end #security_container -->\n\n"; + $out .= "</div><!-- end #security_container -->\n\n" if $secbar; $out .= "</div><!-- end #submitbar -->\n\n"; $out .= "</div><!-- end #entry-form-wrapper -->\n\n"; diff -r 3414d4ea320e -r 188993a48636 htdocs/editjournal.bml --- a/htdocs/editjournal.bml Mon Jan 04 19:40:25 2010 +0000 +++ b/htdocs/editjournal.bml Tue Jan 05 01:00:54 2010 +0000 @@ -29,7 +29,7 @@ body<= return LJ::bad_input( $ML{'error.invalidauth'} ) unless $u; return LJ::bad_input( $ML{'error.person'} ) - unless $u->is_person; + unless $u->is_individual; # are we modify a community post? my $usejournal = $GET{'usejournal'} || $POST{'usejournal'} || $GET{'journal'}; @@ -127,6 +127,11 @@ body<= my $u_for_entry = $usejournal ? $usejournal_u : $u; my $entry_obj = LJ::Entry->new($u_for_entry, ditemid => $ditemid); + # this is a sanity check, make sure the entry we got is visible to the + # person trying to edit it. + return "<?h1 $ML{'Error'} h1?><?p $ML{'.error.nofind'} p?>" + unless $entry_obj->visible_to( $remote ); + # do getevents request my %res = (); LJ::do_request({ 'mode' => 'getevents', @@ -137,7 +142,8 @@ body<= 'itemid' => $itemid }, \%res, { "noauth" => 1, - 'u' => $u } + 'u' => $u, + ignorecanuse => 1 } ); # was there a protocol error? @@ -156,6 +162,7 @@ body<= $disabled_delete = ! LJ::can_delete_journal_item($u, $usejournal_u); $disabled_save++; } + $disabled_save++ if $usejournal && ! $u->can_post_to( $usejournal_u ); $disabled_spamdelete = $disabled_delete || !$usejournal || ($res{'events_1_poster'} eq $u->{'user'}); # read-only posters and journals cannot be edited --------------------------------------------------------------------------------