From 5b9d45cdc04e96ddc97b1610fe45ae56f41a60c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= Date: Fri, 21 Feb 2025 22:07:19 +0100 Subject: editpage: Refactor user right, rate limit and block checks to use Authority Replace EditRightConstraint, UserBlockConstraint, and most of UserRateLimitConstraint with the new AuthorizationConstraint. Instead of many separate checks, everything is now handled by one authorizeWrite() call. Move 'editcontentmodel' rate limit to ContentModelChangeConstraint (by making it use authorizeWrite()). Keep 'linkpurge' rate limit in a separate check, renamed from UserRateLimitConstraint to LinkPurgeRateLimitConstraint, since the way it works in unusual and not portable to Authority without more refactoring in EditPage. AuthorizationConstraint needs some special handling to produce the idiosyncratic result codes required by EditPage, but luckily PermissionStatus gives us everything we need for that. Bug: T271975 Bug: T386346 Change-Id: Ic9f6f2fbd29efa3e349517013da540a363c263b5 --- .../Constraint/AuthorizationConstraintTest.php | 111 ++++++++++++++ .../Constraint/EditConstraintFactoryTest.php | 7 - .../Constraint/EditRightConstraintTest.php | 170 --------------------- .../LinkPurgeRateLimitConstraintTest.php | 70 +++++++++ .../Constraint/UserBlockConstraintTest.php | 72 --------- .../Constraint/UserRateLimitConstraintTest.php | 79 ---------- 6 files changed, 181 insertions(+), 328 deletions(-) create mode 100644 tests/phpunit/unit/includes/editpage/Constraint/AuthorizationConstraintTest.php delete mode 100644 tests/phpunit/unit/includes/editpage/Constraint/EditRightConstraintTest.php create mode 100644 tests/phpunit/unit/includes/editpage/Constraint/LinkPurgeRateLimitConstraintTest.php delete mode 100644 tests/phpunit/unit/includes/editpage/Constraint/UserBlockConstraintTest.php delete mode 100644 tests/phpunit/unit/includes/editpage/Constraint/UserRateLimitConstraintTest.php (limited to 'tests/phpunit/unit/includes/editpage') diff --git a/tests/phpunit/unit/includes/editpage/Constraint/AuthorizationConstraintTest.php b/tests/phpunit/unit/includes/editpage/Constraint/AuthorizationConstraintTest.php new file mode 100644 index 000000000000..68d8c6541858 --- /dev/null +++ b/tests/phpunit/unit/includes/editpage/Constraint/AuthorizationConstraintTest.php @@ -0,0 +1,111 @@ +assertConstraintPassed( $constraint ); + } + + public function provideTestPass(): iterable { + yield 'Edit existing page' => [ + 'performer' => $this->mockAnonAuthorityWithPermissions( [ 'edit' ] ), + 'page' => PageIdentityValue::localIdentity( 123, NS_MAIN, 'AuthorizationConstraintTest' ), + 'new' => false, + ]; + yield 'Create a new page' => [ + 'performer' => $this->mockAnonAuthorityWithPermissions( [ 'edit', 'create' ] ), + 'page' => PageIdentityValue::localIdentity( 0, NS_MAIN, 'AuthorizationConstraintTest' ), + 'new' => true, + ]; + } + + /** + * @dataProvider provideTestFailure + */ + public function testFailure( + Authority $performer, PageIdentity $page, bool $new, int $expectedValue + ): void { + $constraint = new AuthorizationConstraint( + $performer, + $page, + $new + ); + $this->assertConstraintFailed( $constraint, $expectedValue ); + } + + public function provideTestFailure(): iterable { + yield 'Anonymous user' => [ + 'performer' => $this->mockAnonAuthorityWithoutPermissions( [ 'edit' ] ), + 'page' => PageIdentityValue::localIdentity( 123, NS_MAIN, 'AuthorizationConstraintTest' ), + 'new' => false, + 'expectedValue' => IEditConstraint::AS_READ_ONLY_PAGE_ANON, + ]; + yield 'Registered user' => [ + 'performer' => $this->mockRegisteredAuthorityWithoutPermissions( [ 'edit' ] ), + 'page' => PageIdentityValue::localIdentity( 123, NS_MAIN, 'AuthorizationConstraintTest' ), + 'new' => false, + 'expectedValue' => IEditConstraint::AS_READ_ONLY_PAGE_LOGGED, + ]; + yield 'User without create permission creates a page' => [ + 'performer' => $this->mockAnonAuthorityWithoutPermissions( [ 'create' ] ), + 'page' => PageIdentityValue::localIdentity( 0, NS_MAIN, 'AuthorizationConstraintTest' ), + 'new' => true, + 'expectedValue' => IEditConstraint::AS_NO_CREATE_PERMISSION, + ]; + yield 'Blocked user' => [ + 'performer' => $this->mockUserAuthorityWithBlock( + UserIdentityValue::newRegistered( 42, 'AuthorizationConstraintTest User' ), + $this->makeMockBlock( [ + 'decodedExpiry' => 'infinity', + ] ), + [ 'edit' ] + ), + 'page' => PageIdentityValue::localIdentity( 123, NS_MAIN, 'AuthorizationConstraintTest' ), + 'new' => false, + 'expectedValue' => IEditConstraint::AS_BLOCKED_PAGE_FOR_USER, + ]; + } + +} diff --git a/tests/phpunit/unit/includes/editpage/Constraint/EditConstraintFactoryTest.php b/tests/phpunit/unit/includes/editpage/Constraint/EditConstraintFactoryTest.php index dabb27a15ff1..76ce13cb71d5 100644 --- a/tests/phpunit/unit/includes/editpage/Constraint/EditConstraintFactoryTest.php +++ b/tests/phpunit/unit/includes/editpage/Constraint/EditConstraintFactoryTest.php @@ -27,13 +27,11 @@ use MediaWiki\EditPage\Constraint\PageSizeConstraint; use MediaWiki\EditPage\Constraint\ReadOnlyConstraint; use MediaWiki\EditPage\Constraint\SimpleAntiSpamConstraint; use MediaWiki\EditPage\Constraint\SpamRegexConstraint; -use MediaWiki\EditPage\Constraint\UserBlockConstraint; use MediaWiki\EditPage\SpamChecker; use MediaWiki\HookContainer\HookContainer; use MediaWiki\Language\Language; use MediaWiki\Logger\Spi; use MediaWiki\MainConfigNames; -use MediaWiki\Permissions\PermissionManager; use MediaWiki\Permissions\RateLimiter; use MediaWiki\Title\Title; use MediaWiki\User\User; @@ -61,7 +59,6 @@ class EditConstraintFactoryTest extends MediaWikiUnitTestCase { $factory = new EditConstraintFactory( $options, $loggerFactory, - $this->createMock( PermissionManager::class ), $this->createMock( HookContainer::class ), $this->createMock( ReadOnlyMode::class ), $this->createMock( SpamChecker::class ), @@ -107,9 +104,5 @@ class EditConstraintFactoryTest extends MediaWikiUnitTestCase { $title ) ); - $this->assertInstanceOf( - UserBlockConstraint::class, - $factory->newUserBlockConstraint( $title, $user ) - ); } } diff --git a/tests/phpunit/unit/includes/editpage/Constraint/EditRightConstraintTest.php b/tests/phpunit/unit/includes/editpage/Constraint/EditRightConstraintTest.php deleted file mode 100644 index 9d3346e87d01..000000000000 --- a/tests/phpunit/unit/includes/editpage/Constraint/EditRightConstraintTest.php +++ /dev/null @@ -1,170 +0,0 @@ -createMock( Title::class ), - $new - ); - $this->assertConstraintPassed( $constraint ); - } - - public function provideTestPass() { - $title = $this->createMock( Title::class ); - $userEdit = $this->createMock( User::class ); - $permissionManagerEdit = $this->createMock( PermissionManager::class ); - $permissionManagerEdit->expects( $this->once() ) - ->method( 'userCan' ) - ->with( - 'edit', - $userEdit, - $title - ) - ->willReturn( true ); - $userCreateAndEdit = $this->createMock( User::class ); - $userCreateAndEdit->expects( $this->once() ) - ->method( 'authorizeWrite' ) - ->with( - 'create', - $title - ) - ->willReturn( true ); - $permissionManagerCreateAndEdit = $this->createMock( PermissionManager::class ); - $permissionManagerCreateAndEdit->expects( $this->once() ) - ->method( 'userCan' ) - ->with( - 'edit', - $userCreateAndEdit, - $title - ) - ->willReturn( true ); - yield 'Edit existing page' => [ - 'performer' => $userEdit, - 'new' => false, - 'permissionManager' => $permissionManagerEdit - ]; - yield 'Create a new page' => [ - 'performer' => $userCreateAndEdit, - 'new' => true, - 'permissionManager' => $permissionManagerCreateAndEdit - ]; - } - - /** - * @dataProvider provideTestFailure - * @param User $performer - * @param bool $new - * @param PermissionManager $permissionManager - * @param int $expectedValue - */ - public function testFailure( - User $performer, bool $new, PermissionManager $permissionManager, int $expectedValue - ) { - $title = $this->createMock( Title::class ); - $constraint = new EditRightConstraint( - $performer, - $permissionManager, - $title, - $new - ); - $this->assertConstraintFailed( $constraint, $expectedValue ); - } - - public function provideTestFailure() { - $title = $this->createMock( Title::class ); - $anon = $this->createMock( User::class ); - $anon->expects( $this->once() )->method( 'isRegistered' )->willReturn( false ); - $permissionManagerAnon = $this->createMock( PermissionManager::class ); - $permissionManagerAnon->expects( $this->once() ) - ->method( 'userCan' ) - ->with( - 'edit', - $anon, - $title - ) - ->willReturn( false ); - $reg = $this->createMock( User::class ); - $reg->expects( $this->once() )->method( 'isRegistered' )->willReturn( true ); - $permissionManagerReg = $this->createMock( PermissionManager::class ); - $permissionManagerReg->expects( $this->once() ) - ->method( 'userCan' ) - ->with( - 'edit', - $reg, - $title - ) - ->willReturn( false ); - $userWithoutCreatePerm = $this->createMock( User::class ); - $userWithoutCreatePerm->expects( $this->once() ) - ->method( 'authorizeWrite' ) - ->with( - 'create', - $title - ) - ->willReturn( false ); - yield 'Anonymous user' => [ - 'performer' => $anon, - 'new' => false, - 'permissionManager' => $permissionManagerAnon, - 'expectedValue' => IEditConstraint::AS_READ_ONLY_PAGE_ANON, - ]; - yield 'Registered user' => [ - 'performer' => $reg, - 'new' => false, - 'permissionManager' => $permissionManagerReg, - 'expectedValue' => IEditConstraint::AS_READ_ONLY_PAGE_LOGGED, - ]; - yield 'User without create permission creates a page' => [ - 'performer' => $userWithoutCreatePerm, - 'new' => true, - 'permissionManager' => $this->createMock( PermissionManager::class ), - 'expectedValue' => IEditConstraint::AS_NO_CREATE_PERMISSION, - ]; - } - -} diff --git a/tests/phpunit/unit/includes/editpage/Constraint/LinkPurgeRateLimitConstraintTest.php b/tests/phpunit/unit/includes/editpage/Constraint/LinkPurgeRateLimitConstraintTest.php new file mode 100644 index 000000000000..7363601e0f72 --- /dev/null +++ b/tests/phpunit/unit/includes/editpage/Constraint/LinkPurgeRateLimitConstraintTest.php @@ -0,0 +1,70 @@ +createNoOpMock( RateLimiter::class, [ 'limit' ] ); + $mock->expects( $this->once() ) + ->method( 'limit' ) + ->with( self::anything(), 'linkpurge', 0 ) + ->willReturn( $fail ); + return $mock; + } + + public function testPass() { + $limiter = $this->getRateLimiter( false ); + + $subject = new RateLimitSubject( new UserIdentityValue( 1, 'test' ), null, [] ); + + $constraint = new LinkPurgeRateLimitConstraint( $limiter, $subject ); + $this->assertConstraintPassed( $constraint ); + } + + public function testFailure() { + $limiter = $this->getRateLimiter( true ); + + $subject = new RateLimitSubject( new UserIdentityValue( 1, 'test' ), null, [] ); + + $constraint = new LinkPurgeRateLimitConstraint( $limiter, $subject ); + $this->assertConstraintFailed( $constraint, IEditConstraint::AS_RATE_LIMITED ); + } + +} diff --git a/tests/phpunit/unit/includes/editpage/Constraint/UserBlockConstraintTest.php b/tests/phpunit/unit/includes/editpage/Constraint/UserBlockConstraintTest.php deleted file mode 100644 index e231e4a50c66..000000000000 --- a/tests/phpunit/unit/includes/editpage/Constraint/UserBlockConstraintTest.php +++ /dev/null @@ -1,72 +0,0 @@ -createMock( Title::class ); - $user = $this->createMock( User::class ); - $permissionManager = $this->createMock( PermissionManager::class ); - $permissionManager->expects( $this->once() ) - ->method( 'isBlockedFrom' ) - ->with( - $user, - $title - ) - ->willReturn( false ); - - $constraint = new UserBlockConstraint( $permissionManager, $title, $user ); - $this->assertConstraintPassed( $constraint ); - } - - public function testFailure() { - $title = $this->createMock( Title::class ); - $user = $this->createMock( User::class ); - $permissionManager = $this->createMock( PermissionManager::class ); - $permissionManager->expects( $this->once() ) - ->method( 'isBlockedFrom' ) - ->with( - $user, - $title - ) - ->willReturn( true ); - - $constraint = new UserBlockConstraint( $permissionManager, $title, $user ); - $this->assertConstraintFailed( - $constraint, - IEditConstraint::AS_BLOCKED_PAGE_FOR_USER - ); - } - -} diff --git a/tests/phpunit/unit/includes/editpage/Constraint/UserRateLimitConstraintTest.php b/tests/phpunit/unit/includes/editpage/Constraint/UserRateLimitConstraintTest.php deleted file mode 100644 index b832aa7ee5e5..000000000000 --- a/tests/phpunit/unit/includes/editpage/Constraint/UserRateLimitConstraintTest.php +++ /dev/null @@ -1,79 +0,0 @@ -createNoOpMock( RateLimiter::class, [ 'limit' ] ); - $expectedArgs = [ - [ 'edit', 1, false ], - [ 'linkpurge', 0, false ], - [ 'editcontentmodel', 1, $fail ] - ]; - $mock->expects( $this->exactly( 3 ) ) - ->method( 'limit' ) - ->willReturnCallback( function ( $_, $action, $incrBy ) use ( &$expectedArgs ) { - $curExpectedArgs = array_shift( $expectedArgs ); - $this->assertSame( $curExpectedArgs[0], $action ); - $this->assertSame( $curExpectedArgs[1], $incrBy ); - return $curExpectedArgs[2]; - } ); - return $mock; - } - - public function testPass() { - $limiter = $this->getRateLimiter( false ); - - $subject = new RateLimitSubject( new UserIdentityValue( 1, 'test' ), null, [] ); - - $constraint = new UserRateLimitConstraint( $limiter, $subject, 'OldContentModel', 'NewContentModel' ); - $this->assertConstraintPassed( $constraint ); - } - - public function testFailure() { - $limiter = $this->getRateLimiter( true ); - - $subject = new RateLimitSubject( new UserIdentityValue( 1, 'test' ), null, [] ); - - $constraint = new UserRateLimitConstraint( $limiter, $subject, 'OldContentModel', 'NewContentModel' ); - $this->assertConstraintFailed( $constraint, IEditConstraint::AS_RATE_LIMITED ); - } - -} -- cgit v1.2.3