UserContributions endpoint: filter suppressed revisions

ContributionsLookup needs to have the acting user (the authority) passde
in explicitly, so suppressed user contributions can be filtered according
to the user's permissions.

Bug: T252202
Change-Id: I94098f87ae45cd4e1db4a7168bf6e9478e9e32fc

includes/Rest/Handler/UserContributionsHandler.php

@@ -46,7 +46,8 @@ class UserContributionsHandler extends Handler {
 		// can make it segment_size per ticket, but wanted to be consistent with search endpoint.
 		$limit = $this->getValidatedParams()['limit'];
 		$segment = $this->getValidatedParams()['segment'];
-		$contributionsSegment = $this->contributionsLookup->getRevisionsByUser( $user, $limit, $segment );
+		$contributionsSegment =
+			$this->contributionsLookup->getRevisionsByUser( $user, $limit, $user, $segment );
 
 		$revisions = $this->getRevisionsList( $contributionsSegment );
 		$urls = $this->constructURLs( $contributionsSegment );

includes/Revision/ContributionsLookup.php

@@ -6,6 +6,7 @@ use ContribsPager;
 use FauxRequest;
 use MediaWiki\User\UserIdentity;
 use RequestContext;
+use User;
 
 /**
  * @since 1.35
@@ -62,25 +63,28 @@ class ContributionsLookup {
 	}
 
 	/**
-	 * @param UserIdentity $user
-	 *
-	 * @param int $limit
+	 * @param UserIdentity $target the user from whom to retrieve contributions
+	 * @param int $limit the maximum number of revisions to return
+	 * @param User $performer the user used for permission checks
 	 * @param string $segment
+	 *
 	 * @return ContributionsSegment
+	 * @throws \MWException
 	 */
 	public function getRevisionsByUser(
-		UserIdentity $user,
+		UserIdentity $target,
 		int $limit,
+		User $performer,
 		string $segment = ''
 	): ContributionsSegment {
-		// FIXME: set acting user
 		$context = new RequestContext();
+		$context->setUser( $performer );
 		$paramArr = $this->getPagerParams( $limit, $segment );
 		$context->setRequest( new FauxRequest( $paramArr ) );
 
 		// TODO: explore moving this to factory method for testing
 		$pager = new ContribsPager( $context, [
-			'target' => $user->getName(),
+			'target' => $target->getName(),
 		] );
 		$revisions = [];
 		$count = 0;

tests/api-testing/REST/UserContributions.js

@@ -8,9 +8,12 @@ describe( 'GET /me/contributions', () => {
 	const arnoldsRevisions = [];
 	const arnoldsEdits = [];
 	let arnold;
+	let arnoldAction;
+	let samAction;
 
 	before( async () => {
-		const arnoldAction = await action.user( 'arnold' );
+		samAction = await action.user( 'sam', [ 'suppress' ] );
+		arnoldAction = await action.user( 'arnold' );
 		arnold = clientFactory.getRESTClient( basePath, arnoldAction );
 
 		let page = utils.title( 'UserContribution_' );
@@ -173,4 +176,44 @@ describe( 'GET /me/contributions', () => {
 		assert.deepEqual( latestSegment, latestSegment4 );
 	} );
 
+	it( 'Does not return suppressed revisions when requesting user does not have appropriate permissions', async () => {
+		const { body: preDeleteBody } = await arnold.get( '/me/contributions?limit=10' );
+		assert.lengthOf( preDeleteBody.revisions, 5 );
+
+		const pageToDelete = utils.title( 'UserContribution_' );
+
+		const editToDelete = await arnoldAction.edit( pageToDelete, [ { text: 'Delete me 1' } ] );
+		await arnoldAction.edit( pageToDelete, [ { text: 'Delete me 2' } ] );
+
+		await samAction.action( 'revisiondelete',
+			{
+				type: 'revision',
+				token: await samAction.token(),
+				target: editToDelete.title,
+				hide: 'content|comment|user',
+				ids: editToDelete.newrevid
+			},
+			'POST'
+		);
+
+		// Users without appropriate permissions cannot see suppressed revisions (even their own)
+		const { body: arnoldGetBody } = await arnold.get( '/me/contributions?limit=10' );
+		assert.lengthOf( arnoldGetBody.revisions, 6 );
+
+		await samAction.action( 'revisiondelete',
+			{
+				type: 'revision',
+				token: await samAction.token(),
+				target: editToDelete.title,
+				show: 'content|comment|user',
+				ids: editToDelete.newrevid
+			},
+			'POST'
+		);
+
+		// Users with appropriate permissions can see suppressed revisions
+		const { body: arnoldGetBody2 } = await arnold.get( '/me/contributions?limit=10' );
+		assert.lengthOf( arnoldGetBody2.revisions, 7 );
+	} );
+
 } );

tests/phpunit/includes/Revision/ContributionsLookupTest.php

@@ -7,6 +7,7 @@ use MediaWiki\Revision\ContributionsLookup;
 use MediaWiki\Revision\ContributionsSegment;
 use MediaWiki\Revision\RevisionRecord;
 use MediaWikiIntegrationTestCase;
+use User;
 use Wikimedia\Timestamp\ConvertibleTimestamp;
 
 /**
@@ -59,8 +60,10 @@ class ContributionsLookupTest extends MediaWikiIntegrationTestCase {
 
 		$revisionStore = MediaWikiServices::getInstance()->getRevisionStore();
 		$contributionsLookup = new ContributionsLookup( $revisionStore );
+		$performer = self::$testUser;
 
-		$contribs = $contributionsLookup->getRevisionsByUser( self::$testUser, 2 )->getRevisions();
+		$contribs =
+			$contributionsLookup->getRevisionsByUser( self::$testUser, 2, $performer )->getRevisions();
 
 		$this->assertCount( 2, $contribs );
 		$this->assertSame( self::$storedRevisions[4]->getId(), $contribs[0]->getId() );
@@ -85,20 +88,22 @@ class ContributionsLookupTest extends MediaWikiIntegrationTestCase {
 		$revisionStore = MediaWikiServices::getInstance()->getRevisionStore();
 		$contributionsLookup = new ContributionsLookup( $revisionStore );
 
-		$segment1 = $contributionsLookup->getRevisionsByUser( self::$testUser, 2 );
+		$segment1 = $contributionsLookup->getRevisionsByUser( self::$testUser, 2, self::$testUser );
 		$this->assertInstanceOf( ContributionsSegment::class, $segment1 );
 		$this->assertCount( 2, $segment1->getRevisions() );
 		$this->assertNotNull( $segment1->getAfter() );
 		$this->assertNotNull( $segment1->getBefore() );
 		$this->assertFalse( $segment1->isOldest() );
 
-		$segment2 = $contributionsLookup->getRevisionsByUser( self::$testUser, 2, $segment1->getBefore() );
+		$segment2 =
+			$contributionsLookup->getRevisionsByUser( self::$testUser, 2, self::$testUser, $segment1->getBefore() );
 		$this->assertCount( 2, $segment2->getRevisions() );
 		$this->assertNotNull( $segment2->getAfter() );
 		$this->assertNull( $segment2->getBefore() );
 		$this->assertTrue( $segment2->isOldest() );
 
-		$segment3 = $contributionsLookup->getRevisionsByUser( self::$testUser, 2, $segment2->getAfter() );
+		$segment3 =
+			$contributionsLookup->getRevisionsByUser( self::$testUser, 2, self::$testUser, $segment2->getAfter() );
 		$this->assertInstanceOf( ContributionsSegment::class, $segment3 );
 		$this->assertCount( 2, $segment3->getRevisions() );
 		$this->assertNotNull( $segment3->getAfter() );
@@ -143,8 +148,36 @@ class ContributionsLookupTest extends MediaWikiIntegrationTestCase {
 		$revisionStore = MediaWikiServices::getInstance()->getRevisionStore();
 		$contributionsLookup = new ContributionsLookup( $revisionStore );
 		$expectedRevisions = [ self::$storedRevisions[4], self::$storedRevisions[3] ];
-		$segment = $contributionsLookup->getRevisionsByUser( self::$testUser, 2, $segment );
+		$segment = $contributionsLookup->getRevisionsByUser( self::$testUser, 2, self::$testUser, $segment );
 		$this->assertSegmentRevisions( $expectedRevisions, $segment );
 	}
 
+	public function testPermissionChecksAreApplied() {
+		$editingUser = self::$testUser;
+		$sysop = $this->getTestUser( [ 'sysop', 'suppress' ] )->getUser();
+		$anon = User::newFromId( 0 );
+
+		$revisionStore = MediaWikiServices::getInstance()->getRevisionStore();
+		$contributionsLookup = new ContributionsLookup( $revisionStore );
+
+		$revIds = [ self::$storedRevisions[1]->getId(), self::$storedRevisions[2]->getId() ];
+		$this->db->update(
+			'revision',
+			[ 'rev_deleted' => RevisionRecord::DELETED_USER ],
+			[ 'rev_id' => $revIds ],
+			__METHOD__
+		);
+
+		// sanity
+		$this->assertSame( 2, $this->db->affectedRows() );
+
+		// anons should not see suppressed contribs
+		$contribs = $contributionsLookup->getRevisionsByUser( $editingUser, 10, $anon );
+		$this->assertCount( 2, $contribs->getRevisions() );
+
+		// sysop also gets suppressed contribs
+		$contribs = $contributionsLookup->getRevisionsByUser( $editingUser, 10, $sysop );
+		$this->assertCount( 4, $contribs->getRevisions() );
+	}
+
 }

tests/phpunit/unit/includes/Rest/Handler/UserContributionsHandlerTest.php

@@ -108,7 +108,7 @@ class UserContributionsHandlerTest extends \MediaWikiUnitTestCase {
 		$limit = $request->getQueryParams()['limit'] ?? self::DEFAULT_LIMIT;
 		$segment = $request->getQueryParams()['segment'] ?? null;
 		$mockContributionsLookup->method( 'getRevisionsByUser' )
-			->with( $user, $limit, $segment )
+			->with( $user, $limit, $user, $segment )
 			->willReturn( $fakeSegment );
 
 		$handler = new UserContributionsHandler( $mockContributionsLookup );