From bc3df654bd83e30fb7b4bcff285b408138058b03 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Mon, 1 Nov 2010 01:29:02 +0000 Subject: [PATCH] API CHANGE Fixed various controllers to enforce CSRF protection through Form_SecurityToken on GET actions that are not routed through Form->httpSubmission(): AssetAdmin, CMSBatchActionHandler, CMSMain, CommentTableField, LeftAndMain, MemberTableField, PageComment, PageComment_Controller git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/cms/branches/2.4@113282 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- code/AssetAdmin.php | 17 ++++-- code/CMSBatchActionHandler.php | 3 + code/CMSMain.php | 43 ++++++++++---- code/CommentTableField.php | 50 ++++++++++++++++ code/LeftAndMain.php | 15 ++++- code/MemberTableField.php | 19 ++++++- code/SecurityAdmin.php | 2 +- code/sitefeatures/PageComment.php | 61 ++++++++++++++------ javascript/AssetAdmin.js | 3 +- javascript/LeftAndMain_left.js | 14 +++-- javascript/TranslationTab.js | 1 + templates/Includes/AssetAdmin_left.ss | 1 + templates/Includes/AssetTableField.ss | 2 +- templates/Includes/CMSMain_left.ss | 1 + templates/Includes/SecurityAdmin_left.ss | 2 + tests/CMSMainTest.php | 26 +++++++-- tests/CommentAdminTest.php | 2 +- tests/MemberTableFieldTest.php | 72 ++++++++++++++++++------ tests/MemberTableFieldTest.yml | 4 ++ tests/PageCommentsTest.php | 21 ++++--- 20 files changed, 285 insertions(+), 74 deletions(-) diff --git a/code/AssetAdmin.php b/code/AssetAdmin.php index accdac9e..1437addf 100755 --- a/code/AssetAdmin.php +++ b/code/AssetAdmin.php @@ -558,7 +558,10 @@ JS; /** * Add a new folder and return its details suitable for ajax. */ - public function addfolder() { + public function addfolder($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $parent = ($_REQUEST['ParentID'] && is_numeric($_REQUEST['ParentID'])) ? (int)$_REQUEST['ParentID'] : 0; $name = (isset($_REQUEST['Name'])) ? basename($_REQUEST['Name']) : _t('AssetAdmin.NEWFOLDER',"NewFolder"); @@ -626,7 +629,7 @@ JS; /** * Delete a folder */ - public function deletefolder() { + public function deletefolder($data, $ofmr) { $ids = split(' *, *', $_REQUEST['csvIDs']); if(!$ids) return false; @@ -655,7 +658,10 @@ JS; return $script; } - public function removefile(){ + public function removefile($request){ + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + if($fileID = $this->urlParams['ID']) { $file = DataObject::get_by_id('File', $fileID); // Delete the temp verions of this file in assets/_resampled @@ -700,7 +706,10 @@ JS; * Removes all unused thumbnails from the file store * and returns the status of the process to the user. */ - public function deleteunusedthumbnails() { + public function deleteunusedthumbnails($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $count = 0; $thumbnails = $this->getUnusedThumbnails(); diff --git a/code/CMSBatchActionHandler.php b/code/CMSBatchActionHandler.php index ff53a0c5..5682b62b 100644 --- a/code/CMSBatchActionHandler.php +++ b/code/CMSBatchActionHandler.php @@ -53,6 +53,9 @@ class CMSBatchActionHandler extends RequestHandler { Director::redirectBack(); return; } + + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); $actions = Object::get_static($this->class, 'batch_actions'); $actionClass = $actions[$request->param('BatchAction')]; diff --git a/code/CMSMain.php b/code/CMSMain.php index fd98974a..d00145d2 100755 --- a/code/CMSMain.php +++ b/code/CMSMain.php @@ -532,7 +532,10 @@ JS; // Data saving handlers - public function addpage() { + public function addpage($data, $form) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($this->request)) return $this->httpError(400); + $className = isset($_REQUEST['PageType']) ? $_REQUEST['PageType'] : "Page"; $parent = isset($_REQUEST['ParentID']) ? $_REQUEST['ParentID'] : 0; $suffix = isset($_REQUEST['Suffix']) ? "-" . $_REQUEST['Suffix'] : null; @@ -543,6 +546,7 @@ JS; } if(is_numeric($parent)) $parentObj = DataObject::get_by_id("SiteTree", $parent); + else $parentObj = null; if(!$parentObj || !$parentObj->ID) $parent = 0; if($parentObj){ @@ -803,7 +807,7 @@ JS; /** * Roll a page back to a previous version */ - function rollback() { + function rollback($data, $form) { if(isset($_REQUEST['Version']) && (bool)$_REQUEST['Version']) { $this->extend('onBeforeRollback', $_REQUEST['ID']); $record = $this->performRollback($_REQUEST['ID'], $_REQUEST['Version']); @@ -814,7 +818,7 @@ JS; } } - function unpublish() { + function unpublish($data, $form) { $SQL_id = Convert::raw2sql($_REQUEST['ID']); $page = DataObject::get_by_id("SiteTree", $SQL_id); @@ -1147,7 +1151,10 @@ JS; return $form; } - function buildbrokenlinks() { + function buildbrokenlinks($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + if($this->urlParams['ID']) { $newPageSet[] = DataObject::get_by_id("Page", $this->urlParams['ID']); } else { @@ -1231,13 +1238,16 @@ JS; echo '

' . _t('CMSMain.TOTALPAGES',"Total pages: ") . "$count

"; } - function publishall() { + function publishall($request) { ini_set("memory_limit", -1); ini_set('max_execution_time', 0); $response = ""; if(isset($this->requestParams['confirm'])) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $start = 0; $pages = DataObject::get("SiteTree", "", "", "", "$start,30"); $count = 0; @@ -1263,14 +1273,16 @@ JS; $response .= sprintf(_t('CMSMain.PUBPAGES',"Done: Published %d pages"), $count); } else { + $token = SecurityToken::inst(); $response .= '

' . _t('CMSMain.PUBALLFUN','"Publish All" functionality') . '

' . _t('CMSMain.PUBALLFUN2', 'Pressing this button will do the equivalent of going to every page and pressing "publish". It\'s intended to be used after there have been massive edits of the content, such as when the site was first built.') . '

-
'; + . _t('CMSMain.PUBALLCONFIRM',"Please publish every page in the site, copying content stage to live",PR_LOW,'Confirmation button') .'" />' + . $token->getFormField()->FieldHolder() . + ''; } return $response; @@ -1279,7 +1291,7 @@ JS; /** * Restore a completely deleted page from the SiteTree_versions table. */ - function restore() { + function restore($data, $form) { if(($id = $_REQUEST['ID']) && is_numeric($id)) { $restoredPage = Versioned::get_latest_version("SiteTree", $id); if($restoredPage) { @@ -1299,7 +1311,10 @@ JS; } } - function duplicate() { + function duplicate($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + if(($id = $this->urlParams['ID']) && is_numeric($id)) { $page = DataObject::get_by_id("SiteTree", $id); if($page && (!$page->canEdit() || !$page->canCreate())) { @@ -1320,7 +1335,10 @@ JS; } } - function duplicatewithchildren() { + function duplicatewithchildren($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + if(($id = $this->urlParams['ID']) && is_numeric($id)) { $page = DataObject::get_by_id("SiteTree", $id); if($page && (!$page->canEdit() || !$page->canCreate())) { @@ -1340,7 +1358,10 @@ JS; /** * Create a new translation from an existing item, switch to this language and reload the tree. */ - function createtranslation () { + function createtranslation($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $langCode = Convert::raw2sql($_REQUEST['newlang']); $originalLangID = (int)$_REQUEST['ID']; diff --git a/code/CommentTableField.php b/code/CommentTableField.php index f9aaf3ba..5efa791e 100644 --- a/code/CommentTableField.php +++ b/code/CommentTableField.php @@ -85,6 +85,10 @@ class CommentTableField extends ComplexTableField { return $fieldContainer->FieldHolder(); } + + function handleItem($request) { + return new CommentTableField_ItemRequest($this, $request->param('ID')); + } } /** @@ -93,6 +97,7 @@ class CommentTableField extends ComplexTableField { * @subpackage comments */ class CommentTableField_Item extends ComplexTableField_Item { + function HasSpamButton() { return $this->parent()->HasSpamButton(); } @@ -104,6 +109,51 @@ class CommentTableField_Item extends ComplexTableField_Item { function HasHamButton() { return $this->parent()->HasHamButton(); } + + /** + * @return String + */ + function SpamLink() { + return Controller::join_links($this->Link(), "pagecommentaction", 'reportspam', $this->ID); + } + + /** + * @return String + */ + function HamLink() { + return Controller::join_links($this->Link(), "pagecommentaction", 'reportham', $this->ID); + } + + /** + * @return String + */ + function ApproveLink() { + return Controller::join_links($this->Link(), "pagecommentaction", 'approve', $this->ID); + } } +/** + * @package cms + * @subpackage comments + */ +class CommentTableField_ItemRequest extends ComplexTableField_ItemRequest { + + static $url_handlers = array( + 'pagecommentaction/$Action/$ID' => 'handlePageCommentAction', + ); + + /** + * @param SS_HTTPRequest $request + * @return SS_HTTPResponse + */ + function handlePageCommentAction($request) { + $action = $request->param('Action'); + $whitelist = array('approve', 'reportspam', 'reportham'); + if(!in_array($action, $whitelist)) $this->httpError(403); + + $c = new PageComment_Controller($request); + $c->init(); + return $c->$action($request); + } +} ?> \ No newline at end of file diff --git a/code/LeftAndMain.php b/code/LeftAndMain.php index c0f557ae..044b2528 100644 --- a/code/LeftAndMain.php +++ b/code/LeftAndMain.php @@ -869,7 +869,10 @@ JS; /** * Ajax handler for updating the parent of a tree node */ - public function ajaxupdateparent() { + public function ajaxupdateparent($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $id = $_REQUEST['ID']; $parentID = $_REQUEST['ParentID']; if($parentID == 'root'){ @@ -918,7 +921,10 @@ JS; * $_GET[ID]: An array of node ids in the correct order * $_GET[MovedNodeID]: The node that actually got moved */ - public function ajaxupdatesort() { + public function ajaxupdatesort($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $className = $this->stat('tree_class'); $counter = 0; $js = ''; @@ -965,7 +971,10 @@ JS; /** * Delete a number of items */ - public function deleteitems() { + public function deleteitems($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $ids = split(' *, *', $_REQUEST['csvIDs']); $script = "st = \$('sitetree'); \n"; diff --git a/code/MemberTableField.php b/code/MemberTableField.php index 0b515743..f6219bc5 100755 --- a/code/MemberTableField.php +++ b/code/MemberTableField.php @@ -131,7 +131,7 @@ class MemberTableField extends ComplexTableField { } function AddLink() { - return $this->Link() . '/add'; + return Controller::join_links($this->Link(), 'add'); } function SearchForm() { @@ -159,6 +159,10 @@ class MemberTableField extends ComplexTableField { * Add existing member to group rather than creating a new member */ function addtogroup() { + // Protect against CSRF on destructive action + $token = $this->getForm()->getSecurityToken(); + if(!$token->checkRequest($this->controller->getRequest())) return $this->httpError(400); + $data = $_REQUEST; $groupID = (isset($data['ctf']['ID'])) ? $data['ctf']['ID'] : null; @@ -229,6 +233,11 @@ class MemberTableField extends ComplexTableField { * Remove member from group rather than from the database */ function delete() { + // Protect against CSRF on destructive action + $token = $this->getForm()->getSecurityToken(); + // TODO Not sure how this is called, using $_REQUEST to be on the safe side + if(!$token->check($_REQUEST['SecurityID'])) return $this->httpError(400); + $groupID = Convert::raw2sql($_REQUEST['ctf']['ID']); $memberID = Convert::raw2sql($_REQUEST['ctf']['childID']); if(is_numeric($groupID) && is_numeric($memberID)) { @@ -477,7 +486,7 @@ class MemberTableField_Item extends ComplexTableField_Item { function Actions() { $actions = parent::Actions(); - + foreach($actions as $action) { if($action->Name == 'delete') { if($this->parent->getGroup()) { @@ -510,7 +519,11 @@ class MemberTableField_ItemRequest extends ComplexTableField_ItemRequest { /** * Deleting an item from a member table field should just remove that member from the group */ - function delete() { + function delete($request) { + // Protect against CSRF on destructive action + $token = $this->ctf->getForm()->getSecurityToken(); + if(!$token->checkRequest($request)) return $this->httpError('400'); + if($this->ctf->Can('delete') !== true) { return false; } diff --git a/code/SecurityAdmin.php b/code/SecurityAdmin.php index e57bf38d..03847567 100644 --- a/code/SecurityAdmin.php +++ b/code/SecurityAdmin.php @@ -327,7 +327,7 @@ class SecurityAdmin extends LeftAndMain implements PermissionProvider { public function addgroup($request) { // Protect against CSRF on destructive action - if(!Form::get_security_token()->checkRequest($request)) return $this->httpError(400); + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); if(!singleton($this->stat('tree_class'))->canCreate()) return Security::permissionFailure($this); diff --git a/code/sitefeatures/PageComment.php b/code/sitefeatures/PageComment.php index f63dc9fe..49b94fbf 100755 --- a/code/sitefeatures/PageComment.php +++ b/code/sitefeatures/PageComment.php @@ -60,7 +60,10 @@ class PageComment extends DataObject { } function DeleteLink() { - return ($this->canDelete()) ? "PageComment_Controller/deletecomment/$this->ID" : false; + $token = SecurityToken::inst(); + $link = $token->addToUrl("PageComment_Controller/deletecomment/$this->ID"); + + return ($this->canDelete()) ? $link : false; } function CommentTextWithLinks() { @@ -70,15 +73,21 @@ class PageComment extends DataObject { } function SpamLink() { - return ($this->canEdit() && !$this->IsSpam) ? "PageComment_Controller/reportspam/$this->ID" : false; + $token = SecurityToken::inst(); + $link = $token->addToUrl("PageComment_Controller/reportspam/$this->ID"); + return ($this->canEdit() && !$this->IsSpam) ? $link : false; } function HamLink() { - return ($this->canEdit() && $this->IsSpam) ? "PageComment_Controller/reportham/$this->ID" : false; + $token = SecurityToken::inst(); + $link = $token->addToUrl("PageComment_Controller/reportham/$this->ID"); + return ($this->canEdit() && $this->IsSpam) ? $link : false; } function ApproveLink() { - return ($this->canEdit() && $this->NeedsModeration) ? "PageComment_Controller/approve/$this->ID" : false; + $token = SecurityToken::inst(); + $link = $token->addToUrl("PageComment_Controller/approve/$this->ID"); + return ($this->canEdit() && $this->NeedsModeration) ? $link : false; } function SpamClass() { @@ -250,10 +259,14 @@ class PageComment_Controller extends Controller { /** * Deletes all comments on the page referenced by the url param pageid */ - function deleteallcomments() { - $pageId = $_REQUEST['pageid']; + function deleteallcomments($request) { + // Protect against CSRF on destructive action + $token = SecurityToken::inst(); + if(!$token->checkRequest($request)) return $this->httpError(400); + + $pageId = $request->requestVar('pageid'); if(preg_match('/^\d+$/', $pageId)) { - $comments = DataObject::get("PageComment", "\"ParentID\" = $pageId"); + $comments = DataObject::get("PageComment", sprintf("\"ParentID\" = %d", (int)$pageId)); if($comments) foreach($comments as $c) { if($c->canDelete()) $c->delete(); } @@ -266,8 +279,12 @@ class PageComment_Controller extends Controller { } } - function deletecomment() { - $comment = DataObject::get_by_id("PageComment", $this->urlParams['ID']); + function deletecomment($request) { + // Protect against CSRF on destructive action + $token = SecurityToken::inst(); + if(!$token->checkRequest($request)) return $this->httpError(400); + + $comment = DataObject::get_by_id("PageComment", $request->param('ID')); if($comment && $comment->canDelete()) { $comment->delete(); } @@ -279,8 +296,12 @@ class PageComment_Controller extends Controller { } } - function approve() { - $comment = DataObject::get_by_id("PageComment", $this->urlParams['ID']); + function approve($request) { + // Protect against CSRF on destructive action + $token = SecurityToken::inst(); + if(!$token->checkRequest($request)) return $this->httpError(400); + + $comment = DataObject::get_by_id("PageComment", $request->param('ID')); if($comment && $comment->canEdit()) { $comment->NeedsModeration = false; @@ -296,8 +317,12 @@ class PageComment_Controller extends Controller { } } - function reportspam() { - $comment = DataObject::get_by_id("PageComment", $this->urlParams['ID']); + function reportspam($request) { + // Protect against CSRF on destructive action + $token = SecurityToken::inst(); + if(!$token->checkRequest($request)) return $this->httpError(400); + + $comment = DataObject::get_by_id("PageComment", $request->param('ID')); if($comment && $comment->canEdit()) { // if spam protection module exists if(class_exists('SpamProtectorManager')) { @@ -334,8 +359,12 @@ class PageComment_Controller extends Controller { /** * Report a Spam Comment as valid comment (not spam) */ - function reportham() { - $comment = DataObject::get_by_id("PageComment", $this->urlParams['ID']); + function reportham($request) { + // Protect against CSRF on destructive action + $token = SecurityToken::inst(); + if(!$token->checkRequest($request)) return $this->httpError(400); + + $comment = DataObject::get_by_id("PageComment", $request->param('ID')); if($comment && $comment->canEdit()) { // if spam protection module exists if(class_exists('SpamProtectorManager')) { @@ -365,4 +394,4 @@ class PageComment_Controller extends Controller { } -?> +?> \ No newline at end of file diff --git a/javascript/AssetAdmin.js b/javascript/AssetAdmin.js index 4aecd46a..692604b2 100755 --- a/javascript/AssetAdmin.js +++ b/javascript/AssetAdmin.js @@ -524,7 +524,8 @@ Behaviour.register({ method: 'get', onSuccess: function(t) { eval(t.responseText); - } + }, + parameters: {"SecurityID": ($('SecurityID')) ? $('SecurityID').value : null} }; new Ajax.Request('admin/assets/deleteunusedthumbnails',options); } diff --git a/javascript/LeftAndMain_left.js b/javascript/LeftAndMain_left.js index fc660efd..44a6ce9d 100755 --- a/javascript/LeftAndMain_left.js +++ b/javascript/LeftAndMain_left.js @@ -281,8 +281,10 @@ TreeNodeAPI.prototype = { if(this.parentTreeNode && this.parentTreeNode.getIdx) { parentClause = "&parentID=" + this.parentTreeNode.getIdx(); } + var token = $$('input[name=SecurityID]')[0].value; + var url = baseHref() + 'admin/duplicate/' + this.getIdx() + '?ajax=1' + parentClause + '&SecurityID=' + token; - new Ajax.Request(baseHref() + 'admin/duplicate/' + this.getIdx() + '?ajax=1' + parentClause, { + new Ajax.Request(url, { method : 'get', onSuccess : Ajax.Evaluator, onFailure : function(response) { @@ -291,7 +293,9 @@ TreeNodeAPI.prototype = { }); }, duplicatePageWithChildren: function() { - new Ajax.Request(baseHref() + 'admin/duplicatewithchildren/' + this.getIdx() + '?ajax=1', { + var token = $$('input[name=SecurityID]')[0].value; + var url = baseHref() + 'admin/duplicatewithchildren/' + this.getIdx() + '?ajax=1&SecurityID=' + token; + new Ajax.Request(url, { method : 'get', onSuccess : Ajax.Evaluator, onFailure : function(response) { @@ -423,9 +427,10 @@ SiteTreeNode.prototype = { if($('Form_EditForm').elements.ID) currentlyOpenPageID = $('Form_EditForm').elements.ID.value; statusMessage(ss.i18n._t('CMSMAIN.SAVING'), '', true); + var token = $$('input[name=SecurityID]')[0].value; new Ajax.Request(SiteTreeHandlers.parentChanged_url, { method : 'post', - postBody : 'ID=' + node.getIdx() + '&ParentID=' + newParent.getIdx() + '&CurrentlyOpenPageID=' + currentlyOpenPageID, + postBody : 'ID=' + node.getIdx() + '&ParentID=' + newParent.getIdx() + '&CurrentlyOpenPageID=' + currentlyOpenPageID + '&SecurityID=' + token, onSuccess : Ajax.Evaluator, onFailure : function(response) { errorMessage('error saving parent', response); @@ -466,9 +471,10 @@ SiteTreeNode.prototype = { if($('Form_EditForm').elements.ID) currentlyOpenPageID = $('Form_EditForm').elements.ID.value; if(parts) { + var token = $$('input[name=SecurityID]')[0].value; new Ajax.Request(SiteTreeHandlers.orderChanged_url, { method : 'post', - postBody : parts.join('&') + '&CurrentlyOpenPageID=' + currentlyOpenPageID, + postBody : parts.join('&') + '&CurrentlyOpenPageID=' + currentlyOpenPageID + '&SecurityID=' + token, onSuccess : function(response) { movedNode.removeNodeClass('loading'); if( $('sitetree').selected && $('sitetree').selected[0]){ diff --git a/javascript/TranslationTab.js b/javascript/TranslationTab.js index 3aff2269..a1aecfdb 100755 --- a/javascript/TranslationTab.js +++ b/javascript/TranslationTab.js @@ -9,6 +9,7 @@ Behaviour.register({ var url = baseHref() + 'admin/' + this.name.substring(7) + '?ID=' + $('Form_EditForm_ID').value + '&newlang=' + $('Form_EditForm_NewTransLang').value + '&ajax=1'; url += "&locale=" + $('Form_EditForm_Locale').value; + url += "&SecurityID=" + $$('input[name=SecurityID]')[0].value; new Ajax.Request( url, { onSuccess: Ajax.Evaluator, diff --git a/templates/Includes/AssetAdmin_left.ss b/templates/Includes/AssetAdmin_left.ss index 0060f144..47ff763d 100755 --- a/templates/Includes/AssetAdmin_left.ss +++ b/templates/Includes/AssetAdmin_left.ss @@ -13,6 +13,7 @@ diff --git a/templates/Includes/AssetTableField.ss b/templates/Includes/AssetTableField.ss index 6d20f59c..f65947dd 100755 --- a/templates/Includes/AssetTableField.ss +++ b/templates/Includes/AssetTableField.ss @@ -42,7 +42,7 @@ <% end_if %> <% if Can(delete) %> - <% _t('DELFILE', 'Delete this file') %> + <% _t('DELFILE', 'Delete this file') %> <% end_if %> diff --git a/templates/Includes/CMSMain_left.ss b/templates/Includes/CMSMain_left.ss index a6a38a61..3545379a 100755 --- a/templates/Includes/CMSMain_left.ss +++ b/templates/Includes/CMSMain_left.ss @@ -72,6 +72,7 @@

<% _t('SELECTPAGESACTIONS','Select the pages that you want to change & then click an action:') %>

+
+ @@ -15,6 +16,7 @@

<% _t('SELECT','Select the pages that you want to delete and then click the button below') %>

+ diff --git a/tests/CMSMainTest.php b/tests/CMSMainTest.php index 603e2ce9..4736ee13 100644 --- a/tests/CMSMainTest.php +++ b/tests/CMSMainTest.php @@ -9,7 +9,7 @@ class CMSMainTest extends FunctionalTest { protected $autoFollowRedirection = false; static protected $orig = array(); - + static function set_up_once() { self::$orig['CMSBatchActionHandler_batch_actions'] = CMSBatchActionHandler::$batch_actions; CMSBatchActionHandler::$batch_actions = array( @@ -35,7 +35,7 @@ class CMSMainTest extends FunctionalTest { $page2 = $this->objFromFixture('Page', "page2"); $this->session()->inst_set('loggedInAs', $this->idFromFixture('Member', 'admin')); - $response = Director::test("admin/cms/publishall", array('confirm' => 1), $this->session()); + $response = $this->get("admin/cms/publishall?confirm=1"); $this->assertContains( sprintf(_t('CMSMain.PUBPAGES',"Done: Published %d pages"), 8), @@ -44,7 +44,13 @@ class CMSMainTest extends FunctionalTest { // Some modules (e.g., cmsworkflow) will remove this action if(isset(CMSBatchActionHandler::$batch_actions['publish'])) { - $response = Director::test("admin/cms/batchactions/publish", array('csvIDs' => implode(',', array($page1->ID, $page2->ID)), 'ajax' => 1), $this->session()); + $response = $this->post( + "admin/cms/batchactions/publish", + array( + 'csvIDs' => implode(',', array($page1->ID, $page2->ID)), + 'ajax' => 1 + ) + ); $this->assertContains(sprintf('setNodeTitle(%d, \'Page 1\');', $page1->ID), $response->getBody()); $this->assertContains(sprintf('setNodeTitle(%d, \'Page 2\');', $page2->ID), $response->getBody()); @@ -196,13 +202,23 @@ class CMSMainTest extends FunctionalTest { // with insufficient permissions $cmsUser->logIn(); - $response = $this->post('admin/addpage', array('ParentID' => '0', 'PageType' => 'Page', 'Locale' => 'en_US')); + $response = $this->get('admin'); + $response = $this->submitForm( + 'Form_AddPageOptionsForm', + null, + array('ParentID' => '0', 'PageType' => 'Page', 'Locale' => 'en_US') + ); // should redirect, which is a permission error $this->assertEquals(403, $response->getStatusCode(), 'Add TopLevel page must fail for normal user'); // with correct permissions $rootEditUser->logIn(); - $response = $this->post('admin/addpage', array('ParentID' => '0', 'PageType' => 'Page', 'Locale' => 'en_US')); + $response = $this->get('admin'); + $response = $this->submitForm( + 'Form_AddPageOptionsForm', + null, + array('ParentID' => '0', 'PageType' => 'Page', 'Locale' => 'en_US') + ); $this->assertEquals(302, $response->getStatusCode(), 'Must be a redirect on success'); $location=$response->getHeader('Location'); $this->assertContains('/show/',$location, 'Must redirect to /show/ the new page'); diff --git a/tests/CommentAdminTest.php b/tests/CommentAdminTest.php index 9ec92e26..ecf6d68e 100644 --- a/tests/CommentAdminTest.php +++ b/tests/CommentAdminTest.php @@ -29,7 +29,7 @@ class CommentAdminTest extends FunctionalTest { $comm = $this->objFromFixture('PageComment', 'Comment1'); $id = $comm->ID; $this->logInWithPermission('CMS_ACCESS_CommentAdmin'); - $result = $this->get("admin/comments/EditForm/field/Comments/item/$id/delete"); + $response = $this->get("admin/comments/EditForm/field/Comments/item/$id/delete"); $checkComm = DataObject::get_by_id('PageComment',$id); $this->assertFalse($checkComm); diff --git a/tests/MemberTableFieldTest.php b/tests/MemberTableFieldTest.php index 253d99c7..e5e9694b 100644 --- a/tests/MemberTableFieldTest.php +++ b/tests/MemberTableFieldTest.php @@ -1,7 +1,14 @@ objFromFixture('Member', 'member1'); $member2 = $this->objFromFixture('Member', 'member2'); @@ -54,14 +61,12 @@ class MemberTableFieldTest extends SapphireTest { $member1 = $this->objFromFixture('Member', 'member1'); $group1 = $this->objFromFixture('Group', 'group1'); - $tf = new MemberTableField( - $this, - "Members", - $group1 - ); - $tfItem = new MemberTableField_ItemRequest($tf, $member1->ID); - $tfItem->delete(); - + $response = $this->get('MemberTableFieldTest_Controller'); + $token = SecurityToken::inst(); + $url = sprintf('MemberTableFieldTest_Controller/Form/field/Members/item/%d/delete/?usetestmanifest=1', $member1->ID); + $url = $token->addToUrl($url); + $response = $this->get($url); + $group1->flushCache(); $this->assertNotContains($member1->ID, $group1->Members()->column('ID'), @@ -79,14 +84,12 @@ class MemberTableFieldTest extends SapphireTest { $member1ID = $member1->ID; $group1 = $this->objFromFixture('Group', 'group1'); - $tf = new MemberTableField( - $this, - "Members" - // no group assignment - ); - $tfItem = new MemberTableField_ItemRequest($tf, $member1->ID); - $tfItem->delete(); - + $response = $this->get('MemberTableFieldTest_Controller'); + $token = SecurityToken::inst(); + $url = sprintf('MemberTableFieldTest_Controller/FormNoGroup/field/Members/item/%d/delete/?usetestmanifest=1', $member1->ID); + $url = $token->addToUrl($url); + $response = $this->get($url); + $group1->flushCache(); $this->assertNotContains($member1->ID, $group1->Members()->column('ID'), @@ -98,4 +101,39 @@ class MemberTableFieldTest extends SapphireTest { 'Member record is removed from database' ); } +} + +class MemberTableFieldTest_Controller extends Controller implements TestOnly { + + protected $template = 'BlankPage'; + + function Link($action = null) { + return Controller::join_links('MemberTableFieldTest_Controller', $action); + } + + function Form() { + $group1 = DataObject::get_one('Group', '"Code" = \'group1\''); + return new Form( + $this, + 'FormNoGroup', + new FieldSet(new MemberTableField($this, "Members", $group1)), + new FieldSet(new FormAction('submit')) + ); + } + + function FormNoGroup() { + $tf = new MemberTableField( + $this, + "Members" + // no group + ); + + return new Form( + $this, + 'FormNoGroup', + new FieldSet(new MemberTableField($this, "Members")), + new FieldSet(new FormAction('submit')) + ); + } + } \ No newline at end of file diff --git a/tests/MemberTableFieldTest.yml b/tests/MemberTableFieldTest.yml index ee85785d..bc9785f2 100644 --- a/tests/MemberTableFieldTest.yml +++ b/tests/MemberTableFieldTest.yml @@ -1,13 +1,17 @@ Group: admin: Title: Administrators + Code: admin group1: Title: Group1 + Code: group1 group1_child: Title: Group1 Child Parent: =>Group.group1 + Code: group1_child group2: Title: Group2 + Code: group2 Member: admin: Email: admin@example.com diff --git a/tests/PageCommentsTest.php b/tests/PageCommentsTest.php index 4510a31f..dc708422 100644 --- a/tests/PageCommentsTest.php +++ b/tests/PageCommentsTest.php @@ -4,6 +4,8 @@ class PageCommentsTest extends FunctionalTest { static $fixture_file = 'cms/tests/PageCommentsTest.yml'; + static $use_draft_site = true; + function testCanView() { $visitor = $this->objFromFixture('Member', 'visitor'); $admin = $this->objFromFixture('Member', 'commentadmin'); @@ -51,8 +53,11 @@ class PageCommentsTest extends FunctionalTest { $firstComment = $this->objFromFixture('PageComment', 'firstComA'); $firstCommentID = $firstComment->ID; - Director::test($firstPage->RelativeLink(), null, $this->session()); - Director::test('PageComment/deletecomment/'.$firstComment->ID, null, $this->session()); + $response = $this->get($firstPage->RelativeLink()); + $token = SecurityToken::inst(); + $url = sprintf('PageComment/deletecomment/%d/', $firstComment->ID); + $url = $token->addToUrl($url); + $response = $this->get($url); $this->assertFalse(DataObject::get_by_id('PageComment', $firstCommentID)); } @@ -62,11 +67,13 @@ class PageCommentsTest extends FunctionalTest { $this->autoFollowRedirection = false; $this->logInAs('commentadmin'); - Director::test('second-page', null, $this->session()); - Director::test('PageComment/deleteallcomments?pageid='.$second->ID, - null, $this->session()); - Director::test('second-page', null, $this->session()); - + $response = $this->get($second->RelativeLink()); + $token = SecurityToken::inst(); + $url = sprintf('PageComment/deleteallcomments?pageid=%d', $second->ID); + $url = $token->addToUrl($url); + $response = $this->get($url); + $response = $this->get($second->RelativeLink()); + $secondComments = DataObject::get('PageComment', '"ParentID" = '.$second->ID); $this->assertNull($secondComments);