Merge pull request #9277 from tractorcow/pulls/4.4/respect-can-create

BUG Ensure that canCreate() context matches that respected by GridFieldAddNewButton

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)
         );
     }

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_NAME', 'URL is protected'),
-            _t(__CLASS__.'.CONFIRMATION_DESCRIPTION', 'The URL is: "{url}"', ['url' => $url])
+            _t(__CLASS__ . '.CONFIRMATION_DESCRIPTION', 'The URL is: "{url}"', ['url' => $url])
         );
     }
 

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_NAME', 'URL begins with "{path}"', ['path' => $this->getPath()]),
-            _t(__CLASS__.'.CONFIRMATION_DESCRIPTION', 'The complete URL is: "{url}"', ['url' => $url])
+            _t(__CLASS__ . '.CONFIRMATION_DESCRIPTION', 'The complete URL is: "{url}"', ['url' => $url])
         );
     }
 

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;

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
      */

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'));
+        $cancel = FormAction::create('doRefuse', _t(__CLASS__ . '.REFUSE', 'Cancel'));
-        $confirm = FormAction::create('doConfirm', _t(__CLASS__.'.CONFIRM', 'Run the action'))->setAutofocus(true);
+        $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'))
         );
     }
 }

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;

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 : ''
         );
     }
 }

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);

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')]);

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()