API: Migrate Title::userCan() calls to PermissionManager

T208768 introduced the PermissionManager service that can now be used
for page specific permission checks. This change replaces calls to
Title::userCan() with the new service in API classes.

Bug: T220191
Change-Id: I768d07a520ca6473a4eefb88c9f587657bc74357

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index dd1a6db..5687f0f 100644 (file)
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -22,7 +22,9 @@
 
 use MediaWiki\Block\AbstractBlock;
 use MediaWiki\Block\DatabaseBlock;
 
 use MediaWiki\Block\AbstractBlock;
 use MediaWiki\Block\DatabaseBlock;

+use MediaWiki\Linker\LinkTarget;

 use MediaWiki\MediaWikiServices;
 use MediaWiki\MediaWikiServices;

+use MediaWiki\Permissions\PermissionManager;

 use Wikimedia\Rdbms\IDatabase;
 
 /**
 use Wikimedia\Rdbms\IDatabase;
 
 /**


@@ -698,6 +700,16 @@ abstract class ApiBase extends ContextSource {
                $this->getMain()->setContinuationManager( $manager );
        }
 
                $this->getMain()->setContinuationManager( $manager );
        }
 

+       /**
+        * Obtain a PermissionManager instance that subclasses may use in their authorization checks.
+        *
+        * @since 1.34
+        * @return PermissionManager
+        */
+       protected function getPermissionManager(): PermissionManager {
+               return MediaWikiServices::getInstance()->getPermissionManager();
+       }
+

        /**@}*/
 
        /************************************************************************//**
        /**@}*/
 
        /************************************************************************//**


@@ -2120,17 +2132,19 @@ abstract class ApiBase extends ContextSource {
 
        /**
         * Helper function for permission-denied errors
 
        /**
         * Helper function for permission-denied errors

-        * @since 1.29
-        * @since 1.33 Changed the third parameter from $user to $options.
-        * @param Title $title
+        *
+        * @param LinkTarget $linkTarget

         * @param string|string[] $actions
         * @param array $options Additional options
         *   - user: (User) User to use rather than $this->getUser()
         *   - autoblock: (bool, default false) Whether to spread autoblocks
         *  For compatibility, passing a User object is treated as the value for the 'user' option.
         * @throws ApiUsageException if the user doesn't have all of the rights.
         * @param string|string[] $actions
         * @param array $options Additional options
         *   - user: (User) User to use rather than $this->getUser()
         *   - autoblock: (bool, default false) Whether to spread autoblocks
         *  For compatibility, passing a User object is treated as the value for the 'user' option.
         * @throws ApiUsageException if the user doesn't have all of the rights.

+        *
+        * @since 1.29
+        * @since 1.33 Changed the third parameter from $user to $options.

         */
         */

-       public function checkTitleUserPermissions( Title $title, $actions, $options = [] ) {
+       public function checkTitleUserPermissions( LinkTarget $linkTarget, $actions, $options = [] ) {

                if ( !is_array( $options ) ) {
                        wfDeprecated( '$user as the third parameter to ' . __METHOD__, '1.33' );
                        $options = [ 'user' => $options ];
                if ( !is_array( $options ) ) {
                        wfDeprecated( '$user as the third parameter to ' . __METHOD__, '1.33' );
                        $options = [ 'user' => $options ];


@@ -2139,7 +2153,10 @@ abstract class ApiBase extends ContextSource {
 
                $errors = [];
                foreach ( (array)$actions as $action ) {
 
                $errors = [];
                foreach ( (array)$actions as $action ) {

-                       $errors = array_merge( $errors, $title->getUserPermissionsErrors( $action, $user ) );
+                       $errors = array_merge(
+                               $errors,
+                               $this->getPermissionManager()->getPermissionErrors( $action, $user, $linkTarget )
+                       );

                }
 
                if ( $errors ) {
                }
 
                if ( $errors ) {

diff --git a/includes/api/ApiFeedContributions.php b/includes/api/ApiFeedContributions.php
index ed3b7b8..08be8e0 100644 (file)
--- a/includes/api/ApiFeedContributions.php
+++ b/includes/api/ApiFeedContributions.php
@@ -138,7 +138,9 @@ class ApiFeedContributions extends ApiBase {
 
                // Hook completed and did not return a valid feed item
                $title = Title::makeTitle( (int)$row->page_namespace, $row->page_title );
 
                // Hook completed and did not return a valid feed item
                $title = Title::makeTitle( (int)$row->page_namespace, $row->page_title );

-               if ( $title && $title->userCan( 'read', $this->getUser() ) ) {
+               $user = $this->getUser();
+
+               if ( $title && $this->getPermissionManager()->userCan( 'read', $user, $title ) ) {

                        $date = $row->rev_timestamp;
                        $comments = $title->getTalkPage()->getFullURL();
                        $revision = $this->revisionStore->newRevisionFromRow( $row );
                        $date = $row->rev_timestamp;
                        $comments = $title->getTalkPage()->getFullURL();
                        $revision = $this->revisionStore->newRevisionFromRow( $row );

diff --git a/includes/api/ApiQuery.php b/includes/api/ApiQuery.php
index ae6b1a1..7cbd3ef 100644 (file)
--- a/includes/api/ApiQuery.php
+++ b/includes/api/ApiQuery.php
@@ -429,10 +429,9 @@ class ApiQuery extends ApiBase {
                $exportTitles = [];
                $titles = $pageSet->getGoodTitles();
                if ( count( $titles ) ) {
                $exportTitles = [];
                $titles = $pageSet->getGoodTitles();
                if ( count( $titles ) ) {

-                       $user = $this->getUser();

                        /** @var Title $title */
                        foreach ( $titles as $title ) {
                        /** @var Title $title */
                        foreach ( $titles as $title ) {

-                               if ( $title->userCan( 'read', $user ) ) {
+                               if ( $this->getPermissionManager()->userCan( 'read', $this->getUser(), $title ) ) {

                                        $exportTitles[] = $title;
                                }
                        }
                                        $exportTitles[] = $title;
                                }
                        }

diff --git a/includes/api/ApiQueryInfo.php b/includes/api/ApiQueryInfo.php
index 276aafb..90f1340 100644 (file)
--- a/includes/api/ApiQueryInfo.php
+++ b/includes/api/ApiQueryInfo.php
@@ -409,6 +409,8 @@ class ApiQueryInfo extends ApiQueryBase {
                $pageInfo['pagelanguagehtmlcode'] = $pageLanguage->getHtmlCode();
                $pageInfo['pagelanguagedir'] = $pageLanguage->getDir();
 
                $pageInfo['pagelanguagehtmlcode'] = $pageLanguage->getHtmlCode();
                $pageInfo['pagelanguagedir'] = $pageLanguage->getDir();
 

+               $user = $this->getUser();
+

                if ( $titleExists ) {
                        $pageInfo['touched'] = wfTimestamp( TS_ISO_8601, $this->pageTouched[$pageid] );
                        $pageInfo['lastrevid'] = (int)$this->pageLatest[$pageid];
                if ( $titleExists ) {
                        $pageInfo['touched'] = wfTimestamp( TS_ISO_8601, $this->pageTouched[$pageid] );
                        $pageInfo['lastrevid'] = (int)$this->pageLatest[$pageid];


@@ -493,7 +495,9 @@ class ApiQueryInfo extends ApiQueryBase {
                        $pageInfo['canonicalurl'] = wfExpandUrl( $title->getFullURL(), PROTO_CANONICAL );
                }
                if ( $this->fld_readable ) {
                        $pageInfo['canonicalurl'] = wfExpandUrl( $title->getFullURL(), PROTO_CANONICAL );
                }
                if ( $this->fld_readable ) {

-                       $pageInfo['readable'] = $title->userCan( 'read', $this->getUser() );
+                       $pageInfo['readable'] = $this->getPermissionManager()->userCan(
+                               'read', $user, $title
+                       );

                }
 
                if ( $this->fld_preload ) {
                }
 
                if ( $this->fld_preload ) {


@@ -539,10 +543,14 @@ class ApiQueryInfo extends ApiQueryBase {
                                $this->countTestedActions++;
 
                                if ( $detailLevel === 'boolean' ) {
                                $this->countTestedActions++;
 
                                if ( $detailLevel === 'boolean' ) {

-                                       $pageInfo['actions'][$action] = $title->userCan( $action, $user );
+                                       $pageInfo['actions'][$action] = $this->getPermissionManager()->userCan(
+                                               $action, $user, $title
+                                       );

                                } else {
                                        $pageInfo['actions'][$action] = $errorFormatter->arrayFromStatus( $this->errorArrayToStatus(
                                } else {
                                        $pageInfo['actions'][$action] = $errorFormatter->arrayFromStatus( $this->errorArrayToStatus(

-                                               $title->getUserPermissionsErrors( $action, $user, $rigor ),
+                                               $this->getPermissionManager()->getPermissionErrors(
+                                                       $action, $user, $title, $rigor
+                                               ),

                                                $user
                                        ) );
                                }
                                                $user
                                        ) );
                                }

diff --git a/includes/api/ApiQueryRevisions.php b/includes/api/ApiQueryRevisions.php
index ee6a264..fe3ae87 100644 (file)
--- a/includes/api/ApiQueryRevisions.php
+++ b/includes/api/ApiQueryRevisions.php
@@ -201,11 +201,12 @@ class ApiQueryRevisions extends ApiQueryRevisionsBase {
 
                if ( $resultPageSet === null && $this->fetchContent ) {
                        // For each page we will request, the user must have read rights for that page
 
                if ( $resultPageSet === null && $this->fetchContent ) {
                        // For each page we will request, the user must have read rights for that page

-                       $user = $this->getUser();

                        $status = Status::newGood();
                        $status = Status::newGood();

+                       $user = $this->getUser();
+

                        /** @var Title $title */
                        foreach ( $pageSet->getGoodTitles() as $title ) {
                        /** @var Title $title */
                        foreach ( $pageSet->getGoodTitles() as $title ) {

-                               if ( !$title->userCan( 'read', $user ) ) {
+                               if ( !$this->getPermissionManager()->userCan( 'read', $user, $title ) ) {

                                        $status->fatal( ApiMessage::create(
                                                [ 'apierror-cannotviewtitle', wfEscapeWikiText( $title->getPrefixedText() ) ],
                                                'accessdenied'
                                        $status->fatal( ApiMessage::create(
                                                [ 'apierror-cannotviewtitle', wfEscapeWikiText( $title->getPrefixedText() ) ],
                                                'accessdenied'

diff --git a/includes/api/ApiUndelete.php b/includes/api/ApiUndelete.php
index 5f9b97c..ba9be81 100644 (file)
--- a/includes/api/ApiUndelete.php
+++ b/includes/api/ApiUndelete.php
@@ -41,7 +41,7 @@ class ApiUndelete extends ApiBase {
                        $this->dieWithError( [ 'apierror-invalidtitle', wfEscapeWikiText( $params['title'] ) ] );
                }
 
                        $this->dieWithError( [ 'apierror-invalidtitle', wfEscapeWikiText( $params['title'] ) ] );
                }
 

-               if ( !$titleObj->userCan( 'undelete', $user, 'secure' ) ) {
+               if ( !$this->getPermissionManager()->userCan( 'undelete', $this->getUser(), $titleObj ) ) {

                        $this->dieWithError( 'permdenied-undelete' );
                }
 
                        $this->dieWithError( 'permdenied-undelete' );
                }
 

diff --git a/tests/phpunit/includes/api/query/ApiQueryTest.php b/tests/phpunit/includes/api/query/ApiQueryTest.php
index de8d815..20bd855 100644 (file)
--- a/tests/phpunit/includes/api/query/ApiQueryTest.php
+++ b/tests/phpunit/includes/api/query/ApiQueryTest.php
@@ -148,4 +148,28 @@ class ApiQueryTest extends ApiTestCase {
                        );
                }
        }
                        );
                }
        }

+
+       public function testShouldNotExportPagesThatUserCanNotRead() {
+               $title = Title::makeTitle( NS_MAIN, 'Test article' );
+               $this->insertPage( $title );
+
+               $this->setTemporaryHook( 'getUserPermissionsErrors',
+                       function ( Title $page, &$user, $action, &$result ) use ( $title ) {
+                               if ( $page->equals( $title ) && $action === 'read' ) {
+                                       $result = false;
+                                       return false;
+                               }
+                       } );
+
+               $data = $this->doApiRequest( [
+                       'action' => 'query',
+                       'titles' => $title->getPrefixedText(),
+                       'export' => 1,
+               ] );
+
+               $this->assertArrayHasKey( 'query', $data[0] );
+               $this->assertArrayHasKey( 'export', $data[0]['query'] );
+               // This response field contains an XML document even if no pages were exported
+               $this->assertNotContains( $title->getPrefixedText(), $data[0]['query']['export'] );
+       }

 }
 }