ConfirmEditHandler: Use SimpleCaptcha API to invoke CAPTCHA display

Why:

- The previous attempt to integrate AbuseFilter with ConfirmEdit set
  a flag on the request object
  (I110a5f5321649dcf85993a0c209ab70b9886057c) didn't work in WMF
  production because in WMF, we load ConfirmEdit first, followed by
  AbuseFilter. Therefore any flag set in an AbuseFilter hook is ignored
  by ConfirmEdit

What:

- Remove implementation of ConfirmEditTriggersCaptchaHook, as this does
  not work when AbuseFilter is loaded after ConfirmEdit.
- Repurpose onConfirmEditTriggersCaptcha to handle non-edit actions only
- Implement the EditFilterMergedContent hook and call SimpleCaptcha's
  public confirmEditMerged method if CaptchaConsequence has specified
  that a CAPTCHA should be displayed, and if the CAPTCHA has not already
  been solved

Soft-Depends-On: Idc47bdae8007da938f31e1c0f33e9be4813f41d7
Bug: T20110
Change-Id: I7dd3a7c41606dcf5123518c2d3d0f4355f5edfd3

extension.json

@@ -400,7 +400,7 @@
 		}
 	},
 	"Hooks": {
-		"EditFilterMergedContent": "FilteredActions",
+		"EditFilterMergedContent": [ "FilteredActions", "ConfirmEdit" ],
 		"GetAutoPromoteGroups": "AutoPromoteGroups",
 		"TitleMove": "FilteredActions",
 		"ArticleDelete": "FilteredActions",
@@ -418,7 +418,6 @@
 		"CheckUserInsertChangesRow": "CheckUser",
 		"CheckUserInsertPrivateEventRow": "CheckUser",
 		"CheckUserInsertLogEventRow": "CheckUser",
-		"ConfirmEditTriggersCaptcha": "ConfirmEdit",
 		"UserMergeAccountFields": "UserMerge",
 		"BeforeCreateEchoEvent": "Echo",
 		"ParserOutputStashForEdit": "FilteredActions",

includes/Hooks/Handlers/ConfirmEditHandler.php

@@ -2,24 +2,64 @@
 
 namespace MediaWiki\Extension\AbuseFilter\Hooks\Handlers;
 
-use MediaWiki\Context\RequestContext;
-use MediaWiki\Extension\ConfirmEdit\AbuseFilter\CaptchaConsequence;
-use MediaWiki\Extension\ConfirmEdit\Hooks\ConfirmEditTriggersCaptchaHook;
-use MediaWiki\Page\PageIdentity;
+use Content;
+use ExtensionRegistry;
+use MediaWiki\Context\IContextSource;
+use MediaWiki\Extension\ConfirmEdit\Hooks;
+use MediaWiki\Hook\EditFilterMergedContentHook;
+use MediaWiki\Status\Status;
+use MediaWiki\User\User;
 
 /**
  * Integration with Extension:ConfirmEdit, if loaded.
  */
-class ConfirmEditHandler implements ConfirmEditTriggersCaptchaHook {
+class ConfirmEditHandler implements EditFilterMergedContentHook {
 
 	/** @inheritDoc */
-	public function onConfirmEditTriggersCaptcha( string $action, ?PageIdentity $page, bool &$result ) {
-		// Check the request to see if ConfirmEdit's CaptchaConsequence was triggered, as
-		// that sets a property on the request. If so, we know that we want to trigger the CAPTCHA.
-		// Note that users with 'skipcaptcha' right will not be shown the CAPTCHA even if this
-		// return true.
-		if ( RequestContext::getMain()->getRequest()->getBool( CaptchaConsequence::FLAG ) ) {
-			$result = true;
+	public function onEditFilterMergedContent(
+		IContextSource $context, Content $content, Status $status, $summary, User $user, $minoredit
+	) {
+		if ( !ExtensionRegistry::getInstance()->isLoaded( 'ConfirmEdit' ) ) {
+			return true;
 		}
+		$simpleCaptcha = Hooks::getInstance();
+		// FIXME: Remove method_exists checks after I3484d66298bc9f49dfbe003a0605e2ac1a092e10 is merged
+		if ( !method_exists( $simpleCaptcha, 'shouldForceShowCaptcha' ) ||
+			!method_exists( $simpleCaptcha, 'isCaptchaSolved' ) ||
+			!method_exists( $simpleCaptcha, 'editFilterMergedContentHandlerAlreadyInvoked' ) ) {
+			return true;
+		}
+		// In WMF production, AbuseFilter is loaded after ConfirmEdit. That means,
+		// Extension:ConfirmEdit's EditFilterMergedContent hook has already run, and that hook
+		// is responsible for deciding whether to show a CAPTCHA via the SimpleCaptcha::confirmEditMerged
+		// method.
+		// Here, we look to see if:
+		// 1. CaptchaConsequence in AbuseFilter modified the global SimpleCaptcha instance to say that
+		//    we should force showing a Captcha
+		// 2. that the Captcha hasn't yet been solved
+		// 3. ConfirmEdit's EditFilterMergedContent handler has already run (ConfirmEdit was loaded
+		//    ahead of AbuseFilter via wfLoadExtension())
+		// If all conditions are true, we invoke SimpleCaptcha's ConfirmEditMerged method, which
+		// will run in a narrower scope (not invoking ConfirmEdit's onConfirmEditTriggersCaptcha hook,
+		// for example), and will just make sure that the status is modified to present a CAPTCHA to
+		// the user.
+		// FIXME: Remove phan suppression after I3484d66298bc9f49dfbe003a0605e2ac1a092e10 is merged
+		// @phan-suppress-next-line PhanUndeclaredMethod
+		if ( $simpleCaptcha->shouldForceShowCaptcha() &&
+			// @phan-suppress-next-line PhanUndeclaredMethod
+			!$simpleCaptcha->isCaptchaSolved() &&
+			// @phan-suppress-next-line PhanUndeclaredMethod
+			$simpleCaptcha->editFilterMergedContentHandlerAlreadyInvoked() ) {
+			return $simpleCaptcha->confirmEditMerged(
+				$context,
+				$content,
+				$status,
+				$summary,
+				$user,
+				$minoredit
+			);
+		}
+		return true;
 	}
+
 }

tests/phpunit/integration/Hooks/ConfirmEditHandlerTest.php

@@ -2,35 +2,63 @@
 
 namespace MediaWiki\Extension\AbuseFilter\Tests\Integration\Hooks;
 
+use MediaWiki\Context\RequestContext;
 use MediaWiki\Extension\AbuseFilter\Consequences\Parameters;
 use MediaWiki\Extension\AbuseFilter\Hooks\Handlers\ConfirmEditHandler;
 use MediaWiki\Extension\ConfirmEdit\AbuseFilter\CaptchaConsequence;
-use MediaWiki\Page\PageIdentity;
+use MediaWiki\Extension\ConfirmEdit\Hooks;
+use MediaWiki\Status\Status;
+use MediaWiki\Title\Title;
+use MediaWiki\User\User;
 use MediaWikiIntegrationTestCase;
 
 /**
  * @covers \MediaWiki\Extension\AbuseFilter\Hooks\Handlers\ConfirmEditHandler
+ * @group Database
  */
 class ConfirmEditHandlerTest extends MediaWikiIntegrationTestCase {
 
-	public function testConfirmEditTriggersCaptcha() {
+	public function testOnEditFilterMergedContent() {
 		$this->markTestSkippedIfExtensionNotLoaded( 'ConfirmEdit' );
 		$confirmEditHandler = new ConfirmEditHandler();
-		$result = false;
-		$confirmEditHandler->onConfirmEditTriggersCaptcha(
-			'edit',
-			$this->createMock( PageIdentity::class ),
-			$result
+		$status = Status::newGood();
+		$title = $this->createMock( Title::class );
+		$title->method( 'canExist' )->willReturn( true );
+		$context = RequestContext::getMain();
+		$context->setTitle( $title );
+		$confirmEditHandler->onEditFilterMergedContent(
+			$context,
+			$this->createMock( \Content::class ),
+			$status,
+			'',
+			$this->createMock( User::class ),
+			false
 		);
-		$this->assertFalse( $result, 'The default is to not show a CAPTCHA' );
+		$this->assertStatusGood( $status, 'The default is to not show a CAPTCHA' );
 
+		$simpleCaptcha = Hooks::getInstance();
+		// FIXME: Remove this method_exists check after Idc47bdae8007da938f31e1c0f33e9be4813f41d7 is merged
+		if ( method_exists( $simpleCaptcha, 'setEditFilterMergedContentHandlerInvoked' ) ) {
+			$simpleCaptcha->setEditFilterMergedContentHandlerInvoked();
+		}
+		$simpleCaptcha->setAction( 'edit' );
 		$captchaConsequence = new CaptchaConsequence( $this->createMock( Parameters::class ) );
 		$captchaConsequence->execute();
-		$confirmEditHandler->onConfirmEditTriggersCaptcha(
-			'edit',
-			$this->createMock( PageIdentity::class ),
-			$result
+		$confirmEditHandler->onEditFilterMergedContent(
+			$context,
+			$this->createMock( \Content::class ),
+			$status,
+			'',
+			$this->createMock( User::class ),
+			false
 		);
-		$this->assertTrue( $result, 'CaptchaConsequence specifies that a CAPTCHA should be shown' );
+		// FIXME: Remove this method_exists check after Idc47bdae8007da938f31e1c0f33e9be4813f41d7 is merged
+		if ( method_exists( $simpleCaptcha, 'shouldForceShowCaptcha' ) ) {
+			$this->assertStatusError(
+				'captcha-edit-fail',
+				$status,
+				'CaptchaConsequence specifies that a CAPTCHA should be shown'
+			);
+		}
 	}
 }