From 1848d7e90abad4b4b969fa87922a58c97a1026de Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Mon, 17 Dec 2012 00:46:51 +0100 Subject: [PATCH] API Check model permissions in GridField --- docs/en/changelogs/3.1.0.md | 33 +++++++++++++++ docs/en/reference/grid-field.md | 10 +++++ forms/gridfield/GridFieldAddNewButton.php | 6 ++- forms/gridfield/GridFieldDeleteAction.php | 22 ++++++---- forms/gridfield/GridFieldDetailForm.php | 41 +++++++++++++++---- .../gridfield/GridFieldDetailFormTest.php | 4 ++ 6 files changed, 101 insertions(+), 15 deletions(-) diff --git a/docs/en/changelogs/3.1.0.md b/docs/en/changelogs/3.1.0.md index a880275c9..22367db46 100644 --- a/docs/en/changelogs/3.1.0.md +++ b/docs/en/changelogs/3.1.0.md @@ -44,6 +44,39 @@ you'll need to adjust your code. } } +### GridField and ModelAdmin Permission Checks + +`GridFieldDetailForm` now checks for `canEdit()` and `canDelete()` permissions +on your model. `GridFieldAddNewButton` checks `canCreate()`. +The default implementation requires `ADMIN` permissions. +You'll need to loosen those permissions if you want other users with CMS +access to interact with your data. +Since `GridField` is used in `ModelAdmin`, this change will affect both classes. + + Example: Require "CMS: Pages section" access + + :::php + class MyModel extends DataObject { + public function canView($member = null) { + return Permission::check('CMS_ACCESS_CMSMain', 'any', $member); + } + public function canEdit($member = null) { + return Permission::check('CMS_ACCESS_CMSMain', 'any', $member); + } + public function canDelete($member = null) { + return Permission::check('CMS_ACCESS_CMSMain', 'any', $member); + } + public function canCreate($member = null) { + return Permission::check('CMS_ACCESS_CMSMain', 'any', $member); + } + +You can also implement [custom permission codes](/topics/permissions). +For 3.1.0 stable, we aim to further simplify the permission definitions, +in order to reduce the boilerplate code required to get a model editable in the CMS. + +Note: GridField is already relying on the permission checks performed +through the CMS controllers, providing a simple level of security. + ### Other * `TableListField`, `ComplexTableField`, `TableField`, `HasOneComplexTableField`, `HasManyComplexTableField` and `ManyManyComplexTableField` have been removed from the core and placed into a module called "legacytablefields" located at https://github.com/silverstripe-labs/legacytablefields diff --git a/docs/en/reference/grid-field.md b/docs/en/reference/grid-field.md index dfe52bf06..79d0083d4 100644 --- a/docs/en/reference/grid-field.md +++ b/docs/en/reference/grid-field.md @@ -310,6 +310,16 @@ transfered between page requests by being inserted as a hidden field in the form A GridFieldComponent sets and gets data from the GridState. +## Permissions + +Since GridField is mostly used in the CMS, the controller managing a GridField instance +will already do some permission checks for you, and can decline display or executing +any logic on your field. + +If you need more granular control, e.g. to consistently deny non-admins from deleting +records, use the `DataObject->can...()` methods +(see [DataObject permissions](/reference/dataobject#permissions)). + ## Related * [ModelAdmin: A UI driven by GridField](/reference/modeladmin) diff --git a/forms/gridfield/GridFieldAddNewButton.php b/forms/gridfield/GridFieldAddNewButton.php index 2c1e0bf87..ba3089490 100644 --- a/forms/gridfield/GridFieldAddNewButton.php +++ b/forms/gridfield/GridFieldAddNewButton.php @@ -1,6 +1,7 @@ canCreate()} for this record returns true. * * @package framework * @subpackage gridfield @@ -21,9 +22,12 @@ class GridFieldAddNewButton implements GridField_HTMLProvider { } public function getHTMLFragments($gridField) { + $singleton = singleton($gridField->getModelClass()); + if(!$singleton->canCreate()) return array(); + if(!$this->buttonName) { // provide a default button name, can be changed by calling {@link setButtonName()} on this component - $objectName = singleton($gridField->getModelClass())->i18n_singular_name(); + $objectName = $singleton->i18n_singular_name(); $this->buttonName = _t('GridField.Add', 'Add {name}', array('name' => $objectName)); } diff --git a/forms/gridfield/GridFieldDeleteAction.php b/forms/gridfield/GridFieldDeleteAction.php index 16c24a10b..9c2aeb83b 100644 --- a/forms/gridfield/GridFieldDeleteAction.php +++ b/forms/gridfield/GridFieldDeleteAction.php @@ -98,15 +98,16 @@ class GridFieldDeleteAction implements GridField_ColumnProvider, GridField_Actio */ public function getColumnContent($gridField, $record, $columnName) { if($this->removeRelation) { + if(!$record->canEdit()) return; + $field = GridField_FormAction::create($gridField, 'UnlinkRelation'.$record->ID, false, "unlinkrelation", array('RecordID' => $record->ID)) ->addExtraClass('gridfield-button-unlink') ->setAttribute('title', _t('GridAction.UnlinkRelation', "Unlink")) ->setAttribute('data-icon', 'chain--minus'); } else { - if(!$record->canDelete()) { - return; - } + if(!$record->canDelete()) return; + $field = GridField_FormAction::create($gridField, 'DeleteRecord'.$record->ID, false, "deleterecord", array('RecordID' => $record->ID)) ->addExtraClass('gridfield-button-delete') @@ -132,13 +133,20 @@ class GridFieldDeleteAction implements GridField_ColumnProvider, GridField_Actio if(!$item) { return; } - if($actionName == 'deleterecord' && !$item->canDelete()) { - throw new ValidationException( - _t('GridFieldAction_Delete.DeletePermissionsFailure',"No delete permissions"),0); - } + if($actionName == 'deleterecord') { + if(!$item->canDelete()) { + throw new ValidationException( + _t('GridFieldAction_Delete.DeletePermissionsFailure',"No delete permissions"),0); + } + $item->delete(); } else { + if(!$item->canEdit()) { + throw new ValidationException( + _t('GridFieldAction_Delete.EditPermissionsFailure',"No permission to unlink record"),0); + } + $gridField->getList()->remove($item); } } diff --git a/forms/gridfield/GridFieldDetailForm.php b/forms/gridfield/GridFieldDetailForm.php index 23818c217..44c1bd738 100644 --- a/forms/gridfield/GridFieldDetailForm.php +++ b/forms/gridfield/GridFieldDetailForm.php @@ -310,16 +310,31 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler { return $controller->redirect($noActionURL, 302); } + $canView = $this->record->canView(); + $canEdit = $this->record->canEdit(); + $canDelete = $this->record->canDelete(); + $canCreate = $this->record->canCreate(); + + if(!$canView) { + $controller = Controller::curr(); + // TODO More friendly error + return $controller->httpError(403); + } + $actions = new FieldList(); if($this->record->ID !== 0) { - $actions->push(FormAction::create('doSave', _t('GridFieldDetailForm.Save', 'Save')) - ->setUseButtonTag(true) - ->addExtraClass('ss-ui-action-constructive') - ->setAttribute('data-icon', 'accept')); + if($canEdit) { + $actions->push(FormAction::create('doSave', _t('GridFieldDetailForm.Save', 'Save')) + ->setUseButtonTag(true) + ->addExtraClass('ss-ui-action-constructive') + ->setAttribute('data-icon', 'accept')); + } - $actions->push(FormAction::create('doDelete', _t('GridFieldDetailForm.Delete', 'Delete')) - ->setUseButtonTag(true) - ->addExtraClass('ss-ui-action-destructive')); + if($canDelete) { + $actions->push(FormAction::create('doDelete', _t('GridFieldDetailForm.Delete', 'Delete')) + ->setUseButtonTag(true) + ->addExtraClass('ss-ui-action-destructive')); + } }else{ // adding new record //Change the Save label to 'Create' @@ -353,6 +368,14 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler { $form->loadDataFrom($this->record, $this->record->ID == 0 ? Form::MERGE_IGNORE_FALSEISH : Form::MERGE_DEFAULT); + if($this->record->ID && !$canEdit) { + // Restrict editing of existing records + $form->makeReadonly(); + } elseif(!$this->record->ID && !$canCreate) { + // Restrict creation of new records + $form->makeReadonly(); + } + // Load many_many extraData for record. // Fields with the correct 'ManyMany' namespace need to be added manually through getCMSFields(). if($list instanceof ManyManyList) { @@ -429,6 +452,10 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler { $extraData = null; } + if(!$this->record->canEdit()) { + return $controller->httpError(403); + } + try { $form->saveInto($this->record); $this->record->write(); diff --git a/tests/forms/gridfield/GridFieldDetailFormTest.php b/tests/forms/gridfield/GridFieldDetailFormTest.php index c883aefd3..6c50b2e5c 100644 --- a/tests/forms/gridfield/GridFieldDetailFormTest.php +++ b/tests/forms/gridfield/GridFieldDetailFormTest.php @@ -192,6 +192,8 @@ class GridFieldDetailFormTest extends FunctionalTest { } public function testCustomItemRequestClass() { + $this->logInWithPermission('ADMIN'); + $component = new GridFieldDetailForm(); $this->assertEquals('GridFieldDetailForm_ItemRequest', $component->getItemRequestClass()); $component->setItemRequestClass('GridFieldDetailFormTest_ItemRequest'); @@ -199,6 +201,8 @@ class GridFieldDetailFormTest extends FunctionalTest { } public function testItemEditFormCallback() { + $this->logInWithPermission('ADMIN'); + $category = new GridFieldDetailFormTest_Category(); $component = new GridFieldDetailForm(); $component->setItemEditFormCallback(function($form, $component) {