From 8879b4f37e0a23de7b28478a8d5a13e6f45d9039 Mon Sep 17 00:00:00 2001 From: JojoVes Date: Tue, 30 Jul 2024 11:38:05 -0300 Subject: [PATCH 1/6] add http and https schemes as options --- src/Form/Admin.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Form/Admin.php b/src/Form/Admin.php index 9cfaa51..bcc7640 100644 --- a/src/Form/Admin.php +++ b/src/Form/Admin.php @@ -78,12 +78,16 @@ public function getFormId() { public function buildForm(array $form, FormStateInterface $form_state) { $config = $this->config('islandora_spreadsheet_ingest.settings'); $current_whitelist = $config->get('binary_directory_whitelist'); + $scheme_options = $this->streamWrapperManager->getNames(StreamWrapperInterface::READ_VISIBLE) + [ + 'http' => 'HTTP', + 'https' => 'HTTPS', + ]; $form['schemes'] = [ '#type' => 'checkboxes', '#title' => $this->t('Schemes'), '#description' => $this->t('Allowed list of schemes for which binaries can be referenced from.'), '#default_value' => $config->get('schemes') ?? [], - '#options' => $this->streamWrapperManager->getNames(StreamWrapperInterface::READ_VISIBLE), + '#options' => $scheme_options, ]; $form['paths'] = [ '#type' => 'textarea', From db1901db62d63b6931d439b645155d048d209180 Mon Sep 17 00:00:00 2001 From: JojoVes Date: Tue, 30 Jul 2024 12:15:37 -0300 Subject: [PATCH 2/6] add ftp as suggested --- src/Form/Admin.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Form/Admin.php b/src/Form/Admin.php index bcc7640..90febef 100644 --- a/src/Form/Admin.php +++ b/src/Form/Admin.php @@ -81,6 +81,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { $scheme_options = $this->streamWrapperManager->getNames(StreamWrapperInterface::READ_VISIBLE) + [ 'http' => 'HTTP', 'https' => 'HTTPS', + 'ftp' => 'FTP', ]; $form['schemes'] = [ '#type' => 'checkboxes', From 9d30820be4b9fd6c6fede710c43c9021ecfb4f3f Mon Sep 17 00:00:00 2001 From: JojoVes Date: Tue, 30 Jul 2024 17:49:19 -0300 Subject: [PATCH 3/6] message and functionality around missing config --- src/Form/Admin.php | 5 +++++ src/Plugin/migrate/process/AccessibleFile.php | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/Form/Admin.php b/src/Form/Admin.php index 90febef..85f1267 100644 --- a/src/Form/Admin.php +++ b/src/Form/Admin.php @@ -96,6 +96,11 @@ public function buildForm(array $form, FormStateInterface $form_state) { '#default_value' => $current_whitelist ? implode(',', $current_whitelist) : '', '#description' => $this->t('A comma separated list of local locations from which spreadsheet ingests can use binaries.'), ]; + $form['info'] = [ + '#type' => 'item', + '#title' => $this->t('If nothing is whitelisted, everything is whitelisted.'), + '#description' => $this->t('The plugin that expects to use this information will log warnings if this configuration is missing.'), + ]; $form['submit'] = [ '#type' => 'submit', '#value' => $this->t('Save'), diff --git a/src/Plugin/migrate/process/AccessibleFile.php b/src/Plugin/migrate/process/AccessibleFile.php index df155e8..e13497c 100644 --- a/src/Plugin/migrate/process/AccessibleFile.php +++ b/src/Plugin/migrate/process/AccessibleFile.php @@ -6,8 +6,10 @@ use Drupal\Core\File\FileSystemInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; +use Drupal\Core\Url; use Drupal\migrate\MigrateExecutableInterface; use Drupal\migrate\MigrateSkipRowException; +use Drupal\migrate\Plugin\MigrationInterface; use Drupal\migrate\ProcessPluginBase; use Drupal\migrate\Row; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -99,6 +101,20 @@ public function transform($value, MigrateExecutableInterface $migrate_executable $allowed_paths = $this->configFactory->get('islandora_spreadsheet_ingest.settings')->get('binary_directory_whitelist'); $allowed_schemes = $this->configFactory->get('islandora_spreadsheet_ingest.settings')->get('schemes'); + // Ignore this check if relevant configuration has not been set, and advise + // that the configuration should be set. + if (empty($allowed_paths) && empty($allowed_schemes)) { + $migrate_executable->saveMessage( + strtr("Skipping this process to check (:file) since nothing has been configured to check against in the approved list of directories or schemes. For site security, it is highly recommended to set appropriate configuration at :config_url.", [ + ':file' => $value, + ':config_url' => Url::fromRoute('islandora_spreadsheet_ingest.admin')->toString(), + ]), + MigrationInterface::MESSAGE_INFORMATIONAL + ); + + return $value; + } + // If it's a local path check to see if it's in our allowed list. $file_path = $this->fileSystem->realpath($value); if ($file_path) { From 4f75d892d809c38da7711a94b00b6b5099035998 Mon Sep 17 00:00:00 2001 From: JojoVes Date: Thu, 1 Aug 2024 10:59:22 -0300 Subject: [PATCH 4/6] wordsmithing of log message Co-authored-by: Adam <607975+adam-vessey@users.noreply.github.com> --- src/Plugin/migrate/process/AccessibleFile.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Plugin/migrate/process/AccessibleFile.php b/src/Plugin/migrate/process/AccessibleFile.php index e13497c..9677756 100644 --- a/src/Plugin/migrate/process/AccessibleFile.php +++ b/src/Plugin/migrate/process/AccessibleFile.php @@ -105,7 +105,7 @@ public function transform($value, MigrateExecutableInterface $migrate_executable // that the configuration should be set. if (empty($allowed_paths) && empty($allowed_schemes)) { $migrate_executable->saveMessage( - strtr("Skipping this process to check (:file) since nothing has been configured to check against in the approved list of directories or schemes. For site security, it is highly recommended to set appropriate configuration at :config_url.", [ + strtr("File access criteria is not defined; passing check of file (:file). For site security, it is highly recommended to set appropriate configuration at :config_url.", [ ':file' => $value, ':config_url' => Url::fromRoute('islandora_spreadsheet_ingest.admin')->toString(), ]), From dec636807dc37656983564e03b17fcd7366d98c4 Mon Sep 17 00:00:00 2001 From: JojoVes Date: Thu, 1 Aug 2024 11:49:49 -0300 Subject: [PATCH 5/6] change message type to warning --- src/Plugin/migrate/process/AccessibleFile.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Plugin/migrate/process/AccessibleFile.php b/src/Plugin/migrate/process/AccessibleFile.php index 9677756..97b10d6 100644 --- a/src/Plugin/migrate/process/AccessibleFile.php +++ b/src/Plugin/migrate/process/AccessibleFile.php @@ -109,7 +109,7 @@ public function transform($value, MigrateExecutableInterface $migrate_executable ':file' => $value, ':config_url' => Url::fromRoute('islandora_spreadsheet_ingest.admin')->toString(), ]), - MigrationInterface::MESSAGE_INFORMATIONAL + MigrationInterface::MESSAGE_WARNING ); return $value; From 19e58eb1f0c5e07794839a66add2efd909e40f09 Mon Sep 17 00:00:00 2001 From: JojoVes Date: Thu, 1 Aug 2024 11:52:00 -0300 Subject: [PATCH 6/6] remove info text Valid point made in https://github.com/discoverygarden/islandora_spreadsheet_ingest/pull/114#discussion_r1700128193 that this behaviour is more of a deprecation allowance rather than something we should document and keep as an expectation going forward. --- src/Form/Admin.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Form/Admin.php b/src/Form/Admin.php index 85f1267..90febef 100644 --- a/src/Form/Admin.php +++ b/src/Form/Admin.php @@ -96,11 +96,6 @@ public function buildForm(array $form, FormStateInterface $form_state) { '#default_value' => $current_whitelist ? implode(',', $current_whitelist) : '', '#description' => $this->t('A comma separated list of local locations from which spreadsheet ingests can use binaries.'), ]; - $form['info'] = [ - '#type' => 'item', - '#title' => $this->t('If nothing is whitelisted, everything is whitelisted.'), - '#description' => $this->t('The plugin that expects to use this information will log warnings if this configuration is missing.'), - ]; $form['submit'] = [ '#type' => 'submit', '#value' => $this->t('Save'),