Werdna has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/78467


Change subject: Flow: Add CSRF protection
......................................................................

Flow: Add CSRF protection

Change-Id: I15ae4ed95266ad0301fffa00eba6189cfa7dd91b
---
M includes/Block/Block.php
M includes/Block/Summary.php
M includes/Block/Topic.php
M includes/Block/TopicList.php
M includes/api/ApiFlow.php
M includes/api/ApiQueryFlow.php
M special/SpecialFlow.php
M templates/summary.html.php
M templates/topic.html.php
M templates/topiclist.html.php
10 files changed, 34 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow 
refs/changes/67/78467/1

diff --git a/includes/Block/Block.php b/includes/Block/Block.php
index 7975d6a..2053f4e 100644
--- a/includes/Block/Block.php
+++ b/includes/Block/Block.php
@@ -54,8 +54,9 @@
        abstract public function renderAPI( array $options );
        abstract public function commit();
 
-       public function init( $action ) {
+       public function init( $action, $user ) {
                $this->action = $action;
+               $this->user = $user;
        }
 
        public function onSubmit( $action, User $user, array $data  ) {
diff --git a/includes/Block/Summary.php b/includes/Block/Summary.php
index 8724616..9080b6e 100644
--- a/includes/Block/Summary.php
+++ b/includes/Block/Summary.php
@@ -16,8 +16,8 @@
        protected $needCreate = false;
        protected $supportedActions = array( 'edit-summary' );
 
-       public function init( $action ) {
-               parent::init( $action );
+       public function init( $action, $user ) {
+               parent::init( $action, $user );
                if ( $this->workflow->isNew() ) {
                        $this->needCreate = true;
                        return;
@@ -83,12 +83,14 @@
                        'block' => $this,
                        'workflow' => $this->workflow,
                        'summary' => $this->summary,
+                       'user' => $this->user,
                ) );
        }
 
        public function renderAPI( array $options ) {
                $output = array();
                $output['type'] = 'summary';
+
                if ( $this->summary !== null ) {
                        $output['*'] = $this->summary->getContent();
                        $output['summary-id'] = 
$this->summary->getRevisionId()->getHex();
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index b5e9e08..1266d03 100644
--- a/includes/Block/Topic.php
+++ b/includes/Block/Topic.php
@@ -218,6 +218,7 @@
                        'block' => $this,
                        'topic' => $this->workflow,
                        'root' => $this->loadRootPost(),
+                       'user' => $this->user,
                ), $return );
        }
 
diff --git a/includes/Block/TopicList.php b/includes/Block/TopicList.php
index e480297..db548de 100644
--- a/includes/Block/TopicList.php
+++ b/includes/Block/TopicList.php
@@ -61,6 +61,7 @@
                $templating->render( "flow:topiclist.html.php", array(
                        'topicList' => $this,
                        'topics' => $this->getTopics(),
+                       'user' => $this->user,
                ) );
        }
 
@@ -104,7 +105,7 @@
                foreach ( $this->storage->getMulti( 'Workflow', $topicIds ) as 
$workflow ) {
                        $hexId = $workflow->getId()->getHex();
                        $topics[$hexId] = new TopicBlock( $workflow, 
$this->storage, $roots[$hexId] );
-                       $topics[$hexId]->init( $this->action );
+                       $topics[$hexId]->init( $this->action, $this->user );
                }
 
                return $topics;
diff --git a/includes/api/ApiFlow.php b/includes/api/ApiFlow.php
index b872087..aab7c3f 100644
--- a/includes/api/ApiFlow.php
+++ b/includes/api/ApiFlow.php
@@ -28,7 +28,7 @@
                $user = $this->getContext()->getUser();
 
                foreach( $blocks as $block ) {
-                       $block->init( $action );
+                       $block->init( $action, $user );
                }
 
                $blocksToCommit = $this->loader->handleSubmit( $action, 
$blocks, $user, $request );
diff --git a/includes/api/ApiQueryFlow.php b/includes/api/ApiQueryFlow.php
index 5d7f2c5..5059fd9 100644
--- a/includes/api/ApiQueryFlow.php
+++ b/includes/api/ApiQueryFlow.php
@@ -25,7 +25,7 @@
                $blocks = $this->loader->createBlocks();
                $blockOutput = array();
                foreach( $blocks as $block ) {
-                       $block->init( $params['action'] );
+                       $block->init( $params['action'], 
$this->getContext()->getUser() );
 
                        $blockParams = array();
                        if ( isset($passedParams[$block->getName()]) ) {
diff --git a/special/SpecialFlow.php b/special/SpecialFlow.php
index fe17db2..90b74d2 100644
--- a/special/SpecialFlow.php
+++ b/special/SpecialFlow.php
@@ -37,6 +37,7 @@
                $title = $this->loadTitle( $subPage );
                $action = $request->getVal( 'action', 'view' );
                $workflowId = $request->getVal( 'workflow' );
+               $user = $this->getUser();
 
                $definitionRequest = $request->getVal( 'definition', null );
                if ( $definitionRequest !== null ) {
@@ -53,17 +54,23 @@
 
                $blocks = $this->loader->createBlocks();
                foreach ( $blocks as $block ) {
-                       $block->init( $action );
+                       $block->init( $action, $user );
                }
 
                if ( $request->getMethod() === 'POST' ) {
                        $user = $this->container['user'];
-                       $request = $this->getRequest();
-                       $blocksToCommit = $this->loader->handleSubmit( $action, 
$blocks, $user, $request );
-                       if ( $blocksToCommit ) {
-                               $this->loader->commit( $workflow, 
$blocksToCommit );
-                               $this->redirect( $workflow, 'view' );
-                               return;
+                       if ( $request->getVal('wpEditToken') != 
$user->getEditToken('flow') ) {
+                               $error = '<div class="error">' . 
wfMessage('sessionfailure') . '</div>';
+                               $this->getOutput()->addHTML( $error );
+                       } else {
+                               $user = $this->container['user'];
+                               $request = $this->getRequest();
+                               $blocksToCommit = $this->loader->handleSubmit( 
$action, $blocks, $user, $request );
+                               if ( $blocksToCommit ) {
+                                       $this->loader->commit( $workflow, 
$blocksToCommit );
+                                       $this->redirect( $workflow, 'view' );
+                                       return;
+                               }
                        }
                }
 
diff --git a/templates/summary.html.php b/templates/summary.html.php
index e03721e..536db6c 100644
--- a/templates/summary.html.php
+++ b/templates/summary.html.php
@@ -5,6 +5,8 @@
        'method' => 'POST',
        'action' => $this->generateUrl( $workflow, 'edit-summary' ),
 ) );
+$editToken = $user->getEditToken( 'flow' );
+echo Html::element( 'input', array( 'type' => 'hidden', 'name' => 
'wpEditToken', 'value' => $editToken) );
 if ( $block->hasErrors( 'prev_revision' ) ) {
        echo '<p>' . $block->getError( 'prev_revision' )->escaped() . '</p>';
 }
diff --git a/templates/topic.html.php b/templates/topic.html.php
index d5045b8..a0f83b1 100644
--- a/templates/topic.html.php
+++ b/templates/topic.html.php
@@ -5,13 +5,15 @@
 // or some such
 
 $self = $this;
-$postAction = function( $action, array $data = array() ) use( $self, $block, 
$root ) {
+$editToken = $user->getEditToken( 'flow' );
+$postAction = function( $action, array $data = array() ) use( $self, $block, 
$root, $editToken ) {
        // actions that change things must be post requests
        // also, CSRF?
        echo '<li>' . Html::openElement( 'form', array(
                'method' => 'POST',
                'action' => $self->generateUrl( $root->getPostId(), $action )
        ) );
+       echo Html::element( 'input', array( 'type' => 'hidden', 'name' => 
'wpEditToken', 'value' => $editToken) );
        foreach ( $data as $name => $value ) {
                echo Html::element( 'input', array(
                        'type' => 'hidden',
@@ -27,7 +29,7 @@
        ) ) . '</form></li>';
 };
 
-$renderPost = function( $post ) use( $self, $block, $root, $postAction, 
&$renderPost ) {
+$renderPost = function( $post ) use( $self, $block, $root, $postAction, 
&$renderPost, $editToken ) {
        echo '<div style="padding-left: 20px">';
        if ( $post->isFlagged( 'deleted' ) ) {
                echo wfMessage( 'flow-post-deleted' )
@@ -57,6 +59,7 @@
                                // root post id is same as topic workflow id
                                'action' => $self->generateUrl( 
$root->getPostId(), 'reply' ),
                        ) );
+                       echo Html::element( 'input', array( 'type' => 'hidden', 
'name' => 'wpEditToken', 'value' => $editToken) );
                if ( $block->getHexRepliedTo() === $post->getPostId()->getHex() 
) {
                        foreach ( $block->getErrors() as $error ) {
                                echo $error->text() . '<br>'; // the pain ...
diff --git a/templates/topiclist.html.php b/templates/topiclist.html.php
index 7fd3fbd..c720489 100644
--- a/templates/topiclist.html.php
+++ b/templates/topiclist.html.php
@@ -1,9 +1,11 @@
 <?php
 
+$editToken = $user->getEditToken( 'flow' );
 echo Html::openElement( 'form', array(
        'method' => 'POST',
        'action' => $this->generateUrl( $topicList->getWorkflow(), 'new-topic' )
 ) );
+echo Html::element( 'input', array( 'type' => 'hidden', 'name' => 
'wpEditToken', 'value' => $editToken) );
 
 echo '<h3>' . wfMessage( 'flow-topic-title' )->escaped() . '</h3>';
 if ( $topicList->hasErrors( 'topic' ) ) {

-- 
To view, visit https://gerrit.wikimedia.org/r/78467
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I15ae4ed95266ad0301fffa00eba6189cfa7dd91b
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Werdna <agarr...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to