Page MenuHomePhabricator

Info action page creator/last editor
Closed, ResolvedPublic

Description

action=info reveals the page creator/last editor even if it has been revdel'd/suppressed from those revisions.


Version: unspecified
Severity: normal

Details

Reference
bz39981

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 1:02 AM
bzimport added a project: MediaWiki-General.
bzimport set Reference to bz39981.
Krenair created this task.Sep 4 2012, 6:41 PM

Confirmed. Thanks Krenair!

It looks like it should be a simple fix. I'll post a patch soon.

Created attachment 11070
Check if username is hidden, and if viewer has rights

attachment 0001-bug-39981-Restrict-revdel-suppressed-usernames-on-ac.patch ignored as obsolete

I'm not really familiar with how security patches work - are you going to put this in Gerrit?

About the patch itself - we don't really need to have all those variables defined outside of pageCountInfo and then sent in as arguments, so I would -1 this.

Unfortunately, everything in Gerrit is public, so we'll keep it as a patch until we get the patch pushed into production, then the patch will get pushed into all of the release branches.

I passed everything in since that function is static, and I'm not sure how it's being used elsewhere. If you would like to modify it, feel free to upload another patch here.

Created attachment 11075
Check if username is hidden, and if viewer has rights

Improved it a bit, whitespace fixes, etc. Made it use Revision::newFromRow( $row )->getUserText( Revision::FOR_THIS_USER, $user ); instead of that if/elseif/else block.

While making this I noticed that the Revision class can't take rows as arrays if the keys have field prefixes. (It expects 'deleted' instead of 'rev_deleted'. I assume this is to do with newFromArchiveRow.)
I did try getting the row using fetchObject, but then it expects all the fields to be there, instead of just some.
Unfortunately this meant I had to use an ugly workaround instead: $row['deleted'] = $row['rev_deleted'];
A separate bug should probably be made for this.

attachment 0001-bug-39981-Restrict-revdel-suppressed-usernames-on-ac.patch ignored as obsolete

Aaron did a review last week, and thought the patch looked about right. I'll see if we can get this patch deployed on wmf sites this week.

I'm hoping to get one more fix in for a pretty bad bug before we do another patch release. I'm hoping for late this week, or next week for the release.

It actually looks like this was fixed with gerrit 24098.

I think we will want to backport this to 1.19.3, so I'm going to leave this ticket open until I get that patch in.

madman.enwiki wrote:

The fix has now been backported to 1.20; is that what you meant? According to http://www.mediawiki.org/wiki/MediaWiki_1.19, the final version was three months before InfoAction was revamped.

I didn't realize the feature was put in in 1.20, so no need to backport the fix to 1.19. I'll mention it in the announcement for 1.19.3/1.18.5.

(I checked with Chris before making this public. It doesn't affect any releases and has been fixed in master and production.)