diff --git a/SimpleCaptcha/SimpleCaptcha.php b/SimpleCaptcha/SimpleCaptcha.php index 8e3337623..8f4f77e4d 100644 --- a/SimpleCaptcha/SimpleCaptcha.php +++ b/SimpleCaptcha/SimpleCaptcha.php @@ -47,8 +47,14 @@ use WikiPage; class SimpleCaptcha { protected static $messagePrefix = 'captcha-'; + /** @var bool Override to force showing the CAPTCHA to users who don't have "skipcaptcha" right. */ + private bool $forceShowCaptcha = false; + /** @var bool|null Was the CAPTCHA already passed and if yes, with which result? */ - private $captchaSolved = null; + private ?bool $captchaSolved = null; + + /** @var bool Flag to indicate whether the onEditFilterMergedContent hook was invoked. */ + private bool $editFilterMergedContentHandlerCalled = false; /** @var bool[] Activate captchas status list for a pages by key */ private $activatedCaptchas = []; @@ -471,6 +477,11 @@ class SimpleCaptcha { * @return bool True, if the action should trigger a CAPTCHA, false otherwise */ public function triggersCaptcha( $action, $title = null ) { + // Captcha was already solved, we don't need to check anything else. + if ( $this->isCaptchaSolved() ) { + return false; + } + global $wgCaptchaTriggers, $wgCaptchaTriggersOnNamespace; $result = false; @@ -492,6 +503,12 @@ class SimpleCaptcha { $result = $wgCaptchaTriggersOnNamespace[$title->getNamespace()][$action]; } + // SimpleCaptcha has been instructed to force showing the CAPTCHA, no need to + // check what other hook implementations think. + if ( $this->shouldForceShowCaptcha() ) { + return true; + } + $hookRunner = new HookRunner( MediaWikiServices::getInstance()->getHookContainer() ); @@ -647,6 +664,49 @@ class SimpleCaptcha { return false; } + public function isCaptchaSolved(): ?bool { + return $this->captchaSolved; + } + + protected function setCaptchaSolved( ?bool $captchaSolved ): void { + $this->captchaSolved = $captchaSolved; + } + + /** + * @return bool True if an override is set to force showing a CAPTCHA + * to the user. Note that users with "skipcaptcha" right may still + * bypass this override. + */ + public function shouldForceShowCaptcha(): bool { + return $this->forceShowCaptcha; + } + + /** + * @param bool $forceShowCaptcha True if the caller wants to force showing + * a CAPTCHA to the user. Note that users with "skipcaptcha" right may + * still bypass this override. + * @return void + */ + public function setForceShowCaptcha( bool $forceShowCaptcha ): void { + $this->forceShowCaptcha = $forceShowCaptcha; + } + + /** + * @return bool Was the EditFilterMergedContent hook implementation already + * invoked? + */ + public function editFilterMergedContentHandlerAlreadyInvoked(): bool { + return $this->editFilterMergedContentHandlerCalled; + } + + /** + * @return void Set a flag on the class stating that EditFilterMergedContent handler + * was already run. + */ + public function setEditFilterMergedContentHandlerInvoked(): void { + $this->editFilterMergedContentHandlerCalled = true; + } + /** * Filter callback function for URL whitelisting * @param string $url string to check @@ -985,8 +1045,8 @@ class SimpleCaptcha { protected function passCaptcha( $index, $word ) { // Don't check the same CAPTCHA twice in one session, // if the CAPTCHA was already checked - Bug T94276 - if ( $this->captchaSolved !== null ) { - return $this->captchaSolved; + if ( $this->isCaptchaSolved() !== null ) { + return (bool)$this->isCaptchaSolved(); } $info = $this->retrieveCaptcha( $index ); @@ -994,12 +1054,12 @@ class SimpleCaptcha { if ( $this->keyMatch( $word, $info ) ) { $this->log( "passed" ); $this->clearCaptcha( $index ); - $this->captchaSolved = true; + $this->setCaptchaSolved( true ); return true; } else { $this->clearCaptcha( $index ); $this->log( "bad form input" ); - $this->captchaSolved = false; + $this->setCaptchaSolved( false ); return false; } } else { diff --git a/includes/AbuseFilter/CaptchaConsequence.php b/includes/AbuseFilter/CaptchaConsequence.php index f08239c40..22c7fe4ac 100644 --- a/includes/AbuseFilter/CaptchaConsequence.php +++ b/includes/AbuseFilter/CaptchaConsequence.php @@ -2,21 +2,18 @@ namespace MediaWiki\Extension\ConfirmEdit\AbuseFilter; -use MediaWiki\Context\RequestContext; use MediaWiki\Extension\AbuseFilter\Consequences\Consequence\Consequence; +use MediaWiki\Extension\ConfirmEdit\Hooks; /** - * Will inform ConfirmEdit extension to show a CAPTCHA. + * Show a CAPTCHA to the user before they can proceed with an action. */ class CaptchaConsequence extends Consequence { - public const FLAG = 'wgAbuseFilterCaptchaConsequence'; - public function execute(): bool { - // This consequence was triggered, so we need to set a global flag - // which Extension:ConfirmEdit will read in order to decide if a - // CAPTCHA should be shown to the user in onConfirmEditTriggersCaptcha - RequestContext::getMain()->getRequest()->setVal( self::FLAG, true ); + // This consequence was triggered, so we need to set a flag + // on the SimpleCaptcha instance to force showing the CAPTCHA. + Hooks::getInstance()->setForceShowCaptcha( true ); return true; } } diff --git a/includes/Hooks.php b/includes/Hooks.php index 5c856e3bf..76c540cdc 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -92,7 +92,12 @@ class Hooks implements public function onEditFilterMergedContent( IContextSource $context, Content $content, Status $status, $summary, User $user, $minorEdit ) { - return self::getInstance()->confirmEditMerged( $context, $content, $status, $summary, + $simpleCaptcha = self::getInstance(); + // Set a flag indicating that ConfirmEdit's implementation of + // EditFilterMergedContent ran. This can be checked by other extensions + // e.g. AbuseFilter. + $simpleCaptcha->setEditFilterMergedContentHandlerInvoked(); + return $simpleCaptcha->confirmEditMerged( $context, $content, $status, $summary, $user, $minorEdit ); } diff --git a/tests/phpunit/SimpleCaptcha/CaptchaTest.php b/tests/phpunit/SimpleCaptcha/CaptchaTest.php index 91cd9802e..06783abfd 100644 --- a/tests/phpunit/SimpleCaptcha/CaptchaTest.php +++ b/tests/phpunit/SimpleCaptcha/CaptchaTest.php @@ -8,6 +8,7 @@ use MediaWiki\Request\WebRequest; use MediaWiki\Title\Title; use MediaWiki\User\User; use Wikimedia\ScopedCallback; +use Wikimedia\TestingAccessWrapper; /** * @covers \MediaWiki\Extension\ConfirmEdit\SimpleCaptcha\SimpleCaptcha @@ -186,4 +187,33 @@ class CaptchaTest extends MediaWikiIntegrationTestCase { ] ); } + + public function testTriggersCaptchaReturnsEarlyIfCaptchaSolved() { + $this->setMwGlobals( [ + 'wgCaptchaTriggers' => [ + 'edit' => true, + ] + ] ); + $testObject = new SimpleCaptcha(); + /** @var SimpleCaptcha|TestingAccessWrapper $wrapper */ + $wrapper = TestingAccessWrapper::newFromObject( $testObject ); + $wrapper->captchaSolved = true; + $this->assertFalse( $wrapper->triggersCaptcha( 'edit' ), 'CAPTCHA is not triggered if already solved' ); + } + + public function testForceShowCaptcha() { + $this->setMwGlobals( [ + 'wgCaptchaTriggers' => [ + 'edit' => false, + ] + ] ); + $testObject = new SimpleCaptcha(); + /** @var SimpleCaptcha|TestingAccessWrapper $wrapper */ + $wrapper = TestingAccessWrapper::newFromObject( $testObject ); + $this->assertFalse( + $wrapper->triggersCaptcha( 'edit' ), 'CAPTCHA is not triggered by edit action in this configuration' + ); + $wrapper->setForceShowCaptcha( true ); + $this->assertTrue( $wrapper->triggersCaptcha( 'edit' ), 'Force showing a CAPTCHA if flag is set' ); + } } diff --git a/tests/phpunit/integration/AbuseFilter/CaptchaConsequenceTest.php b/tests/phpunit/integration/AbuseFilter/CaptchaConsequenceTest.php index 45e36065e..1767fb5c6 100644 --- a/tests/phpunit/integration/AbuseFilter/CaptchaConsequenceTest.php +++ b/tests/phpunit/integration/AbuseFilter/CaptchaConsequenceTest.php @@ -2,9 +2,9 @@ namespace MediaWiki\Extension\ConfirmEdit\Test\Integration\AbuseFilter; -use MediaWiki\Context\RequestContext; use MediaWiki\Extension\AbuseFilter\Consequences\Parameters; use MediaWiki\Extension\ConfirmEdit\AbuseFilter\CaptchaConsequence; +use MediaWiki\Extension\ConfirmEdit\Hooks; use MediaWikiIntegrationTestCase; /** @@ -16,16 +16,10 @@ class CaptchaConsequenceTest extends MediaWikiIntegrationTestCase { $parameters = $this->createMock( Parameters::class ); $parameters->method( 'getAction' )->willReturn( 'edit' ); $captchaConsequence = new CaptchaConsequence( $parameters ); - $request = RequestContext::getMain(); - $this->assertNull( $request->getRequest()->getVal( - CaptchaConsequence::FLAG - ) ); + $simpleCaptcha = Hooks::getInstance(); + $this->assertFalse( $simpleCaptcha->shouldForceShowCaptcha() ); $captchaConsequence->execute(); - $this->assertTrue( - $request->getRequest()->getBool( - CaptchaConsequence::FLAG - ) - ); + $this->assertTrue( $simpleCaptcha->shouldForceShowCaptcha() ); } }