Merge "Adding token validation to the edit handler"

5 files changed, 65 insertions, 25 deletions

diff --git a/includes/Rest/Handler/EditHandler.php b/includes/Rest/Handler/EditHandler.php
index 8eb14196b4a5..6f73f12ca8f7 100644
--- a/includes/Rest/Handler/EditHandler.php
+++ b/includes/Rest/Handler/EditHandler.php
@@ -10,6 +10,7 @@ use MediaWiki\Request\WebResponse;
 use MediaWiki\Rest\LocalizedHttpException;
 use MediaWiki\Rest\Response;
 use MediaWiki\Rest\TokenAwareHandlerTrait;
+use MediaWiki\Rest\Validator\Validator;
 use MediaWiki\Revision\RevisionLookup;
 use MediaWiki\Revision\SlotRecord;
 use MediaWiki\Title\TitleFormatter;
@@ -57,6 +58,14 @@ abstract class EditHandler extends ActionModuleBasedHandler {
 	/**
 	 * @inheritDoc
 	 */
+	public function validate( Validator $restValidator ) {
+			parent::validate( $restValidator );
+			$this->validateToken( true );
+	}
+
+	/**
+	 * @inheritDoc
+	 */
 	protected function mapActionModuleResult( array $data ) {
 		if ( isset( $data['error'] ) ) {
 			throw new LocalizedHttpException( new MessageValue( 'apierror-' . $data['error'] ), 400 );
diff --git a/tests/api-testing/REST/content.v1/Creation.js b/tests/api-testing/REST/content.v1/Creation.js
index 0cccbffd79d3..e592ed107e3d 100644
--- a/tests/api-testing/REST/content.v1/Creation.js
+++ b/tests/api-testing/REST/content.v1/Creation.js
@@ -151,7 +151,7 @@ describe( 'POST /page', () => {
 			const { status: editStatus, body: editBody, header: editHeader } =
 				await client.post( '/page', reqBody );
 
-			assert.equal( editStatus, 400 );
+			assert.equal( editStatus, 403 );
 			assert.match( editHeader[ 'content-type' ], /^application\/json/ );
 			assert.nestedProperty( editBody, 'messageTranslations' );
 		} );
diff --git a/tests/api-testing/REST/content.v1/Update.js b/tests/api-testing/REST/content.v1/Update.js
index 6f4eba1eea8e..c7e24880746b 100644
--- a/tests/api-testing/REST/content.v1/Update.js
+++ b/tests/api-testing/REST/content.v1/Update.js
@@ -203,7 +203,7 @@ describe( 'PUT /page/{title}', () => {
 			const { status: editStatus, body: editBody, header: editHeader } =
 				await client.put( `/page/${ title }`, reqBody );
 
-			assert.equal( editStatus, 400 );
+			assert.equal( editStatus, 403 );
 			assert.match( editHeader[ 'content-type' ], /^application\/json/ );
 			assert.nestedProperty( editBody, 'messageTranslations' );
 		} );
@@ -247,7 +247,8 @@ describe( 'PUT /page/{title}', () => {
 			const reqBody = {
 				source: 'Lörem Ipsüm',
 				comment: 'tästing',
-				content_model: 'wikitext'
+				content_model: 'wikitext',
+				token: mindyToken
 			};
 			const { status: editStatus, body: editBody, header: editHeader } =
 				await client.put( `/page/${ title }`, reqBody );
@@ -261,7 +262,8 @@ describe( 'PUT /page/{title}', () => {
 			const reqBody = {
 				source: 'Lörem Ipsüm',
 				comment: 'tästing',
-				content_model: 'wikitext'
+				content_model: 'wikitext',
+				token: mindyToken
 			};
 			const { status: editStatus, body: editBody, header: editHeader } =
 				await client.put( '/page/', reqBody );
diff --git a/tests/phpunit/integration/includes/Rest/Handler/CreationHandlerTest.php b/tests/phpunit/integration/includes/Rest/Handler/CreationHandlerTest.php
index ca6373ddf85a..6cb3ba46c280 100644
--- a/tests/phpunit/integration/includes/Rest/Handler/CreationHandlerTest.php
+++ b/tests/phpunit/integration/includes/Rest/Handler/CreationHandlerTest.php
@@ -13,6 +13,7 @@ use MediaWiki\Rest\RequestData;
 use MediaWiki\Revision\MutableRevisionRecord;
 use MediaWiki\Revision\RevisionLookup;
 use MediaWiki\Revision\SlotRecord;
+use MediaWiki\Session\Token;
 use MediaWiki\Status\Status;
 use MediaWiki\Tests\Unit\DummyServicesTrait;
 use MediaWikiIntegrationTestCase;
@@ -81,6 +82,7 @@ class CreationHandlerTest extends MediaWikiIntegrationTestCase {
 	public static function provideExecute() {
 		// NOTE: Prefix hard coded in a fake for Router::getRouteUrl() in HandlerTestTrait
 		$baseUrl = 'https://wiki.example.com/rest/v1/page/';
+		$token = strval( new Token( 'TOKEN', '' ) );
 
 		yield "create with token" => [
 			[ // Request data received by CreationHandler
@@ -89,7 +91,7 @@ class CreationHandlerTest extends MediaWikiIntegrationTestCase {
 					'Content-Type' => 'application/json',
 				],
 				'bodyContents' => json_encode( [
-					'token' => 'TOKEN',
+					'token' => $token,
 					'title' => 'Foo',
 					'source' => 'Lorem Ipsum',
 					'comment' => 'Testing'
@@ -129,7 +131,8 @@ class CreationHandlerTest extends MediaWikiIntegrationTestCase {
 				'source' => 'Content of revision 371707'
 			],
 			$baseUrl . 'Foo',
-			false
+			false,
+			true,
 		];
 
 		yield "create with model" => [
@@ -181,7 +184,8 @@ class CreationHandlerTest extends MediaWikiIntegrationTestCase {
 				'source' => 'Content of revision 371707'
 			],
 			$baseUrl . 'Talk:Foo',
-			true
+			true,
+			false,
 		];
 
 		yield "create without token" => [
@@ -233,7 +237,8 @@ class CreationHandlerTest extends MediaWikiIntegrationTestCase {
 				'source' => 'Content of revision 371707'
 			],
 			$baseUrl . 'Foo%2Fbar',
-			true
+			true,
+			false,
 		];
 
 		yield "create with space" => [
@@ -283,7 +288,8 @@ class CreationHandlerTest extends MediaWikiIntegrationTestCase {
 				'source' => 'Content of revision 371707'
 			],
 			$baseUrl . 'Foo_(ba%2Br)',
-			true
+			true,
+			false
 		];
 	}
 
@@ -296,13 +302,20 @@ class CreationHandlerTest extends MediaWikiIntegrationTestCase {
 		$actionResult,
 		$expectedResponse,
 		$expectedRedirect,
-		$csrfSafe
+		$csrfSafe,
+		$hasToken
 	) {
 		$request = new RequestData( $requestData );
 
 		$handler = $this->newHandler( $actionResult, null, $csrfSafe );
 
-		$response = $this->executeHandler( $handler, $request, [], [], [], [], null, $this->getSession( $csrfSafe ) );
+		$session = $this->getSession( $csrfSafe );
+
+		$session->method( 'hasToken' )->willReturn( $hasToken );
+
+		$session->method( 'getToken' )->willReturn( new Token( 'TOKEN', '' ) );
+
+		$response = $this->executeHandler( $handler, $request, [], [], [], [], null, $session );
 
 		$this->assertSame( 201, $response->getStatusCode() );
 		$this->assertSame(
diff --git a/tests/phpunit/integration/includes/Rest/Handler/UpdateHandlerTest.php b/tests/phpunit/integration/includes/Rest/Handler/UpdateHandlerTest.php
index e224928e0de9..cc0ed35fbbb9 100644
--- a/tests/phpunit/integration/includes/Rest/Handler/UpdateHandlerTest.php
+++ b/tests/phpunit/integration/includes/Rest/Handler/UpdateHandlerTest.php
@@ -20,6 +20,7 @@ use MediaWiki\Rest\RequestData;
 use MediaWiki\Revision\MutableRevisionRecord;
 use MediaWiki\Revision\RevisionLookup;
 use MediaWiki\Revision\SlotRecord;
+use MediaWiki\Session\Token;
 use MediaWiki\Status\Status;
 use MediaWiki\Tests\Unit\DummyServicesTrait;
 use MediaWiki\Title\Title;
@@ -111,6 +112,8 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 	}
 
 	public static function provideExecute() {
+		$token = strval( new Token( 'TOKEN', '' ) );
+
 		yield "create with token" => [
 			[ // Request data received by UpdateHandler
 				'method' => 'PUT',
@@ -119,7 +122,7 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 					'Content-Type' => 'application/json',
 				],
 				'bodyContents' => json_encode( [
-					'token' => 'TOKEN',
+					'token' => $token,
 					'source' => 'Lorem Ipsum',
 					'comment' => 'Testing'
 				] ),
@@ -129,7 +132,7 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 				'text' => 'Lorem Ipsum',
 				'summary' => 'Testing',
 				'createonly' => '1',
-				'token' => 'TOKEN',
+				'token' => $token,
 			],
 			[ // Mock response returned by ApiEditPage
 				"edit" => [
@@ -158,7 +161,8 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 				],
 				'source' => 'Content of revision 371707'
 			],
-			false
+			false,
+			true,
 		];
 
 		yield "create with model" => [
@@ -169,7 +173,7 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 					'Content-Type' => 'application/json',
 				],
 				'bodyContents' => json_encode( [
-					'token' => 'TOKEN',
+					'token' => $token,
 					'source' => 'Lorem Ipsum',
 					'comment' => 'Testing',
 					'content_model' => CONTENT_MODEL_WIKITEXT,
@@ -181,7 +185,7 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 				'summary' => 'Testing',
 				'contentmodel' => 'wikitext',
 				'createonly' => '1',
-				'token' => 'TOKEN',
+				'token' => $token,
 			],
 			[ // Mock response returned by ApiEditPage
 				"edit" => [
@@ -210,7 +214,8 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 				],
 				'source' => 'Content of revision 371707'
 			],
-			false
+			false,
+			true,
 		];
 
 		yield "update with token" => [
@@ -221,7 +226,7 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 					'Content-Type' => 'application/json',
 				],
 				'bodyContents' => json_encode( [
-					'token' => 'TOKEN',
+					'token' => $token,
 					'source' => 'Lorem Ipsum',
 					'comment' => 'Testing',
 					'latest' => [ 'id' => 789123 ],
@@ -233,7 +238,7 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 				'summary' => 'Testing',
 				'nocreate' => '1',
 				'baserevid' => '789123',
-				'token' => 'TOKEN',
+				'token' => $token,
 			],
 			[ // Mock response returned by ApiEditPage
 				"edit" => [
@@ -261,7 +266,8 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 				],
 				'source' => 'Content of revision 371707'
 			],
-			false
+			false,
+			true,
 		];
 
 		yield "update with model" => [
@@ -313,7 +319,8 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 				],
 				'source' => 'Content of revision 371707'
 			],
-			true
+			true,
+			false,
 		];
 
 		yield "update without token" => [
@@ -362,7 +369,8 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 					'title' => 'CC-BY-SA 4.0'
 				],
 			],
-			true
+			true,
+			false,
 		];
 
 		yield "null-edit (unchanged)" => [
@@ -409,7 +417,8 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 					'title' => 'CC-BY-SA 4.0'
 				],
 			],
-			true
+			true,
+			false,
 		];
 	}
 
@@ -421,14 +430,21 @@ class UpdateHandlerTest extends MediaWikiLangTestCase {
 		$expectedActionParams,
 		$actionResult,
 		$expectedResponse,
-		$csrfSafe
+		$csrfSafe,
+		$hasToken
 	) {
 		$request = new RequestData( $requestData );
 
 		$handler = $this->newHandler( $actionResult, null, $csrfSafe );
 
+		$session = $this->getSession( $csrfSafe );
+
+		$session->method( 'hasToken' )->willReturn( $hasToken );
+
+		$session->method( 'getToken' )->willReturn( new Token( 'TOKEN', '' ) );
+
 		$responseData = $this->executeHandlerAndGetBodyData(
-			$handler, $request, [], [], [], [], null, $this->getSession( $csrfSafe )
+			$handler, $request, [], [], [], [], null, $session
 		);
 
 		// Check parameters passed to ApiEditPage by UpdateHandler based on $requestData