diff --git a/extension.json b/extension.json index 849dda5..ac75851 100644 --- a/extension.json +++ b/extension.json @@ -1,7 +1,8 @@ { "name": "CrawlerProtection", "author": "MyWikis LLC", - "version": "1.2.0", + "version": "1.3.0", + "url": "https://www.mediawiki.org/wiki/Extension:CrawlerProtection", "description": "Suite of protective measures to protect wikis from crawlers.", "type": "hook", "requires": { @@ -23,6 +24,11 @@ "SpecialPageBeforeExecute": "main" }, "config": { + "CrawlerProtectedActions": { + "value": [ + "history" + ] + }, "CrawlerProtectedSpecialPages": { "value": [ "mobilediff", diff --git a/includes/CrawlerProtectionService.php b/includes/CrawlerProtectionService.php index 43feda7..102a649 100644 --- a/includes/CrawlerProtectionService.php +++ b/includes/CrawlerProtectionService.php @@ -40,6 +40,7 @@ class CrawlerProtectionService { /** @var string[] List of constructor options this class accepts */ public const CONSTRUCTOR_OPTIONS = [ + 'CrawlerProtectedActions', 'CrawlerProtectedSpecialPages', ]; @@ -89,7 +90,7 @@ public function checkPerformAction( if ( $type === 'revision' - || $action === 'history' + || $this->isProtectedAction( $action ) || $diffId > 0 || $oldId > 0 ) { @@ -100,6 +101,29 @@ public function checkPerformAction( return true; } + /** + * Determine whether the given action name is in the configured + * list of protected actions. + * + * The comparison is case-insensitive so that configured values + * like "History" or "HISTORY" match the action parameter. + * + * @param string|null $action + * @return bool + */ + public function isProtectedAction( ?string $action ): bool { + if ( $action === null ) { + return false; + } + + $protectedActions = array_map( + 'strtolower', + $this->options->get( 'CrawlerProtectedActions' ) + ); + + return in_array( strtolower( $action ), $protectedActions, true ); + } + /** * Check whether a special page request should be blocked. * diff --git a/tests/phpunit/unit/CrawlerProtectionServiceTest.php b/tests/phpunit/unit/CrawlerProtectionServiceTest.php index 1e92f5e..e7fd061 100644 --- a/tests/phpunit/unit/CrawlerProtectionServiceTest.php +++ b/tests/phpunit/unit/CrawlerProtectionServiceTest.php @@ -41,16 +41,21 @@ public static function setUpBeforeClass(): void { * a mock ResponseFactory. * * @param array $protectedPages + * @param array $protectedActions * @param ResponseFactory|\PHPUnit\Framework\MockObject\MockObject|null $responseFactory * @return CrawlerProtectionService */ private function buildService( array $protectedPages = [ 'recentchangeslinked', 'whatlinkshere', 'mobilediff' ], + array $protectedActions = [ 'history' ], $responseFactory = null ): CrawlerProtectionService { $options = new ServiceOptions( CrawlerProtectionService::CONSTRUCTOR_OPTIONS, - [ 'CrawlerProtectedSpecialPages' => $protectedPages ] + [ + 'CrawlerProtectedActions' => $protectedActions, + 'CrawlerProtectedSpecialPages' => $protectedPages, + ] ); $responseFactory ??= $this->createMock( ResponseFactory::class ); @@ -78,7 +83,7 @@ public function testCheckPerformActionAllowsRegisteredUser() { $responseFactory = $this->createMock( ResponseFactory::class ); $responseFactory->expects( $this->never() )->method( 'denyAccess' ); - $service = $this->buildService( [], $responseFactory ); + $service = $this->buildService( [], [ 'history' ], $responseFactory ); $this->assertTrue( $service->checkPerformAction( $output, $user, $request ) ); } @@ -100,7 +105,7 @@ public function testCheckPerformActionBlocksAnonymous( array $getValMap, string $responseFactory = $this->createMock( ResponseFactory::class ); $responseFactory->expects( $this->once() )->method( 'denyAccess' )->with( $output ); - $service = $this->buildService( [], $responseFactory ); + $service = $this->buildService( [], [ 'history' ], $responseFactory ); $this->assertFalse( $service->checkPerformAction( $output, $user, $request ), $msg ); } @@ -169,10 +174,96 @@ public function testCheckPerformActionAllowsNormalAnonymousView() { $responseFactory = $this->createMock( ResponseFactory::class ); $responseFactory->expects( $this->never() )->method( 'denyAccess' ); - $service = $this->buildService( [], $responseFactory ); + $service = $this->buildService( [], [ 'history' ], $responseFactory ); $this->assertTrue( $service->checkPerformAction( $output, $user, $request ) ); } + /** + * @covers ::checkPerformAction + */ + public function testCheckPerformActionBlocksConfiguredAction() { + $output = $this->createMock( self::$outputPageClassName ); + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( false ); + + $request = $this->createMock( self::$webRequestClassName ); + $request->method( 'getVal' )->willReturnMap( [ + [ 'type', null, null ], + [ 'action', null, 'edit' ], + [ 'diff', null, null ], + [ 'oldid', null, null ], + ] ); + + $responseFactory = $this->createMock( ResponseFactory::class ); + $responseFactory->expects( $this->once() )->method( 'denyAccess' )->with( $output ); + + $service = $this->buildService( [], [ 'edit', 'history' ], $responseFactory ); + $this->assertFalse( $service->checkPerformAction( $output, $user, $request ) ); + } + + /** + * @covers ::checkPerformAction + */ + public function testCheckPerformActionAllowsActionNotInConfig() { + $output = $this->createMock( self::$outputPageClassName ); + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( false ); + + $request = $this->createMock( self::$webRequestClassName ); + $request->method( 'getVal' )->willReturnMap( [ + [ 'type', null, null ], + [ 'action', null, 'history' ], + [ 'diff', null, null ], + [ 'oldid', null, null ], + ] ); + + $responseFactory = $this->createMock( ResponseFactory::class ); + $responseFactory->expects( $this->never() )->method( 'denyAccess' ); + + $service = $this->buildService( [], [], $responseFactory ); + $this->assertTrue( $service->checkPerformAction( $output, $user, $request ) ); + } + + // --------------------------------------------------------------- + // isProtectedAction tests + // --------------------------------------------------------------- + + /** + * @covers ::isProtectedAction + */ + public function testIsProtectedActionReturnsTrueForConfiguredAction() { + $service = $this->buildService( [], [ 'history', 'edit' ] ); + $this->assertTrue( $service->isProtectedAction( 'history' ) ); + $this->assertTrue( $service->isProtectedAction( 'edit' ) ); + } + + /** + * @covers ::isProtectedAction + */ + public function testIsProtectedActionReturnsFalseForUnconfiguredAction() { + $service = $this->buildService( [], [ 'history' ] ); + $this->assertFalse( $service->isProtectedAction( 'view' ) ); + $this->assertFalse( $service->isProtectedAction( 'edit' ) ); + } + + /** + * @covers ::isProtectedAction + */ + public function testIsProtectedActionReturnsFalseForNull() { + $service = $this->buildService( [], [ 'history' ] ); + $this->assertFalse( $service->isProtectedAction( null ) ); + } + + /** + * @covers ::isProtectedAction + */ + public function testIsProtectedActionIsCaseInsensitive() { + $service = $this->buildService( [], [ 'History' ] ); + $this->assertTrue( $service->isProtectedAction( 'history' ) ); + $this->assertTrue( $service->isProtectedAction( 'HISTORY' ) ); + $this->assertTrue( $service->isProtectedAction( 'History' ) ); + } + // --------------------------------------------------------------- // checkSpecialPage tests // --------------------------------------------------------------- @@ -193,6 +284,7 @@ public function testCheckSpecialPageBlocksAnonymous( string $specialPageName ) { $service = $this->buildService( [ 'RecentChangesLinked', 'WhatLinksHere', 'MobileDiff' ], + [], $responseFactory ); $this->assertFalse( $service->checkSpecialPage( $specialPageName, $output, $user ) ); @@ -214,6 +306,7 @@ public function testCheckSpecialPageAllowsRegistered( string $specialPageName ) $service = $this->buildService( [ 'RecentChangesLinked', 'WhatLinksHere', 'MobileDiff' ], + [], $responseFactory ); $this->assertTrue( $service->checkSpecialPage( $specialPageName, $output, $user ) ); @@ -232,6 +325,7 @@ public function testCheckSpecialPageAllowsUnprotected() { $service = $this->buildService( [ 'RecentChangesLinked', 'WhatLinksHere', 'MobileDiff' ], + [], $responseFactory ); $this->assertTrue( $service->checkSpecialPage( 'Search', $output, $user ) );