Issue #3351247 by omkar.podey, phenaproxima, tedbow, Wim Leers, catch: Harden our HTTPS requirement

package_manager/src/Validator/ComposerSettingsValidator.php

@@ -44,20 +44,37 @@ final class ComposerSettingsValidator implements EventSubscriberInterface {
       ? $event->stage->getStageDirectory()
       : $this->pathLocator->getProjectRoot();
 
-    try {
-      $setting = ComposerInspector::toBoolean($this->inspector->getConfig('secure-http', $dir) ?: '0');
+    $settings = [];
+    foreach (['disable-tls', 'secure-http'] as $key) {
+      try {
+        $settings[$key] = ComposerInspector::toBoolean($this->inspector->getConfig($key, $dir) ?: '0');
+      }
+      catch (\Throwable $throwable) {
+        $event->addErrorFromThrowable($throwable, $this->t('Unable to determine Composer <code>@key</code> setting.', [
+          '@key' => $key,
+        ]));
+        return;
+      }
     }
-    catch (\Throwable $throwable) {
-      $event->addErrorFromThrowable($throwable, $this->t('Unable to determine Composer <code>secure-http</code> setting.'));
-      return;
+
+    // If disable-tls is enabled, it overrides secure-http and sets its value to
+    // FALSE, even if secure-http is set to TRUE explicitly.
+    $messages = [];
+    if ($settings['disable-tls'] === TRUE) {
+      $messages[] = $this->t('TLS must be enabled for HTTPS Composer downloads. See <a href=":url">the Composer documentation</a> for more information.', [
+        ':url' => 'https://getcomposer.org/doc/06-config.md#disable-tls',
+      ]);
+      $messages[] = $this->t('You should also check the value of <code>secure-http</code> and make sure that it is set to <code>true</code> or not set at all.');
     }
-    if ($setting === FALSE) {
-      $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',
-        ]),
+    elseif ($settings['secure-http'] !== TRUE) {
+      $messages[] = $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',
       ]);
     }
+
+    if ($messages) {
+      $event->addError($messages, $this->t("Composer settings don't satisfy Package Manager's requirements."));
+    }
   }
 
   /**

package_manager/tests/src/Kernel/ComposerSettingsValidatorTest.php

@@ -17,67 +17,115 @@ use Drupal\package_manager\ValidationResult;
 class ComposerSettingsValidatorTest extends PackageManagerKernelTestBase {
 
   /**
-   * Data provider for testSecureHttpValidation().
+   * Data provider for testComposerSettingsValidation().
    *
    * @return mixed[][]
    *   The test cases.
    */
-  public function providerSecureHttpValidation(): array {
-    $error = ValidationResult::createError([
+  public function providerComposerSettingsValidation(): array {
+    $summary = t("Composer settings don't satisfy Package Manager's requirements.");
+
+    $secure_http_error = ValidationResult::createError([
       t('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.'),
-    ]);
+    ], $summary);
+    $tls_error = ValidationResult::createError([
+      t('TLS must be enabled for HTTPS Composer downloads. See <a href="https://getcomposer.org/doc/06-config.md#disable-tls">the Composer documentation</a> for more information.'),
+      t('You should also check the value of <code>secure-http</code> and make sure that it is set to <code>true</code> or not set at all.'),
+    ], $summary);
 
     return [
-      'disabled' => [
+      'secure-http set to FALSE' => [
         [
           'secure-http' => FALSE,
         ],
-        [$error],
+        [$secure_http_error],
       ],
-      'explicitly enabled' => [
+      'secure-http explicitly set to TRUE' => [
         [
           'secure-http' => TRUE,
         ],
         [],
       ],
-      'implicitly enabled' => [
+      'secure-http implicitly set to TRUE' => [
         [
           'extra.unrelated' => TRUE,
         ],
         [],
       ],
+      'disable-tls set to TRUE' => [
+        [
+          'disable-tls' => TRUE,
+        ],
+        [$tls_error],
+      ],
+      'disable-tls implicitly set to FALSE' => [
+        [
+          'extra.unrelated' => TRUE,
+        ],
+        [],
+      ],
+      'explicitly set disable-tls to FALSE' => [
+        [
+          'disable-tls' => FALSE,
+        ],
+        [],
+      ],
+      'disable-tls set to TRUE + secure-http set to TRUE, message only for TLS, secure-http overridden' => [
+        [
+          'disable-tls' => TRUE,
+          'secure-http' => TRUE,
+        ],
+        [$tls_error],
+      ],
+      'disable-tls set to TRUE + secure-http set to FALSE, message only for TLS' => [
+        [
+          'disable-tls' => TRUE,
+          'secure-http' => FALSE,
+        ],
+        [$tls_error],
+      ],
     ];
   }
 
   /**
-   * Tests that Composer's secure-http setting is validated.
+   * Tests that Composer's settings are validated.
    *
    * @param array $config
    *   The config to set.
    * @param \Drupal\package_manager\ValidationResult[] $expected_results
    *   The expected validation results, if any.
    *
-   * @dataProvider providerSecureHttpValidation
+   * @dataProvider providerComposerSettingsValidation
    */
-  public function testSecureHttpValidation(array $config, array $expected_results): void {
+  public function testComposerSettingsValidation(array $config, array $expected_results): void {
     (new ActiveFixtureManipulator())->addConfig($config)->commitChanges();
     $this->assertStatusCheckResults($expected_results);
     $this->assertResults($expected_results, PreCreateEvent::class);
   }
 
   /**
-   * Tests that Composer's secure-http setting is validated during pre-apply.
+   * Tests that Composer's settings are validated during pre-apply.
    *
    * @param array $config
    *   The config to set.
    * @param \Drupal\package_manager\ValidationResult[] $expected_results
    *   The expected validation results, if any.
    *
-   * @dataProvider providerSecureHttpValidation
+   * @dataProvider providerComposerSettingsValidation
    */
-  public function testSecureHttpValidationDuringPreApply(array $config, array $expected_results): void {
+  public function testComposerSettingsValidationDuringPreApply(array $config, array $expected_results): void {
     $this->getStageFixtureManipulator()->addConfig($config);
     $this->assertResults($expected_results, PreApplyEvent::class);
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function tearDown(): void {
+    // Ensure that any warnings arising from Composer settings (which we expect
+    // in this test) will not fail the test during tear-down.
+    $this->failureLogger->reset();
+    parent::tearDown();
+  }
+
 }

package_manager/tests/src/Kernel/LockFileValidatorTest.php

@@ -52,6 +52,7 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase {
     $arguments = Argument::cetera();
     $inspector->getConfig('allow-plugins', $arguments)->willReturn('[]');
     $inspector->getConfig('secure-http', $arguments)->willReturn('1');
+    $inspector->getConfig('disable-tls', $arguments)->willReturn('0');
     $inspector->getConfig('extra', $arguments)->willReturn('{}');
     $inspector->getConfig('minimum-stability', $arguments)->willReturn('stable');
     $inspector->getInstalledPackagesList($arguments)->willReturn(new InstalledPackagesList());