Make UserNotLoggedIn redirect to login page
authorTyler Romeo <tylerromeo@gmail.com>
Tue, 15 Jul 2014 18:48:09 +0000 (14:48 -0400)
committerTyler Romeo <tylerromeo@gmail.com>
Thu, 7 Aug 2014 17:38:16 +0000 (13:38 -0400)
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
includes/actions/WatchAction.php
includes/exception/UserNotLoggedIn.php
includes/specialpage/SpecialPage.php
includes/specials/SpecialUserlogin.php
languages/i18n/en.json
languages/i18n/qqq.json
tests/phpunit/includes/SpecialPageTest.php

index 92b34c6..ae2ab4f 100644 (file)
@@ -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
index a54b7c5..cc18d3d 100644 (file)
@@ -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 );
index 9d89009..03ba0b2 100644 (file)
  */
 
 /**
- * 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
  * }
  * @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();
+       }
 }
index 0070c74..df8d9de 100644 (file)
@@ -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 );
                }
        }
index 46b4999..bd7457e 100644 (file)
@@ -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 );
        }
 
        /**
index 0897006..4352f36 100644 (file)
        "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: <em>$1</em>",
        "virus-scanfailed": "scan failed (code $1)",
        "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",
        "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.",
index 0ec9349..785587d 100644 (file)
        "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).",
        "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}}",
index 0ee335a..245cdff 100644 (file)
@@ -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 = '<a href="/index.php?title=Special:UserLogin&amp;returnto=Special%3AWatchlist"'
-                       . ' title="Special:UserLogin">' . $msg . '</a>';
-
-               $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' )
                );
        }