From 5ccb1a42c7752778db2381d180dbee0ed9b7e779 Mon Sep 17 00:00:00 2001 From: Petr Pchelko Date: Thu, 22 Aug 2019 15:53:05 -0700 Subject: [PATCH] Make DefaultPreferencesFactory depend on PermissionManager. Bug: T220191 Change-Id: I3f5c4340501d59b5ca63b096364b5cc8388cff80 --- includes/ServiceWiring.php | 3 +- .../preferences/DefaultPreferencesFactory.php | 60 ++++++++-------- .../DefaultPreferencesFactoryTest.php | 68 ++++++++++++------- 3 files changed, 80 insertions(+), 51 deletions(-) diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index c7ad75c127..689a98e482 100644 --- 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 66c2bc33e5..56db81222e 100644 --- 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 e7f7067adb..a45944167a 100644 --- 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' ) ); -- 2.20.1