added Ruebots patch for filehash 3

SUP-6725: Hardening fixity

.github/workflows/lint.yml

@@ -3,7 +3,7 @@ name: Code Linting
 on:
   pull_request:
     branches:
-      - 1.x
+      - main
   workflow_dispatch:
 
 jobs:

.github/workflows/semver.yml

@@ -4,7 +4,7 @@ on:
   pull_request_target:
     types: closed
     branches:
-      - 1.x
+      - main
 jobs:
   update:
     if: github.event.pull_request.merged == true

README.md

@@ -6,10 +6,10 @@ Perform periodic fixity checks on selected files.
 
 This module defines a new content entity type `fixity_check`. This entity is
 used as an audit trail for fixity checks performed on a related `file` entity.
-Wherein the revisions of the `fixity_check` record the results of previous 
+Wherein the revisions of the `fixity_check` record the results of previous
 checks against that `file` entity.
 
-This modules requires and enforces the following constraints on `fixity_check` 
+This modules requires and enforces the following constraints on `fixity_check`
 entities:
 
 - **Must** be related to a `file`
@@ -29,7 +29,7 @@ Users with the permission `View Fixity Checks` can:
 - View fixity audit log of Media entities
 
 A `cron` hook is setup to automatically mark files as _requiring_ periodic
-checks. As well as performing those checks on a regular basis. Email 
+checks. As well as performing those checks on a regular basis. Email
 notifications can be configured to alert the selected user of the status
 of all performed checks on a regular basis or only when an error occurs.
 
@@ -111,5 +111,5 @@ and or contact [discoverygarden].
 [discoverygarden]: http://support.discoverygarden.ca
 [filehash]: https://www.drupal.org/project/filehash
 [gplv2]: http://www.gnu.org/licenses/gpl-2.0.txt
-[install]: https://drupal.org/documentation/install/modules-themes/modules-8
+[install]: https://www.drupal.org/docs/extending-drupal/installing-modules
 [CTDA: Connecticut Digital Archive]: https://lib.uconn.edu/find/connecticut-digital-archive/

composer.json

@@ -1,8 +1,8 @@
 {
-  "name": "discoverygarden/dgi_fixity",
+  "name": "roblib/dgi_fixity",
   "type": "drupal-module",
   "license": "GPL-2.0-or-later",
   "require": {
-    "drupal/filehash": "^2.0"
+      "drupal/filehash": "^3.0"
   }
 }

dgi_fixity.info.yml

@@ -2,7 +2,7 @@ name: 'Fixity'
 description: "Performs fixity checks on files."
 type: module
 package: DGI
-core_version_requirement: ^8 || ^9
+core_version_requirement: ^9 || ^10
 configure: dgi_fixity.settings
 dependencies:
   - drupal:file

dgi_fixity.module

@@ -17,6 +17,7 @@ use Drupal\Core\Mail\MailFormatHelper;
 use Drupal\Core\Render\Element;
 use Drupal\Core\Routing\RouteMatchInterface;
 use Drupal\Core\Url;
+use Drupal\dgi_fixity\FixityCheckServiceInterface;
 use Drupal\dgi_fixity\Form\SettingsForm;
 use Drupal\file\Plugin\Field\FieldWidget\FileWidget;
 use Drupal\user\Entity\User;
@@ -145,15 +146,16 @@ function dgi_fixity_cron() {
  * Implements hook_entity_type_alter().
  */
 function dgi_fixity_entity_type_alter(array &$entity_types) {
-  /** @var \Drupal\dgi_fixity\FixityCheckServiceInterface $fixity */
+  // XXX: Cannot reference dgi_fixity.fixity_check:fromEntityTypes() due to
-  $fixity = \Drupal::service('dgi_fixity.fixity_check');
+  // circular dependencies, as dgi_fixity.fixity_check makes use of the
-  $supported_entity_types = $fixity->fromEntityTypes();
+  // entity_type.manager that we are in the middle of trying to build.
-  foreach ($supported_entity_types as $entity_type_id) {
+  foreach (FixityCheckServiceInterface::ENTITY_TYPES as $entity_type_id) {
     $entity_type = &$entity_types[$entity_type_id];
     $entity_type->setLinkTemplate('fixity-audit', "/fixity/$entity_type_id/{{$entity_type_id}}");
     $entity_type->setLinkTemplate('fixity-check', "/fixity/$entity_type_id/{{$entity_type_id}}/check");
     $entity_type->setFormClass('fixity-check', 'Drupal\dgi_fixity\Form\CheckForm');
   }
+  unset($entity_type);
 }
 
 /**

dgi_fixity.services.yml

@@ -1,11 +1,10 @@
 services:
   logger.channel.dgi_fixity:
-    class: Drupal\Core\Logger\LoggerChannel
+    parent: logger.channel_base
-    factory: logger.factory:get
     arguments: ['dgi_fixity']
   dgi_fixity.fixity_check:
     class: Drupal\dgi_fixity\FixityCheckService
-    arguments: ['@string_translation', '@config.factory', '@entity_type.manager', '@datetime.time', '@plugin.manager.mail', '@logger.channel.dgi_fixity', '@filehash']
+    arguments: ['@string_translation', '@config.factory', '@entity_type.manager', '@datetime.time', '@logger.channel.dgi_fixity', '@filehash']
   dgi_fixity.route_subscriber:
     class: Drupal\dgi_fixity\Routing\FixityCheckRouteSubscriber
     arguments: ['@entity_type.manager', '@dgi_fixity.fixity_check']

src/Commands/FixityCheck.php

@@ -17,13 +17,6 @@ class FixityCheck extends DrushCommands {
 
   use StringTranslationTrait;
 
-  /**
-   * A logger instance.
-   *
-   * @var \Psr\Log\LoggerInterface
-   */
-  protected $logger;
-
   /**
    * The entity type manager.
    *
@@ -57,7 +50,7 @@ class FixityCheck extends DrushCommands {
     /** @var \Drupal\dgi_fixity\FixityCheckStorageInterface $storage */
     $storage = $this->entityTypeManager->getStorage('fixity_check');
     $count = $storage->countPeriodic();
-    if ($this->io()->confirm("This will remove periodic checks on ${count} files, are you sure?", FALSE)) {
+    if ($this->io()->confirm("This will remove periodic checks on {$count} files, are you sure?", FALSE)) {
       $storage->clearPeriodic();
     }
   }

src/Controller/FixityCheckController.php

@@ -235,6 +235,7 @@ class FixityCheckController extends ControllerBase {
       ->condition('id', $fixity_check->id())
       ->sort('performed', 'DESC')
       ->pager(50)
+      ->accessCheck()
       ->execute();
     return array_keys($result);
   }

src/Entity/FixityCheck.php

@@ -208,7 +208,13 @@ class FixityCheck extends ContentEntityBase implements FixityCheckInterface {
   public function getFile(): ?File {
     /** @var \Drupal\Core\Field\EntityReferenceFieldItemList $file */
     $file = $this->file;
-    return $file->isEmpty() ? NULL : $file->referencedEntities()[0];
+
+    if ($file->isEmpty()) {
+      return NULL;
+    }
+
+    $referenced_entities = $file->referencedEntities();
+    return !empty($referenced_entities) ? reset($referenced_entities) : NULL;
   }
 
   /**

src/FixityCheckBatchCheck.php

@@ -170,7 +170,7 @@ class FixityCheckBatchCheck {
     $results = &$context['results'];
     if (!isset($sandbox['offset'])) {
       $sandbox['offset'] = 0;
-      $sandbox['remaining'] = $storage->countPeriodic();
+      $sandbox['count'] = $storage->countPeriodic();
       $results['successful'] = 0;
       $results['ignored'] = 0;
       $results['skipped'] = 0;
@@ -179,7 +179,8 @@ class FixityCheckBatchCheck {
     }
 
     $files = $storage->getPeriodic($sandbox['offset'], $batch_size);
-    $end = min($sandbox['remaining'], $sandbox['offset'] + count($files));
+
+    $end = min($sandbox['count'], $sandbox['offset'] + count($files));
     $context['message'] = \t('Processing @start to @end', [
       '@start' => $sandbox['offset'],
       '@end' => $end,
@@ -187,12 +188,7 @@ class FixityCheckBatchCheck {
     static::check($files, $force, $results);
     $sandbox['offset'] = $end;
 
-    $remaining = $storage->countPeriodic();
+    $context['finished'] = ($sandbox['count'] <= $end);
-    $progress_halted = $sandbox['remaining'] == $remaining;
-    $sandbox['remaining'] = $remaining;
-
-    // End when we have exhausted all inputs or progress has halted.
-    $context['finished'] = empty($files) || $progress_halted;
   }
 
   /**

src/FixityCheckService.php

@@ -7,7 +7,6 @@ use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Link;
-use Drupal\Core\Mail\MailManagerInterface;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
 use Drupal\Core\StringTranslation\TranslationInterface;
 use Drupal\dgi_fixity\Entity\FixityCheck;
@@ -48,17 +47,10 @@ class FixityCheckService implements FixityCheckServiceInterface {
    */
   protected $time;
 
-  /**
-   * The mail manager service.
-   *
-   * @var \Drupal\Core\Mail\MailManagerInterface
-   */
-  protected $mailManager;
-
   /**
    * The logger for this service.
    *
-   * @var Psr\Log\LoggerInterface
+   * @var \Psr\Log\LoggerInterface
    */
   protected $logger;
 
@@ -72,12 +64,18 @@ class FixityCheckService implements FixityCheckServiceInterface {
   /**
    * Constructor.
    */
-  public function __construct(TranslationInterface $string_translation, ConfigFactoryInterface $config, EntityTypeManagerInterface $entity_type_manager, TimeInterface $time, MailManagerInterface $mail_manager, LoggerInterface $logger, FileHash $filehash) {
+  public function __construct(
+    TranslationInterface $string_translation,
+    ConfigFactoryInterface $config,
+    EntityTypeManagerInterface $entity_type_manager,
+    TimeInterface $time,
+    LoggerInterface $logger,
+    FileHash $filehash,
+  ) {
     $this->stringTranslation = $string_translation;
     $this->config = $config;
     $this->entityTypeManager = $entity_type_manager;
     $this->time = $time;
-    $this->mailManager = $mail_manager;
     $this->logger = $logger;
     $this->filehash = $filehash;
   }
@@ -86,10 +84,7 @@ class FixityCheckService implements FixityCheckServiceInterface {
    * {@inheritdoc}
    */
   public function fromEntityTypes(): array {
-    return [
+    return static::ENTITY_TYPES;
-      'media',
-      'file',
-    ];
   }
 
   /**
@@ -223,7 +218,7 @@ class FixityCheckService implements FixityCheckServiceInterface {
     // Assume success until proven untrue.
     $state = FixityCheck::STATE_MATCHES;
     // If column is set, only generate that hash.
-    foreach ($this->filehash->algos() as $column => $algo) {
+    foreach ($this->filehash->getEnabledAlgorithms() as $column => $algo) {
       // Nothing to do if the previous checksum value is not known.
       if (!isset($file->{$column})) {
         $state = FixityCheck::STATE_NO_CHECKSUM;
@@ -293,6 +288,7 @@ class FixityCheckService implements FixityCheckServiceInterface {
       ->condition('performed', 0, '!=')
       ->groupBy('state')
       ->aggregate('id', 'COUNT')
+      ->accessCheck(FALSE)
       ->execute();
 
     $failed = 0;
@@ -312,12 +308,14 @@ class FixityCheckService implements FixityCheckServiceInterface {
     $periodic = (int) $storage->getQuery('AND')
       ->count('id')
       ->condition('periodic', TRUE)
+      ->accessCheck(FALSE)
       ->execute();
 
     // All checks performed ever.
     $revisions = (int) $storage->getQuery('AND')
       ->allRevisions()
       ->count('id')
+      ->accessCheck(FALSE)
       ->execute();
 
     // Checks which have exceeded the threshold and should be performed again.
@@ -326,6 +324,7 @@ class FixityCheckService implements FixityCheckServiceInterface {
       ->condition('periodic', TRUE)
       ->condition('performed', $threshold, '>=')
       ->count('id')
+      ->accessCheck(FALSE)
       ->execute();
 
     // Up to date checks.

src/FixityCheckServiceInterface.php

@@ -12,6 +12,11 @@ use Drupal\views\ViewExecutable;
  */
 interface FixityCheckServiceInterface {
 
+  const ENTITY_TYPES = [
+    'media',
+    'file',
+  ];
+
   /**
    * A list of entity types which be converted into a fixity_check entity.
    *

src/Form/SettingsForm.php

@@ -141,7 +141,7 @@ class SettingsForm extends ConfigFormBase {
         '#title' => $this->t('Time elapsed'),
         '#description' => $this->t('
           <p>Time threshold is relative to "<em>now</em>". For example "<em>-1 month</em>" would prevent any checks that occurred less than a month ago.</p>
-          <p>Check <a href="https://www.php.net/manual/en/datetime.formats.relative.php">Relative Formats</a> for acceptable values</p>
+          <p>Check <a href="https://www.php.net/manual/en/datetime.formats.php#datetime.formats.relative">Relative Formats</a> for acceptable values</p>
         '),
         '#default_value' => $config->get(static::THRESHOLD) ?: '-1 month',
         '#element_validate' => [
@@ -155,7 +155,7 @@ class SettingsForm extends ConfigFormBase {
         '#description' => $this->t('
           <p>Set how many files will be processed at once when performing a batch / cron job</p>
         '),
-        '#default_value' => 100,
+        '#default_value' => $config->get(static::BATCH_SIZE) ?: 100,
       ],
     ];
 
@@ -209,7 +209,7 @@ class SettingsForm extends ConfigFormBase {
         '#title' => $this->t('Time elapsed'),
         '#description' => $this->t('
           <p>Time threshold is relative to "<em>now</em>". For example "<em>-1 week</em>" would mean a week must pass between notifications.</p>
-          <p>Check <a href="https://www.php.net/manual/en/datetime.formats.relative.php">Relative Formats</a> for acceptable values</p>
+          <p>Check <a href="https://www.php.net/manual/en/datetime.formats.php#datetime.formats.relative">Relative Formats</a> for acceptable values</p>
         '),
         '#default_value' => $notification_threshold,
         '#element_validate' => [

src/Plugin/QueueWorker/FixityCheckWorker.php

@@ -4,8 +4,9 @@ namespace Drupal\dgi_fixity\Plugin\QueueWorker;
 
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Drupal\Core\Queue\QueueWorkerBase;
-use Drupal\dgi_fixity\FixityCheckServiceInterface;
 use Drupal\dgi_fixity\FixityCheckInterface;
+use Drupal\dgi_fixity\FixityCheckServiceInterface;
+use Drupal\file\FileInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -61,7 +62,12 @@ class FixityCheckWorker extends QueueWorkerBase implements ContainerFactoryPlugi
   public function processItem($data) {
     if ($data instanceof FixityCheckInterface) {
       /** @var \Drupal\dgi_fixity\FixityCheckInterface $data */
-      $this->fixity->check($data->getFile());
+      if ($data->getFile() instanceof FileInterface) {
+        $this->fixity->check($data->getFile());
+      }
+      else {
+        $data->setState(FixityCheckInterface::STATE_MISSING);
+      }
     }
   }
 

src/Plugin/QueueWorker/ProcessSourceWorker.php

@@ -2,9 +2,12 @@
 
 namespace Drupal\dgi_fixity\Plugin\QueueWorker;
 
+use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Drupal\Core\Queue\QueueWorkerBase;
 use Drupal\Core\Queue\RequeueException;
+use Drupal\Core\Session\AccountInterface;
+use Drupal\Core\Session\AccountSwitcherInterface;
 use Drupal\dgi_fixity\FixityCheckServiceInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
@@ -26,6 +29,20 @@ class ProcessSourceWorker extends QueueWorkerBase implements ContainerFactoryPlu
    */
   protected $fixity;
 
+  /**
+   * The account switcher service.
+   *
+   * @var \Drupal\Core\Session\AccountSwitcherInterface
+   */
+  protected $accountSwitcher;
+
+  /**
+   * The entity type manager.
+   *
+   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
+   */
+  protected $entityTypeManager;
+
   /**
    * Constructs a new FixityCheckWorker instance.
    *
@@ -37,10 +54,16 @@ class ProcessSourceWorker extends QueueWorkerBase implements ContainerFactoryPlu
    *   The plugin implementation definition.
    * @param \Drupal\dgi_fixity\FixityCheckServiceInterface $fixity
    *   The fixity check service.
+   * @param \Drupal\Core\Session\AccountSwitcherInterface $account_switcher
+   *   The account switcher service.
+   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
+   *   The entity type manager.
    */
-  public function __construct(array $configuration, $plugin_id, $plugin_definition, FixityCheckServiceInterface $fixity) {
+  public function __construct(array $configuration, $plugin_id, $plugin_definition, FixityCheckServiceInterface $fixity, AccountSwitcherInterface $account_switcher, EntityTypeManagerInterface $entity_type_manager) {
     parent::__construct($configuration, $plugin_id, $plugin_definition);
     $this->fixity = $fixity;
+    $this->accountSwitcher = $account_switcher;
+    $this->entityTypeManager = $entity_type_manager;
   }
 
   /**
@@ -52,6 +75,8 @@ class ProcessSourceWorker extends QueueWorkerBase implements ContainerFactoryPlu
       $plugin_id,
       $plugin_definition,
       $container->get('dgi_fixity.fixity_check'),
+      $container->get('account_switcher'),
+      $container->get('entity_type.manager'),
     );
   }
 
@@ -59,20 +84,31 @@ class ProcessSourceWorker extends QueueWorkerBase implements ContainerFactoryPlu
    * {@inheritdoc}
    */
   public function processItem($data) {
-    /** @var \Drupal\dgi_fixity\FixityCheckServiceInterface $fixity */
+    // To avoid expensive access calls.
-    $fixity = \Drupal::service('dgi_fixity.fixity_check');
+    $user_storage = $this->entityTypeManager->getStorage('user');
-    $view = $fixity->source($data, 1000);
+    $account = $user_storage->load(1);
-    $view->execute();
+
-    // Only processes those which have not already enabled periodic checks.
+    if ($account instanceof AccountInterface) {
-    foreach ($view->result as $row) {
+      $this->accountSwitcher->switchTo($account);
-      /** @var \Drupal\dgi_fixity\FixityCheckInterface $check */
+
-      $check = $view->field['periodic']->getEntity($row);
+      /** @var \Drupal\dgi_fixity\FixityCheckServiceInterface $fixity */
-      $check->setPeriodic(TRUE);
+      $fixity = \Drupal::service('dgi_fixity.fixity_check');
-      $check->save();
+      $view = $fixity->source($data, 1000);
-    }
+      $view->execute();
-    // Not finished processing.
+      // Only processes those which have not already enabled periodic checks.
-    if (count($view->result) !== 0) {
+      foreach ($view->result as $row) {
-      throw new RequeueException();
+        /** @var \Drupal\dgi_fixity\FixityCheckInterface $check */
+        $check = $view->field['periodic']->getEntity($row);
+        $check->setPeriodic(TRUE);
+        $check->save();
+      }
+      // Not finished processing.
+      if (count($view->result) !== 0) {
+        $this->accountSwitcher->switchBack();
+        throw new RequeueException();
+      }
+
+      $this->accountSwitcher->switchBack();
     }
   }
 

src/Routing/FixityCheckRouteSubscriber.php

@@ -21,6 +21,13 @@ class FixityCheckRouteSubscriber extends RouteSubscriberBase {
    */
   protected $fixity;
 
+  /**
+   * The entity type manager service.
+   *
+   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
+   */
+  protected EntityTypeManagerInterface $entityTypeManager;
+
   /**
    * Subscriber for Fixity Check routes.
    *