diff --git a/core/modules/auto_updates/auto_updates.services.yml b/core/modules/auto_updates/auto_updates.services.yml index d2b80b6847de8efaf6d1003f62689300486728b7..de0987e1a9d5e20cc3d002cc26ee40300f28f9d5 100644 --- a/core/modules/auto_updates/auto_updates.services.yml +++ b/core/modules/auto_updates/auto_updates.services.yml @@ -30,6 +30,12 @@ services: - '@string_translation' tags: - { name: event_subscriber } + auto_updates.validator.settings: + class: Drupal\auto_updates\Validator\SettingsValidator + arguments: + - '@string_translation' + tags: + - { name: event_subscriber } auto_updates.update_version_validator: class: Drupal\auto_updates\Validator\UpdateVersionValidator arguments: @@ -42,6 +48,12 @@ services: - '@package_manager.validator.composer_executable' tags: - { name: event_subscriber } + auto_updates.validator.composer_settings: + class: Drupal\auto_updates\Validator\PackageManagerReadinessCheck + arguments: + - '@package_manager.validator.composer_settings' + tags: + - { name: event_subscriber } auto_updates.disk_space_validator: class: Drupal\auto_updates\Validator\PackageManagerReadinessCheck arguments: diff --git a/core/modules/auto_updates/src/Validator/SettingsValidator.php b/core/modules/auto_updates/src/Validator/SettingsValidator.php new file mode 100644 index 0000000000000000000000000000000000000000..571bdc3442afbe3a53f00a0c5feaa4b0994081f4 --- /dev/null +++ b/core/modules/auto_updates/src/Validator/SettingsValidator.php @@ -0,0 +1,52 @@ +<?php + +namespace Drupal\auto_updates\Validator; + +use Drupal\auto_updates\Event\ReadinessCheckEvent; +use Drupal\auto_updates\Updater; +use Drupal\Core\Site\Settings; +use Drupal\Core\StringTranslation\StringTranslationTrait; +use Drupal\Core\StringTranslation\TranslationInterface; +use Drupal\package_manager\Event\PreCreateEvent; +use Drupal\package_manager\Event\PreOperationStageEvent; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +class SettingsValidator implements EventSubscriberInterface { + + use StringTranslationTrait; + + /** + * Constructs a SettingsValidator object. + * + * @param \Drupal\Core\StringTranslation\TranslationInterface $translation + * The string translation service. + */ + public function __construct(TranslationInterface $translation) { + $this->setStringTranslation($translation); + } + + /** + * Validates site settings before an update starts. + * + * @param \Drupal\package_manager\Event\PreOperationStageEvent $event + * The event object. + */ + public function checkSettings(PreOperationStageEvent $event): void { + if ($event->getStage() instanceof Updater && Settings::get('update_fetch_with_http_fallback')) { + $event->addError([ + $this->t('The <code>update_fetch_with_http_fallback</code> setting must be disabled for automatic updates.'), + ]); + } + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() { + return [ + ReadinessCheckEvent::class => 'checkSettings', + PreCreateEvent::class => 'checkSettings', + ]; + } + +} diff --git a/core/modules/auto_updates/tests/src/Kernel/ReadinessValidation/PackageManagerReadinessChecksTest.php b/core/modules/auto_updates/tests/src/Kernel/ReadinessValidation/PackageManagerReadinessChecksTest.php index 2a385f1775129f695b0488ff13ea706633945ffe..553570872117ff715bf8c95d2989007fe43c55f7 100644 --- a/core/modules/auto_updates/tests/src/Kernel/ReadinessValidation/PackageManagerReadinessChecksTest.php +++ b/core/modules/auto_updates/tests/src/Kernel/ReadinessValidation/PackageManagerReadinessChecksTest.php @@ -49,6 +49,7 @@ public function providerValidatorInvoked(): array { ['package_manager.validator.disk_space'], ['package_manager.validator.pending_updates'], ['package_manager.validator.file_system'], + ['package_manager.validator.composer_settings'], ]; } diff --git a/core/modules/auto_updates/tests/src/Kernel/ReadinessValidation/SettingsValidatorTest.php b/core/modules/auto_updates/tests/src/Kernel/ReadinessValidation/SettingsValidatorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..18e88d7726418425dd2d387089c89c773325f919 --- /dev/null +++ b/core/modules/auto_updates/tests/src/Kernel/ReadinessValidation/SettingsValidatorTest.php @@ -0,0 +1,68 @@ +<?php + +namespace Drupal\Tests\auto_updates\Kernel\ReadinessValidation; + +use Drupal\package_manager\Exception\StageValidationException; +use Drupal\package_manager\ValidationResult; +use Drupal\Tests\auto_updates\Kernel\AutoUpdatesKernelTestBase; + +/** + * @covers \Drupal\Tests\auto_updates\Kernel\ReadinessValidation\SettingsValidatorTest + * + * @group auto_updates + */ +class SettingsValidatorTest extends AutoUpdatesKernelTestBase { + + /** + * {@inheritdoc} + */ + protected static $modules = [ + 'auto_updates', + 'package_manager', + 'package_manager_bypass', + ]; + + /** + * Data provider for ::testSettingsValidation(). + * + * @return array[] + * Sets of arguments to pass to the test method. + */ + public function providerSettingsValidation(): array { + $result = ValidationResult::createError([ + 'The <code>update_fetch_with_http_fallback</code> setting must be disabled for automatic updates.', + ]); + + return [ + [TRUE, [$result]], + [FALSE, []], + ]; + } + + /** + * Tests settings validation before starting an update. + * + * @param bool $setting + * The value of the update_fetch_with_http_fallback setting. + * @param \Drupal\package_manager\ValidationResult[] $expected_results + * The expected validation results. + * + * @dataProvider providerSettingsValidation + */ + public function testSettingsValidation(bool $setting, array $expected_results): void { + $this->setSetting('update_fetch_with_http_fallback', $setting); + + $this->assertCheckerResultsFromManager($expected_results, TRUE); + try { + $this->container->get('auto_updates.updater')->begin([ + 'drupal' => '9.8.1', + ]); + // If there was no exception, ensure we're not expecting any errors. + $this->assertSame([], $expected_results); + } + catch (StageValidationException $e) { + $this->assertValidationResultsEqual($expected_results, $e->getResults()); + } + } + +} diff --git a/core/modules/package_manager/package_manager.services.yml b/core/modules/package_manager/package_manager.services.yml index 803c02b7c61ffdd2c92efc86d5bdec5fd83e9f66..d2b01442f16af05d284209fe16f8e0cf5a73f0ff 100644 --- a/core/modules/package_manager/package_manager.services.yml +++ b/core/modules/package_manager/package_manager.services.yml @@ -118,6 +118,12 @@ services: - '@string_translation' tags: - { name: event_subscriber } + package_manager.validator.composer_settings: + class: Drupal\package_manager\EventSubscriber\ComposerSettingsValidator + arguments: + - '@string_translation' + tags: + - { name: event_subscriber } package_manager.excluded_paths_subscriber: class: Drupal\package_manager\EventSubscriber\ExcludedPathsSubscriber arguments: diff --git a/core/modules/package_manager/src/ComposerUtility.php b/core/modules/package_manager/src/ComposerUtility.php index 6460b9e13c2aea78085a0468ae5220deb0333b4a..6a111b74a911b77919483aa09bf9673d5d91030d 100644 --- a/core/modules/package_manager/src/ComposerUtility.php +++ b/core/modules/package_manager/src/ComposerUtility.php @@ -37,6 +37,16 @@ public function __construct(Composer $composer) { $this->composer = $composer; } + /** + * Returns the underlying Composer instance. + * + * @return \Composer\Composer + * The Composer instance. + */ + public function getComposer(): Composer { + return $this->composer; + } + /** * Creates a utility object using the files in a given directory. * diff --git a/core/modules/package_manager/src/EventSubscriber/ComposerSettingsValidator.php b/core/modules/package_manager/src/EventSubscriber/ComposerSettingsValidator.php new file mode 100644 index 0000000000000000000000000000000000000000..64c7249564e83520e06f7cc857311859ebaf640d --- /dev/null +++ b/core/modules/package_manager/src/EventSubscriber/ComposerSettingsValidator.php @@ -0,0 +1,54 @@ +<?php + +namespace Drupal\package_manager\EventSubscriber; + +use Drupal\Core\StringTranslation\StringTranslationTrait; +use Drupal\Core\StringTranslation\TranslationInterface; +use Drupal\package_manager\Event\PreCreateEvent; +use Drupal\package_manager\Event\PreOperationStageEvent; + +/** + * Validates certain Composer settings. + */ +class ComposerSettingsValidator implements PreOperationStageValidatorInterface { + + use StringTranslationTrait; + + /** + * Constructs a ComposerSettingsValidator object. + * + * @param \Drupal\Core\StringTranslation\TranslationInterface $translation + * The string translation service. + */ + public function __construct(TranslationInterface $translation) { + $this->setStringTranslation($translation); + } + + /** + * {@inheritdoc} + */ + public function validateStagePreOperation(PreOperationStageEvent $event): void { + $config = $event->getStage() + ->getActiveComposer() + ->getComposer() + ->getConfig(); + + if ($config->get('secure-http') !== TRUE) { + $event->addError([ + $this->t('HTTPS must be enabled for Composer downloads. See <a href=":url">the Composer documentation</a> for more information.', [ + ':url' => 'https://getcomposer.org/doc/06-config.md#secure-http', + ]), + ]); + } + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() { + return [ + PreCreateEvent::class => 'validateStagePreOperation', + ]; + } + +} diff --git a/core/modules/package_manager/tests/src/Kernel/ComposerSettingsValidatorTest.php b/core/modules/package_manager/tests/src/Kernel/ComposerSettingsValidatorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..fae703f6e0efa0781cc5c3b2f74ea34c9b02da92 --- /dev/null +++ b/core/modules/package_manager/tests/src/Kernel/ComposerSettingsValidatorTest.php @@ -0,0 +1,100 @@ +<?php + +namespace Drupal\Tests\package_manager\Kernel; + +use Drupal\Component\Serialization\Json; +use Drupal\Core\DependencyInjection\ContainerBuilder; +use Drupal\package_manager\Exception\StageValidationException; +use Drupal\package_manager\PathLocator; +use Drupal\package_manager\ValidationResult; +use org\bovigo\vfs\vfsStream; + +/** + * @covers \Drupal\package_manager\EventSubscriber\ComposerSettingsValidator + * + * @group package_manager + */ +class ComposerSettingsValidatorTest extends PackageManagerKernelTestBase { + + /** + * {@inheritdoc} + */ + protected function disableValidators(ContainerBuilder $container): void { + parent::disableValidators($container); + + // Disable the disk space validator, since it tries to inspect the file + // system in ways that vfsStream doesn't support, like calling stat() and + // disk_free_space(). + $container->removeDefinition('package_manager.validator.disk_space'); + + // Disable the lock file validator, since the mock file system we create in + // this test doesn't have any lock files to validate. + $container->removeDefinition('package_manager.validator.lock_file'); + } + + /** + * Data provider for ::testSecureHttpValidation(). + * + * @return array[] + * Sets of arguments to pass to the test method. + */ + public function providerSecureHttpValidation(): array { + $error = ValidationResult::createError([ + 'HTTPS must be enabled for Composer downloads. See <a href="https://getcomposer.org/doc/06-config.md#secure-http">the Composer documentation</a> for more information.', + ]); + + return [ + 'disabled' => [ + Json::encode([ + 'config' => [ + 'secure-http' => FALSE, + ], + ]), + [$error], + ], + 'explicitly enabled' => [ + Json::encode([ + 'config' => [ + 'secure-http' => TRUE, + ], + ]), + [], + ], + 'implicitly enabled' => [ + '{}', + [], + ], + ]; + } + + /** + * Tests that Composer's secure-http setting is validated. + * + * @param string $contents + * The contents of the composer.json file. + * @param \Drupal\package_manager\ValidationResult[] $expected_results + * The expected validation results, if any. + * + * @dataProvider providerSecureHttpValidation + */ + public function testSecureHttpValidation(string $contents, array $expected_results): void { + $file = vfsStream::newFile('composer.json')->setContent($contents); + $this->vfsRoot->addChild($file); + + $active_dir = $this->vfsRoot->url(); + $locator = $this->prophesize(PathLocator::class); + $locator->getActiveDirectory()->willReturn($active_dir); + $locator->getProjectRoot()->willReturn($active_dir); + $locator->getVendorDirectory()->willReturn($active_dir); + $this->container->set('package_manager.path_locator', $locator->reveal()); + + try { + $this->createStage()->create(); + $this->assertSame([], $expected_results); + } + catch (StageValidationException $e) { + $this->assertValidationResultsEqual($expected_results, $e->getResults()); + } + } + +} diff --git a/core/modules/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php b/core/modules/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php index c4ee293ee0cace75c07a900acd227c0994807fcf..ea15bb086ef4e3816f74f0ef7288ed275766a7be 100644 --- a/core/modules/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php +++ b/core/modules/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php @@ -33,9 +33,11 @@ public function register(ContainerBuilder $container) { protected function disableValidators(ContainerBuilder $container): void { parent::disableValidators($container); - // Disable the lock file validator, since in this test we are validating an - // imaginary file system which doesn't have any lock files. + // Disable the lock file and Composer settings validators, since in this + // test we are validating an imaginary file system which doesn't have any + // Composer files. $container->removeDefinition('package_manager.validator.lock_file'); + $container->removeDefinition('package_manager.validator.composer_settings'); } /** diff --git a/core/modules/package_manager/tests/src/Kernel/LockFileValidatorTest.php b/core/modules/package_manager/tests/src/Kernel/LockFileValidatorTest.php index 62c0142462432c6237f26c06f5df49c11d16d543..c71a6d27e46af85c4cba3296a14554184ab09e98 100644 --- a/core/modules/package_manager/tests/src/Kernel/LockFileValidatorTest.php +++ b/core/modules/package_manager/tests/src/Kernel/LockFileValidatorTest.php @@ -45,6 +45,10 @@ protected function disableValidators(ContainerBuilder $container): void { // system in ways that vfsStream doesn't support, like calling stat() and // disk_free_space(). $container->removeDefinition('package_manager.validator.disk_space'); + + // Disable the Composer settings validator, since it tries to read Composer + // files that may not exist in this test. + $container->removeDefinition('package_manager.validator.composer_settings'); } /** diff --git a/core/modules/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php b/core/modules/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php index d5db9b1afbb01f7a357817e1c220b1c38cd05b31..d6e504ea138c38d28cad20ff73ad518f8524f027 100644 --- a/core/modules/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php +++ b/core/modules/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php @@ -44,9 +44,11 @@ protected function disableValidators(ContainerBuilder $container): void { // disk_free_space(). $container->removeDefinition('package_manager.validator.disk_space'); - // Disable the lock file validator, since the mock file system we create in - // this test doesn't have any lock files to validate. + // Disable the lock file and Composer settings validators, since in this + // test we are validating an imaginary file system which doesn't have any + // Composer files. $container->removeDefinition('package_manager.validator.lock_file'); + $container->removeDefinition('package_manager.validator.composer_settings'); } /**