Update code for voice and tone reasons

* Reduces use of whitelist
* Deprecates and provides new $wg to replace $wgCaptchaWhitelistIP and $wgCaptchaWhitelist

Bug: T277936
Change-Id: I9c4a572321bb06e5a1d4208a295e21b73e75b787

README.md

@@ -45,9 +45,9 @@ Additional maintenance work was done by Yaron Koren.
  *
  * Specific IP addresses or CIDR-style ranges may be used,
  * for instance:
- * $wgCaptchaWhitelistIP = array('192.168.1.0/24', '10.1.0.0/16');
+ * $wgCaptchaBypassIPs = [ '192.168.1.0/24', '10.1.0.0/16' ];
  */
-$wgCaptchaWhitelistIP = false;
+$wgCaptchaBypassIPs = false;
 
 /**
  * Actions which can trigger a captcha
@@ -132,12 +132,12 @@ $wgAllowConfirmedEmail = false;
 $wgCaptchaBadLoginAttempts = 3;
 
 /**
- * Regex to whitelist URLs to known-good sites...
+ * Regex to ignore URLs to known-good sites...
  * For instance:
- * $wgCaptchaWhitelist = '#^https?://([a-z0-9-]+\\.)?(wikimedia|wikipedia)\.org/#i';
- * Local admins can define a whitelist under [[MediaWiki:captcha-addurl-whitelist]]
+ * $wgCaptchaIgnoredUrls = '#^https?://([a-z0-9-]+\\.)?(wikimedia|wikipedia)\.org/#i';
+ * Local admins can define a local allow list under [[MediaWiki:captcha-addurl-whitelist]]
  */
-$wgCaptchaWhitelist = false;
+$wgCaptchaIgnoredUrls = false;
 
 /**
  * Additional regexes to check for. Use full regexes; can match things

extension.json

@@ -119,6 +119,11 @@
 	},
 	"config": {
 		"CaptchaWhitelistIP": {
+			"description": "DEPRECATED! Use CaptchaBypassIPs",
+			"value": false
+		},
+		"CaptchaBypassIPs": {
+			"description": "A list of IP addresses that can skip the captcha",
 			"value": false
 		},
 		"Captcha": {
@@ -162,6 +167,11 @@
 			"value": 20
 		},
 		"CaptchaWhitelist": {
+			"description": "DEPRECATED: Use CaptchaIgnoredUrls",
+			"value": false
+		},
+		"CaptchaIgnoredUrls": {
+			"description": "Urls that won't trigger a captcha",
 			"value": false
 		},
 		"CaptchaRegexes": {

includes/Hooks.php

@@ -94,7 +94,7 @@ class Hooks implements
 	) {
 		$title = $wikiPage->getTitle();
 		if ( $title->getText() === 'Captcha-ip-whitelist' && $title->getNamespace() === NS_MEDIAWIKI ) {
-			$this->cache->delete( $this->cache->makeKey( 'confirmedit', 'ipwhitelist' ) );
+			$this->cache->delete( $this->cache->makeKey( 'confirmedit', 'ipbypasslist' ) );
 		}
 
 		return true;

includes/SimpleCaptcha/SimpleCaptcha.php

@@ -357,27 +357,30 @@ class SimpleCaptcha {
 	}
 
 	/**
-	 * Check if the current IP is allowed to skip captchas. This checks
-	 * the whitelist from two sources.
-	 *  1) From the server-side config array $wgCaptchaWhitelistIP
+	 * Check if the current IP is allowed to skip solving a captcha.
+	 * This checks the bypass list from two sources.
+	 *  1) From the server-side config array $wgCaptchaWhitelistIP (deprecated) or $wgCaptchaBypassIPs
 	 *  2) From the local [[MediaWiki:Captcha-ip-whitelist]] message
 	 *
-	 * @return bool true if whitelisted, false if not
+	 * @return bool true if the IP can bypass a captcha, false if not
 	 */
-	private function isIPWhitelisted() {
-		global $wgCaptchaWhitelistIP, $wgRequest;
+	private function canIPBypassCaptcha() {
+		global $wgCaptchaWhitelistIP, $wgCaptchaBypassIPs, $wgRequest;
 		$ip = $wgRequest->getIP();
 
-		if ( $wgCaptchaWhitelistIP ) {
-			if ( IPUtils::isInRanges( $ip, $wgCaptchaWhitelistIP ) ) {
-				return true;
-			}
+		// Deprecated; to be removed later
+		if ( $wgCaptchaWhitelistIP && IPUtils::isInRanges( $ip, $wgCaptchaWhitelistIP ) ) {
+			return true;
 		}
 
-		$whitelistMsg = wfMessage( 'captcha-ip-whitelist' )->inContentLanguage();
-		if ( !$whitelistMsg->isDisabled() ) {
-			$whitelistedIPs = $this->getWikiIPWhitelist( $whitelistMsg );
-			if ( IPUtils::isInRanges( $ip, $whitelistedIPs ) ) {
+		if ( $wgCaptchaBypassIPs && IPUtils::isInRanges( $ip, $wgCaptchaBypassIPs ) ) {
+			return true;
+		}
+
+		$msg = wfMessage( 'captcha-ip-whitelist' )->inContentLanguage();
+		if ( !$msg->isDisabled() ) {
+			$allowedIPs = $this->getWikiIPBypassList( $msg );
+			if ( IPUtils::isInRanges( $ip, $allowedIPs ) ) {
 				return true;
 			}
 		}
@@ -386,40 +389,38 @@ class SimpleCaptcha {
 	}
 
 	/**
-	 * Get the on-wiki IP whitelist stored in [[MediaWiki:Captcha-ip-whitelist]]
-	 * page from cache if possible.
+	 * Get the on-wiki IP bypass list stored on a MediaWiki page from cache if possible.
 	 *
-	 * @param Message $msg whitelist Message on wiki
-	 * @return array whitelisted IP addresses or IP ranges, empty array if no whitelist
+	 * @param Message $msg Message on wiki with IP lists
+	 * @return array Allowed IP addresses or IP ranges, empty array if none
 	 */
-	private function getWikiIPWhitelist( Message $msg ) {
+	private function getWikiIPBypassList( Message $msg ) {
 		$cache = MediaWikiServices::getInstance()->getMainWANObjectCache();
-		$cacheKey = $cache->makeKey( 'confirmedit', 'ipwhitelist' );
+		$cacheKey = $cache->makeKey( 'confirmedit', 'ipbypasslist' );
 
-		$cachedWhitelist = $cache->get( $cacheKey );
-		if ( $cachedWhitelist === false ) {
-			// Could not retrieve from cache so build the whitelist directly
-			// from the wikipage
-			$whitelist = $this->buildValidIPs(
-				explode( "\n", $msg->plain() )
-			);
-			// And then store it in cache for one day. This cache is cleared on
-			// modifications to the whitelist page.
-			// @see MediaWiki\Extension\ConfirmEdit\Hooks::onPageSaveComplete()
-			$cache->set( $cacheKey, $whitelist, 86400 );
-		} else {
-			// Whitelist from the cache
-			$whitelist = $cachedWhitelist;
+		$cached = $cache->get( $cacheKey );
+		if ( $cached !== false ) {
+			return $cached;
 		}
 
-		return $whitelist;
+		// Could not retrieve from cache, so build the list directly
+		// from the MediaWiki page
+		$list = $this->buildValidIPs(
+			explode( "\n", $msg->plain() )
+		);
+		// And then store it in cache for one day.
+		// This cache is cleared on modifications to the wiki page.
+		// @see MediaWiki\Extension\ConfirmEdit\Hooks::onPageSaveComplete()
+		$cache->set( $cacheKey, $list, 86400 );
+
+		return $list;
 	}
 
 	/**
 	 * From a list of unvalidated input, get all the valid
 	 * IP addresses and IP ranges from it.
 	 *
-	 * Note that only lines with just the IP address or IP range is considered
+	 * Note that only lines with just the IP address or the IP range is considered
 	 * as valid. Whitespace is allowed, but if there is any other character on
 	 * the line, it's not considered as a valid entry.
 	 *
@@ -706,12 +707,12 @@ class SimpleCaptcha {
 	}
 
 	/**
-	 * Filter callback function for URL whitelisting
+	 * Filter callback function for URL allow-listing
 	 * @param string $url string to check
-	 * @return bool true if unknown, false if whitelisted
+	 * @return bool true if unknown, false if allowed
 	 */
 	private function filterLink( $url ) {
-		global $wgCaptchaWhitelist;
+		global $wgCaptchaWhitelist, $wgCaptchaIgnoredUrls;
 		static $regexes = null;
 
 		if ( $regexes === null ) {
@@ -721,9 +722,13 @@ class SimpleCaptcha {
 				? []
 				: $this->buildRegexes( explode( "\n", $source->plain() ) );
 
+			// DEPRECATED
 			if ( $wgCaptchaWhitelist !== false ) {
 				array_unshift( $regexes, $wgCaptchaWhitelist );
 			}
+			if ( $wgCaptchaIgnoredUrls !== false ) {
+				array_unshift( $regexes, $wgCaptchaIgnoredUrls );
+			}
 		}
 
 		foreach ( $regexes as $regex ) {
@@ -736,8 +741,8 @@ class SimpleCaptcha {
 	}
 
 	/**
-	 * Build regex from whitelist
-	 * @param string[] $lines string from [[MediaWiki:Captcha-addurl-whitelist]]
+	 * Build regex from list of URLs
+	 * @param string[] $lines string from MediaWiki page
 	 * @return string[] Regexes
 	 * @private
 	 */
@@ -749,12 +754,12 @@ class SimpleCaptcha {
 		$lines = array_filter( array_map( 'trim', preg_replace( '/#.*$/', '', $lines ) ) );
 
 		# No lines, don't make a regex which will match everything
-		if ( count( $lines ) == 0 ) {
+		if ( count( $lines ) === 0 ) {
 			wfDebug( "No lines\n" );
 			return [];
 		}
 
-	# Make regex
+		# Make regex
 		# It's faster using the S modifier even though it will usually only be run once
 		// $regex = 'http://+[a-z0-9_\-.]*(' . implode( '|', $lines ) . ')';
 		// return '/' . str_replace( '/', '\/', preg_replace('|\\\*/|', '/', $regex) ) . '/Si';
@@ -1241,8 +1246,8 @@ class SimpleCaptcha {
 			return true;
 		}
 
-		if ( $this->isIPWhitelisted() ) {
-			wfDebug( "ConfirmEdit: user IP is whitelisted" );
+		if ( $this->canIPBypassCaptcha() ) {
+			wfDebug( "ConfirmEdit: user IP can bypass captcha" );
 			return true;
 		}
 

tests/phpunit/SimpleCaptcha/CaptchaTest.php

@@ -140,9 +140,9 @@ class CaptchaTest extends MediaWikiIntegrationTestCase {
 	}
 
 	/**
-	 * @dataProvider provideCanSkipCaptchaIPWhitelisted
+	 * @dataProvider provideCanSkipCaptchaBypassIPList
 	 */
-	public function testCanSkipCaptchaIPWhitelisted( $requestIP, $IPWhitelist, $expected ) {
+	public function testCanSkipCaptchaBypassIP( $requestIP, $list, $expected ) {
 		$testObject = new SimpleCaptcha();
 		$config = new HashConfig( [ 'AllowConfirmedEmail' => false ] );
 		$request = $this->createMock( WebRequest::class );
@@ -151,14 +151,14 @@ class CaptchaTest extends MediaWikiIntegrationTestCase {
 		$this->setMwGlobals( [
 			'wgRequest' => $request,
 		] );
-		$this->overrideConfigValue( 'CaptchaWhitelistIP', $IPWhitelist );
+		$this->overrideConfigValue( 'CaptchaBypassIPs', $list );
 
 		$actual = $testObject->canSkipCaptcha( RequestContext::getMain()->getUser(), $config );
 
 		$this->assertEquals( $expected, $actual );
 	}
 
-	public static function provideCanSkipCaptchaIPWhitelisted() {
+	public static function provideCanSkipCaptchaBypassIPList() {
 		return ( [
 			[ '127.0.0.1', [ '127.0.0.1', '127.0.0.2' ], true ],
 			[ '127.0.0.1', [], false ]