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