WatchAction requires token (BREAKING CHANGE)

* (bug 27655) Require token for watching/unwatching pages
* Previously done for API (bug 29070) in r88522
* As with markpatrolled, the tokens are not compatible and made that way on purpose. The API requires the POST method and uses a universal token per-session. Since the front-end is all GET based (also per convention like in markpatrolled and rollback) they are stronger salted (title / action specific)
* ajax.watch used the API already and was switched in r88554.
* The actual watching/unwatching code was moved from WatchAction->onView to WatchAction::doWatch. This was done to allow the API to do the action without needing to generate a token like the front-end needs (or having to duplicate code). It is now similar to RecentChange::markPatrolled (in that it  also a "central" function that does not care about tokens, it's called after the token-handling)
* JavaScript / Gadgets that utilize action=watch in their scripts:
** Effects should be minimal as they should be using the API (see r88522 and wikitech-l)
** If they use index.php and scrap the link from the page, they can continue to do so.

* There are links to the watch action all over the place. I've tried to catch most of them, but there may be some I miss. Migration in most cases is just a matter of adding an array item to the $query for:
 'token' => WatchAction::getWatchToken( $title, $user [, $action] )
or changing:
 Action::factory( 'watch', $article )->execute();
to:
 WatchAction::doWatch( $title, $user );

While replacing the usages in some cases an instance of Article() no longer had to be created, in others $wgUser had to be retrieved from global (which was implied before but needs to be given directly now)

Other notes:
* Article->unwatch() and Article->watch(), which were deprecated as of 1.18 and are no longer used in core, may be broken in scenarios where the Request does not have a 'token' but is making a call to $article->watch()
* Some extensions need to be fixed, I'm currently running a grep search and will fix them a.s.a.p

[1] http://www.mediawiki.org/wiki/ResourceLoader/Default_modules?mw.user#tokens

diff --git a/includes/Article.php b/includes/Article.php
index 073dd99..aef0e91 100644 (file)
--- a/includes/Article.php
+++ b/includes/Article.php
@@ -2382,7 +2382,8 @@ class Article {
        }
 
        /**
-        * User-interface handler for the "watch" action
+        * User-interface handler for the "watch" action.
+        * Requires Request to pass a token as of 1.19.
         * @deprecated since 1.18
         */
        public function watch() {
@@ -2398,11 +2399,13 @@ class Article {
         * @deprecated since 1.18
         */
        public function doWatch() {
-               return Action::factory( 'watch', $this )->execute();
+               global $wgUser;
+               return WatchAction:doWatch( $this->mTitle, $wgUser );
        }
 
        /**
         * User interface handler for the "unwatch" action.
+        * Requires Request to pass a token as of 1.19.
         * @deprecated since 1.18
         */
        public function unwatch() {
@@ -2415,7 +2418,8 @@ class Article {
         * @deprecated since 1.18
         */
        public function doUnwatch() {
-               return Action::factory( 'unwatch', $this )->execute();
+               global $wgUser;
+               return WatchAction:doUnwatch( $this->mTitle, $wgUser );
        }
 
        /**

diff --git a/includes/EditPage.php b/includes/EditPage.php
index fad212c..e979c61 100644 (file)
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -1176,13 +1176,14 @@ class EditPage {
         * Commit the change of watch status
         */
        protected function commitWatch() {
+               global $wgUser;
                if ( $this->watchthis xor $this->mTitle->userIsWatching() ) {
                        $dbw = wfGetDB( DB_MASTER );
                        $dbw->begin();
                        if ( $this->watchthis ) {
-                               Action::factory( 'watch', $this->mArticle )->execute();
+                               WatchAction::doWatch( $this->mTitle, $wgUser );
                        } else {
-                               Action::factory( 'unwatch', $this->mArticle )->execute();
+                               WatchAction::doUnwatch( $this->mTitle, $wgUser );
                        }
                        $dbw->commit();
                }

diff --git a/includes/FileDeleteForm.php b/includes/FileDeleteForm.php
index f16036b..854005b 100644 (file)
--- a/includes/FileDeleteForm.php
+++ b/includes/FileDeleteForm.php
@@ -123,10 +123,10 @@ class FileDeleteForm {
                                // delete the associated article first
                                if( $article->doDeleteArticle( $reason, $suppress, $id, false ) ) {
                                        global $wgRequest;
-                                       if( $wgRequest->getCheck( 'wpWatch' ) && $wgUser->isLoggedIn() ) {
-                                               Action::factory( 'watch', $article )->execute();
-                                       } elseif( $title->userIsWatching() ) {
-                                               Action::factory( 'unwatch', $article )->execute();
+                                       if ( $wgRequest->getCheck( 'wpWatch' ) && $wgUser->isLoggedIn() ) {
+                                               WatchAction::doWatch( $title, $wgUser );
+                                       } elseif ( $title->userIsWatching() ) {
+                                               WatchAction::doUnwatch( $title, $wgUser );
                                        }
                                        $status = $file->delete( $reason, $suppress );
                                        if( $status->ok ) {

diff --git a/includes/ProtectionForm.php b/includes/ProtectionForm.php
index 2e305c3..63f23a3 100644 (file)
--- a/includes/ProtectionForm.php
+++ b/includes/ProtectionForm.php
@@ -318,10 +318,10 @@ class ProtectionForm {
                        return false;
                }
 
-               if( $wgRequest->getCheck( 'mwProtectWatch' ) && $wgUser->isLoggedIn() ) {
-                       Action::factory( 'watch', $this->mArticle )->execute();
-               } elseif( $this->mTitle->userIsWatching() ) {
-                       Action::factory( 'unwatch', $this->mArticle )->execute();
+               if ( $wgRequest->getCheck( 'mwProtectWatch' ) && $wgUser->isLoggedIn() ) {
+                       WatchAction::doWatch( $this->mTitle, $wgUser );
+               } elseif ( $this->mTitle->userIsWatching() ) {
+                       WatchAction::doUnwatch( $this->mTitle, $wgUser );
                }
                return $ok;
        }

diff --git a/includes/SkinLegacy.php b/includes/SkinLegacy.php
index 8163891..29c4450 100644 (file)
--- a/includes/SkinLegacy.php
+++ b/includes/SkinLegacy.php
@@ -672,22 +672,31 @@ class LegacyTemplate extends BaseTemplate {
        }
 
        function watchThisPage() {
-               global $wgOut;
+               global $wgOut, $wgUser;
                ++$this->mWatchLinkNum;
 
+               // Cache
+               $title = $this->getSkin()->getTitle();
+
                if ( $wgOut->isArticleRelated() ) {
-                       if ( $this->getSkin()->getTitle()->userIsWatching() ) {
+                       if ( $title->userIsWatching() ) {
                                $text = wfMsg( 'unwatchthispage' );
-                               $query = array( 'action' => 'unwatch' );
+                               $query = array(
+                                       'action' => 'unwatch',
+                                       'token' => UnwatchAction::getUnwatchToken( $title, $wgUser ),
+                               );
                                $id = 'mw-unwatch-link' . $this->mWatchLinkNum;
                        } else {
                                $text = wfMsg( 'watchthispage' );
-                               $query = array( 'action' => 'watch' );
+                               $query = array(
+                                       'action' => 'watch',
+                                       'token' => WatchAction::getWatchToken( $title, $wgUser ),
+                               );
                                $id = 'mw-watch-link' . $this->mWatchLinkNum;
                        }
 
                        $s = $this->getSkin()->link(
-                               $this->getSkin()->getTitle(),
+                               $title,
                                $text,
                                array( 'id' => $id ),
                                $query,

diff --git a/includes/SkinTemplate.php b/includes/SkinTemplate.php
index ac112b6..b7b71bd 100644 (file)
--- a/includes/SkinTemplate.php
+++ b/includes/SkinTemplate.php
@@ -1015,10 +1015,11 @@ class SkinTemplate extends Skin {
                                 * the global versions.
                                 */
                                $mode = $title->userIsWatching() ? 'unwatch' : 'watch';
+                               $token = WatchAction::getWatchToken( $title, $wgUser, $mode );
                                $content_navigation['actions'][$mode] = array(
                                        'class' => $onPage && ( $action == 'watch' || $action == 'unwatch' ) ? 'selected' : false,
                                        'text' => wfMsg( $mode ), // uses 'watch' or 'unwatch' message
-                                       'href' => $title->getLocalURL( 'action=' . $mode )
+                                       'href' => $title->getLocalURL( array( 'action' => $mode, 'token' => $token ) )
                                );
                        }
 

diff --git a/includes/actions/DeleteAction.php b/includes/actions/DeleteAction.php
index f0a3a9b..136a4eb 100644 (file)
--- a/includes/actions/DeleteAction.php
+++ b/includes/actions/DeleteAction.php
@@ -1,6 +1,6 @@
 <?php
 /**
- * Performs the watch and unwatch actions on a page
+ * Performs the delete action on a page
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -196,9 +196,9 @@ class DeleteAction extends Action {
        public function onSuccess(){
                // Watch or unwatch, if requested
                if( $this->getRequest()->getCheck( 'wpWatch' ) && $this->getUser()->isLoggedIn() ) {
-                       Action::factory( 'watch', $this->page )->execute();
+                       WatchAction::doWatch( $this->getTitle(), $this->getUser() );
                } elseif ( $this->getTitle()->userIsWatching() ) {
-                       Action::factory( 'unwatch', $this->page )->execute();
+                       WatchAction::doUnwatch( $this->getTitle(), $this->getUser() );
                }
 
                $this->getOutput()->setPagetitle( wfMsg( 'actioncomplete' ) );

diff --git a/includes/actions/WatchAction.php b/includes/actions/WatchAction.php
index 5dace86..cce0b15 100644 (file)
--- a/includes/actions/WatchAction.php
+++ b/includes/actions/WatchAction.php
@@ -39,9 +39,20 @@ class WatchAction extends FormlessAction {
        }
 
        protected function checkCanExecute( User $user ) {
+
+               // Must be logged in
                if ( $user->isAnon() ) {
                        throw new ErrorPageError( 'watchnologin', 'watchnologintext' );
                }
+
+               // Must have valid token for this action/title
+               $salt = array( $this->getName(), $this->getTitle()->getDBkey() );
+
+               if ( !$user->matchEditToken( $this->getRequest()->getVal( 'token' ), $salt ) ) {
+                       throw new ErrorPageError( 'sessionfailure-title', 'sessionfailure' );
+                       return;
+               }
+               
                return parent::checkCanExecute( $user );
        }
 
@@ -49,15 +60,66 @@ class WatchAction extends FormlessAction {
                wfProfileIn( __METHOD__ );
 
                $user = $this->getUser();
-               if ( wfRunHooks( 'WatchArticle', array( &$user, &$this->page ) ) ) {
-                       $this->getUser()->addWatch( $this->getTitle() );
-                       wfRunHooks( 'WatchArticleComplete', array( &$user, &$this->page ) );
-               }
+               self::doWatch( $this->getTitle(), $user );
 
                wfProfileOut( __METHOD__ );
 
                return wfMessage( 'addedwatchtext', $this->getTitle()->getPrefixedText() )->parse();
        }
+
+       public static function doWatch( Title $title, User $user  ) {
+               $page = new Article( $title );
+
+               if ( wfRunHooks( 'WatchArticle', array( &$user, &$page ) ) ) {
+                       $user->addWatch( $title );
+                       wfRunHooks( 'WatchArticleComplete', array( &$user, &$page ) );
+               }
+               return true;
+       }
+
+       public static function doUnwatch( Title $title, User $user  ) {
+               $page = new Article( $title );
+
+               if ( wfRunHooks( 'UnwatchArticle', array( &$user, &$page ) ) ) {
+                       $user->removeWatch( $title );
+                       wfRunHooks( 'UnwatchArticleComplete', array( &$user, &$page ) );
+               }
+               return true;
+       }
+
+       /**
+        * Get token to watch (or unwatch) a page for a user
+        *
+        * @param Title $title Title object of page to watch
+        * @param User $title User for whom the action is going to be performed
+        * @param string $action Optionally override the action to 'unwatch'
+        * @return string Token
+        * @since 1.19
+        */
+       public static function getWatchToken( Title $title, User $user, $action = 'watch' ) {
+               if ( $action != 'unwatch' ) {
+                       $action = 'watch';
+               }
+               $salt = array( $action, $title->getDBkey() );
+
+               // This token stronger salted and not compatible with ApiWatch
+               // It's title/action specific because index.php is GET and API is POST
+               return $user->editToken( $salt );
+       }
+
+       /**
+        * Get token to unwatch (or watch) a page for a user
+        *
+        * @param Title $title Title object of page to unwatch
+        * @param User $title User for whom the action is going to be performed
+        * @param string $action Optionally override the action to 'watch'
+        * @return string Token
+        * @since 1.19
+        */
+       public static function getUnwatchToken( Title $title, User $user, $action = 'unwatch' ) {
+               return self::getWatchToken( $title, $user, $action );
+       }
+
 }
 
 class UnwatchAction extends WatchAction {
@@ -74,10 +136,7 @@ class UnwatchAction extends WatchAction {
                wfProfileIn( __METHOD__ );
 
                $user = $this->getUser();
-               if ( wfRunHooks( 'UnwatchArticle', array( &$user, &$this->page ) ) ) {
-                       $this->getUser()->removeWatch( $this->getTitle() );
-                       wfRunHooks( 'UnwatchArticleComplete', array( &$user, &$this->page ) );
-               }
+               self::doUnwatch( $this->getTitle(), $user );
 
                wfProfileOut( __METHOD__ );
 

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 25e2701..ce7fd48 100644 (file)
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -645,17 +645,17 @@ abstract class ApiBase {
         * @param $titleObj Title the article's title to change
         * @param $userOption String The user option to consider when $watch=preferences
         */
-       protected function setWatch ( $watch, $titleObj, $userOption = null ) {
+       protected function setWatch( $watch, $titleObj, $userOption = null ) {
                $value = $this->getWatchlistValue( $watch, $titleObj, $userOption );
                if ( $value === null ) {
                        return;
                }
 
-               $articleObj = new Article( $titleObj );
+               global $wgUser;
                if ( $value ) {
-                       Action::factory( 'watch', $articleObj )->execute();
+                       WatchAction::doWatch( $titleObj, $wgUser );
                } else {
-                       Action::factory( 'unwatch', $articleObj )->execute();
+                       WatchAction::doUnwatch( $titleObj, $wgUser );
                }
        }
 

diff --git a/includes/api/ApiWatch.php b/includes/api/ApiWatch.php
index bb38b42..8e76eeb 100644 (file)
--- a/includes/api/ApiWatch.php
+++ b/includes/api/ApiWatch.php
@@ -59,11 +59,11 @@ class ApiWatch extends ApiBase {
                if ( $params['unwatch'] ) {
                        $res['unwatched'] = '';
                        $res['message'] = wfMsgExt( 'removedwatchtext', array( 'parse' ), $title->getPrefixedText() );
-                       $success = Action::factory( 'unwatch', $article )->execute();
+                       $success = WatchAction::doWatch( $title, $wgUser );
                } else {
                        $res['watched'] = '';
                        $res['message'] = wfMsgExt( 'addedwatchtext', array( 'parse' ), $title->getPrefixedText() );
-                       $success = Action::factory( 'watch', $article )->execute();
+                       $success = UnwatchAction::doUnwatch( $title, $wgUser );
                }
                if ( !$success ) {
                        $this->dieUsageMsg( 'hookaborted' );