Merge "Allow other extensions to setup triggers using attributes"

SimpleCaptcha/Captcha.php

@@ -243,10 +243,9 @@ class SimpleCaptcha {
 	 * @return bool true to keep running callbacks
 	 */
 	function injectEmailUser( &$form ) {
-		global $wgCaptchaTriggers;
 		$out = $form->getOutput();
 		$user = $form->getUser();
-		if ( $wgCaptchaTriggers['sendemail'] ) {
+		if ( $this->triggersCaptcha( CaptchaTriggers::SENDEMAIL ) ) {
 			$this->action = 'sendemail';
 			if ( $user->isAllowed( 'skipcaptcha' ) ) {
 				wfDebug( "ConfirmEdit: user group allows skipping captcha on email sending\n" );
@@ -272,11 +271,11 @@ class SimpleCaptcha {
 	 * TODO use Throttler
 	 */
 	public function increaseBadLoginCounter( $username ) {
-		global $wgCaptchaTriggers, $wgCaptchaBadLoginExpiration,
-			$wgCaptchaBadLoginPerUserExpiration;
+		global $wgCaptchaBadLoginExpiration, $wgCaptchaBadLoginPerUserExpiration;
+
 		$cache = ObjectCache::getLocalClusterInstance();
 
-		if ( $wgCaptchaTriggers['badlogin'] ) {
+		if ( $this->triggersCaptcha( CaptchaTriggers::BAD_LOGIN ) ) {
 			$key = $this->badLoginKey();
 			$count = ObjectCache::getLocalClusterInstance()->get( $key );
 			if ( !$count ) {
@@ -286,7 +285,7 @@ class SimpleCaptcha {
 			$cache->incr( $key );
 		}
 
-		if ( $wgCaptchaTriggers['badloginperuser'] && $username ) {
+		if ( $this->triggersCaptcha( CaptchaTriggers::BAD_LOGIN_PER_USER ) && $username ) {
 			$key = $this->badLoginPerUserKey( $username );
 			$count = $cache->get( $key );
 			if ( !$count ) {
@@ -302,9 +301,7 @@ class SimpleCaptcha {
 	 * @param string $username
 	 */
 	public function resetBadLoginCounter( $username ) {
-		global $wgCaptchaTriggers;
-
-		if ( $wgCaptchaTriggers['badloginperuser'] && $username ) {
+		if ( $this->triggersCaptcha( CaptchaTriggers::BAD_LOGIN_PER_USER ) && $username ) {
 			$cache = ObjectCache::getLocalClusterInstance();
 			$cache->delete( $this->badLoginPerUserKey( $username ) );
 		}
@@ -317,9 +314,10 @@ class SimpleCaptcha {
 	 * @access private
 	 */
 	public function isBadLoginTriggered() {
-		global $wgCaptchaTriggers, $wgCaptchaBadLoginAttempts;
+		global $wgCaptchaBadLoginAttempts;
+
 		$cache = ObjectCache::getLocalClusterInstance();
-		return $wgCaptchaTriggers['badlogin']
+		return $this->triggersCaptcha( CaptchaTriggers::BAD_LOGIN )
 			&& (int)$cache->get( $this->badLoginKey() ) >= $wgCaptchaBadLoginAttempts;
 	}
 
@@ -330,13 +328,14 @@ class SimpleCaptcha {
 	 * @return bool|null False: no, null: no, but it will be triggered next time
 	 */
 	public function isBadLoginPerUserTriggered( $u ) {
-		global $wgCaptchaTriggers, $wgCaptchaBadLoginPerUserAttempts;
+		global $wgCaptchaBadLoginPerUserAttempts;
+
 		$cache = ObjectCache::getLocalClusterInstance();
 
 		if ( is_object( $u ) ) {
 			$u = $u->getName();
 		}
-		return $wgCaptchaTriggers['badloginperuser']
+		return $this->triggersCaptcha( CaptchaTriggers::BAD_LOGIN_PER_USER )
 			&& (int)$cache->get( $this->badLoginPerUserKey( $u ) ) >= $wgCaptchaBadLoginPerUserAttempts;
 	}
 
@@ -465,15 +464,45 @@ class SimpleCaptcha {
 	 * @param Title $title
 	 * @param string $action (edit/create/addurl...)
 	 * @return bool true if action triggers captcha on $title's namespace
+	 * @deprecated since 1.5.1 Use triggersCaptcha instead
 	 */
-	function captchaTriggers( $title, $action ) {
-		global $wgCaptchaTriggers, $wgCaptchaTriggersOnNamespace;
-		// Special config for this NS?
-		if ( isset( $wgCaptchaTriggersOnNamespace[$title->getNamespace()][$action] ) ) {
-			return $wgCaptchaTriggersOnNamespace[$title->getNamespace()][$action];
+	public function captchaTriggers( $title, $action ) {
+		return $this->triggersCaptcha( $action, $title );
 	}
 
-		return ( !empty( $wgCaptchaTriggers[$action] ) ); // Default
+	/**
+	 * Checks, whether the passed action should trigger a CAPTCHA. The optional $title parameter
+	 * will be used to check namespace specific CAPTCHA triggers.
+	 *
+	 * @param string $action The CAPTCHA trigger to check (see CaptchaTriggers for ConfirmEdit
+	 * built-in triggers)
+	 * @param Title|null $title An optional Title object, if the namespace specific triggers
+	 * should be checked, too.
+	 * @return bool True, if the action should trigger a CAPTCHA, false otherwise
+	 */
+	public function triggersCaptcha( $action, $title = null ) {
+		global $wgCaptchaTriggers, $wgCaptchaTriggersOnNamespace;
+
+		$result = false;
+		$triggers = $wgCaptchaTriggers;
+		$attributeCaptchaTriggers = ExtensionRegistry::getInstance()
+			->getAttribute( CaptchaTriggers::EXT_REG_ATTRIBUTE_NAME );
+		if ( is_array( $attributeCaptchaTriggers ) ) {
+			$triggers += $attributeCaptchaTriggers;
+		}
+
+		if ( isset( $triggers[$action] ) ) {
+			$result = $triggers[$action];
+		}
+
+		if (
+			$title !== null &&
+			isset( $wgCaptchaTriggersOnNamespace[$title->getNamespace()][$action] )
+		) {
+			$result = $wgCaptchaTriggersOnNamespace[$title->getNamespace()][$action];
+		}
+
+		return $result;
 	}
 
 	/**
@@ -522,7 +551,7 @@ class SimpleCaptcha {
 			$isEmpty = $content === '';
 		}
 
-		if ( $this->captchaTriggers( $title, 'edit' ) ) {
+		if ( $this->triggersCaptcha( 'edit', $title ) ) {
 			// Check on all edits
 			$this->trigger = sprintf( "edit trigger by '%s' at [[%s]]",
 				$user->getName(),
@@ -532,7 +561,7 @@ class SimpleCaptcha {
 			return true;
 		}
 
-		if ( $this->captchaTriggers( $title, 'create' ) && !$title->exists() ) {
+		if ( $this->triggersCaptcha( 'create', $title ) && !$title->exists() ) {
 			// Check if creating a page
 			$this->trigger = sprintf( "Create trigger by '%s' at [[%s]]",
 				$user->getName(),
@@ -551,7 +580,7 @@ class SimpleCaptcha {
 			return false;
 		}
 
-		if ( !$isEmpty && $this->captchaTriggers( $title, 'addurl' ) ) {
+		if ( !$isEmpty && $this->triggersCaptcha( 'addurl', $title ) ) {
 			// Only check edits that add URLs
 			if ( $content instanceof Content ) {
 				// Get links from the database
@@ -833,10 +862,10 @@ class SimpleCaptcha {
 	 * @return bool true to show captcha, false to skip captcha
 	 */
 	public function needCreateAccountCaptcha( User $creatingUser = null ) {
-		global $wgCaptchaTriggers, $wgUser;
+		global $wgUser;
 		$creatingUser = $creatingUser ?: $wgUser;
 
-		if ( $wgCaptchaTriggers['createaccount'] ) {
+		if ( $this->triggersCaptcha( CaptchaTriggers::CREATE_ACCOUNT ) ) {
 			if ( $creatingUser->isAllowed( 'skipcaptcha' ) ) {
 				wfDebug( "ConfirmEdit: user group allows skipping captcha on account creation\n" );
 				return false;
@@ -859,9 +888,9 @@ class SimpleCaptcha {
 	 * @return bool true to continue saving, false to abort and show a captcha form
 	 */
 	function confirmEmailUser( $from, $to, $subject, $text, &$error ) {
-		global $wgCaptchaTriggers, $wgUser, $wgRequest;
+		global $wgUser, $wgRequest;
 
-		if ( $wgCaptchaTriggers['sendemail'] ) {
+		if ( $this->triggersCaptcha( CaptchaTriggers::SENDEMAIL ) ) {
 			if ( $wgUser->isAllowed( 'skipcaptcha' ) ) {
 				wfDebug( "ConfirmEdit: user group allows skipping captcha on email sending\n" );
 				return true;

extension.json

@@ -1,7 +1,7 @@
 {
 	"@doc": "Please read README.md",
 	"name": "ConfirmEdit",
-	"version": "1.5.0",
+	"version": "1.5.1",
 	"author": [
 		"Brion Vibber",
 		"Florian Schmidt",
@@ -56,6 +56,7 @@
 		"CaptchaSessionStore": "includes/CaptchaStore.php",
 		"CaptchaCacheStore": "includes/CaptchaStore.php",
 		"CaptchaHashStore": "includes/CaptchaStore.php",
+		"CaptchaTriggers": "includes/CaptchaTriggers.php",
 		"CaptchaSpecialPage": "includes/specials/SpecialCaptcha.php",
 		"CaptchaPreAuthenticationProvider": "includes/auth/CaptchaPreAuthenticationProvider.php",
 		"CaptchaAuthenticationRequest": "includes/auth/CaptchaAuthenticationRequest.php"
@@ -93,7 +94,6 @@
 	"config": {
 		"CaptchaWhitelistIP": false,
 		"Captcha": null,
-		"CaptchaClass": "SimpleCaptcha",
 		"CaptchaTriggers": {
 			"edit": false,
 			"create": false,

includes/CaptchaTriggers.php

@@ -0,0 +1,17 @@
+<?php
+
+/**
+ * A class with constants of the CAPTCHA triggers built-in in ConfirmEdit. Other extensions may
+ * add more possible triggers, which are not included in this class.
+ */
+abstract class CaptchaTriggers {
+	const EDIT = 'edit';
+	const CREATE = 'create';
+	const SENDEMAIL = 'sendemail';
+	const ADD_URL = 'addurl';
+	const CREATE_ACCOUNT = 'createaccount';
+	const BAD_LOGIN = 'badlogin';
+	const BAD_LOGIN_PER_USER = 'badloginperuser';
+
+	const EXT_REG_ATTRIBUTE_NAME = 'CaptchaTriggers';
+}

includes/ConfirmEditHooks.php

@@ -11,9 +11,13 @@ class ConfirmEditHooks {
 	public static function getInstance() {
 		global $wgCaptcha, $wgCaptchaClass;
 
+		$class = $wgCaptchaClass;
+		if ( $class == null ) {
+			$class = 'SimpleCaptcha';
+		}
 		if ( !static::$instanceCreated ) {
 			static::$instanceCreated = true;
-			$wgCaptcha = new $wgCaptchaClass;
+			$wgCaptcha = new $class;
 		}
 
 		return $wgCaptcha;
@@ -81,9 +85,6 @@ class ConfirmEditHooks {
 		self::getInstance()->onAuthChangeFormFields( $requests, $fieldInfo, $formDescriptor, $action );
 	}
 
-	/**
-	 * Set up $wgWhitelistRead
-	 */
 	public static function confirmEditSetup() {
 		// @codingStandardsIgnoreStart MediaWiki.NamingConventions.ValidGlobalName.wgPrefix
 		global $wgCaptchaTriggers, $wgAllowConfirmedEmail,

tests/phpunit/SimpleCaptcha/CaptchaTest.php

@@ -0,0 +1,113 @@
+<?php
+
+class CaptchaTest extends MediaWikiTestCase {
+	/**
+	 * @dataProvider provideSimpleTriggersCaptcha
+	 */
+	public function testTriggersCaptcha( $action, $expectedResult ) {
+		$captcha = new SimpleCaptcha();
+		$this->setMwGlobals( [
+			'wgCaptchaTriggers' => [
+				$action => $expectedResult,
+			]
+		] );
+		$this->assertEquals( $expectedResult, $captcha->triggersCaptcha( $action ) );
+	}
+
+	public function provideSimpleTriggersCaptcha() {
+		$data = [];
+		$captchaTriggers = new ReflectionClass( CaptchaTriggers::class );
+		$constants = $captchaTriggers->getConstants();
+		foreach ( $constants as $const ) {
+			$data[] = [ $const, true ];
+			$data[] = [ $const, false ];
+		}
+		return $data;
+	}
+
+	/**
+	 * @dataProvider provideNamespaceOverwrites
+	 */
+	public function testNamespaceTriggersOverwrite( $trigger, $expected ) {
+		$captcha = new SimpleCaptcha();
+		$this->setMwGlobals( [
+			'wgCaptchaTriggers' => [
+				$trigger => !$expected,
+			],
+			'wgCaptchaTriggersOnNamespace' => [
+				0 => [
+					$trigger => $expected,
+				],
+			],
+		] );
+		$title = Title::newFromText( 'Main' );
+		$this->assertEquals( $expected, $captcha->triggersCaptcha( $trigger, $title ) );
+	}
+
+	public function provideNamespaceOverwrites() {
+		return [
+			[ 'edit', true ],
+			[ 'edit', false ],
+		];
+	}
+
+	private function setCaptchaTriggersAttribute( $trigger, $value ) {
+		$info = [
+			'globals' => [],
+			'callbacks' => [],
+			'defines' => [],
+			'credits' => [],
+			'attributes' => [
+				'CaptchaTriggers' => [
+					$trigger => $value
+				]
+			],
+			'autoloaderPaths' => []
+		];
+		$registry = new ExtensionRegistry();
+		$class = new ReflectionClass( 'ExtensionRegistry' );
+		$instanceProperty = $class->getProperty( 'instance' );
+		$instanceProperty->setAccessible( true );
+		$instanceProperty->setValue( $registry );
+		$method = $class->getMethod( 'exportExtractedData' );
+		$method->setAccessible( true );
+		$method->invokeArgs( $registry, [ $info ] );
+	}
+
+	/**
+	 * @dataProvider provideAttributeSet
+	 */
+	public function testCaptchaTriggersAttributeSetTrue( $trigger, $value ) {
+		$this->setCaptchaTriggersAttribute( $trigger, $value );
+		$captcha = new SimpleCaptcha();
+		$this->assertEquals( $value, $captcha->triggersCaptcha( $trigger ) );
+	}
+
+	public function provideAttributeSet() {
+		return [
+			[ 'test', true ],
+			[ 'test', false ],
+		];
+	}
+
+	/**
+	 * @dataProvider provideAttributeOverwritten
+	 */
+	public function testCaptchaTriggersAttributeGetsOverwritten( $trigger, $expected ) {
+		$this->setMwGlobals( [
+			'wgCaptchaTriggers' => [
+				$trigger => $expected
+			]
+		] );
+		$this->setCaptchaTriggersAttribute( $trigger, !$expected );
+		$captcha = new SimpleCaptcha();
+		$this->assertEquals( $expected, $captcha->triggersCaptcha( $trigger ) );
+	}
+
+	public function provideAttributeOverwritten() {
+		return [
+			[ 'edit', true ],
+			[ 'edit', false ],
+		];
+	}
+}