diff --git a/includes/CrawlerProtectionService.php b/includes/CrawlerProtectionService.php index 3df4426..0dd0360 100644 --- a/includes/CrawlerProtectionService.php +++ b/includes/CrawlerProtectionService.php @@ -90,11 +90,20 @@ public function checkPerformAction( $diffId = (int)$request->getVal( 'diff' ); $oldId = (int)$request->getVal( 'oldid' ); + // The type=revision, diff and oldid parameters are all ways of + // viewing page history. They are only blocked when 'history' is + // in the configured list of protected actions, so that operators + // can fully disable history blocking by removing 'history' from + // $wgCrawlerProtectedActions. + $historyProtected = $this->isProtectedAction( 'history' ); + if ( - $type === 'revision' - || $this->isProtectedAction( $action ) - || $diffId > 0 - || $oldId > 0 + $this->isProtectedAction( $action ) + || ( $historyProtected && ( + $type === 'revision' + || $diffId > 0 + || $oldId > 0 + ) ) ) { $this->responseFactory->denyAccess( $output ); return false; diff --git a/tests/phpunit/namespaced-stubs.php b/tests/phpunit/namespaced-stubs.php index eff675a..b173c54 100644 --- a/tests/phpunit/namespaced-stubs.php +++ b/tests/phpunit/namespaced-stubs.php @@ -100,9 +100,13 @@ public function getContext() { namespace MediaWiki\User { class User { - public function isRegistered() { + public function isRegistered(): bool { return false; } + + public function getName(): string { + return ''; + } } } diff --git a/tests/phpunit/unit/CrawlerProtectionServiceTest.php b/tests/phpunit/unit/CrawlerProtectionServiceTest.php index 9349363..8d99287 100644 --- a/tests/phpunit/unit/CrawlerProtectionServiceTest.php +++ b/tests/phpunit/unit/CrawlerProtectionServiceTest.php @@ -204,6 +204,83 @@ public function testCheckPerformActionBlocksConfiguredAction() { $this->assertFalse( $service->checkPerformAction( $output, $user, $request ) ); } + /** + * When 'history' is removed from CrawlerProtectedActions, the + * history-related parameters (type=revision, diff, oldid) should + * also be allowed, so that operators can fully disable history + * blocking via configuration (see issue: history can't be unblocked). + * + * @covers ::checkPerformAction + * @dataProvider provideHistoryRelatedRequestParams + * + * @param array $getValMap + * @param string $msg + */ + public function testCheckPerformActionAllowsHistoryRelatedWhenNotConfigured( + array $getValMap, string $msg + ) { + $output = $this->createMock( self::$outputPageClassName ); + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( false ); + $user->method( 'getName' )->willReturn( '127.0.0.1' ); + + $request = $this->createMock( self::$webRequestClassName ); + $request->method( 'getVal' )->willReturnMap( $getValMap ); + + $responseFactory = $this->createMock( ResponseFactory::class ); + $responseFactory->expects( $this->never() )->method( 'denyAccess' ); + + $service = $this->buildService( [], [], [], $responseFactory ); + $this->assertTrue( $service->checkPerformAction( $output, $user, $request ), $msg ); + } + + /** + * Data provider for history-related parameters that should be + * allowed when 'history' is not in CrawlerProtectedActions. + * + * @return array + */ + public function provideHistoryRelatedRequestParams(): array { + return [ + 'action=history' => [ + [ + [ 'type', null, null ], + [ 'action', null, 'history' ], + [ 'diff', null, null ], + [ 'oldid', null, null ], + ], + 'action=history should be allowed when history not protected', + ], + 'type=revision' => [ + [ + [ 'type', null, 'revision' ], + [ 'action', null, null ], + [ 'diff', null, null ], + [ 'oldid', null, null ], + ], + 'type=revision should be allowed when history not protected', + ], + 'diff=42' => [ + [ + [ 'type', null, null ], + [ 'action', null, null ], + [ 'diff', null, '42' ], + [ 'oldid', null, null ], + ], + 'diff=42 should be allowed when history not protected', + ], + 'oldid=99' => [ + [ + [ 'type', null, null ], + [ 'action', null, null ], + [ 'diff', null, null ], + [ 'oldid', null, '99' ], + ], + 'oldid=99 should be allowed when history not protected', + ], + ]; + } + /** * @covers ::checkPerformAction */