From 48a60aa76235613c140aa66634105f8c26336ec1 Mon Sep 17 00:00:00 2001 From: Reedy Date: Fri, 25 Oct 2024 19:24:07 +0100 Subject: [PATCH] Various minor code cleanup Change-Id: I75f34c66f1c1968cfb9a3e1932068ec2420e0fa6 --- .../Auth/CaptchaAuthenticationRequest.php | 24 ++- .../Auth/CaptchaPreAuthenticationProvider.php | 31 ++-- includes/CaptchaTriggers.php | 1 - includes/CaptchaValue.php | 6 +- .../FancyCaptcha/ApiFancyCaptchaReload.php | 1 - includes/FancyCaptcha/FancyCaptcha.php | 38 +++-- .../FancyCaptcha/HTMLFancyCaptchaField.php | 18 +-- includes/Hooks.php | 112 ++------------ includes/Hooks/HookRunner.php | 7 +- includes/QuestyCaptcha/QuestyCaptcha.php | 19 +-- .../HTMLReCaptchaNoCaptchaField.php | 4 +- .../ReCaptchaNoCaptcha/ReCaptchaNoCaptcha.php | 20 +-- ...eCaptchaNoCaptchaAuthenticationRequest.php | 10 +- includes/SimpleCaptcha/SimpleCaptcha.php | 146 +++++++++--------- includes/Specials/SpecialCaptcha.php | 4 +- includes/Store/CaptchaCacheStore.php | 15 +- includes/Store/CaptchaHashStore.php | 14 +- includes/Store/CaptchaSessionStore.php | 13 +- includes/Turnstile/HTMLTurnstileField.php | 8 +- includes/Turnstile/Turnstile.php | 32 ++-- .../TurnstileAuthenticationRequest.php | 8 +- includes/hCaptcha/HCaptcha.php | 28 ++-- .../HCaptchaAuthenticationRequest.php | 8 +- includes/hCaptcha/HTMLHCaptchaField.php | 4 +- tests/phpunit/QuestyCaptchaTest.php | 1 - 25 files changed, 185 insertions(+), 387 deletions(-) diff --git a/includes/Auth/CaptchaAuthenticationRequest.php b/includes/Auth/CaptchaAuthenticationRequest.php index a81e6fe58..cf1ca1d7f 100644 --- a/includes/Auth/CaptchaAuthenticationRequest.php +++ b/includes/Auth/CaptchaAuthenticationRequest.php @@ -7,7 +7,9 @@ use MediaWiki\Auth\AuthManager; use MediaWiki\Extension\ConfirmEdit\Hooks; /** - * Generic captcha authentication request class. A captcha consist some data stored in the session + * Generic captcha authentication request class. + * + * A captcha consists of some data stored in the session * (e.g. a question and its answer), an ID that references the data, and a solution. */ class CaptchaAuthenticationRequest extends AuthenticationRequest { @@ -36,13 +38,11 @@ class CaptchaAuthenticationRequest extends AuthenticationRequest { return 'CaptchaAuthenticationRequest'; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function loadFromSubmission( array $data ) { $success = parent::loadFromSubmission( $data ); if ( $success ) { - // captchaId and captchaWord was set from the submission but captchaData was not. + // The captchaId and captchaWord was set from the submission, but captchaData was not. $captcha = Hooks::getInstance(); $this->captchaData = $captcha->retrieveCaptcha( $this->captchaId ); if ( !$this->captchaData ) { @@ -52,13 +52,11 @@ class CaptchaAuthenticationRequest extends AuthenticationRequest { return $success; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function getFieldInfo() { $captcha = Hooks::getInstance(); - // doesn't actually exist but *Captcha::getMessage will handle that + // generic action doesn't exist, but *Captcha::getMessage will handle that $action = 'generic'; switch ( $this->action ) { case AuthManager::ACTION_LOGIN: @@ -69,7 +67,7 @@ class CaptchaAuthenticationRequest extends AuthenticationRequest { break; } - $fields = [ + return [ 'captchaId' => [ 'type' => 'hidden', 'value' => $this->captchaId, @@ -88,13 +86,9 @@ class CaptchaAuthenticationRequest extends AuthenticationRequest { 'help' => wfMessage( 'captcha-help' ), ], ]; - - return $fields; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function getMetadata() { return ( Hooks::getInstance() )->describeCaptchaType(); } diff --git a/includes/Auth/CaptchaPreAuthenticationProvider.php b/includes/Auth/CaptchaPreAuthenticationProvider.php index 9f3bf1c30..2d98223fb 100644 --- a/includes/Auth/CaptchaPreAuthenticationProvider.php +++ b/includes/Auth/CaptchaPreAuthenticationProvider.php @@ -13,9 +13,7 @@ use MediaWiki\Status\Status; use MediaWiki\User\User; class CaptchaPreAuthenticationProvider extends AbstractPreAuthenticationProvider { - /** - * @inheritDoc - */ + /** @inheritDoc */ public function getAuthenticationRequests( $action, array $options ) { $captcha = Hooks::getInstance(); $user = User::newFromName( $options['username'] ); @@ -47,7 +45,7 @@ class CaptchaPreAuthenticationProvider extends AbstractPreAuthenticationProvider // failed login attempt using a username that needs a captcha, set a session flag // to display a captcha on login from that point on. This will result in confusing // error messages if the browser cannot persist the session (because we'll never show - // a required captcha field), but then login would be impossible anyway so no big deal. + // a required captcha field), but then login would be impossible anyway, so no big deal. // If the username ends to be one that does not trigger the captcha, that will // result in weird behavior (if the user leaves the captcha field empty, they get @@ -57,8 +55,8 @@ class CaptchaPreAuthenticationProvider extends AbstractPreAuthenticationProvider $userProbablyNeedsCaptcha = $session->get( 'ConfirmEdit:loginCaptchaPerUserTriggered' ); $suggestedUsername = $session->suggestLoginUsername(); if ( - $loginCounter->isBadLoginTriggered() - || $userProbablyNeedsCaptcha + $userProbablyNeedsCaptcha + || $loginCounter->isBadLoginTriggered() || ( $suggestedUsername && $loginCounter->isBadLoginPerUserTriggered( $suggestedUsername ) ) ) { $needed = true; @@ -77,21 +75,18 @@ class CaptchaPreAuthenticationProvider extends AbstractPreAuthenticationProvider if ( $needed ) { return [ $captcha->createAuthenticationRequest() ]; - } else { - return []; } + + return []; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function testForAuthentication( array $reqs ) { $captcha = Hooks::getInstance(); $username = AuthenticationRequest::getUsernameFromRequests( $reqs ); $loginCounter = $this->getLoginAttemptCounter( $captcha ); $success = true; - $isBadLoginPerUserTriggered = $username ? - $loginCounter->isBadLoginPerUserTriggered( $username ) : false; + $isBadLoginPerUserTriggered = $username && $loginCounter->isBadLoginPerUserTriggered( $username ); if ( $loginCounter->isBadLoginTriggered() || $isBadLoginPerUserTriggered ) { $captcha->setAction( 'badlogin' ); @@ -118,9 +113,7 @@ class CaptchaPreAuthenticationProvider extends AbstractPreAuthenticationProvider return $success ? Status::newGood() : $this->makeError( 'wrongpassword', $captcha ); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function testForAccountCreation( $user, $creator, array $reqs ) { $captcha = Hooks::getInstance(); @@ -146,9 +139,7 @@ class CaptchaPreAuthenticationProvider extends AbstractPreAuthenticationProvider return Status::newGood(); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function postAuthentication( $user, AuthenticationResponse $response ) { $captcha = Hooks::getInstance(); $loginCounter = $this->getLoginAttemptCounter( $captcha ); @@ -167,7 +158,7 @@ class CaptchaPreAuthenticationProvider extends AbstractPreAuthenticationProvider /** * Verify submitted captcha. - * Assumes that the user has to pass the capctha (permission checks are caller's responsibility). + * Assumes that the user has to pass the captcha (permission checks are caller's responsibility). * @param SimpleCaptcha $captcha * @param AuthenticationRequest[] $reqs * @param User $user diff --git a/includes/CaptchaTriggers.php b/includes/CaptchaTriggers.php index 30f4b18ef..6733adea3 100644 --- a/includes/CaptchaTriggers.php +++ b/includes/CaptchaTriggers.php @@ -14,6 +14,5 @@ abstract class CaptchaTriggers { public const CREATE_ACCOUNT = 'createaccount'; public const BAD_LOGIN = 'badlogin'; public const BAD_LOGIN_PER_USER = 'badloginperuser'; - public const EXT_REG_ATTRIBUTE_NAME = 'CaptchaTriggers'; } diff --git a/includes/CaptchaValue.php b/includes/CaptchaValue.php index 8ca26f0e8..eddcfb48c 100644 --- a/includes/CaptchaValue.php +++ b/includes/CaptchaValue.php @@ -8,15 +8,13 @@ namespace MediaWiki\Extension\ConfirmEdit; class CaptchaValue { /** * ID that is used to store the captcha in cache. - * @var string */ - protected $id; + protected string $id; /** * Answer to the captcha. - * @var string */ - protected $solution; + protected string $solution; /** * @var mixed diff --git a/includes/FancyCaptcha/ApiFancyCaptchaReload.php b/includes/FancyCaptcha/ApiFancyCaptchaReload.php index 925279623..dd88e5590 100644 --- a/includes/FancyCaptcha/ApiFancyCaptchaReload.php +++ b/includes/FancyCaptcha/ApiFancyCaptchaReload.php @@ -12,7 +12,6 @@ use MediaWiki\Api\ApiBase; */ class ApiFancyCaptchaReload extends ApiBase { public function execute() { - # Get a new FancyCaptcha form data $captcha = new FancyCaptcha(); $info = $captcha->getCaptcha(); $captchaIndex = $captcha->storeCaptcha( $info ); diff --git a/includes/FancyCaptcha/FancyCaptcha.php b/includes/FancyCaptcha/FancyCaptcha.php index 9f8c58dfe..e208e7014 100644 --- a/includes/FancyCaptcha/FancyCaptcha.php +++ b/includes/FancyCaptcha/FancyCaptcha.php @@ -19,7 +19,7 @@ use Wikimedia\FileBackend\FileBackend; use Wikimedia\FileBackend\FSFileBackend; /** - * FancyCaptcha for displaying captchas precomputed by captcha.py + * FancyCaptcha for displaying captcha images precomputed by captcha.py */ class FancyCaptcha extends SimpleCaptcha { /** @@ -114,9 +114,7 @@ class FancyCaptcha extends SimpleCaptcha { $resultArr['captcha']['url'] = $title->getLocalURL( 'wpCaptchaId=' . urlencode( $index ) ); } - /** - * @return array - */ + /** @inheritDoc */ public function describeCaptchaType() { return [ 'type' => 'image', @@ -124,10 +122,7 @@ class FancyCaptcha extends SimpleCaptcha { ]; } - /** - * @param int $tabIndex - * @return array - */ + /** @inheritDoc */ public function getFormInformation( $tabIndex = 1 ) { $title = SpecialPage::getTitleFor( 'Captcha', 'image' ); $info = $this->getCaptcha(); @@ -163,7 +158,7 @@ class FancyCaptcha extends SimpleCaptcha { 'class' => 'cdx-text-input__input', 'id' => 'wpCaptchaWord', 'type' => 'text', - // max_length in captcha.py plus fudge factor + // max_length in captcha.py plus some fudge factor 'size' => '12', 'autocomplete' => 'off', 'autocorrect' => 'off', @@ -175,7 +170,7 @@ class FancyCaptcha extends SimpleCaptcha { ] ) . Html::closeElement( 'div' ); if ( $this->action == 'createaccount' ) { - // use raw element, because the message can contain links or some other html + // use a raw element, because the message can contain links or some other html $form .= Html::rawElement( 'small', [ 'class' => 'mw-createacct-captcha-assisted' ], wfMessage( 'createacct-imgcaptcha-help' )->parse() @@ -236,7 +231,7 @@ class FancyCaptcha extends SimpleCaptcha { if ( !is_array( $dirs ) || !count( $dirs ) ) { // cache miss $dirs = []; - // subdirs actually present... + // subdirs are actually present... foreach ( $backend->getTopDirectoryList( [ 'dir' => $directory ] ) as $entry ) { if ( ctype_xdigit( $entry ) && strlen( $entry ) == 1 ) { $dirs[] = $entry; @@ -249,7 +244,7 @@ class FancyCaptcha extends SimpleCaptcha { } if ( !count( $dirs ) ) { - // Remove this directory if empty so callers don't keep looking here + // Remove this directory if empty, so callers don't keep looking here $backend->clean( [ 'dir' => $directory ] ); // none found return false; @@ -257,7 +252,7 @@ class FancyCaptcha extends SimpleCaptcha { // pick a random subdir $place = mt_rand( 0, count( $dirs ) - 1 ); - // In case all dirs are not filled, cycle through next digits... + // In case all dirs are not filled, cycle through the next digits... $fancyCount = count( $dirs ); for ( $j = 0; $j < $fancyCount; $j++ ) { $char = $dirs[( $place + $j ) % count( $dirs )]; @@ -311,7 +306,7 @@ class FancyCaptcha extends SimpleCaptcha { } if ( !count( $files ) ) { - // Remove this directory if empty so callers don't keep looking here + // Remove this directory if empty, so callers don't keep looking here $backend->clean( [ 'dir' => $directory ] ); return false; } @@ -370,7 +365,7 @@ class FancyCaptcha extends SimpleCaptcha { // listing cache too stale? break out so it will be cleared break; } - // try next file + // try the next file continue; } return [ @@ -434,11 +429,14 @@ class FancyCaptcha extends SimpleCaptcha { * @return array (salt, hash) */ public function hashFromImageName( $basename ) { - if ( preg_match( '/^image_([0-9a-f]+)_([0-9a-f]+)\\.png$/', $basename, $matches ) ) { - return [ $matches[1], $matches[2] ]; - } else { + if ( !preg_match( '/^image_([0-9a-f]+)_([0-9a-f]+)\\.png$/', $basename, $matches ) ) { throw new InvalidArgumentException( "Invalid filename '$basename'.\n" ); } + + return [ + $matches[1], + $matches[2] + ]; } /** @@ -481,8 +479,8 @@ class FancyCaptcha extends SimpleCaptcha { * @return string */ public function getCaptchaInfo( $captchaData, $id ) { - $title = SpecialPage::getTitleFor( 'Captcha', 'image' ); - return $title->getLocalURL( 'wpCaptchaId=' . urlencode( $id ) ); + return SpecialPage::getTitleFor( 'Captcha', 'image' ) + ->getLocalURL( 'wpCaptchaId=' . urlencode( $id ) ); } /** diff --git a/includes/FancyCaptcha/HTMLFancyCaptchaField.php b/includes/FancyCaptcha/HTMLFancyCaptchaField.php index 6be96803f..72e461f88 100644 --- a/includes/FancyCaptcha/HTMLFancyCaptchaField.php +++ b/includes/FancyCaptcha/HTMLFancyCaptchaField.php @@ -31,9 +31,7 @@ class HTMLFancyCaptchaField extends HTMLFormField { $this->showCreateHelp = !empty( $params['showCreateHelp'] ); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function getInputHTML( $value ) { $out = $this->mParent->getOutput(); @@ -54,7 +52,7 @@ class HTMLFancyCaptchaField extends HTMLFormField { 'id' => $this->mID, 'name' => $this->mName, 'class' => 'cdx-text-input__input', - // max_length in captcha.py plus fudge factor + // max_length in captcha.py plus a fudge factor 'size' => '12', 'dir' => $this->mDir, 'autocomplete' => 'off', @@ -76,7 +74,7 @@ class HTMLFancyCaptchaField extends HTMLFormField { . Html::element( 'input', $attribs ) . Html::closeElement( 'div' ); if ( $this->showCreateHelp ) { - // use raw element, the message will contain a link + // use a raw element, the message will contain a link $html .= Html::rawElement( 'small', [ 'class' => 'mw-createacct-captcha-assisted' ], $this->mParent->msg( 'createacct-imgcaptcha-help' )->parse() ); @@ -87,9 +85,7 @@ class HTMLFancyCaptchaField extends HTMLFormField { return $html; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function getLabel() { // slight abuse of what getLabel() should mean; $mLabel is used for the pre-label text // as the actual label is always the same @@ -97,13 +93,11 @@ class HTMLFancyCaptchaField extends HTMLFormField { . $this->mParent->msg( 'fancycaptcha-captcha' )->text(); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function getLabelHtml( $cellAttributes = [] ) { $labelHtml = parent::getLabelHtml( $cellAttributes ); if ( $this->mLabel ) { - // use raw element, the message will contain a link + // use a raw element, the message will contain a link $labelHtml = Html::rawElement( 'p', [], $this->mLabel ) . $labelHtml; } return $labelHtml; diff --git a/includes/Hooks.php b/includes/Hooks.php index 1ba47da98..fd2e5f5eb 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -4,12 +4,9 @@ namespace MediaWiki\Extension\ConfirmEdit; -use MailAddress; -use MediaWiki\Api\ApiBase; use MediaWiki\Api\Hook\APIGetAllowedParamsHook; use MediaWiki\Content\Content; use MediaWiki\Context\IContextSource; -use MediaWiki\EditPage\EditPage; use MediaWiki\Extension\ConfirmEdit\SimpleCaptcha\SimpleCaptcha; use MediaWiki\Hook\AlternateEditPreviewHook; use MediaWiki\Hook\EditFilterMergedContentHook; @@ -18,26 +15,18 @@ use MediaWiki\Hook\EditPageBeforeEditButtonsHook; use MediaWiki\Hook\EmailUserFormHook; use MediaWiki\Hook\EmailUserHook; use MediaWiki\Html\Html; -use MediaWiki\HTMLForm\HTMLForm; -use MediaWiki\Output\OutputPage; -use MediaWiki\Parser\ParserOutput; use MediaWiki\Permissions\Hook\TitleReadWhitelistHook; use MediaWiki\Registration\ExtensionRegistry; use MediaWiki\ResourceLoader\Hook\ResourceLoaderRegisterModulesHook; use MediaWiki\ResourceLoader\ResourceLoader; -use MediaWiki\Revision\RevisionRecord; use MediaWiki\SpecialPage\Hook\AuthChangeFormFieldsHook; use MediaWiki\SpecialPage\SpecialPage; use MediaWiki\Status\Status; -use MediaWiki\Storage\EditResult; use MediaWiki\Storage\Hook\PageSaveCompleteHook; use MediaWiki\Title\Title; use MediaWiki\User\User; -use MediaWiki\User\UserIdentity; use Wikimedia\IPUtils; -use Wikimedia\Message\MessageSpecifier; use Wikimedia\ObjectCache\WANObjectCache; -use WikiPage; class Hooks implements AlternateEditPreviewHook, @@ -81,38 +70,20 @@ class Hooks implements return $wgCaptcha; } - /** - * @param IContextSource $context - * @param Content $content - * @param Status $status - * @param string $summary - * @param User $user - * @param bool $minorEdit - * @return bool - */ + /** @inheritDoc */ public function onEditFilterMergedContent( IContextSource $context, Content $content, Status $status, $summary, User $user, $minorEdit ) { $simpleCaptcha = self::getInstance(); // Set a flag indicating that ConfirmEdit's implementation of - // EditFilterMergedContent ran. This can be checked by other extensions - // e.g. AbuseFilter. + // EditFilterMergedContent ran. + // This can be checked by other MediaWiki extensions, e.g. AbuseFilter. $simpleCaptcha->setEditFilterMergedContentHandlerInvoked(); return $simpleCaptcha->confirmEditMerged( $context, $content, $status, $summary, $user, $minorEdit ); } - /** - * @see https://www.mediawiki.org/wiki/Manual:Hooks/PageSaveComplete - * - * @param WikiPage $wikiPage - * @param UserIdentity $user - * @param string $summary - * @param int $flags - * @param RevisionRecord $revisionRecord - * @param EditResult $editResult - * @return bool|void - */ + /** @inheritDoc */ public function onPageSaveComplete( $wikiPage, $user, @@ -129,68 +100,32 @@ class Hooks implements return true; } - /** - * @see https://www.mediawiki.org/wiki/Manual:Hooks/EditPageBeforeEditButtons - * - * @param EditPage $editpage Current EditPage object - * @param array &$buttons Array of edit buttons, "Save", "Preview", "Live", and "Diff" - * @param int &$tabindex HTML tabindex of the last edit check/button - */ + /** @inheritDoc */ public function onEditPageBeforeEditButtons( $editpage, &$buttons, &$tabindex ) { self::getInstance()->editShowCaptcha( $editpage ); } - /** - * @param EditPage $editPage - * @param OutputPage $out - */ + /** @inheritDoc */ public function onEditPage__showEditForm_fields( $editPage, $out ) { self::getInstance()->showEditFormFields( $editPage, $out ); } - /** - * @see https://www.mediawiki.org/wiki/Manual:Hooks/EmailUserForm - * - * @param HTMLForm &$form HTMLForm object - * @return bool|void True or no return value to continue or false to abort - */ + /** @inheritDoc */ public function onEmailUserForm( &$form ) { return self::getInstance()->injectEmailUser( $form ); } - /** - * @see https://www.mediawiki.org/wiki/Manual:Hooks/EmailUser - * - * @param MailAddress &$to MailAddress object of receiving user - * @param MailAddress &$from MailAddress object of sending user - * @param string &$subject subject of the mail - * @param string &$text text of the mail - * @param bool|Status|MessageSpecifier|array &$error Out-param for an error. - * Should be set to a Status object or boolean false. - * @return bool|void True or no return value to continue or false to abort - */ + /** @inheritDoc */ public function onEmailUser( &$to, &$from, &$subject, &$text, &$error ) { return self::getInstance()->confirmEmailUser( $from, $to, $subject, $text, $error ); } - /** - * APIGetAllowedParams hook handler - * Default $flags to 1 for backwards-compatible behavior - * @param ApiBase $module - * @param array &$params - * @param int $flags - * @return bool - */ + /** @inheritDoc */ public function onAPIGetAllowedParams( $module, &$params, $flags ) { return self::getInstance()->apiGetAllowedParams( $module, $params, $flags ); } - /** - * @param array $requests - * @param array $fieldInfo - * @param array &$formDescriptor - * @param string $action - */ + /** @inheritDoc */ public function onAuthChangeFormFields( $requests, $fieldInfo, &$formDescriptor, $action ) { @@ -206,14 +141,7 @@ class Hooks implements } } - /** - * @see https://www.mediawiki.org/wiki/Manual:Hooks/TitleReadWhitelist - * - * @param Title $title - * @param User $user - * @param bool &$whitelisted - * @return bool|void - */ + /** @inheritDoc */ public function onTitleReadWhitelist( $title, $user, &$whitelisted ) { $image = SpecialPage::getTitleFor( 'Captcha', 'image' ); $help = SpecialPage::getTitleFor( 'Captcha', 'help' ); @@ -223,7 +151,6 @@ class Hooks implements } /** - * * Callback for extension.json of FancyCaptcha to set a default captcha directory, * which depends on wgUploadDirectory */ @@ -234,15 +161,7 @@ class Hooks implements } } - /** - * @see https://www.mediawiki.org/wiki/Manual:Hooks/AlternateEditPreview - * - * @param EditPage $editPage - * @param Content &$content - * @param string &$previewHTML - * @param ParserOutput &$parserOutput - * @return bool|void - */ + /** @inheritDoc */ public function onAlternateEditPreview( $editPage, &$content, &$previewHTML, &$parserOutput ) { @@ -312,12 +231,7 @@ class Hooks implements return false; } - /** - * @see https://www.mediawiki.org/wiki/Manual:Hooks/ResourceLoaderRegisterModules - * - * @param ResourceLoader $resourceLoader - * @return void - */ + /** @inheritDoc */ public function onResourceLoaderRegisterModules( ResourceLoader $resourceLoader ): void { $extensionRegistry = ExtensionRegistry::getInstance(); $messages = []; diff --git a/includes/Hooks/HookRunner.php b/includes/Hooks/HookRunner.php index 72694d6b1..6f0d13b09 100644 --- a/includes/Hooks/HookRunner.php +++ b/includes/Hooks/HookRunner.php @@ -31,8 +31,7 @@ use MediaWiki\Page\PageIdentity; class HookRunner implements ConfirmEditTriggersCaptchaHook { - /** @var HookContainer */ - private $hookContainer; + private HookContainer $hookContainer; /** * @param HookContainer $hookContainer @@ -41,9 +40,7 @@ class HookRunner implements $this->hookContainer = $hookContainer; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function onConfirmEditTriggersCaptcha( string $action, ?PageIdentity $page, diff --git a/includes/QuestyCaptcha/QuestyCaptcha.php b/includes/QuestyCaptcha/QuestyCaptcha.php index bf8e27795..0df60ebd4 100644 --- a/includes/QuestyCaptcha/QuestyCaptcha.php +++ b/includes/QuestyCaptcha/QuestyCaptcha.php @@ -42,20 +42,7 @@ class QuestyCaptcha extends SimpleCaptcha { } } - /** - * @param array &$resultArr - */ - protected function addCaptchaAPI( &$resultArr ) { - $captcha = $this->getCaptcha(); - $index = $this->storeCaptcha( $captcha ); - $resultArr['captcha'] = $this->describeCaptchaType(); - $resultArr['captcha']['id'] = $index; - $resultArr['captcha']['question'] = $captcha['question']; - } - - /** - * @return array - */ + /** @inheritDoc */ public function describeCaptchaType() { return [ 'type' => 'question', @@ -63,9 +50,7 @@ class QuestyCaptcha extends SimpleCaptcha { ]; } - /** - * @return array - */ + /** @inheritDoc */ public function getCaptcha() { global $wgCaptchaQuestions; diff --git a/includes/ReCaptchaNoCaptcha/HTMLReCaptchaNoCaptchaField.php b/includes/ReCaptchaNoCaptcha/HTMLReCaptchaNoCaptchaField.php index d6e4765fd..ffacae20c 100644 --- a/includes/ReCaptchaNoCaptcha/HTMLReCaptchaNoCaptchaField.php +++ b/includes/ReCaptchaNoCaptcha/HTMLReCaptchaNoCaptchaField.php @@ -32,9 +32,7 @@ class HTMLReCaptchaNoCaptchaField extends HTMLFormField { $this->mName = 'g-recaptcha-response'; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function getInputHTML( $value ) { $out = $this->mParent->getOutput(); $lang = htmlspecialchars( urlencode( $this->mParent->getLanguage()->getCode() ) ); diff --git a/includes/ReCaptchaNoCaptcha/ReCaptchaNoCaptcha.php b/includes/ReCaptchaNoCaptcha/ReCaptchaNoCaptcha.php index ce533ff51..5ee4d47fd 100644 --- a/includes/ReCaptchaNoCaptcha/ReCaptchaNoCaptcha.php +++ b/includes/ReCaptchaNoCaptcha/ReCaptchaNoCaptcha.php @@ -77,9 +77,7 @@ HTML; ]; } - /** - * @return string[] - */ + /** @inheritDoc */ public static function getCSPUrls() { return [ 'https://www.recaptcha.net/recaptcha/api.js' ]; } @@ -213,33 +211,25 @@ HTML; return true; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function getError() { return $this->error; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function storeCaptcha( $info ) { // ReCaptcha is stored by Google; the ID will be generated at that time as well, and // the one returned here won't be used. Just pretend this worked. return 'not used'; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function retrieveCaptcha( $index ) { // just pretend it worked return [ 'index' => $index ]; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function getCaptcha() { // ReCaptcha is handled by frontend code + an external provider; nothing to do here. return []; diff --git a/includes/ReCaptchaNoCaptcha/ReCaptchaNoCaptchaAuthenticationRequest.php b/includes/ReCaptchaNoCaptcha/ReCaptchaNoCaptchaAuthenticationRequest.php index 101350b72..c4197f464 100644 --- a/includes/ReCaptchaNoCaptcha/ReCaptchaNoCaptchaAuthenticationRequest.php +++ b/includes/ReCaptchaNoCaptcha/ReCaptchaNoCaptchaAuthenticationRequest.php @@ -6,7 +6,7 @@ use MediaWiki\Auth\AuthenticationRequest; use MediaWiki\Extension\ConfirmEdit\Auth\CaptchaAuthenticationRequest; /** - * Authentication request for ReCaptcha v2. Unlike the parent class, no session storage is used + * Authentication request for ReCaptcha v2. Unlike the parent class, no session storage is used, * and there is no ID; Google provides a single proof string after successfully solving a captcha. */ class ReCaptchaNoCaptchaAuthenticationRequest extends CaptchaAuthenticationRequest { @@ -14,17 +14,13 @@ class ReCaptchaNoCaptchaAuthenticationRequest extends CaptchaAuthenticationReque parent::__construct( '', [] ); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function loadFromSubmission( array $data ) { // unhack the hack in parent return AuthenticationRequest::loadFromSubmission( $data ); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function getFieldInfo() { $fieldInfo = parent::getFieldInfo(); diff --git a/includes/SimpleCaptcha/SimpleCaptcha.php b/includes/SimpleCaptcha/SimpleCaptcha.php index e79cf68bc..2ed68d4e3 100644 --- a/includes/SimpleCaptcha/SimpleCaptcha.php +++ b/includes/SimpleCaptcha/SimpleCaptcha.php @@ -115,7 +115,7 @@ class SimpleCaptcha { } /** - * Returns a list of activate captchas for a page by key. + * Returns a list of activated captchas for a page by key. * @return bool[] */ public function getActivatedCaptchas() { @@ -153,7 +153,7 @@ class SimpleCaptcha { * Override this! * * It is not guaranteed that the CAPTCHA will load synchronously with the main page - * content. So you can not rely on registering handlers before page load. E.g.: + * content. So you cannot rely on registering handlers before page load. E.g.: * * NOT SAFE: $( window ).on( 'load', handler ) * SAFE: $( handler ) @@ -214,7 +214,7 @@ class SimpleCaptcha { } /** - * Adds the CSP policies necessary for the captcha module to work in a CSP enforced + * Adds the necessary CSP policies for the captcha module to work in a CSP enforced * setup. * * @param ContentSecurityPolicy $csp The CSP instance to add the policies to, usually @@ -275,7 +275,7 @@ class SimpleCaptcha { } /** - * Show error message for missing or incorrect captcha on EditPage. + * Show the error message for missing or incorrect captcha on EditPage. * @param EditPage $editPage * @param OutputPage $out */ @@ -420,7 +420,7 @@ class SimpleCaptcha { * IP addresses and IP ranges from it. * * Note that only lines with just the IP address or IP range is considered - * as valid. Whitespace is allowed but if there is any other character on + * as valid. Whitespace is allowed, but if there is any other character on * the line, it's not considered as a valid entry. * * @param string[] $input @@ -645,6 +645,7 @@ class SimpleCaptcha { $numHits = count( $addedMatches ); if ( $numHits > 0 ) { + // TODO: last parameter to sprintf() isn't used $this->trigger = sprintf( "%dx %s at [[%s]]: %s", $numHits, $regex, @@ -751,61 +752,62 @@ class SimpleCaptcha { if ( count( $lines ) == 0 ) { wfDebug( "No lines\n" ); return []; - } else { - # 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'; - $regexes = []; - $regexStart = [ - 'normal' => '/^(?:https?:)?\/\/+[a-z0-9_\-.]*(?:', - 'noprotocol' => '/^(?:', - ]; - $regexEnd = [ - 'normal' => ')/Si', - 'noprotocol' => ')/Si', - ]; - $regexMax = 4096; - $build = []; - foreach ( $lines as $line ) { - # Extract flags from the line - $options = []; - if ( preg_match( '/^(.*?)\s*<([^<>]*)>$/', $line, $matches ) ) { - if ( $matches[1] === '' ) { - wfDebug( "Line with empty regex\n" ); - continue; - } - $line = $matches[1]; - $opts = preg_split( '/\s*\|\s*/', trim( $matches[2] ) ); - foreach ( $opts as $opt ) { - $opt = strtolower( $opt ); - if ( $opt == 'noprotocol' ) { - $options['noprotocol'] = true; - } - } - } - - $key = isset( $options['noprotocol'] ) ? 'noprotocol' : 'normal'; - - // FIXME: not very robust size check, but should work. :) - if ( !isset( $build[$key] ) ) { - $build[$key] = $line; - } elseif ( strlen( $build[$key] ) + strlen( $line ) > $regexMax ) { - $regexes[] = $regexStart[$key] . - str_replace( '/', '\/', preg_replace( '|\\\*/|', '/', $build[$key] ) ) . - $regexEnd[$key]; - $build[$key] = $line; - } else { - $build[$key] .= '|' . $line; - } - } - foreach ( $build as $key => $value ) { - $regexes[] = $regexStart[$key] . - str_replace( '/', '\/', preg_replace( '|\\\*/|', '/', $value ) ) . - $regexEnd[$key]; - } - return $regexes; } + + # 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'; + $regexes = []; + $regexStart = [ + 'normal' => '/^(?:https?:)?\/\/+[a-z0-9_\-.]*(?:', + 'noprotocol' => '/^(?:', + ]; + $regexEnd = [ + 'normal' => ')/Si', + 'noprotocol' => ')/Si', + ]; + $regexMax = 4096; + $build = []; + foreach ( $lines as $line ) { + # Extract flags from the line + $options = []; + if ( preg_match( '/^(.*?)\s*<([^<>]*)>$/', $line, $matches ) ) { + if ( $matches[1] === '' ) { + wfDebug( "Line with empty regex\n" ); + continue; + } + $line = $matches[1]; + $opts = preg_split( '/\s*\|\s*/', trim( $matches[2] ) ); + foreach ( $opts as $opt ) { + $opt = strtolower( $opt ); + if ( $opt == 'noprotocol' ) { + $options['noprotocol'] = true; + } + } + } + + $key = isset( $options['noprotocol'] ) ? 'noprotocol' : 'normal'; + + // FIXME: not very robust size check, but should work. :) + if ( !isset( $build[$key] ) ) { + $build[$key] = $line; + } elseif ( strlen( $build[$key] ) + strlen( $line ) > $regexMax ) { + $regexes[] = $regexStart[$key] . + str_replace( '/', '\/', preg_replace( '|\\\*/|', '/', $build[$key] ) ) . + $regexEnd[$key]; + $build[$key] = $line; + } else { + $build[$key] .= '|' . $line; + } + } + foreach ( $build as $key => $value ) { + $regexes[] = $regexStart[$key] . + str_replace( '/', '\/', preg_replace( '|\\\*/|', '/', $value ) ) . + $regexEnd[$key]; + } + + return $regexes; } /** @@ -839,10 +841,10 @@ class SimpleCaptcha { } if ( $this->shouldCheck( $page, $newtext, $section, $context ) ) { return $this->passCaptchaLimitedFromRequest( $wgRequest, $user ); - } else { - wfDebug( "ConfirmEdit: no need to show captcha.\n" ); - return true; } + + wfDebug( "ConfirmEdit: no need to show captcha.\n" ); + return true; } /** @@ -857,7 +859,7 @@ class SimpleCaptcha { */ public function confirmEditMerged( $context, $content, $status, $summary, $user, $minorEdit ) { $title = $context->getTitle(); - if ( !( $title->canExist() ) ) { + if ( !$title->canExist() ) { // we check WikiPage only // try to get an appropriate title for this page $title = $context->getTitle(); @@ -888,7 +890,7 @@ class SimpleCaptcha { // that the user already submitted an edit, and so the 'captcha-edit' // message is more appropriate. $message = 'captcha-edit'; - [ $_index, $word ] = $this->getCaptchaParamsFromRequest( + [ , $word ] = $this->getCaptchaParamsFromRequest( RequestContext::getMain()->getRequest() ); // But if there's a word supplied in the request, then we should @@ -917,11 +919,7 @@ class SimpleCaptcha { */ public function needCreateAccountCaptcha( User $creatingUser ) { if ( $this->triggersCaptcha( CaptchaTriggers::CREATE_ACCOUNT ) ) { - if ( $this->canSkipCaptcha( $creatingUser, - \MediaWiki\MediaWikiServices::getInstance()->getMainConfig() ) ) { - return false; - } - return true; + return !$this->canSkipCaptcha( $creatingUser, MediaWikiServices::getInstance()->getMainConfig() ); } return false; } @@ -941,7 +939,7 @@ class SimpleCaptcha { $user = RequestContext::getMain()->getUser(); if ( $this->triggersCaptcha( CaptchaTriggers::SENDEMAIL ) ) { if ( $this->canSkipCaptcha( $user, - \MediaWiki\MediaWikiServices::getInstance()->getMainConfig() ) ) { + MediaWikiServices::getInstance()->getMainConfig() ) ) { return true; } @@ -988,7 +986,7 @@ class SimpleCaptcha { } /** - * Checks, if the user reached the amount of false CAPTCHAs and give him some vacation + * Checks, if the user reached the number of false CAPTCHAs and give him some vacation * or run self::passCaptcha() and clear counter if correct. * * @param WebRequest $request @@ -1011,7 +1009,7 @@ class SimpleCaptcha { } /** - * Checks, if the user reached the amount of false CAPTCHAs and give him some vacation + * Checks, if the user reached the number of false CAPTCHAs and give him some vacation * or run self::passCaptcha() and clear counter if correct. * * @param string $index Captcha idenitifier @@ -1023,7 +1021,7 @@ class SimpleCaptcha { public function passCaptchaLimited( $index, $word, User $user ) { // don't increase pingLimiter here, just check, if CAPTCHA limit exceeded if ( $user->pingLimiter( 'badcaptcha', 0 ) ) { - // for debugging add an proper error message, the user just see an false captcha error message + // for debugging add a proper error message, the user will just see a false captcha error message $this->log( 'User reached RateLimit, preventing action' ); return false; } @@ -1032,7 +1030,7 @@ class SimpleCaptcha { return true; } - // captcha was not solved: increase limit and return false + // captcha was not solved: increase the limit and return false $user->pingLimiter( 'badcaptcha' ); return false; } diff --git a/includes/Specials/SpecialCaptcha.php b/includes/Specials/SpecialCaptcha.php index 9cfe884f3..5ef128747 100644 --- a/includes/Specials/SpecialCaptcha.php +++ b/includes/Specials/SpecialCaptcha.php @@ -10,9 +10,7 @@ class SpecialCaptcha extends UnlistedSpecialPage { parent::__construct( 'Captcha' ); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function execute( $par ) { $this->setHeaders(); diff --git a/includes/Store/CaptchaCacheStore.php b/includes/Store/CaptchaCacheStore.php index ef60e6a0f..33c9c9bdf 100644 --- a/includes/Store/CaptchaCacheStore.php +++ b/includes/Store/CaptchaCacheStore.php @@ -14,9 +14,7 @@ class CaptchaCacheStore extends CaptchaStore { $this->store = MediaWikiServices::getInstance()->getMicroStash(); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function store( $index, $info ) { global $wgCaptchaSessionExpiration; @@ -24,26 +22,23 @@ class CaptchaCacheStore extends CaptchaStore { $this->store->makeKey( 'captcha', $index ), $info, $wgCaptchaSessionExpiration, - // Assume the write will reach the master DC before the user sends the + // Assume the write action will reach the primary DC before the user sends the // HTTP POST request attempted to solve the captcha and perform an action $this->store::WRITE_BACKGROUND ); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function retrieve( $index ) { return $this->store->get( $this->store->makeKey( 'captcha', $index ) ); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function clear( $index ) { $this->store->delete( $this->store->makeKey( 'captcha', $index ) ); } + /** @inheritDoc */ public function cookiesNeeded() { return false; } diff --git a/includes/Store/CaptchaHashStore.php b/includes/Store/CaptchaHashStore.php index f66617f4d..abc5dc8d5 100644 --- a/includes/Store/CaptchaHashStore.php +++ b/includes/Store/CaptchaHashStore.php @@ -6,16 +6,12 @@ class CaptchaHashStore extends CaptchaStore { /** @var array */ protected $data = []; - /** - * @inheritDoc - */ + /** @inheritDoc */ public function store( $index, $info ) { $this->data[$index] = $info; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function retrieve( $index ) { if ( array_key_exists( $index, $this->data ) ) { return $this->data[$index]; @@ -23,17 +19,17 @@ class CaptchaHashStore extends CaptchaStore { return false; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function clear( $index ) { unset( $this->data[$index] ); } + /** @inheritDoc */ public function cookiesNeeded() { return false; } + /** @inheritDoc */ public function clearAll() { $this->data = []; } diff --git a/includes/Store/CaptchaSessionStore.php b/includes/Store/CaptchaSessionStore.php index d6ae35d0b..673ad6023 100644 --- a/includes/Store/CaptchaSessionStore.php +++ b/includes/Store/CaptchaSessionStore.php @@ -10,27 +10,22 @@ class CaptchaSessionStore extends CaptchaStore { SessionManager::getGlobalSession()->persist(); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function store( $index, $info ) { SessionManager::getGlobalSession()->set( 'captcha' . $index, $info ); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function retrieve( $index ) { return SessionManager::getGlobalSession()->get( 'captcha' . $index, false ); } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function clear( $index ) { SessionManager::getGlobalSession()->remove( 'captcha' . $index ); } + /** @inheritDoc */ public function cookiesNeeded() { return true; } diff --git a/includes/Turnstile/HTMLTurnstileField.php b/includes/Turnstile/HTMLTurnstileField.php index 00f00de04..fe4e1d88c 100644 --- a/includes/Turnstile/HTMLTurnstileField.php +++ b/includes/Turnstile/HTMLTurnstileField.php @@ -19,7 +19,7 @@ class HTMLTurnstileField extends HTMLFormField { /** * Parameters: * - key: (string, required) Turnstile public key - * - error: (string) Turnstile error from previous round + * - error: (string) Turnstile error from the previous round * @param array $params */ public function __construct( array $params ) { @@ -32,14 +32,12 @@ class HTMLTurnstileField extends HTMLFormField { $this->mName = 'cf-turnstile-response'; } - /** - * @inheritDoc - */ + /** @inheritDoc */ public function getInputHTML( $value ) { $out = $this->mParent->getOutput(); $lang = htmlspecialchars( urlencode( $this->mParent->getLanguage()->getCode() ) ); - // Insert Turnstile script, in display language, if available. + // Insert the Turnstile script, in display language, if available. // Language falls back to the browser's display language. // See https://developers.cloudflare.com/turnstile/reference/supported-languages/ $out->addHeadItem( diff --git a/includes/Turnstile/Turnstile.php b/includes/Turnstile/Turnstile.php index 7d2264152..6cc090b97 100644 --- a/includes/Turnstile/Turnstile.php +++ b/includes/Turnstile/Turnstile.php @@ -44,7 +44,7 @@ class Turnstile extends SimpleCaptcha { return [ 'html' => $output, 'headitems' => [ - // Insert Turnstile script, in display language, if available. + // Insert the Turnstile script, in display language, if available. // Language falls back to the browser's display language. // See https://developers.cloudflare.com/turnstile/reference/supported-languages/ "