From 0df763f71d418485eaca2c1addc3693c21b454ba Mon Sep 17 00:00:00 2001 From: Petr Pchelko Date: Fri, 13 Sep 2019 14:07:59 -0700 Subject: [PATCH] Use UserIdentity instead of User in REST Change-Id: Ia6a517c6a64664be2363492108f9497fc949f299 --- .../Rest/BasicAccess/MWBasicAuthorizer.php | 8 +++--- .../BasicAccess/MWBasicRequestAuthorizer.php | 6 ++--- includes/Rest/EntryPoint.php | 6 ++++- .../Validator/ParamValidatorCallbacks.php | 17 +++++++++--- includes/Rest/Validator/Validator.php | 15 +++++++---- .../MWBasicRequestAuthorizerTest.php | 26 +++++++++---------- .../phpunit/includes/Rest/EntryPointTest.php | 4 ++- .../Rest/Handler/HelloHandlerTest.php | 5 ++-- .../phpunit/unit/includes/Rest/RouterTest.php | 4 ++- 9 files changed, 56 insertions(+), 35 deletions(-) diff --git a/includes/Rest/BasicAccess/MWBasicAuthorizer.php b/includes/Rest/BasicAccess/MWBasicAuthorizer.php index 43014f1379..92529b3679 100644 --- a/includes/Rest/BasicAccess/MWBasicAuthorizer.php +++ b/includes/Rest/BasicAccess/MWBasicAuthorizer.php @@ -2,24 +2,24 @@ namespace MediaWiki\Rest\BasicAccess; -use User; use MediaWiki\Permissions\PermissionManager; use MediaWiki\Rest\Handler; use MediaWiki\Rest\RequestInterface; +use MediaWiki\User\UserIdentity; /** - * A factory for MWBasicRequestAuthorizer which passes through a User object + * A factory for MWBasicRequestAuthorizer which passes through a UserIdentity. * * @internal */ class MWBasicAuthorizer extends BasicAuthorizerBase { - /** @var User */ + /** @var UserIdentity */ private $user; /** @var PermissionManager */ private $permissionManager; - public function __construct( User $user, PermissionManager $permissionManager ) { + public function __construct( UserIdentity $user, PermissionManager $permissionManager ) { $this->user = $user; $this->permissionManager = $permissionManager; } diff --git a/includes/Rest/BasicAccess/MWBasicRequestAuthorizer.php b/includes/Rest/BasicAccess/MWBasicRequestAuthorizer.php index 8c459c63f4..671488aaf2 100644 --- a/includes/Rest/BasicAccess/MWBasicRequestAuthorizer.php +++ b/includes/Rest/BasicAccess/MWBasicRequestAuthorizer.php @@ -2,7 +2,7 @@ namespace MediaWiki\Rest\BasicAccess; -use User; +use MediaWiki\User\UserIdentity; use MediaWiki\Permissions\PermissionManager; use MediaWiki\Rest\Handler; use MediaWiki\Rest\RequestInterface; @@ -13,14 +13,14 @@ use MediaWiki\Rest\RequestInterface; * @internal */ class MWBasicRequestAuthorizer extends BasicRequestAuthorizer { - /** @var User */ + /** @var UserIdentity */ private $user; /** @var PermissionManager */ private $permissionManager; public function __construct( RequestInterface $request, Handler $handler, - User $user, PermissionManager $permissionManager + UserIdentity $user, PermissionManager $permissionManager ) { parent::__construct( $request, $handler ); $this->user = $user; diff --git a/includes/Rest/EntryPoint.php b/includes/Rest/EntryPoint.php index 4fdd1f8c5a..070451d9ce 100644 --- a/includes/Rest/EntryPoint.php +++ b/includes/Rest/EntryPoint.php @@ -57,7 +57,11 @@ class EntryPoint { $services->getPermissionManager() ); // @phan-suppress-next-line PhanAccessMethodInternal - $restValidator = new Validator( $objectFactory, $request, RequestContext::getMain()->getUser() ); + $restValidator = new Validator( $objectFactory, + $services->getPermissionManager(), + $request, + RequestContext::getMain()->getUser() + ); global $IP; $router = new Router( diff --git a/includes/Rest/Validator/ParamValidatorCallbacks.php b/includes/Rest/Validator/ParamValidatorCallbacks.php index 6c54a504d7..93de9112c0 100644 --- a/includes/Rest/Validator/ParamValidatorCallbacks.php +++ b/includes/Rest/Validator/ParamValidatorCallbacks.php @@ -3,21 +3,30 @@ namespace MediaWiki\Rest\Validator; use InvalidArgumentException; +use MediaWiki\Permissions\PermissionManager; use MediaWiki\Rest\RequestInterface; +use MediaWiki\User\UserIdentity; use Psr\Http\Message\UploadedFileInterface; -use User; use Wikimedia\ParamValidator\Callbacks; use Wikimedia\ParamValidator\ValidationException; class ParamValidatorCallbacks implements Callbacks { + /** @var PermissionManager */ + private $permissionManager; + /** @var RequestInterface */ private $request; - /** @var User */ + /** @var UserIdentity */ private $user; - public function __construct( RequestInterface $request, User $user ) { + public function __construct( + PermissionManager $permissionManager, + RequestInterface $request, + UserIdentity $user + ) { + $this->permissionManager = $permissionManager; $this->request = $request; $this->user = $user; } @@ -76,7 +85,7 @@ class ParamValidatorCallbacks implements Callbacks { } public function useHighLimits( array $options ) { - return $this->user->isAllowed( 'apihighlimits' ); + return $this->permissionManager->userHasRight( $this->user, 'apihighlimits' ); } } diff --git a/includes/Rest/Validator/Validator.php b/includes/Rest/Validator/Validator.php index cee1cdb359..be8d7a4e66 100644 --- a/includes/Rest/Validator/Validator.php +++ b/includes/Rest/Validator/Validator.php @@ -2,10 +2,11 @@ namespace MediaWiki\Rest\Validator; +use MediaWiki\Permissions\PermissionManager; use MediaWiki\Rest\Handler; use MediaWiki\Rest\HttpException; use MediaWiki\Rest\RequestInterface; -use User; +use MediaWiki\User\UserIdentity; use Wikimedia\ObjectFactory; use Wikimedia\ParamValidator\ParamValidator; use Wikimedia\ParamValidator\TypeDef\BooleanDef; @@ -62,16 +63,20 @@ class Validator { private $paramValidator; /** - * @internal * @param ObjectFactory $objectFactory + * @param PermissionManager $permissionManager * @param RequestInterface $request - * @param User $user + * @param UserIdentity $user + * @internal */ public function __construct( - ObjectFactory $objectFactory, RequestInterface $request, User $user + ObjectFactory $objectFactory, + PermissionManager $permissionManager, + RequestInterface $request, + UserIdentity $user ) { $this->paramValidator = new ParamValidator( - new ParamValidatorCallbacks( $request, $user ), + new ParamValidatorCallbacks( $permissionManager, $request, $user ), $objectFactory, [ 'typeDefs' => self::$typeDefs, diff --git a/tests/phpunit/includes/Rest/BasicAccess/MWBasicRequestAuthorizerTest.php b/tests/phpunit/includes/Rest/BasicAccess/MWBasicRequestAuthorizerTest.php index a3102420c8..d5bbb11d78 100644 --- a/tests/phpunit/includes/Rest/BasicAccess/MWBasicRequestAuthorizerTest.php +++ b/tests/phpunit/includes/Rest/BasicAccess/MWBasicRequestAuthorizerTest.php @@ -3,7 +3,7 @@ namespace MediaWiki\Tests\Rest\BasicAccess; use GuzzleHttp\Psr7\Uri; -use MediaWiki\MediaWikiServices; +use MediaWiki\Permissions\PermissionManager; use MediaWiki\Rest\BasicAccess\MWBasicAuthorizer; use MediaWiki\Rest\Handler; use MediaWiki\Rest\RequestData; @@ -26,20 +26,18 @@ use Wikimedia\ObjectFactory; class MWBasicRequestAuthorizerTest extends MediaWikiTestCase { private function createRouter( $userRights, $request ) { $user = User::newFromName( 'Test user' ); - // Don't allow the rights to everybody so that user rights kick in. - $this->mergeMwGlobalArrayValue( 'wgGroupPermissions', [ '*' => $userRights ] ); - $this->overrideUserPermissions( - $user, - array_keys( array_filter( $userRights ), function ( $value ) { - return $value === true; - } ) - ); - - global $IP; - $objectFactory = new ObjectFactory( $this->getMockForAbstractClass( ContainerInterface::class ) ); + $permissionManager = $this->createMock( PermissionManager::class ); + // Don't allow the rights to everybody so that user rights kick in. + $permissionManager->method( 'isEveryoneAllowed' )->willReturn( false ); + $permissionManager->method( 'userHasRight' ) + ->will( $this->returnCallback( function ( $user, $action ) use ( $userRights ) { + return isset( $userRights[$action] ) && $userRights[$action]; + } ) ); + + global $IP; return new Router( [ "$IP/tests/phpunit/unit/includes/Rest/testRoutes.json" ], @@ -47,9 +45,9 @@ class MWBasicRequestAuthorizerTest extends MediaWikiTestCase { '/rest', new \EmptyBagOStuff(), new ResponseFactory( [] ), - new MWBasicAuthorizer( $user, MediaWikiServices::getInstance()->getPermissionManager() ), + new MWBasicAuthorizer( $user, $permissionManager ), $objectFactory, - new Validator( $objectFactory, $request, $user ) + new Validator( $objectFactory, $permissionManager, $request, $user ) ); } diff --git a/tests/phpunit/includes/Rest/EntryPointTest.php b/tests/phpunit/includes/Rest/EntryPointTest.php index 1c9bc41608..d05c7973c2 100644 --- a/tests/phpunit/includes/Rest/EntryPointTest.php +++ b/tests/phpunit/includes/Rest/EntryPointTest.php @@ -5,6 +5,7 @@ namespace MediaWiki\Tests\Rest; use EmptyBagOStuff; use GuzzleHttp\Psr7\Uri; use GuzzleHttp\Psr7\Stream; +use MediaWiki\Permissions\PermissionManager; use MediaWiki\Rest\BasicAccess\StaticBasicAuthorizer; use MediaWiki\Rest\Handler; use MediaWiki\Rest\EntryPoint; @@ -32,6 +33,7 @@ class EntryPointTest extends \MediaWikiTestCase { $objectFactory = new ObjectFactory( $this->getMockForAbstractClass( ContainerInterface::class ) ); + $permissionManager = $this->createMock( PermissionManager::class ); return new Router( [ "$IP/tests/phpunit/unit/includes/Rest/testRoutes.json" ], @@ -41,7 +43,7 @@ class EntryPointTest extends \MediaWikiTestCase { new ResponseFactory( [] ), new StaticBasicAuthorizer(), $objectFactory, - new Validator( $objectFactory, $request, new User ) + new Validator( $objectFactory, $permissionManager, $request, new User ) ); } diff --git a/tests/phpunit/unit/includes/Rest/Handler/HelloHandlerTest.php b/tests/phpunit/unit/includes/Rest/Handler/HelloHandlerTest.php index 7d682fda62..ca3cf105f2 100644 --- a/tests/phpunit/unit/includes/Rest/Handler/HelloHandlerTest.php +++ b/tests/phpunit/unit/includes/Rest/Handler/HelloHandlerTest.php @@ -4,6 +4,7 @@ namespace MediaWiki\Tests\Rest\Handler; use EmptyBagOStuff; use GuzzleHttp\Psr7\Uri; +use MediaWiki\Permissions\PermissionManager; use MediaWiki\Rest\BasicAccess\StaticBasicAuthorizer; use MediaWiki\Rest\RequestData; use MediaWiki\Rest\ResponseFactory; @@ -55,7 +56,7 @@ class HelloHandlerTest extends \MediaWikiUnitTestCase { $objectFactory = new ObjectFactory( $this->getMockForAbstractClass( ContainerInterface::class ) ); - + $permissionManager = $this->createMock( PermissionManager::class ); $request = new RequestData( $requestInfo ); $router = new Router( [ __DIR__ . '/../testRoutes.json' ], @@ -65,7 +66,7 @@ class HelloHandlerTest extends \MediaWikiUnitTestCase { new ResponseFactory( [] ), new StaticBasicAuthorizer(), $objectFactory, - new Validator( $objectFactory, $request, new User ) + new Validator( $objectFactory, $permissionManager, $request, new User ) ); $response = $router->execute( $request ); if ( isset( $responseInfo['statusCode'] ) ) { diff --git a/tests/phpunit/unit/includes/Rest/RouterTest.php b/tests/phpunit/unit/includes/Rest/RouterTest.php index 58039ea846..270bcfc483 100644 --- a/tests/phpunit/unit/includes/Rest/RouterTest.php +++ b/tests/phpunit/unit/includes/Rest/RouterTest.php @@ -3,6 +3,7 @@ namespace MediaWiki\Tests\Rest; use GuzzleHttp\Psr7\Uri; +use MediaWiki\Permissions\PermissionManager; use MediaWiki\Rest\BasicAccess\StaticBasicAuthorizer; use MediaWiki\Rest\Handler; use MediaWiki\Rest\HttpException; @@ -24,6 +25,7 @@ class RouterTest extends \MediaWikiUnitTestCase { $objectFactory = new ObjectFactory( $this->getMockForAbstractClass( ContainerInterface::class ) ); + $permissionManager = $this->createMock( PermissionManager::class ); return new Router( [ __DIR__ . '/testRoutes.json' ], [], @@ -32,7 +34,7 @@ class RouterTest extends \MediaWikiUnitTestCase { new ResponseFactory( [] ), new StaticBasicAuthorizer( $authError ), $objectFactory, - new Validator( $objectFactory, $request, new User ) + new Validator( $objectFactory, $permissionManager, $request, new User ) ); } -- 2.20.1