Merge "Make DefaultPreferencesFactory depend on PermissionManager."

diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index c7ad75c..689a98e 100644 (file)
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -542,7 +542,8 @@ return [
                        $services->getContentLanguage(),
                        AuthManager::singleton(),
                        $services->getLinkRendererFactory()->create(),
-                       $services->getNamespaceInfo()
+                       $services->getNamespaceInfo(),
+                       $services->getPermissionManager()
                );
                $factory->setLogger( LoggerFactory::getInstance( 'preferences' ) );
 

diff --git a/includes/preferences/DefaultPreferencesFactory.php b/includes/preferences/DefaultPreferencesFactory.php
index 66c2bc3..56db812 100644 (file)
--- a/includes/preferences/DefaultPreferencesFactory.php
+++ b/includes/preferences/DefaultPreferencesFactory.php
@@ -20,7 +20,6 @@
 
 namespace MediaWiki\Preferences;
 
-use Config;
 use DateTime;
 use DateTimeZone;
 use Exception;
@@ -37,6 +36,7 @@ use MediaWiki\Auth\PasswordAuthenticationRequest;
 use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Linker\LinkRenderer;
 use MediaWiki\MediaWikiServices;
+use MediaWiki\Permissions\PermissionManager;
 use MessageLocalizer;
 use MWException;
 use MWTimestamp;
@@ -77,6 +77,9 @@ class DefaultPreferencesFactory implements PreferencesFactory {
        /** @var NamespaceInfo */
        protected $nsInfo;
 
+       /** @var PermissionManager */
+       protected $permissionManager;
+
        /**
         * TODO Make this a const when we drop HHVM support (T192166)
         *
@@ -114,35 +117,34 @@ class DefaultPreferencesFactory implements PreferencesFactory {
        /**
         * Do not call this directly.  Get it from MediaWikiServices.
         *
-        * @param ServiceOptions|Config $options Config accepted for backwards compatibility
+        * @param ServiceOptions $options
         * @param Language $contLang
         * @param AuthManager $authManager
         * @param LinkRenderer $linkRenderer
-        * @param NamespaceInfo|null $nsInfo
+        * @param NamespaceInfo $nsInfo
+        * @param PermissionManager|null $permissionManager
         */
        public function __construct(
-               $options,
+               ServiceOptions $options,
                Language $contLang,
                AuthManager $authManager,
                LinkRenderer $linkRenderer,
-               NamespaceInfo $nsInfo = null
+               NamespaceInfo $nsInfo,
+               PermissionManager $permissionManager = null
        ) {
-               if ( $options instanceof Config ) {
-                       wfDeprecated( __METHOD__ . ' with Config parameter', '1.34' );
-                       $options = new ServiceOptions( self::$constructorOptions, $options );
-               }
-
                $options->assertRequiredOptions( self::$constructorOptions );
 
-               if ( !$nsInfo ) {
-                       wfDeprecated( __METHOD__ . ' with no NamespaceInfo argument', '1.34' );
-                       $nsInfo = MediaWikiServices::getInstance()->getNamespaceInfo();
+               if ( !$permissionManager ) {
+                       // TODO: this is actually hard-deprecated, left for jenkins to pass
+                       // together with GlobalPreferences extension. Will be removed in a followup.
+                       $permissionManager = MediaWikiServices::getInstance()->getPermissionManager();
                }
                $this->options = $options;
                $this->contLang = $contLang;
                $this->authManager = $authManager;
                $this->linkRenderer = $linkRenderer;
                $this->nsInfo = $nsInfo;
+               $this->permissionManager = $permissionManager;
                $this->logger = new NullLogger();
        }
 
@@ -209,7 +211,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                # # Make sure that form fields have their parent set. See T43337.
                $dummyForm = new HTMLForm( [], $context );
 
-               $disable = !$user->isAllowed( 'editmyoptions' );
+               $disable = !$this->permissionManager->userHasRight( $user, 'editmyoptions' );
 
                $defaultOptions = User::getDefaultOptions();
                $userOptions = $user->getOptions();
@@ -390,8 +392,8 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                        ];
                }
 
-               $canViewPrivateInfo = $user->isAllowed( 'viewmyprivateinfo' );
-               $canEditPrivateInfo = $user->isAllowed( 'editmyprivateinfo' );
+               $canViewPrivateInfo = $this->permissionManager->userHasRight( $user, 'viewmyprivateinfo' );
+               $canEditPrivateInfo = $this->permissionManager->userHasRight( $user, 'editmyprivateinfo' );
 
                // Actually changeable stuff
                $defaultPreferences['realname'] = [
@@ -631,7 +633,9 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                                ];
                        }
 
-                       if ( $this->options->get( 'EnableUserEmail' ) && $user->isAllowed( 'sendemail' ) ) {
+                       if ( $this->options->get( 'EnableUserEmail' ) &&
+                               $this->permissionManager->userHasRight( $user, 'sendemail' )
+                       ) {
                                $defaultPreferences['disablemail'] = [
                                        'id' => 'wpAllowEmail',
                                        'type' => 'toggle',
@@ -921,7 +925,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                        'label-message' => 'tog-numberheadings',
                ];
 
-               if ( $user->isAllowed( 'rollback' ) ) {
+               if ( $this->permissionManager->userHasRight( $user, 'rollback' ) ) {
                        $defaultPreferences['showrollbackconfirmation'] = [
                                'type' => 'toggle',
                                'section' => 'rendering/advancedrendering',
@@ -961,7 +965,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                        ];
                }
 
-               if ( $user->isAllowed( 'minoredit' ) ) {
+               if ( $this->permissionManager->userHasRight( $user, 'minoredit' ) ) {
                        $defaultPreferences['minordefault'] = [
                                'type' => 'toggle',
                                'section' => 'editing/editor',
@@ -1107,7 +1111,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                $watchlistdaysMax = ceil( $this->options->get( 'RCMaxAge' ) / ( 3600 * 24 ) );
 
                # # Watchlist #####################################
-               if ( $user->isAllowed( 'editmywatchlist' ) ) {
+               if ( $this->permissionManager->userHasRight( $user, 'editmywatchlist' ) ) {
                        $editWatchlistLinks = '';
                        $editWatchlistModes = [
                                'edit' => [ 'subpage' => false, 'flags' => [] ],
@@ -1221,20 +1225,20 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                ];
 
                // Kinda hacky
-               if ( $user->isAllowed( 'createpage' ) || $user->isAllowed( 'createtalk' ) ) {
+               if ( $this->permissionManager->userHasAnyRight( $user, 'createpage', 'createtalk' ) ) {
                        $watchTypes['read'] = 'watchcreations';
                }
 
-               if ( $user->isAllowed( 'rollback' ) ) {
+               if ( $this->permissionManager->userHasRight( $user, 'rollback' ) ) {
                        $watchTypes['rollback'] = 'watchrollback';
                }
 
-               if ( $user->isAllowed( 'upload' ) ) {
+               if ( $this->permissionManager->userHasRight( $user, 'upload' ) ) {
                        $watchTypes['upload'] = 'watchuploads';
                }
 
                foreach ( $watchTypes as $action => $pref ) {
-                       if ( $user->isAllowed( $action ) ) {
+                       if ( $this->permissionManager->userHasRight( $user, $action ) ) {
                                // Messages:
                                // tog-watchdefault, tog-watchmoves, tog-watchdeletion, tog-watchcreations, tog-watchuploads
                                // tog-watchrollback
@@ -1606,7 +1610,9 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                $hiddenPrefs = $this->options->get( 'HiddenPrefs' );
                $result = true;
 
-               if ( !$user->isAllowedAny( 'editmyprivateinfo', 'editmyoptions' ) ) {
+               if ( !$this->permissionManager
+                               ->userHasAnyRight( $user, 'editmyprivateinfo', 'editmyoptions' )
+               ) {
                        return Status::newFatal( 'mypreferencesprotected' );
                }
 
@@ -1617,14 +1623,14 @@ class DefaultPreferencesFactory implements PreferencesFactory {
                // (not really "private", but still shouldn't be edited without permission)
 
                if ( !in_array( 'realname', $hiddenPrefs )
-                       && $user->isAllowed( 'editmyprivateinfo' )
+                       && $this->permissionManager->userHasRight( $user, 'editmyprivateinfo' )
                        && array_key_exists( 'realname', $formData )
                ) {
                        $realName = $formData['realname'];
                        $user->setRealName( $realName );
                }
 
-               if ( $user->isAllowed( 'editmyoptions' ) ) {
+               if ( $this->permissionManager->userHasRight( $user, 'editmyoptions' ) ) {
                        $oldUserOptions = $user->getOptions();
 
                        foreach ( $this->getSaveBlacklist() as $b ) {

diff --git a/tests/phpunit/includes/preferences/DefaultPreferencesFactoryTest.php b/tests/phpunit/includes/preferences/DefaultPreferencesFactoryTest.php
index e7f7067..a459441 100644 (file)
--- a/tests/phpunit/includes/preferences/DefaultPreferencesFactoryTest.php
+++ b/tests/phpunit/includes/preferences/DefaultPreferencesFactoryTest.php
@@ -2,6 +2,7 @@
 
 use MediaWiki\Auth\AuthManager;
 use MediaWiki\MediaWikiServices;
+use MediaWiki\Permissions\PermissionManager;
 use MediaWiki\Preferences\DefaultPreferencesFactory;
 use Wikimedia\TestingAccessWrapper;
 
@@ -49,9 +50,10 @@ class DefaultPreferencesFactoryTest extends \MediaWikiTestCase {
 
        /**
         * Get a basic PreferencesFactory for testing with.
+        * @param PermissionManager|null $manager
         * @return DefaultPreferencesFactory
         */
-       protected function getPreferencesFactory() {
+       protected function getPreferencesFactory( PermissionManager $manager = null ) {
                $mockNsInfo = $this->createMock( NamespaceInfo::class );
                $mockNsInfo->method( 'getValidNamespaces' )->willReturn( [
                        NS_MAIN, NS_TALK, NS_USER, NS_USER_TALK
@@ -59,13 +61,16 @@ class DefaultPreferencesFactoryTest extends \MediaWikiTestCase {
                $mockNsInfo->expects( $this->never() )
                        ->method( $this->anythingBut( 'getValidNamespaces', '__destruct' ) );
 
+               $mockPermissionManager = $manager ?? $this->createMock( PermissionManager::class );
+
                return new DefaultPreferencesFactory(
                        new LoggedServiceOptions( self::$serviceOptionsAccessLog,
                                DefaultPreferencesFactory::$constructorOptions, $this->config ),
                        new Language(),
                        AuthManager::singleton(),
                        MediaWikiServices::getInstance()->getLinkRenderer(),
-                       $mockNsInfo
+                       $mockNsInfo,
+                       $mockPermissionManager
                );
        }
 
@@ -88,7 +93,9 @@ class DefaultPreferencesFactoryTest extends \MediaWikiTestCase {
         * @dataProvider emailAuthenticationProvider
         */
        public function testEmailAuthentication( $user, $cssClass ) {
-               $prefs = $this->getPreferencesFactory()->getFormDescriptor( $user, $this->context );
+               $pm = $this->createMock( PermissionManager::class );
+               $pm->method( 'userHasRight' )->willReturn( true );
+               $prefs = $this->getPreferencesFactory( $pm )->getFormDescriptor( $user, $this->context );
                $this->assertArrayHasKey( 'cssclass', $prefs['emailauthentication'] );
                $this->assertEquals( $cssClass, $prefs['emailauthentication']['cssclass'] );
        }
@@ -100,16 +107,18 @@ class DefaultPreferencesFactoryTest extends \MediaWikiTestCase {
                $userMock = $this->getMockBuilder( User::class )
                        ->disableOriginalConstructor()
                        ->getMock();
-               $userMock->method( 'isAllowed' )
-                       ->willReturn( false );
                $userMock->method( 'getEffectiveGroups' )
                        ->willReturn( [] );
                $userMock->method( 'getGroupMemberships' )
                        ->willReturn( [] );
                $userMock->method( 'getOptions' )
                        ->willReturn( [ 'test' => 'yes' ] );
-
-               $prefs = $this->getPreferencesFactory()->getFormDescriptor( $userMock, $this->context );
+               $pm = $this->createMock( PermissionManager::class );
+               $pm->method( 'userHasRight' )
+                       ->will( $this->returnValueMap( [
+                               [ $userMock, 'editmyoptions', true ]
+                       ] ) );
+               $prefs = $this->getPreferencesFactory( $pm )->getFormDescriptor( $userMock, $this->context );
                $this->assertArrayNotHasKey( 'showrollbackconfirmation', $prefs );
        }
 
@@ -120,16 +129,19 @@ class DefaultPreferencesFactoryTest extends \MediaWikiTestCase {
                $userMock = $this->getMockBuilder( User::class )
                        ->disableOriginalConstructor()
                        ->getMock();
-               $userMock->method( 'isAllowed' )
-                       ->willReturn( true );
                $userMock->method( 'getEffectiveGroups' )
                        ->willReturn( [] );
                $userMock->method( 'getGroupMemberships' )
                        ->willReturn( [] );
                $userMock->method( 'getOptions' )
                        ->willReturn( [ 'test' => 'yes' ] );
-
-               $prefs = $this->getPreferencesFactory()->getFormDescriptor( $userMock, $this->context );
+               $pm = $this->createMock( PermissionManager::class );
+               $pm->method( 'userHasRight' )
+                       ->will( $this->returnValueMap( [
+                               [ $userMock, 'editmyoptions', true ],
+                               [ $userMock, 'rollback', true ]
+                       ] ) );
+               $prefs = $this->getPreferencesFactory( $pm )->getFormDescriptor( $userMock, $this->context );
                $this->assertArrayHasKey( 'showrollbackconfirmation', $prefs );
                $this->assertEquals(
                        'rendering/advancedrendering',
@@ -181,10 +193,6 @@ class DefaultPreferencesFactoryTest extends \MediaWikiTestCase {
                        ->getMock();
                $userMock->method( 'getOptions' )
                        ->willReturn( $oldOptions );
-               $userMock->method( 'isAllowedAny' )
-                       ->willReturn( true );
-               $userMock->method( 'isAllowed' )
-                       ->willReturn( true );
 
                $userMock->expects( $this->exactly( 2 ) )
                        ->method( 'setOption' )
@@ -193,18 +201,25 @@ class DefaultPreferencesFactoryTest extends \MediaWikiTestCase {
                                [ $this->equalTo( 'option' ), $this->equalTo( $newOptions[ 'option' ] ) ]
                        );
 
-               $form->expects( $this->any() )
-                       ->method( 'getModifiedUser' )
+               $form->method( 'getModifiedUser' )
                        ->willReturn( $userMock );
 
-               $form->expects( $this->any() )
-                       ->method( 'getContext' )
+               $form->method( 'getContext' )
                        ->willReturn( $this->context );
 
-               $form->expects( $this->any() )
-                       ->method( 'getConfig' )
+               $form->method( 'getConfig' )
                        ->willReturn( $configMock );
 
+               $pm = $this->createMock( PermissionManager::class );
+               $pm->method( 'userHasAnyRight' )
+                       ->will( $this->returnValueMap( [
+                               [ $userMock, 'editmyprivateinfo', 'editmyoptions', true ]
+                       ] ) );
+               $pm->method( 'userHasRight' )
+                       ->will( $this->returnValueMap( [
+                               [ $userMock, 'editmyoptions', true ]
+                       ] ) );
+
                $this->setTemporaryHook( 'PreferencesFormPreSave',
                        function ( $formData, $form, $user, &$result, $oldUserOptions )
                                use ( $newOptions, $oldOptions, $userMock ) {
@@ -220,7 +235,7 @@ class DefaultPreferencesFactoryTest extends \MediaWikiTestCase {
                );
 
                /** @var DefaultPreferencesFactory $factory */
-               $factory = TestingAccessWrapper::newFromObject( $this->getPreferencesFactory() );
+               $factory = TestingAccessWrapper::newFromObject( $this->getPreferencesFactory( $pm ) );
                $factory->saveFormData( $newOptions, $form, [] );
        }
 
@@ -233,7 +248,14 @@ class DefaultPreferencesFactoryTest extends \MediaWikiTestCase {
                // Test a string with leading zeros (i.e. not octal) and spaces.
                $this->context->getRequest()->setVal( 'wprclimit', ' 0012 ' );
                $user = new User;
-               $form = $this->getPreferencesFactory()->getForm( $user, $this->context );
+               $pm = $this->createMock( PermissionManager::class );
+               $pm->method( 'userHasAnyRight' )
+                       ->willReturn( true );
+               $pm->method( 'userHasRight' )
+                       ->will( $this->returnValueMap( [
+                               [ $user, 'editmyoptions', true ]
+                       ] ) );
+               $form = $this->getPreferencesFactory( $pm )->getForm( $user, $this->context );
                $form->show();
                $form->trySubmit();
                $this->assertEquals( 12, $user->getOption( 'rclimit' ) );