[ss-2016-002] Ensure Gridfield actions respect CSRF

silverstripe · Feb 18, 2016 · 56e92f5 · 56e92f5
1 parent 3398f67
commit 56e92f5
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 13 deletions.
diff --git a/forms/gridfield/GridField.php b/forms/gridfield/GridField.php
@@ -833,6 +833,18 @@ protected function buildColumnDispatch() {
 	 */
 	public function gridFieldAlterAction($data, $form, SS_HTTPRequest $request) {
 		$data = $request->requestVars();
+
+		// Protection against CSRF attacks
+		$token = $this
+			->getForm()
+			->getSecurityToken();
+		if(!$token->checkRequest($request)) {
+			$this->httpError(400, _t("Form.CSRF_FAILED_MESSAGE",
+				"There seems to have been a technical problem. Please click the back button, ".
+				"refresh your browser, and try again."
+			));
+		}
+
 		$name = $this->getName();
 
 		$fieldData = null;

diff --git a/tests/forms/gridfield/GridFieldDeleteActionTest.php b/tests/forms/gridfield/GridFieldDeleteActionTest.php
@@ -42,15 +42,54 @@ public function testShowDeleteButtonsWithAdminPermission() {
 		$this->assertEquals(3, count($deleteButtons), 'Delete buttons should show when logged in.');
 	}
 
+	public function testActionsRequireCSRF() {
+		$this->logInWithPermission('ADMIN');
+		$this->setExpectedException(
+			'SS_HTTPResponse_Exception',
+			_t("Form.CSRF_FAILED_MESSAGE",
+				"There seems to have been a technical problem. Please click the back button, ".
+				"refresh your browser, and try again."
+			),
+			400
+		);
+		$stateID = 'testGridStateActionField';
+		$request = new SS_HTTPRequest(
+			'POST',
+			'url',
+			array(),
+			array(
+				'action_gridFieldAlterAction?StateID='.$stateID,
+				'SecurityID' => null,
+			)
+		);
+		$this->gridField->gridFieldAlterAction(array('StateID'=>$stateID), $this->form, $request);
+	}
+
 	public function testDeleteActionWithoutCorrectPermission() {
 		if(Member::currentUser()) { Member::currentUser()->logOut(); }
 		$this->setExpectedException('ValidationException');
 
 		$stateID = 'testGridStateActionField';
-		Session::set($stateID, array('grid'=>'', 'actionName'=>'deleterecord',
-			'args'=>array('RecordID'=>$this->idFromFixture('GridFieldAction_Delete_Team', 'team1'))));
-		$request = new SS_HTTPRequest('POST', 'url', array(),
-			array('action_gridFieldAlterAction?StateID='.$stateID=>true));
+		Session::set(
+			$stateID,
+			array(
+				'grid' => '',
+				'actionName' => 'deleterecord',
+				'args' => array(
+					'RecordID' => $this->idFromFixture('GridFieldAction_Delete_Team', 'team1')
+				)
+			)
+		);
+		$token = SecurityToken::inst();
+		$request = new SS_HTTPRequest(
+			'POST',
+			'url',
+			array(),
+			array(
+				'action_gridFieldAlterAction?StateID='.$stateID => true,
+				$token->getName() => $token->getValue(),
+			)
+		);
 		$this->gridField->gridFieldAlterAction(array('StateID'=>$stateID), $this->form, $request);
 		$this->assertEquals(3, $this->list->count(),
 			'User should\'t be able to delete records without correct permissions.');
@@ -59,10 +98,26 @@ public function testDeleteActionWithoutCorrectPermission() {
 	public function testDeleteActionWithAdminPermission() {
 		$this->logInWithPermission('ADMIN');
 		$stateID = 'testGridStateActionField';
-		Session::set($stateID, array('grid'=>'', 'actionName'=>'deleterecord',
-			'args'=>array('RecordID'=>$this->idFromFixture('GridFieldAction_Delete_Team', 'team1'))));
-		$request = new SS_HTTPRequest('POST', 'url', array(),
-			array('action_gridFieldAlterAction?StateID='.$stateID=>true));
+		Session::set(
+			$stateID,
+			array(
+				'grid'=>'',
+				'actionName'=>'deleterecord',
+				'args' => array(
+					'RecordID' => $this->idFromFixture('GridFieldAction_Delete_Team', 'team1')
+				)
+			)
+		);
+		$token = SecurityToken::inst();
+		$request = new SS_HTTPRequest(
+			'POST',
+			'url',
+			array(),
+			array(
+				'action_gridFieldAlterAction?StateID='.$stateID=>true,
+				$token->getName() => $token->getValue(),
+			)
+		);
 		$this->gridField->gridFieldAlterAction(array('StateID'=>$stateID), $this->form, $request);
 		$this->assertEquals(2, $this->list->count(), 'User should be able to delete records with ADMIN permission.');
 	}
@@ -76,11 +131,26 @@ public function testDeleteActionRemoveRelation() {
 		$form = new Form(new Controller(), 'mockform', new FieldList(array($this->gridField)), new FieldList());
 
 		$stateID = 'testGridStateActionField';
-		Session::set($stateID, array('grid'=>'', 'actionName'=>'deleterecord',
-			'args'=>array('RecordID'=>$this->idFromFixture('GridFieldAction_Delete_Team', 'team1'))));
-		$request = new SS_HTTPRequest('POST', 'url', array(),
-			array('action_gridFieldAlterAction?StateID='.$stateID=>true));
-
+		Session::set(
+			$stateID,
+			array(
+				'grid'=>'',
+				'actionName'=>'deleterecord',
+				'args' => array(
+					'RecordID' => $this->idFromFixture('GridFieldAction_Delete_Team', 'team1')
+				)
+			)
+		);
+		$token = SecurityToken::inst();
+		$request = new SS_HTTPRequest(
+			'POST',
+			'url',
+			array(),
+			array(
+				'action_gridFieldAlterAction?StateID='.$stateID=>true,
+				$token->getName() => $token->getValue(),
+			)
+		);
 		$this->gridField->gridFieldAlterAction(array('StateID'=>$stateID), $this->form, $request);
 		$this->assertEquals(2, $this->list->count(), 'User should be able to delete records with ADMIN permission.');