From c8e3c424f9ddd929b4ddfae61f39f94b2894b467 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 17 Jan 2014 12:26:41 -0800 Subject: [PATCH] Add Status outparam for AbortNewAccount hook to fix API error handling With this fix and relevant fix for ConfirmEdit in, an API account creation attempt that fails the captcha check will return a much cleaner error such as: { 'error': { 'code': 'captcha-createaccount-fail', 'info': 'Incorrect or missing CAPTCHA.' } } Abort hooks that use the old interface and send a text message will now be reported with the generic 'createaccount-hook-abort' message code, with the string passed back intact. Previously, the returned result would list the contents of the message _as_ the error code, making it hard for a client to determine the error. 'AbortNewAccount' hook clients can add a '&$status=null' function paramater on their signature, and along with the back-compat message parameter you can set something like: $msg = wfMessage( 'captcha-createaccount-fail' )->text(); // back-compat $status = Status::newFatal( 'captcha-createaccount-fail' ); // new This is done for ConfirmEdit in If9cc08e Bug: 60008 Change-Id: I6ae34c00d1051d34363b6d654424be17dcb1ea30 --- docs/hooks.txt | 3 +++ includes/specials/SpecialUserlogin.php | 19 +++++++++++++++---- languages/messages/MessagesEn.php | 1 + languages/messages/MessagesQqq.php | 1 + maintenance/language/messageTypes.inc | 1 + 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/docs/hooks.txt b/docs/hooks.txt index b61743000a..ec04a750c4 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -269,6 +269,9 @@ $reason: the reason for the move (added in 1.13) 'AbortNewAccount': Return false to cancel explicit account creation. $user: the User object about to be created (read-only, incomplete) &$msg: out parameter: HTML to display on abort +&$status: out parameter: Status object to return, replaces the older $msg param (added in 1.23) + Create the object with Status::newFatal() to ensure proper API error messages + are returned when creating account through API clients. 'AbortTalkPageEmailNotification': Return false to cancel talk page email notification $targetUser: the user whom to send talk page email notification diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php index 8e5ef58126..eca1839864 100644 --- a/includes/specials/SpecialUserlogin.php +++ b/includes/specials/SpecialUserlogin.php @@ -444,12 +444,23 @@ class LoginForm extends SpecialPage { $u->setRealName( $this->mRealName ); $abortError = ''; - if ( !wfRunHooks( 'AbortNewAccount', array( $u, &$abortError ) ) ) { + $abortStatus = null; + if ( !wfRunHooks( 'AbortNewAccount', array( $u, &$abortError, &$abortStatus ) ) ) { // Hook point to add extra creation throttles and blocks wfDebug( "LoginForm::addNewAccountInternal: a hook blocked creation\n" ); - $abortError = new RawMessage( $abortError ); - $abortError->text(); - return Status::newFatal( $abortError ); + if ( $abortStatus === null ) { + // Report back the old string as a raw message status. + // This will report the error back as 'createaccount-hook-aborted' + // with the given string as the message. + // To return a different error code, return a Status object. + $abortError = new Message( 'createaccount-hook-aborted', array( $abortError ) ); + $abortError->text(); + return Status::newFatal( $abortError ); + } else { + // For MediaWiki 1.23+ and updated hooks, return the Status object + // returned from the hook. + return $abortStatus; + } } // Hook point to check for exempt from account creation throttle diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index d7b300cace..dbda2980ee 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -1625,6 +1625,7 @@ The reason given by $3 is ''$2''", 'cantcreateaccount-text' => "Account creation from this IP address ('''$1''') has been blocked by [[User:$3|$3]]. The reason given by $3 is ''$2''", +'createaccount-hook-aborted' => '$1', # do not translate or duplicate this message to other languages # History pages 'viewpagelogs' => 'View logs for this page', diff --git a/languages/messages/MessagesQqq.php b/languages/messages/MessagesQqq.php index 64214d2d8b..575b40c5cd 100644 --- a/languages/messages/MessagesQqq.php +++ b/languages/messages/MessagesQqq.php @@ -2282,6 +2282,7 @@ See also: * $2 - reason or {{msg-mw|Blockednoreason}} * $3 - username * $4 - current user's IP address", +'createaccount-hook-aborted' => 'Placeholder message to return with API errors on account create; passes through the message from a hook {{notranslate}}', # History pages 'viewpagelogs' => 'Link displayed in history of pages', diff --git a/maintenance/language/messageTypes.inc b/maintenance/language/messageTypes.inc index a7d1ceb67c..77ffe798b2 100644 --- a/maintenance/language/messageTypes.inc +++ b/maintenance/language/messageTypes.inc @@ -271,6 +271,7 @@ $wgIgnoredMessages = array( 'autocomment-prefix', 'move-redirect-text', 'interlanguage-link-title-langonly', + 'createaccount-hook-abort', ); /** Optional messages, which may be translated only if changed in the target language. */ -- 2.20.1