diff --git a/includes/Rest/Handler/UserContributionsHandler.php b/includes/Rest/Handler/UserContributionsHandler.php index edfbbf46fb0..e488bcea7ad 100644 --- a/includes/Rest/Handler/UserContributionsHandler.php +++ b/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 ); diff --git a/includes/Revision/ContributionsLookup.php b/includes/Revision/ContributionsLookup.php index 59164f85370..2f58c5dfea0 100644 --- a/includes/Revision/ContributionsLookup.php +++ b/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; diff --git a/tests/api-testing/REST/UserContributions.js b/tests/api-testing/REST/UserContributions.js index 5dded0d5d0f..8210212acaa 100644 --- a/tests/api-testing/REST/UserContributions.js +++ b/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 ); + } ); + } ); diff --git a/tests/phpunit/includes/Revision/ContributionsLookupTest.php b/tests/phpunit/includes/Revision/ContributionsLookupTest.php index e201ca5ac20..1e8a4974e31 100644 --- a/tests/phpunit/includes/Revision/ContributionsLookupTest.php +++ b/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() ); + } + } diff --git a/tests/phpunit/unit/includes/Rest/Handler/UserContributionsHandlerTest.php b/tests/phpunit/unit/includes/Rest/Handler/UserContributionsHandlerTest.php index a23fd44744c..faea3809fb0 100644 --- a/tests/phpunit/unit/includes/Rest/Handler/UserContributionsHandlerTest.php +++ b/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 );