From 50bc57a20007fbce9b598bcde68e8c70bac2f3ec Mon Sep 17 00:00:00 2001 From: Florian Schmidt Date: Mon, 14 Aug 2017 17:39:20 +0200 Subject: [PATCH] Allow other extensions to setup triggers using attributes Instead of misusing the config section of extension.json to declare captcha triggers in the ConfirmEdits CaptchaTriggers config variable, other extensions can now use the CaptchaTriggers attribute for the exact same thing. E.g., to declare a new trigger, the following addition to the own extension.json will register the trigger in ConfirmEdit: "CaptchaTriggers": { "wikiforum": true } This also removes the CaptchaClass config from the main extension.json config section, and automatically sets the SimpleCaptcha module in the getInstance() method of ConfirmEditHooks, which is a pre-requirement for the mediawiki/core change Ieeb26011e42c741041d2c3252238ca0823b99eb4. Bug: T152929 Change-Id: I4c5eaf87657f5dc07787480a2f1a56a1db8c714f --- SimpleCaptcha/Captcha.php | 79 +++++++++----- extension.json | 4 +- includes/CaptchaTriggers.php | 17 +++ includes/ConfirmEditHooks.php | 9 +- tests/phpunit/SimpleCaptcha/CaptchaTest.php | 113 ++++++++++++++++++++ 5 files changed, 191 insertions(+), 31 deletions(-) create mode 100644 includes/CaptchaTriggers.php create mode 100644 tests/phpunit/SimpleCaptcha/CaptchaTest.php diff --git a/SimpleCaptcha/Captcha.php b/SimpleCaptcha/Captcha.php index 661ff7b2a..881ad3b7c 100644 --- a/SimpleCaptcha/Captcha.php +++ b/SimpleCaptcha/Captcha.php @@ -244,10 +244,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" ); @@ -273,11 +272,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 ) { @@ -287,7 +286,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 ) { @@ -303,9 +302,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 ) ); } @@ -318,9 +315,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; } @@ -331,13 +329,14 @@ class SimpleCaptcha { * @return boolean|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; } @@ -466,15 +465,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 ) { + public function captchaTriggers( $title, $action ) { + return $this->triggersCaptcha( $action, $title ); + } + + /** + * 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; - // Special config for this NS? - if ( isset( $wgCaptchaTriggersOnNamespace[$title->getNamespace()][$action] ) ) { - return $wgCaptchaTriggersOnNamespace[$title->getNamespace()][$action]; + + $result = false; + $triggers = $wgCaptchaTriggers; + $attributeCaptchaTriggers = ExtensionRegistry::getInstance() + ->getAttribute( CaptchaTriggers::EXT_REG_ATTRIBUTE_NAME ); + if ( is_array( $attributeCaptchaTriggers ) ) { + $triggers += $attributeCaptchaTriggers; } - return ( !empty( $wgCaptchaTriggers[$action] ) ); // Default + if ( isset( $triggers[$action] ) ) { + $result = $triggers[$action]; + } + + if ( + $title !== null && + isset( $wgCaptchaTriggersOnNamespace[$title->getNamespace()][$action] ) + ) { + $result = $wgCaptchaTriggersOnNamespace[$title->getNamespace()][$action]; + } + + return $result; } /** @@ -523,7 +552,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(), @@ -533,7 +562,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(), @@ -552,7 +581,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 @@ -834,10 +863,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; @@ -860,9 +889,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; diff --git a/extension.json b/extension.json index 61ec7af36..ce6ef61a5 100644 --- a/extension.json +++ b/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, diff --git a/includes/CaptchaTriggers.php b/includes/CaptchaTriggers.php new file mode 100644 index 000000000..d88ca223b --- /dev/null +++ b/includes/CaptchaTriggers.php @@ -0,0 +1,17 @@ +onAuthChangeFormFields( $requests, $fieldInfo, $formDescriptor, $action ); } - /** - * Set up $wgWhitelistRead - */ public static function confirmEditSetup() { // @codingStandardsIgnoreStart MediaWiki.NamingConventions.ValidGlobalName.wgPrefix global $wgCaptchaTriggers, $wgAllowConfirmedEmail, diff --git a/tests/phpunit/SimpleCaptcha/CaptchaTest.php b/tests/phpunit/SimpleCaptcha/CaptchaTest.php new file mode 100644 index 000000000..261a47e67 --- /dev/null +++ b/tests/phpunit/SimpleCaptcha/CaptchaTest.php @@ -0,0 +1,113 @@ +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 ], + ]; + } +}