From 2d03eedf8aa584ce3c3ec62373b618699831dbe3 Mon Sep 17 00:00:00 2001 From: Krinkle Date: Mon, 6 Jun 2011 00:09:03 +0000 Subject: [PATCH] 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 --- includes/Article.php | 10 +++-- includes/EditPage.php | 5 ++- includes/FileDeleteForm.php | 8 ++-- includes/ProtectionForm.php | 8 ++-- includes/SkinLegacy.php | 19 +++++--- includes/SkinTemplate.php | 3 +- includes/actions/DeleteAction.php | 6 +-- includes/actions/WatchAction.php | 75 +++++++++++++++++++++++++++---- includes/api/ApiBase.php | 8 ++-- includes/api/ApiWatch.php | 4 +- 10 files changed, 110 insertions(+), 36 deletions(-) diff --git a/includes/Article.php b/includes/Article.php index 073dd997bd..aef0e91caa 100644 --- 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 fad212cfc0..e979c61dbf 100644 --- 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 f16036b432..854005b0a1 100644 --- 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 2e305c3ee0..63f23a3ae5 100644 --- 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 81638915ca..29c4450b71 100644 --- 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 ac112b6097..b7b71bd183 100644 --- 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 f0a3a9bc8b..136a4eb6ea 100644 --- a/includes/actions/DeleteAction.php +++ b/includes/actions/DeleteAction.php @@ -1,6 +1,6 @@ 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 5dace86304..cce0b15e5b 100644 --- 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 25e2701fde..ce7fd484c1 100644 --- 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 bb38b4232e..8e76eeb182 100644 --- 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' ); -- 2.20.1