WatchAction: Require POST for index.php action=watch

The GET variant was already rarely used because our frontend enchances these links
with a click handler that uses AJAX to make a POST request to the API.
The index.php url, nor its token, were used for the majority of users.

Simplify this by stripping the 'token' query from these urls and requiring a POST
request for index.php?action=watch and unwatch.

* FormAction: Actually set a proper '<form action>' instead of letting HTMLForm
  default to a confusing title path (e.g. /wiki/Pagename). Article path should
  not be used for POST requests.

* WatchAction: Group all FormAction-related methods together.

* WatchAction: Make token consistent with other actions now that it is POST-only
  (no "stronger" salt containing the page title).

* Remove ununsed mediawiki.page.startup dependency from mediawiki.page.watch.ajax.

* WatchAction: If accessed over GET directly (e.g. for users without javascript)
  display a confirmation form that submits the token. Similar to PurgeAction.

Change-Id: I504f457e68a133bcfc418cff13b838080fec1008

diff --git a/includes/actions/Action.php b/includes/actions/Action.php
index 43f03c3..6ddc596 100644 (file)
--- a/includes/actions/Action.php
+++ b/includes/actions/Action.php
@@ -370,7 +370,7 @@ abstract class Action {
         * Returns the description that goes below the \<h1\> tag
         * @since 1.17
         *
-        * @return string
+        * @return string HTML
         */
        protected function getDescription() {
                return $this->msg( strtolower( $this->getName() ) )->escaped();

diff --git a/includes/actions/FormAction.php b/includes/actions/FormAction.php
index 26f43cb..9decf90 100644 (file)
--- a/includes/actions/FormAction.php
+++ b/includes/actions/FormAction.php
@@ -68,8 +68,9 @@ abstract class FormAction extends Action {
                $form = new HTMLForm( $this->fields, $this->getContext(), $this->getName() );
                $form->setSubmitCallback( array( $this, 'onSubmit' ) );
 
+               $title = $this->getTitle();
+               $form->setAction( $title->getLocalURL( array( 'action' => $this->getName() ) ) );
                // Retain query parameters (uselang etc)
-               $form->addHiddenField( 'action', $this->getName() ); // Might not be the same as the query string
                $params = array_diff_key(
                        $this->getRequest()->getQueryValues(),
                        array( 'action' => null, 'title' => null )

diff --git a/includes/actions/WatchAction.php b/includes/actions/WatchAction.php
index 8b6e329..51108c0 100644 (file)
--- a/includes/actions/WatchAction.php
+++ b/includes/actions/WatchAction.php
@@ -35,6 +35,9 @@ class WatchAction extends FormAction {
                return false;
        }
 
+       /**
+        * @return string HTML
+        */
        protected function getDescription() {
                return $this->msg( 'addwatch' )->escaped();
        }
@@ -63,17 +66,9 @@ class WatchAction extends FormAction {
                // This will throw exceptions if there's a problem
                $this->checkCanExecute( $user );
 
-               // Must have valid token for this action/title
-               $salt = array( $this->getName(), $this->getTitle()->getPrefixedDBkey() );
-
-               if ( $user->matchEditToken( $this->getRequest()->getVal( 'token' ), $salt ) ) {
-                       $this->onSubmit( array() );
+               $form = $this->getForm();
+               if ( $form->show() ) {
                        $this->onSuccess();
-               } else {
-                       $form = $this->getForm();
-                       if ( $form->show() ) {
-                               $this->onSuccess();
-                       }
                }
        }
 
@@ -86,6 +81,21 @@ class WatchAction extends FormAction {
                parent::checkCanExecute( $user );
        }
 
+       protected function alterForm( HTMLForm $form ) {
+               $form->setSubmitTextMsg( 'confirm-watch-button' );
+               $form->setTokenSalt( 'watch' );
+       }
+
+       protected function preText() {
+               return $this->msg( 'confirm-watch-top' )->parse();
+       }
+
+       public function onSuccess() {
+               $this->getOutput()->addWikiMsg( 'addedwatchtext', $this->getTitle()->getPrefixedText() );
+       }
+
+       /* Static utility methods */
+
        /**
         * Watch or unwatch a page
         * @since 1.22
@@ -176,11 +186,8 @@ class WatchAction extends FormAction {
                if ( $action != 'unwatch' ) {
                        $action = 'watch';
                }
-               $salt = array( $action, $title->getPrefixedDBkey() );
-
-               // 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->getEditToken( $salt );
+               // Match ApiWatch and ResourceLoaderUserTokensModule
+               return $user->getEditToken( $action );
        }
 
        /**
@@ -195,16 +202,4 @@ class WatchAction extends FormAction {
        public static function getUnwatchToken( Title $title, User $user, $action = 'unwatch' ) {
                return self::getWatchToken( $title, $user, $action );
        }
-
-       protected function alterForm( HTMLForm $form ) {
-               $form->setSubmitTextMsg( 'confirm-watch-button' );
-       }
-
-       protected function preText() {
-               return $this->msg( 'confirm-watch-top' )->parse();
-       }
-
-       public function onSuccess() {
-               $this->getOutput()->addWikiMsg( 'addedwatchtext', $this->getTitle()->getPrefixedText() );
-       }
 }

diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php
index ae69934..4d7c03a 100644 (file)
--- a/includes/skins/SkinTemplate.php
+++ b/includes/skins/SkinTemplate.php
@@ -1019,12 +1019,11 @@ class SkinTemplate extends Skin {
                                         * the global versions.
                                         */
                                        $mode = $user->isWatched( $title ) ? 'unwatch' : 'watch';
-                                       $token = WatchAction::getWatchToken( $title, $user, $mode );
                                        $content_navigation['actions'][$mode] = array(
                                                'class' => $onPage && ( $action == 'watch' || $action == 'unwatch' ) ? 'selected' : false,
                                                // uses 'watch' or 'unwatch' message
                                                'text' => $this->msg( $mode )->text(),
-                                               'href' => $title->getLocalURL( array( 'action' => $mode, 'token' => $token ) )
+                                               'href' => $title->getLocalURL( array( 'action' => $mode ) )
                                        );
                                }
                        }

diff --git a/includes/specials/SpecialUnwatchedpages.php b/includes/specials/SpecialUnwatchedpages.php
index b3ca006..4ad6ac0 100644 (file)
--- a/includes/specials/SpecialUnwatchedpages.php
+++ b/includes/specials/SpecialUnwatchedpages.php
@@ -96,12 +96,11 @@ class UnwatchedpagesPage extends QueryPage {
                $text = $wgContLang->convert( $nt->getPrefixedText() );
 
                $plink = Linker::linkKnown( $nt, htmlspecialchars( $text ) );
-               $token = WatchAction::getWatchToken( $nt, $this->getUser() );
                $wlink = Linker::linkKnown(
                        $nt,
                        $this->msg( 'watch' )->escaped(),
                        array( 'class' => 'mw-watch-link' ),
-                       array( 'action' => 'watch', 'token' => $token )
+                       array( 'action' => 'watch' )
                );
 
                return $this->getLanguage()->specialList( $plink, $wlink );

diff --git a/resources/Resources.php b/resources/Resources.php
index 335775f..107ccec 100644 (file)
--- a/resources/Resources.php
+++ b/resources/Resources.php
@@ -1532,7 +1532,6 @@ return array(
                'dependencies' => array(
                        'mediawiki.api.watch',
                        'mediawiki.notify',
-                       'mediawiki.page.startup',
                        'mediawiki.util',
                        'jquery.accessKeyLabel',
                        'mediawiki.RegExp',