diff --git a/code/GroupImportForm.php b/code/GroupImportForm.php index e0f524711..ce4404ee2 100644 --- a/code/GroupImportForm.php +++ b/code/GroupImportForm.php @@ -89,6 +89,7 @@ public function __construct($controller, $name, $fields = null, $actions = null, public function doImport($data, $form) { $loader = new GroupCsvBulkLoader(); + $loader->setCheckPermissions(true); // load file $result = $loader->load($data['CsvFile']['tmp_name']); diff --git a/code/MemberImportForm.php b/code/MemberImportForm.php index cb09ffbbb..a29fec586 100644 --- a/code/MemberImportForm.php +++ b/code/MemberImportForm.php @@ -93,6 +93,7 @@ public function __construct($controller, $name, $fields = null, $actions = null, public function doImport($data, $form) { $loader = new MemberCsvBulkLoader(); + $loader->setCheckPermissions(true); // optionally set group relation if ($this->group) { diff --git a/code/ModelAdmin.php b/code/ModelAdmin.php index a283f9236..bf85b5d4e 100644 --- a/code/ModelAdmin.php +++ b/code/ModelAdmin.php @@ -6,6 +6,7 @@ use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; +use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Convert; use SilverStripe\Core\Injector\Injector; @@ -620,7 +621,11 @@ public function getModelImporters() $importers = []; foreach ($importerClasses as $modelClass => $importerClass) { - $importers[$modelClass] = new $importerClass($modelClass); + $importer = new $importerClass($modelClass); + if (ClassInfo::hasMethod($importer, 'setCheckPermissions')) { + $importer->setCheckPermissions(true); + } + $importers[$modelClass] = $importer; } return $importers; @@ -647,19 +652,14 @@ public function ImportForm() return false; } - if (!$modelSNG->canCreate(Security::getCurrentUser())) { - return false; - } - $fields = new FieldList( new HiddenField('ClassName', false, $this->modelClass), new FileField('_CsvFile', false) ); // get HTML specification for each import (column names etc.) - $importerClass = $importers[$this->modelTab]; /** @var BulkLoader $importer */ - $importer = new $importerClass($this->modelClass); + $importer = $importers[$this->modelTab]; $spec = $importer->getImportSpec(); $specFields = new ArrayList(); foreach ($spec['fields'] as $name => $desc) { @@ -744,7 +744,13 @@ public function import($data, $form, $request) if (!empty($data['EmptyBeforeImport']) && $data['EmptyBeforeImport']) { //clear database before import $loader->deleteExistingRecords = true; } - $results = $loader->load($_FILES['_CsvFile']['tmp_name']); + try { + $results = $loader->load($_FILES['_CsvFile']['tmp_name']); + } catch (HTTPResponse_Exception $e) { + $form->sessionMessage($e->getMessage(), ValidationResult::TYPE_ERROR); + $this->redirectBack(); + return false; + } $message = ''; diff --git a/composer.json b/composer.json index eb6bca508..cf6ad7a9e 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ } ], "require": { - "silverstripe/framework": "^4.10", + "silverstripe/framework": "^4.13.39", "silverstripe/versioned": "^1@dev", "silverstripe/vendor-plugin": "^1.0", "php": "^7.4 || ^8.0" @@ -54,4 +54,4 @@ }, "minimum-stability": "dev", "prefer-stable": true -} \ No newline at end of file +} diff --git a/tests/behat/features/files/import-company-create.csv b/tests/behat/features/files/import-company-create.csv new file mode 100644 index 000000000..b93a85888 --- /dev/null +++ b/tests/behat/features/files/import-company-create.csv @@ -0,0 +1,2 @@ +Name,Category,Revenue,CEO +"New Company",Retail,123,"New person" diff --git a/tests/behat/features/files/import-company-delete.csv b/tests/behat/features/files/import-company-delete.csv new file mode 100644 index 000000000..08eae7e47 --- /dev/null +++ b/tests/behat/features/files/import-company-delete.csv @@ -0,0 +1 @@ +ID,Name,Category,Revenue,CEO diff --git a/tests/behat/features/files/import-company-edit.csv b/tests/behat/features/files/import-company-edit.csv new file mode 100644 index 000000000..e4dc83f32 --- /dev/null +++ b/tests/behat/features/files/import-company-edit.csv @@ -0,0 +1,2 @@ +ID,Name,Category,Revenue,CEO +1,"Some other company",Retail,123,"some person" diff --git a/tests/behat/features/gridfield-import.feature b/tests/behat/features/gridfield-import.feature new file mode 100644 index 000000000..61281b74b --- /dev/null +++ b/tests/behat/features/gridfield-import.feature @@ -0,0 +1,83 @@ +Feature: Import in GridField + As a site owner + I want confidence that only users with permission can import records + So that I can sleep at night + + Background: + Given the "Company" "Walmart" with "Category"="Retail" + And I am logged in with "ADMIN" permissions + + Scenario: I can create new records via the import form + When I go to "/admin/test" + Then I should see 1 ".col-Name" elements + And I press the "Import CSV" button + Then I should see the "#Form_ImportForm" element + When I attach the file "import-company-create.csv" to the "_CsvFile" field + And I press the "Import from CSV" button + Then I should see "Imported 1 record" in the "#Form_ImportForm" element + And I should see 2 ".col-Name" elements + + Scenario: I cannot create new records without permission + Given I add an extension "SilverStripe\Admin\Tests\Behat\Context\Extension\CannotCreateExtension" to the "SilverStripe\FrameworkTest\Model\Company" class + When I go to "/admin/test" + And I press the "Import CSV" button + Then I should see the "#Form_ImportForm" element + When I attach the file "import-company-create.csv" to the "_CsvFile" field + And I press the "Import from CSV" button + Then I should see "Not allowed to create 'Company' records" in the "#Form_ImportForm" element + And I should see 1 ".col-Name" elements + And I should see "Walmart" in the ".col-Name" element + + Scenario: I can edit existing records via the import form + # To edit, we have to rely on IDs - so start by validating that a record with that ID exists at all. + When I go to "/admin/test/SilverStripe-FrameworkTest-Model-Company/EditForm/field/SilverStripe-FrameworkTest-Model-Company/item/1" + # Check we are viewing a record (we don't know or care what it's called) + Then I should see "Main" in the "div.cms-content-header-tabs" element + # Check we didn't get a not found error + And I should not see "Not Found" + When I go to "/admin/test" + And I press the "Import CSV" button + Then I should see the "#Form_ImportForm" element + When I attach the file "import-company-edit.csv" to the "_CsvFile" field + And I press the "Import from CSV" button + Then I should see "Updated 1 record" in the "#Form_ImportForm" element + And I should see 1 ".col-Name" elements + And I should not see "Walmart" in the ".col-Name" element + And I should see "Some other company" in the ".col-Name" element + + Scenario: I cannot edit existing records without permission + Given I add an extension "SilverStripe\Admin\Tests\Behat\Context\Extension\CannotEditExtension" to the "SilverStripe\FrameworkTest\Model\Company" class + # We can't check if the record exists here because we don't have permission! But if it doesn't, this test will fail anyway. + When I go to "/admin/test" + And I press the "Import CSV" button + Then I should see the "#Form_ImportForm" element + When I attach the file "import-company-edit.csv" to the "_CsvFile" field + And I press the "Import from CSV" button + Then I should see "Not allowed to edit 'Company' records" in the "#Form_ImportForm" element + And I should see 1 ".col-Name" elements + And I should see "Walmart" in the ".col-Name" element + And I should not see "Some other company" in the ".col-Name" element + + Scenario: I can delete records via the import form + When I go to "/admin/test" + And I press the "Import CSV" button + Then I should see the "#Form_ImportForm" element + When I attach the file "import-company-delete.csv" to the "_CsvFile" field + And I check "Replace data" + And I press the "Import from CSV" button + Then I should see "Nothing to import" in the "#Form_ImportForm" element + And I should see "No items found" in the ".ss-gridfield-items" element + And I should not see the ".col-Name" element + + Scenario: I cannot delete records without permission + Given I add an extension "SilverStripe\Admin\Tests\Behat\Context\Extension\CannotDeleteExtension" to the "SilverStripe\FrameworkTest\Model\Company" class + When I go to "/admin/test" + And I press the "Import CSV" button + Then I should see the "#Form_ImportForm" element + When I attach the file "import-company-delete.csv" to the "_CsvFile" field + And I check "Replace data" + And I press the "Import from CSV" button + Then I should see "Not allowed to delete 'Company' records" in the "#Form_ImportForm" element + And I should not see "No items found" in the ".ss-gridfield-items" element + And I should see 1 ".col-Name" elements + And I should see "Walmart" in the ".col-Name" element diff --git a/tests/behat/src/Extension/CannotCreateExtension.php b/tests/behat/src/Extension/CannotCreateExtension.php new file mode 100644 index 000000000..b4836238f --- /dev/null +++ b/tests/behat/src/Extension/CannotCreateExtension.php @@ -0,0 +1,14 @@ +