From d0439af89f6b254cea09b3773ab139f04f81a97d Mon Sep 17 00:00:00 2001 From: Tyler Romeo Date: Tue, 15 Jul 2014 14:48:09 -0400 Subject: [PATCH] Make UserNotLoggedIn redirect to login page For pages like Special:Watchlist that throw a UserNotLoggedIn exception when the user is anonymous, this patch makes the page redirect to the login page automatically. This is instead of the current behavior of showing a link to the login page that the user must click. (Also, Special:Userlogin has existing functionality that will redirect the user back once they are logged in.) Bug: 15484 Change-Id: Idd9325374cb5dc13c4c057f45f88a33bdff523a9 --- RELEASE-NOTES-1.24 | 3 + includes/actions/WatchAction.php | 9 +-- includes/exception/UserNotLoggedIn.php | 43 +++++++++++++- includes/specialpage/SpecialPage.php | 40 +++---------- includes/specials/SpecialUserlogin.php | 66 +++++++++++++++++++++- languages/i18n/en.json | 6 +- languages/i18n/qqq.json | 4 +- tests/phpunit/includes/SpecialPageTest.php | 21 ++----- 8 files changed, 128 insertions(+), 64 deletions(-) diff --git a/RELEASE-NOTES-1.24 b/RELEASE-NOTES-1.24 index 92b34c66dd..ae2ab4f5ef 100644 --- a/RELEASE-NOTES-1.24 +++ b/RELEASE-NOTES-1.24 @@ -143,6 +143,9 @@ production. the $wgResourceModuleSkinStyles global. See the Vector skin for examples. * (bug 4488) There is now a preference to watch pages where the user has rollbacked an edit by default. +* (bug 15484) Users will now be redirected to the login page when they need to + log in, rather than being shown a page asking them to log in and having to click + another link to actually get to the login page. === Bug fixes in 1.24 === * (bug 49116) Footer copyright notice is now always displayed in user language diff --git a/includes/actions/WatchAction.php b/includes/actions/WatchAction.php index a54b7c57cd..cc18d3dda5 100644 --- a/includes/actions/WatchAction.php +++ b/includes/actions/WatchAction.php @@ -82,14 +82,7 @@ class WatchAction extends FormAction { protected function checkCanExecute( User $user ) { // Must be logged in if ( $user->isAnon() ) { - $loginreqlink = Linker::linkKnown( - SpecialPage::getTitleFor( 'Userlogin' ), - $this->msg( 'loginreqlink' )->escaped(), - array(), - array( 'returnto' => $this->getPageTitle(), 'returntoquery' => 'action=' . $this->getName() ) - ); - $reasonMsg = $this->msg( 'watchlistanontext' )->rawParams( $loginreqlink ); - throw new UserNotLoggedIn( $reasonMsg, 'watchnologin' ); + throw new UserNotLoggedIn( 'watchlistanontext', 'watchnologin' ); } return parent::checkCanExecute( $user ); diff --git a/includes/exception/UserNotLoggedIn.php b/includes/exception/UserNotLoggedIn.php index 9d89009100..03ba0b2039 100644 --- a/includes/exception/UserNotLoggedIn.php +++ b/includes/exception/UserNotLoggedIn.php @@ -19,12 +19,14 @@ */ /** - * Shows a generic "user is not logged in" error page. + * Redirect a user to the login page * * This is essentially an ErrorPageError exception which by default uses the * 'exception-nologin' as a title and 'exception-nologin-text' for the message. - * @see bug 37627 - * @since 1.20 + * + * @note In order for this exception to redirect, the error message passed to the + * constructor has to be explicitly added to LoginForm::validErrorMessages. Otherwise, + * the user will just be shown the message rather than redirected. * * @par Example: * @code @@ -43,11 +45,16 @@ * } * @endcode * + * @see bug 37627 + * @since 1.20 * @ingroup Exception */ class UserNotLoggedIn extends ErrorPageError { /** + * @note The value of the $reasonMsg parameter must be put into LoginForm::validErrorMessages + * if you want the user to be automatically redirected to the login form. + * * @param string $reasonMsg A message key containing the reason for the error. * Optional, default: 'exception-nologin-text' * @param string $titleMsg A message key to set the page title. @@ -62,4 +69,34 @@ class UserNotLoggedIn extends ErrorPageError { ) { parent::__construct( $titleMsg, $reasonMsg, $params ); } + + /** + * Redirect to Special:Userlogin if the specified message is compatible. Otherwise, + * show an error page as usual. + */ + public function report() { + // If an unsupported message is used, don't try redirecting to Special:Userlogin, + // since the message may not be compatible. + if ( !in_array( $this->msg, LoginForm::$validErrorMessages ) ) { + parent::report(); + } + + // Message is valid. Redirec to Special:Userlogin + + $context = RequestContext::getMain(); + + $output = $context->getOutput(); + $query = $context->getRequest()->getValues(); + // Title will be overridden by returnto + unset( $query['title'] ); + // Redirect to Special:Userlogin + $output->redirect( SpecialPage::getTitleFor( 'Userlogin' )->getFullURL( array( + // Return to this page when the user logs in + 'returnto' => $context->getTitle()->getFullText(), + 'returntoquery' => wfArrayToCgi( $query ), + 'warning' => $this->msg, + ) ) ); + + $output->output(); + } } diff --git a/includes/specialpage/SpecialPage.php b/includes/specialpage/SpecialPage.php index 0070c74b04..df8d9de816 100644 --- a/includes/specialpage/SpecialPage.php +++ b/includes/specialpage/SpecialPage.php @@ -274,44 +274,20 @@ class SpecialPage { } /** - * If the user is not logged in, throws UserNotLoggedIn error. + * If the user is not logged in, throws UserNotLoggedIn error * - * Default error message includes a link to Special:Userlogin with properly set 'returnto' query - * parameter. + * The user will be redirected to Special:Userlogin with the given message as an error on + * the form. * * @since 1.23 - * @param string|Message $reasonMsg [optional] Passed on to UserNotLoggedIn constructor. Strings - * will be used as message keys. If a string is given, the message will also receive a - * formatted login link (generated using the 'loginreqlink' message) as first parameter. If a - * Message is given, it will be passed on verbatim. - * @param string|Message $titleMsg [optional] Passed on to UserNotLoggedIn constructor. Strings - * will be used as message keys. + * @param string $reasonMsg [optional] Message key to be displayed on login page + * @param string $titleMsg [optional] Passed on to UserNotLoggedIn constructor * @throws UserNotLoggedIn */ - public function requireLogin( $reasonMsg = null, $titleMsg = null ) { + public function requireLogin( + $reasonMsg = 'exception-nologin-text', $titleMsg = 'exception-nologin' + ) { if ( $this->getUser()->isAnon() ) { - // Use default messages if not given or explicit null passed - if ( !$reasonMsg ) { - $reasonMsg = 'exception-nologin-text-manual'; - } - if ( !$titleMsg ) { - $titleMsg = 'exception-nologin'; - } - - // Convert to Messages with current context - if ( is_string( $reasonMsg ) ) { - $loginreqlink = Linker::linkKnown( - SpecialPage::getTitleFor( 'Userlogin' ), - $this->msg( 'loginreqlink' )->escaped(), - array(), - array( 'returnto' => $this->getPageTitle()->getPrefixedText() ) - ); - $reasonMsg = $this->msg( $reasonMsg )->rawParams( $loginreqlink ); - } - if ( is_string( $titleMsg ) ) { - $titleMsg = $this->msg( $titleMsg ); - } - throw new UserNotLoggedIn( $reasonMsg, $titleMsg ); } } diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php index 46b499989a..bd7457ee17 100644 --- a/includes/specials/SpecialUserlogin.php +++ b/includes/specials/SpecialUserlogin.php @@ -42,6 +42,28 @@ class LoginForm extends SpecialPage { const NEED_TOKEN = 12; const WRONG_TOKEN = 13; + /** + * Valid error and warning messages + * + * Special:Userlogin can show an error or warning message on the form when + * coming from another page. This is done via the ?error= or ?warning= GET + * parameters. + * + * This array is the list of valid message keys. All other values will be + * ignored. + * + * @since 1.24 + * @var string[] + */ + public static $validErrorMessages = array( + 'exception-nologin-text', + 'watchlistanontext', + 'changeemail-no-info', + 'resetpass-no-info', + 'confirmemail_needlogin', + 'prefsnologintext2', + ); + public $mAbortLoginErrorMsg = null; protected $mUsername; @@ -65,6 +87,8 @@ class LoginForm extends SpecialPage { protected $mType; protected $mReason; protected $mRealName; + protected $mEntryError = ''; + protected $mEntryErrorType = 'error'; private $mTempPasswordUsed; private $mLoaded = false; @@ -128,6 +152,37 @@ class LoginForm extends SpecialPage { $this->mReturnTo = $request->getVal( 'returnto', '' ); $this->mReturnToQuery = $request->getVal( 'returntoquery', '' ); + // Show an error or warning passed on from a previous page + $entryError = $this->msg( $request->getVal( 'error', '' ) ); + $entryWarning = $this->msg( $request->getVal( 'warning', '' ) ); + // bc: provide login link as a parameter for messages where the translation + // was not updated + $loginreqlink = Linker::linkKnown( + $this->getPageTitle(), + $this->msg( 'loginreqlink' )->escaped(), + array(), + array( + 'returnto' => $this->mReturnTo, + 'returntoquery' => $this->mReturnToQuery, + 'uselang' => $this->mLanguage, + 'fromhttp' => $this->mFromHTTP ? '1' : '0', + ) + ); + + // Only show valid error or warning messages. + if ( $entryError->exists() + && in_array( $entryError->getKey(), self::$validErrorMessages ) + ) { + $this->mEntryErrorType = 'error'; + $this->mEntryError = $entryError->rawParams( $loginreqlink )->escaped(); + + } elseif ( $entryWarning->exists() + && in_array( $entryWarning->getKey(), self::$validErrorMessages ) + ) { + $this->mEntryErrorType = 'warning'; + $this->mEntryError = $entryWarning->rawParams( $loginreqlink )->escaped(); + } + if ( $wgEnableEmail ) { $this->mEmail = $request->getText( 'wpEmail' ); } else { @@ -182,6 +237,14 @@ class LoginForm extends SpecialPage { } $this->setHeaders(); + // In the case where the user is already logged in, do not show the login page. + // The use case scenario for this is when a user opens a large number of tabs, is + // redirected to the login page on all of them, and then logs in on one, expecting + // all the others to work properly. + if ( $this->mType !== 'signup' && !$this->mPosted && $this->getUser()->isLoggedIn() ) { + $this->successfulLogin(); + } + // If logging in and not on HTTPS, either redirect to it or offer a link. global $wgSecureLogin; if ( $this->mRequest->getProtocol() !== 'https' ) { @@ -191,6 +254,7 @@ class LoginForm extends SpecialPage { 'returntoquery' => $this->mReturnToQuery !== '' ? $this->mReturnToQuery : null, 'title' => null, + ( $this->mEntryErrorType === 'error' ? 'error' : 'warning' ) => $this->mEntryError, ) + $this->mRequest->getQueryValues(); $url = $title->getFullURL( $query, false, PROTO_HTTPS ); if ( $wgSecureLogin @@ -232,7 +296,7 @@ class LoginForm extends SpecialPage { return; } } - $this->mainLoginForm( '' ); + $this->mainLoginForm( $this->mEntryError, $this->mEntryErrorType ); } /** diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 0897006e45..4352f361f0 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -360,7 +360,7 @@ "invalidtitle-knownnamespace": "Invalid title with namespace \"$2\" and text \"$3\"", "invalidtitle-unknownnamespace": "Invalid title with unknown namespace number $1 and text \"$2\"", "exception-nologin": "Not logged in", - "exception-nologin-text": "Please [[Special:Userlogin|log in]] to be able to access this page or action.", + "exception-nologin-text": "Please log in to be able to access this page or action.", "exception-nologin-text-manual": "Please $1 to be able to access this page or action.", "virus-badscanner": "Bad configuration: Unknown virus scanner: $1", "virus-scanfailed": "scan failed (code $1)", @@ -915,7 +915,7 @@ "preferences-summary": "", "mypreferences": "Preferences", "prefs-edits": "Number of edits:", - "prefsnologintext2": "Please $1 to change your preferences.", + "prefsnologintext2": "Please login to change your preferences.", "prefs-skin": "Skin", "skin-preview": "Preview", "datedefault": "No preference", @@ -1812,7 +1812,7 @@ "mywatchlist": "Watchlist", "watchlistfor2": "For $1 $2", "nowatchlist": "You have no items on your watchlist.", - "watchlistanontext": "Please $1 to view or edit items on your watchlist.", + "watchlistanontext": "Please login to view or edit items on your watchlist.", "watchnologin": "Not logged in", "addwatch": "Add to watchlist", "addedwatchtext": "The page \"[[:$1]]\" has been added to your [[Special:Watchlist|watchlist]].\nFuture changes to this page and its associated talk page will be listed there.", diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index 0ec934958f..785587df13 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -1078,7 +1078,7 @@ "preferences-summary": "{{doc-specialpagesummary|preferences}}", "mypreferences": "Action link label that leads to [[Special:Preferences]]; appears in the top menu (e.g. \"Username Talk Preferences Watchlist Contributions Log out\").\n\nSee also:\n* {{msg-mw|Mypreferences}}\n* {{msg-mw|Accesskey-pt-preferences}}\n* {{msg-mw|Tooltip-pt-preferences}}\n{{Identical|Preferences}}", "prefs-edits": "In user preferences.", - "prefsnologintext2": "Parameters:\n* $1 - a link to [[Special:UserLogin]] with {{msg-mw|loginreqlink}} as link description", + "prefsnologintext2": "Showed on Special:Userlogin when user tries to access their preferences before logging in", "prefs-skin": "Used in user preferences.\n{{Identical|Skin}}", "skin-preview": "{{doc-actionlink}}\nThe link beside each skin name in [[Special:Preferences|your user preferences]], tab \"skin\".\n{{Identical|Preview}}", "datedefault": "Used as checkbox label in [[Special:Preferences#mw-prefsection-datetime|user preferences]], {{msg-mw|prefs-datetime}} tab.\n\nThis message indicates {{msg-mw|prefs-dateformat}} is default (= not specified).", @@ -1975,7 +1975,7 @@ "mywatchlist": "Link at the upper right corner of the screen.\n\nSee also:\n* {{msg-mw|Mywatchlist}}\n* {{msg-mw|Accesskey-pt-watchlist}}\n* {{msg-mw|Tooltip-pt-watchlist}}\n{{Identical|Watchlist}}", "watchlistfor2": "Subtitle on [[Special:Watchlist]].\nParameters:\n* $1 - Username of current user\n* $2 - Tool links (View relevant changes | View and edit watchlist | Edit raw watchlist)\n{{Identical|For $1}}", "nowatchlist": "Displayed when there is no pages in the watchlist.", - "watchlistanontext": "Parameters:\n* $1 - a link to [[Special:UserLogin]] with {{msg-mw|loginreqlink}} as link description", + "watchlistanontext": "Shown on Special:Userlogin when user tries to access their watchlist before logging in", "watchnologin": "Used as error page title.\n\nThe error message for this title is:\n* {{msg-mw|Watchnologintext}}\n{{Identical|Not logged in}}", "addwatch": "Link to a dialog box, displayed at the end of the list of categories at the foot of each page.\n\nSee also:\n* {{msg-mw|Removewatch}}", "addedwatchtext": "Explanation shown when clicking on the {{msg-mw|Watch}} tab. Parameters:\n* $1 - page title\nSee also:\n* {{msg-mw|Addedwatch}}", diff --git a/tests/phpunit/includes/SpecialPageTest.php b/tests/phpunit/includes/SpecialPageTest.php index 0ee335a609..245cdffdd4 100644 --- a/tests/phpunit/includes/SpecialPageTest.php +++ b/tests/phpunit/includes/SpecialPageTest.php @@ -70,32 +70,23 @@ class SpecialPageTest extends MediaWikiTestCase { $this->setExpectedException( 'UserNotLoggedIn', $expected ); - if ( $reason === 'blank' && $title === 'blank' ) { - $specialPage->requireLogin(); - } else { - $specialPage->requireLogin( $reason, $title ); - } + // $specialPage->requireLogin( [ $reason [, $title ] ] ) + call_user_func_array( + array( $specialPage, 'requireLogin' ), + array_filter( array( $reason, $title ) ) + ); } public function requireLoginAnonProvider() { $lang = 'en'; - $msg = wfMessage( 'loginreqlink' )->inLanguage( $lang )->escaped(); - $loginLink = '' . $msg . ''; - - $expected1 = wfMessage( 'exception-nologin-text-manual' ) - ->params( $loginLink )->inLanguage( $lang )->text(); - + $expected1 = wfMessage( 'exception-nologin-text' )->inLanguage( $lang )->text(); $expected2 = wfMessage( 'about' )->inLanguage( $lang )->text(); return array( array( $expected1, null, null ), array( $expected2, 'about', null ), - array( $expected2, wfMessage( 'about' ), null ), array( $expected2, 'about', 'about' ), - array( $expected2, 'about', wfMessage( 'about' ) ), - array( $expected1, 'blank', 'blank' ) ); } -- 2.20.1