Convert PermissionManager constructor to use ServiceOptions.

Change-Id: I36a3a2f338506ef14cc5d65b8bee2961a92d60da

diff --git a/includes/Permissions/PermissionManager.php b/includes/Permissions/PermissionManager.php
index 248ba14..3c0a6e2 100644 (file)
--- a/includes/Permissions/PermissionManager.php
+++ b/includes/Permissions/PermissionManager.php
@@ -22,6 +22,7 @@ namespace MediaWiki\Permissions;
 use Action;
 use Exception;
 use Hooks;
+use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Linker\LinkTarget;
 use MediaWiki\Revision\RevisionLookup;
 use MediaWiki\Revision\RevisionRecord;
@@ -54,36 +55,34 @@ class PermissionManager {
        /** @var string Does cheap and expensive checks, using the master as needed */
        const RIGOR_SECURE = 'secure';
 
+       /**
+        * TODO Make this const when HHVM support is dropped (T192166)
+        *
+        * @since 1.34
+        * @var array
+        */
+       public static $constructorOptions = [
+               'WhitelistRead',
+               'WhitelistReadRegexp',
+               'EmailConfirmToEdit',
+               'BlockDisablesLogin',
+               'GroupPermissions',
+               'RevokePermissions',
+               'AvailableRights'
+       ];
+
+       /** @var ServiceOptions */
+       private $options;
+
        /** @var SpecialPageFactory */
        private $specialPageFactory;
 
        /** @var RevisionLookup */
        private $revisionLookup;
 
-       /** @var string[] List of pages names anonymous user may see */
-       private $whitelistRead;
-
-       /** @var string[] Whitelists publicly readable titles with regular expressions */
-       private $whitelistReadRegexp;
-
-       /** @var bool Require users to confirm email address before they can edit */
-       private $emailConfirmToEdit;
-
-       /** @var bool If set to true, blocked users will no longer be allowed to log in */
-       private $blockDisablesLogin;
-
        /** @var NamespaceInfo */
        private $nsInfo;
 
-       /** @var string[][] Access rights for groups and users in these groups */
-       private $groupPermissions;
-
-       /** @var string[][] Permission keys revoked from users in each group */
-       private $revokePermissions;
-
-       /** @var string[] A list of available rights, in addition to the ones defined by the core */
-       private $availableRights;
-
        /** @var string[] Cached results of getAllRights() */
        private $allRights = false;
 
@@ -189,38 +188,21 @@ class PermissionManager {
        ];
 
        /**
+        * @param ServiceOptions $options
         * @param SpecialPageFactory $specialPageFactory
         * @param RevisionLookup $revisionLookup
-        * @param string[] $whitelistRead
-        * @param string[] $whitelistReadRegexp
-        * @param bool $emailConfirmToEdit
-        * @param bool $blockDisablesLogin
-        * @param string[][] $groupPermissions
-        * @param string[][] $revokePermissions
-        * @param string[] $availableRights
         * @param NamespaceInfo $nsInfo
         */
        public function __construct(
+               ServiceOptions $options,
                SpecialPageFactory $specialPageFactory,
                RevisionLookup $revisionLookup,
-               $whitelistRead,
-               $whitelistReadRegexp,
-               $emailConfirmToEdit,
-               $blockDisablesLogin,
-               $groupPermissions,
-               $revokePermissions,
-               $availableRights,
                NamespaceInfo $nsInfo
        ) {
+               $options->assertRequiredOptions( self::$constructorOptions );
+               $this->options = $options;
                $this->specialPageFactory = $specialPageFactory;
                $this->revisionLookup = $revisionLookup;
-               $this->whitelistRead = $whitelistRead;
-               $this->whitelistReadRegexp = $whitelistReadRegexp;
-               $this->emailConfirmToEdit = $emailConfirmToEdit;
-               $this->blockDisablesLogin = $blockDisablesLogin;
-               $this->groupPermissions = $groupPermissions;
-               $this->revokePermissions = $revokePermissions;
-               $this->availableRights = $availableRights;
                $this->nsInfo = $nsInfo;
        }
 
@@ -500,6 +482,7 @@ class PermissionManager {
                // TODO: remove when LinkTarget usage will expand further
                $title = Title::newFromLinkTarget( $page );
 
+               $whiteListRead = $this->options->get( 'WhitelistRead' );
                $whitelisted = false;
                if ( $this->isEveryoneAllowed( 'read' ) ) {
                        # Shortcut for public wikis, allows skipping quite a bit of code
@@ -514,20 +497,20 @@ class PermissionManager {
                        # Always grant access to the login page.
                        # Even anons need to be able to log in.
                        $whitelisted = true;
-               } elseif ( is_array( $this->whitelistRead ) && count( $this->whitelistRead ) ) {
+               } elseif ( is_array( $whiteListRead ) && count( $whiteListRead ) ) {
                        # Time to check the whitelist
                        # Only do these checks is there's something to check against
                        $name = $title->getPrefixedText();
                        $dbName = $title->getPrefixedDBkey();
 
                        // Check for explicit whitelisting with and without underscores
-                       if ( in_array( $name, $this->whitelistRead, true )
-                                || in_array( $dbName, $this->whitelistRead, true ) ) {
+                       if ( in_array( $name, $whiteListRead, true )
+                                || in_array( $dbName, $whiteListRead, true ) ) {
                                $whitelisted = true;
                        } elseif ( $title->getNamespace() == NS_MAIN ) {
                                # Old settings might have the title prefixed with
                                # a colon for main-namespace pages
-                               if ( in_array( ':' . $name, $this->whitelistRead ) ) {
+                               if ( in_array( ':' . $name, $whiteListRead ) ) {
                                        $whitelisted = true;
                                }
                        } elseif ( $title->isSpecialPage() ) {
@@ -537,18 +520,19 @@ class PermissionManager {
                                        $this->specialPageFactory->resolveAlias( $name );
                                if ( $name ) {
                                        $pure = SpecialPage::getTitleFor( $name )->getPrefixedText();
-                                       if ( in_array( $pure, $this->whitelistRead, true ) ) {
+                                       if ( in_array( $pure, $whiteListRead, true ) ) {
                                                $whitelisted = true;
                                        }
                                }
                        }
                }
 
-               if ( !$whitelisted && is_array( $this->whitelistReadRegexp )
-                        && !empty( $this->whitelistReadRegexp ) ) {
+               $whitelistReadRegexp = $this->options->get( 'WhitelistReadRegexp' );
+               if ( !$whitelisted && is_array( $whitelistReadRegexp )
+                        && !empty( $whitelistReadRegexp ) ) {
                        $name = $title->getPrefixedText();
                        // Check for regex whitelisting
-                       foreach ( $this->whitelistReadRegexp as $listItem ) {
+                       foreach ( $whitelistReadRegexp as $listItem ) {
                                if ( preg_match( $listItem, $name ) ) {
                                        $whitelisted = true;
                                        break;
@@ -636,11 +620,11 @@ class PermissionManager {
                }
 
                // Optimize for a very common case
-               if ( $action === 'read' && !$this->blockDisablesLogin ) {
+               if ( $action === 'read' && !$this->options->get( 'BlockDisablesLogin' ) ) {
                        return $errors;
                }
 
-               if ( $this->emailConfirmToEdit
+               if ( $this->options->get( 'EmailConfirmToEdit' )
                         && !$user->isEmailConfirmed()
                         && $action === 'edit'
                ) {
@@ -1246,7 +1230,7 @@ class PermissionManager {
 
                        if (
                                $user->isLoggedIn() &&
-                               $this->blockDisablesLogin &&
+                               $this->options->get( 'BlockDisablesLogin' ) &&
                                $user->getBlock()
                        ) {
                                $anon = new User;
@@ -1296,10 +1280,10 @@ class PermissionManager {
         * @return bool
         */
        public function groupHasPermission( $group, $role ) {
-               return isset( $this->groupPermissions[$group][$role] ) &&
-                          $this->groupPermissions[$group][$role] &&
-                          !( isset( $this->revokePermissions[$group][$role] ) &&
-                                 $this->revokePermissions[$group][$role] );
+               $groupPermissions = $this->options->get( 'GroupPermissions' );
+               $revokePermissions = $this->options->get( 'RevokePermissions' );
+               return isset( $groupPermissions[$group][$role] ) && $groupPermissions[$group][$role] &&
+                          !( isset( $revokePermissions[$group][$role] ) && $revokePermissions[$group][$role] );
        }
 
        /**
@@ -1314,17 +1298,17 @@ class PermissionManager {
                $rights = [];
                // grant every granted permission first
                foreach ( $groups as $group ) {
-                       if ( isset( $this->groupPermissions[$group] ) ) {
+                       if ( isset( $this->options->get( 'GroupPermissions' )[$group] ) ) {
                                $rights = array_merge( $rights,
                                        // array_filter removes empty items
-                                       array_keys( array_filter( $this->groupPermissions[$group] ) ) );
+                                       array_keys( array_filter( $this->options->get( 'GroupPermissions' )[$group] ) ) );
                        }
                }
                // now revoke the revoked permissions
                foreach ( $groups as $group ) {
-                       if ( isset( $this->revokePermissions[$group] ) ) {
+                       if ( isset( $this->options->get( 'RevokePermissions' )[$group] ) ) {
                                $rights = array_diff( $rights,
-                                       array_keys( array_filter( $this->revokePermissions[$group] ) ) );
+                                       array_keys( array_filter( $this->options->get( 'RevokePermissions' )[$group] ) ) );
                        }
                }
                return array_unique( $rights );
@@ -1340,7 +1324,7 @@ class PermissionManager {
         */
        public function getGroupsWithPermission( $role ) {
                $allowedGroups = [];
-               foreach ( array_keys( $this->groupPermissions ) as $group ) {
+               foreach ( array_keys( $this->options->get( 'GroupPermissions' ) ) as $group ) {
                        if ( $this->groupHasPermission( $group, $role ) ) {
                                $allowedGroups[] = $group;
                        }
@@ -1370,14 +1354,14 @@ class PermissionManager {
                        return $this->cachedRights[$right];
                }
 
-               if ( !isset( $this->groupPermissions['*'][$right] )
-                        || !$this->groupPermissions['*'][$right] ) {
+               if ( !isset( $this->options->get( 'GroupPermissions' )['*'][$right] )
+                        || !$this->options->get( 'GroupPermissions' )['*'][$right] ) {
                        $this->cachedRights[$right] = false;
                        return false;
                }
 
                // If it's revoked anywhere, then everyone doesn't have it
-               foreach ( $this->revokePermissions as $rights ) {
+               foreach ( $this->options->get( 'RevokePermissions' ) as $rights ) {
                        if ( isset( $rights[$right] ) && $rights[$right] ) {
                                $this->cachedRights[$right] = false;
                                return false;
@@ -1415,10 +1399,10 @@ class PermissionManager {
         */
        public function getAllPermissions() {
                if ( $this->allRights === false ) {
-                       if ( count( $this->availableRights ) ) {
+                       if ( count( $this->options->get( 'AvailableRights' ) ) ) {
                                $this->allRights = array_unique( array_merge(
                                        $this->coreRights,
-                                       $this->availableRights
+                                       $this->options->get( 'AvailableRights' )
                                ) );
                        } else {
                                $this->allRights = $this->coreRights;

diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index 663f371..7c13ac8 100644 (file)
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -497,17 +497,12 @@ return [
        },
 
        'PermissionManager' => function ( MediaWikiServices $services ) : PermissionManager {
-               $config = $services->getMainConfig();
                return new PermissionManager(
+                       new ServiceOptions(
+                               PermissionManager::$constructorOptions, $services->getMainConfig()
+                       ),
                        $services->getSpecialPageFactory(),
                        $services->getRevisionLookup(),
-                       $config->get( 'WhitelistRead' ),
-                       $config->get( 'WhitelistReadRegexp' ),
-                       $config->get( 'EmailConfirmToEdit' ),
-                       $config->get( 'BlockDisablesLogin' ),
-                       $config->get( 'GroupPermissions' ),
-                       $config->get( 'RevokePermissions' ),
-                       $config->get( 'AvailableRights' ),
                        $services->getNamespaceInfo()
                );
        },

diff --git a/tests/phpunit/includes/Permissions/PermissionManagerTest.php b/tests/phpunit/includes/Permissions/PermissionManagerTest.php
index 86942fd..3c5f43b 100644 (file)
--- a/tests/phpunit/includes/Permissions/PermissionManagerTest.php
+++ b/tests/phpunit/includes/Permissions/PermissionManagerTest.php
@@ -5,6 +5,7 @@ namespace MediaWiki\Tests\Permissions;
 use Action;
 use ContentHandler;
 use FauxRequest;
+use LoggedServiceOptions;
 use MediaWiki\Block\DatabaseBlock;
 use MediaWiki\Block\Restriction\NamespaceRestriction;
 use MediaWiki\Block\Restriction\PageRestriction;
@@ -14,6 +15,7 @@ use MediaWiki\MediaWikiServices;
 use MediaWiki\Permissions\PermissionManager;
 use MediaWiki\Revision\MutableRevisionRecord;
 use MediaWiki\Revision\RevisionLookup;
+use TestAllServiceOptionsUsed;
 use Wikimedia\ScopedCallback;
 use MediaWiki\Session\SessionId;
 use MediaWiki\Session\TestUtils;
@@ -30,6 +32,7 @@ use Wikimedia\TestingAccessWrapper;
  * @covers \MediaWiki\Permissions\PermissionManager
  */
 class PermissionManagerTest extends MediaWikiLangTestCase {
+       use TestAllServiceOptionsUsed;
 
        /**
         * @var string
@@ -725,15 +728,21 @@ class PermissionManagerTest extends MediaWikiLangTestCase {
                                }
                        } );
                $permissionManager = new PermissionManager(
+                       new LoggedServiceOptions(
+                               self::$serviceOptionsAccessLog,
+                               PermissionManager::$constructorOptions,
+                               [
+                                       'WhitelistRead' => [],
+                                       'WhitelistReadRegexp' => [],
+                                       'EmailConfirmToEdit' => false,
+                                       'BlockDisablesLogin' => false,
+                                       'GroupPermissions' => [],
+                                       'RevokePermissions' => [],
+                                       'AvailableRights' => []
+                               ]
+                       ),
                        $services->getSpecialPageFactory(),
                        $revisionLookup,
-                       [],
-                       [],
-                       false,
-                       false,
-                       [],
-                       [],
-                       [],
                        MediaWikiServices::getInstance()->getNamespaceInfo()
                );
                $this->setService( 'PermissionManager', $permissionManager );

diff --git a/tests/phpunit/includes/Rest/BasicAccess/MWBasicRequestAuthorizerTest.php b/tests/phpunit/includes/Rest/BasicAccess/MWBasicRequestAuthorizerTest.php
index 076ff36..49dcf07 100644 (file)
--- a/tests/phpunit/includes/Rest/BasicAccess/MWBasicRequestAuthorizerTest.php
+++ b/tests/phpunit/includes/Rest/BasicAccess/MWBasicRequestAuthorizerTest.php
@@ -3,13 +3,12 @@
 namespace MediaWiki\Tests\Rest\BasicAccess;
 
 use GuzzleHttp\Psr7\Uri;
-use MediaWiki\Permissions\PermissionManager;
+use MediaWiki\MediaWikiServices;
 use MediaWiki\Rest\BasicAccess\MWBasicAuthorizer;
 use MediaWiki\Rest\Handler;
 use MediaWiki\Rest\RequestData;
 use MediaWiki\Rest\ResponseFactory;
 use MediaWiki\Rest\Router;
-use MediaWiki\User\UserIdentity;
 use MediaWikiTestCase;
 use User;
 
@@ -24,23 +23,15 @@ use User;
 class MWBasicRequestAuthorizerTest extends MediaWikiTestCase {
        private function createRouter( $userRights ) {
                $user = User::newFromName( 'Test user' );
-
-               $pm = new class( $user, $userRights ) extends PermissionManager {
-                       private $testUser;
-                       private $testUserRights;
-
-                       public function __construct( $user, $userRights ) {
-                               $this->testUser = $user;
-                               $this->testUserRights = $userRights;
-                       }
-
-                       public function userHasRight( UserIdentity $user, $action = '' ) {
-                               if ( $user === $this->testUser ) {
-                                       return $this->testUserRights[$action] ?? false;
-                               }
-                               return parent::userHasRight( $user, $action );
-                       }
-               };
+               // Don't allow the rights to everybody so that user rights kick in.
+               $this->mergeMwGlobalArrayValue( 'wgGroupPermissions', [ '*' => $userRights ] );
+               $this->resetServices();
+               $this->overrideUserPermissions(
+                       $user,
+                       array_keys( array_filter( $userRights ), function ( $value ) {
+                               return $value === true;
+                       } )
+               );
 
                global $IP;
 
@@ -50,7 +41,7 @@ class MWBasicRequestAuthorizerTest extends MediaWikiTestCase {
                        '/rest',
                        new \EmptyBagOStuff(),
                        new ResponseFactory(),
-                       new MWBasicAuthorizer( $user, $pm ) );
+                       new MWBasicAuthorizer( $user, MediaWikiServices::getInstance()->getPermissionManager() ) );
        }
 
        public function testReadDenied() {