diff --git a/src/Control/Middleware/ConfirmationMiddleware/GetParameter.php b/src/Control/Middleware/ConfirmationMiddleware/GetParameter.php index 9d6c617a6..44ceaf1a2 100644 --- a/src/Control/Middleware/ConfirmationMiddleware/GetParameter.php +++ b/src/Control/Middleware/ConfirmationMiddleware/GetParameter.php @@ -61,7 +61,7 @@ class GetParameter implements Rule, Bypass { return new Confirmation\Item( $token, - _t(__CLASS__.'.CONFIRMATION_NAME', '"{key}" GET parameter', ['key' => $this->name]), + _t(__CLASS__ . '.CONFIRMATION_NAME', '"{key}" GET parameter', ['key' => $this->name]), sprintf('%s = "%s"', $this->name, $value) ); } diff --git a/src/Control/Middleware/ConfirmationMiddleware/Url.php b/src/Control/Middleware/ConfirmationMiddleware/Url.php index 297bfdeea..0e1cf0075 100644 --- a/src/Control/Middleware/ConfirmationMiddleware/Url.php +++ b/src/Control/Middleware/ConfirmationMiddleware/Url.php @@ -178,8 +178,8 @@ class Url implements Rule, Bypass { return new Confirmation\Item( $token, - _t(__CLASS__.'.CONFIRMATION_NAME', 'URL is protected'), - _t(__CLASS__.'.CONFIRMATION_DESCRIPTION', 'The URL is: "{url}"', ['url' => $url]) + _t(__CLASS__ . '.CONFIRMATION_NAME', 'URL is protected'), + _t(__CLASS__ . '.CONFIRMATION_DESCRIPTION', 'The URL is: "{url}"', ['url' => $url]) ); } diff --git a/src/Control/Middleware/ConfirmationMiddleware/UrlPathStartswith.php b/src/Control/Middleware/ConfirmationMiddleware/UrlPathStartswith.php index fc01488d5..efea0c528 100644 --- a/src/Control/Middleware/ConfirmationMiddleware/UrlPathStartswith.php +++ b/src/Control/Middleware/ConfirmationMiddleware/UrlPathStartswith.php @@ -34,8 +34,8 @@ class UrlPathStartswith implements Rule, Bypass { return new Confirmation\Item( $token, - _t(__CLASS__.'.CONFIRMATION_NAME', 'URL begins with "{path}"', ['path' => $this->getPath()]), - _t(__CLASS__.'.CONFIRMATION_DESCRIPTION', 'The complete URL is: "{url}"', ['url' => $url]) + _t(__CLASS__ . '.CONFIRMATION_NAME', 'URL begins with "{path}"', ['path' => $this->getPath()]), + _t(__CLASS__ . '.CONFIRMATION_DESCRIPTION', 'The complete URL is: "{url}"', ['url' => $url]) ); } diff --git a/src/Dev/DevConfirmationController.php b/src/Dev/DevConfirmationController.php index 71f376bf6..2a64b4b4c 100644 --- a/src/Dev/DevConfirmationController.php +++ b/src/Dev/DevConfirmationController.php @@ -23,9 +23,9 @@ class DevConfirmationController extends Confirmation\Handler $renderer = DebugView::create(); echo $renderer->renderHeader(); echo $renderer->renderInfo( - _t(__CLASS__.".INFO_TITLE", "Security Confirmation"), + _t(__CLASS__ . ".INFO_TITLE", "Security Confirmation"), Director::absoluteBaseURL(), - _t(__CLASS__.".INFO_DESCRIPTION", "Confirm potentially dangerous operation") + _t(__CLASS__ . ".INFO_DESCRIPTION", "Confirm potentially dangerous operation") ); return $response; diff --git a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php index 188bac218..21159de4e 100644 --- a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php +++ b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php @@ -18,6 +18,7 @@ use SilverStripe\ORM\DataObject; use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\ORM\HasManyList; use SilverStripe\ORM\ManyManyList; +use SilverStripe\ORM\RelationList; use SilverStripe\ORM\SS_List; use SilverStripe\ORM\ValidationException; use SilverStripe\ORM\ValidationResult; @@ -177,20 +178,6 @@ 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 = $this->getToplevelController(); - // TODO More friendly error - return $controller->httpError(403); - } - - // Build actions - $actions = $this->getFormActions(); - // If we are creating a new record in a has-many list, then // pre-populate the record's foreign key. if ($list instanceof HasManyList && !$this->record->isInDB()) { @@ -199,6 +186,12 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler $this->record->$key = $id; } + if (!$this->record->canView()) { + $controller = $this->getToplevelController(); + // TODO More friendly error + return $controller->httpError(403); + } + $fields = $this->component->getFields(); if (!$fields) { $fields = $this->record->getCMSFields(); @@ -218,20 +211,22 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler $this, 'ItemEditForm', $fields, - $actions, + $this->getFormActions(), $this->component->getValidator() ); $form->loadDataFrom($this->record, $this->record->ID == 0 ? Form::MERGE_IGNORE_FALSEISH : Form::MERGE_DEFAULT); - if ($this->record->ID && !$canEdit) { + if ($this->record->ID && !$this->record->canEdit()) { // Restrict editing of existing records $form->makeReadonly(); // Hack to re-enable delete button if user can delete - if ($canDelete) { + if ($this->record->canDelete()) { $form->Actions()->fieldByName('action_doDelete')->setReadonly(false); } - } elseif (!$this->record->ID && !$canCreate) { + } elseif (!$this->record->ID + && !$this->record->canCreate(null, $this->getCreateContext()) + ) { // Restrict creation of new records $form->makeReadonly(); } @@ -271,6 +266,25 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler return $form; } + /** + * Build context for verifying canCreate + * @see GridFieldAddNewButton::getHTMLFragments() + * + * @return array + */ + protected function getCreateContext() + { + $gridField = $this->gridField; + $context = []; + if ($gridField->getList() instanceof RelationList) { + $record = $gridField->getForm()->getRecord(); + if ($record && $record instanceof DataObject) { + $context['Parent'] = $record; + } + } + return $context; + } + /** * @return CompositeField Returns the right aligned toolbar group field along with its FormAction's */ diff --git a/src/Security/Confirmation/Form.php b/src/Security/Confirmation/Form.php index f3ded39fd..bfbc560ac 100644 --- a/src/Security/Confirmation/Form.php +++ b/src/Security/Confirmation/Form.php @@ -99,8 +99,8 @@ class Form extends BaseForm protected function buildActionList(Storage $storage) { - $cancel = FormAction::create('doRefuse', _t(__CLASS__.'.REFUSE', 'Cancel')); - $confirm = FormAction::create('doConfirm', _t(__CLASS__.'.CONFIRM', 'Run the action'))->setAutofocus(true); + $cancel = FormAction::create('doRefuse', _t(__CLASS__ . '.REFUSE', 'Cancel')); + $confirm = FormAction::create('doConfirm', _t(__CLASS__ . '.CONFIRM', 'Run the action'))->setAutofocus(true); if ($storage->getHttpMethod() === 'POST') { $confirm->setAttribute('formaction', htmlspecialchars($storage->getSuccessUrl())); @@ -165,7 +165,7 @@ class Form extends BaseForm protected function buildEmptyFieldList() { return FieldList::create( - HeaderField::create(null, _t(__CLASS__.'.EMPTY_TITLE', 'Nothing to confirm')) + HeaderField::create(null, _t(__CLASS__ . '.EMPTY_TITLE', 'Nothing to confirm')) ); } } diff --git a/src/Security/Confirmation/Handler.php b/src/Security/Confirmation/Handler.php index 77de39713..c6f0833e5 100644 --- a/src/Security/Confirmation/Handler.php +++ b/src/Security/Confirmation/Handler.php @@ -47,7 +47,7 @@ class Handler extends RequestHandler public function index() { return [ - 'Title' => _t(__CLASS__.'.FORM_TITLE', 'Confirm potentially dangerous action'), + 'Title' => _t(__CLASS__ . '.FORM_TITLE', 'Confirm potentially dangerous action'), 'Form' => $this->Form() ]; return $this; diff --git a/src/Security/Confirmation/Storage.php b/src/Security/Confirmation/Storage.php index 0794e3646..ea52686c9 100644 --- a/src/Security/Confirmation/Storage.php +++ b/src/Security/Confirmation/Storage.php @@ -125,7 +125,7 @@ class Storage $token = $item->getToken(); $salt = $this->getSessionSalt(); - $salted = $salt.$token; + $salted = $salt . $token; return hash(static::HASH_ALGO, $salted, true); } @@ -139,7 +139,7 @@ class Storage { $salt = $this->getSessionSalt(); - return bin2hex(hash(static::HASH_ALGO, $salt.'cookie key', true)); + return bin2hex(hash(static::HASH_ALGO, $salt . 'cookie key', true)); } /** @@ -151,7 +151,7 @@ class Storage { $salt = $this->getSessionSalt(); - return base64_encode(hash(static::HASH_ALGO, $salt.'csrf token', true)); + return base64_encode(hash(static::HASH_ALGO, $salt . 'csrf token', true)); } /** @@ -441,7 +441,7 @@ class Storage '%s.%s%s', str_replace('\\', '.', __CLASS__), $this->id, - $key ? '.'.$key : '' + $key ? '.' . $key : '' ); } } diff --git a/tests/php/Control/HttpRequestMockBuilder.php b/tests/php/Control/HttpRequestMockBuilder.php index 50b522147..f7e9680ae 100644 --- a/tests/php/Control/HttpRequestMockBuilder.php +++ b/tests/php/Control/HttpRequestMockBuilder.php @@ -29,7 +29,7 @@ trait HttpRequestMockBuilder $request->method('getSession')->willReturn($session); $request->method('getURL')->will($this->returnCallback(static function ($addParams) use ($url, $getVars) { - return $addParams && count($getVars) ? $url.'?'.http_build_query($getVars) : $url; + return $addParams && count($getVars) ? $url . '?' . http_build_query($getVars) : $url; })); $request->method('getVars')->willReturn($getVars); diff --git a/tests/php/Control/Middleware/ConfirmationMiddlewareTest.php b/tests/php/Control/Middleware/ConfirmationMiddlewareTest.php index 9b2f43f51..957ef46cf 100644 --- a/tests/php/Control/Middleware/ConfirmationMiddlewareTest.php +++ b/tests/php/Control/Middleware/ConfirmationMiddlewareTest.php @@ -68,7 +68,7 @@ class ConfirmationMiddlewareTest extends SapphireTest $this->assertFalse($next); $this->assertInstanceOf(HTTPResponse::class, $response); $this->assertEquals(302, $response->getStatusCode()); - $this->assertEquals(Director::baseURL().'dev/confirm/middleware', $response->getHeader('location')); + $this->assertEquals(Director::baseURL() . 'dev/confirm/middleware', $response->getHeader('location')); // Test bypasses have more priority than rules $middleware->setBypasses([new Url('dev/build')]); diff --git a/tests/php/Security/Confirmation/StorageTest.php b/tests/php/Security/Confirmation/StorageTest.php index a3075769d..d785b6d68 100644 --- a/tests/php/Security/Confirmation/StorageTest.php +++ b/tests/php/Security/Confirmation/StorageTest.php @@ -14,7 +14,7 @@ class StorageTest extends SapphireTest private function getNamespace($id) { - return str_replace('\\', '.', Storage::class).'.'.$id; + return str_replace('\\', '.', Storage::class) . '.' . $id; } public function testNewStorage()