CLI-1415: [auth:acsf-login] validate factory URL (#1827)

* CLI-1415: [auth:acsf-login] validate factory URL * add tests * fix tests * fix tests * fix mutants * kill mutant
acquia · Nov 14, 2024 · 489a8b4 · 489a8b4
1 parent b12cc94
commit 489a8b4
Show file tree

Hide file tree

Showing 6 changed files with 131 additions and 6 deletions.
diff --git a/src/Command/Auth/AuthAcsfLoginCommand.php b/src/Command/Auth/AuthAcsfLoginCommand.php
@@ -22,10 +22,14 @@ protected function configure(): void
             ->addOption('factory-url', 'f', InputOption::VALUE_REQUIRED, "Your Site Factory URL (including https://)");
     }
 
+    /**
+     * @throws \Acquia\Cli\Exception\AcquiaCliException
+     */
     protected function execute(InputInterface $input, OutputInterface $output): int
     {
         if ($input->getOption('factory-url')) {
             $factoryUrl = $input->getOption('factory-url');
+            self::validateUrl($factoryUrl);
         } elseif ($input->isInteractive() && $this->datastoreCloud->get('acsf_factories')) {
             $factories = $this->datastoreCloud->get('acsf_factories');
             $factoryChoices = $factories;
@@ -37,7 +41,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             ];
             $factory = $this->promptChooseFromObjectsOrArrays($factoryChoices, 'url', 'url', 'Choose a Factory to login to');
             if ($factory['url'] === 'Enter a new factory URL') {
-                $factoryUrl = $this->io->ask('Enter the full URL of the factory');
+                $factoryUrl = $this->determineOption('factory-url', false, $this->validateUrl(...));
                 $factory = [
                     'url' => $factoryUrl,
                     'users' => [],
@@ -61,11 +65,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 return Command::SUCCESS;
             }
         } else {
-            $factoryUrl = $this->determineOption('factory-url');
+            $factoryUrl = $this->determineOption('factory-url', false, $this->validateUrl(...));
         }
 
         $username = $this->determineOption('username');
-        $key = $this->determineOption('key', true);
+        $key = $this->determineOption('key', true, $this->validateApiKey(...));
 
         $this->writeAcsfCredentialsToDisk($factoryUrl, $username, $key);
         $output->writeln("<info>Saved credentials</info>");

diff --git a/src/Command/CommandBase.php b/src/Command/CommandBase.php
@@ -67,6 +67,7 @@
 use Symfony\Component\Validator\Constraints\Length;
 use Symfony\Component\Validator\Constraints\NotBlank;
 use Symfony\Component\Validator\Constraints\Regex;
+use Symfony\Component\Validator\Constraints\Url;
 use Symfony\Component\Validator\Exception\ValidatorException;
 use Symfony\Component\Validator\Validation;
 use Symfony\Component\Yaml\Yaml;
@@ -610,6 +611,9 @@ private function promptChooseDatabases(
         return [$environmentDatabases[$chosenDatabaseIndex]];
     }
 
+    /**
+     * @throws \Acquia\Cli\Exception\AcquiaCliException
+     */
     protected function determineEnvironment(InputInterface $input, OutputInterface $output, bool $allowProduction = false, bool $allowNode = false): array|string|EnvironmentResponse
     {
         if ($input->getArgument('environmentId')) {
@@ -628,6 +632,10 @@ protected function determineEnvironment(InputInterface $input, OutputInterface $
     }
 
     // Todo: obviously combine this with promptChooseEnvironment.
+
+    /**
+     * @throws \Acquia\Cli\Exception\AcquiaCliException
+     */
     private function promptChooseEnvironmentConsiderProd(Client $acquiaCloudClient, string $applicationUuid, bool $allowProduction, bool $allowNode): EnvironmentResponse
     {
         $environmentResource = new Environments($acquiaCloudClient);
@@ -1657,12 +1665,15 @@ protected function promptOpenBrowserToCreateToken(
         }
     }
 
+    /**
+     * @throws \Acquia\Cli\Exception\AcquiaCliException
+     */
     protected function determineApiKey(): string
     {
         return $this->determineOption('key', false, $this->validateApiKey(...));
     }
 
-    private function validateApiKey(mixed $key): string
+    protected static function validateApiKey(mixed $key): string
     {
         $violations = Validation::createValidator()->validate($key, [
             new Length(['min' => 10]),
@@ -1678,6 +1689,18 @@ private function validateApiKey(mixed $key): string
         return $key;
     }
 
+    protected static function validateUrl(?string $url): string
+    {
+        $violations = Validation::createValidator()->validate($url, [
+            new NotBlank(),
+            new Url(),
+        ]);
+        if (count($violations)) {
+            throw new ValidatorException($violations->get(0)->getMessage());
+        }
+        return $url;
+    }
+
     protected function determineApiSecret(): string
     {
         return $this->determineOption('secret', true, $this->validateApiKey(...));

diff --git a/src/Command/Email/ConfigurePlatformEmailCommand.php b/src/Command/Email/ConfigurePlatformEmailCommand.php
@@ -225,7 +225,7 @@ private function addDomainToSubscriptionApplications(Client $client, Subscriptio
     /**
      * Validates the URL entered as the base domain name.
      */
-    public static function validateUrl(string $url): string
+    protected static function validateUrl(?string $url): string
     {
         $constraintsList = [new NotBlank()];
         $urlParts = parse_url($url);

diff --git a/src/Command/Remote/DrushCommand.php b/src/Command/Remote/DrushCommand.php
@@ -31,6 +31,9 @@ protected function configure(): void
             ->addUsage('myapp.dev -- status --fields=db-status');
     }
 
+    /**
+     * @throws \Acquia\Cli\Exception\AcquiaCliException
+     */
     protected function execute(InputInterface $input, OutputInterface $output): ?int
     {
         $environment = $this->determineEnvironment($input, $output, true);

diff --git a/tests/phpunit/src/Commands/Acsf/AcsfAuthLoginCommandTest.php b/tests/phpunit/src/Commands/Acsf/AcsfAuthLoginCommandTest.php
@@ -9,6 +9,8 @@
 use Acquia\Cli\Command\CommandBase;
 use Acquia\Cli\Config\CloudDataConfig;
 use Acquia\Cli\DataStore\CloudDataStore;
+use Symfony\Component\Console\Exception\MissingInputException;
+use Symfony\Component\Validator\Exception\ValidatorException;
 
 /**
  * @property \Acquia\Cli\Command\Auth\AuthLoginCommand $command
@@ -85,6 +87,37 @@ public static function providerTestAuthLoginCommand(): array
                 // $config.
                 AcsfCommandTestBase::getAcsfCredentialsFileContents(),
             ],
+            // Data set 3.
+            [
+                true,
+                [
+                    'Enter a new factory URL',
+                    self::$acsfCurrentFactoryUrl,
+                    'Enter a new user',
+                    self::$acsfUsername,
+                    self::$acsfKey,
+                ],
+                [],
+                // Output to assert.
+                'Enter your Site Factory URL (including https://) (option -f, --factory-url):',
+                // $config.
+                AcsfCommandTestBase::getAcsfCredentialsFileContents(),
+            ],
+            // Data set 4.
+            [
+                false,
+                [
+                    'Enter a new factory URL',
+                    self::$acsfCurrentFactoryUrl,
+                    self::$acsfUsername,
+                    self::$acsfKey,
+                ],
+                [],
+                // Output to assert.
+                'Enter your Site Factory URL (including https://) (option -f, --factory-url):',
+                // $config.
+                AcsfCommandTestBase::getAcsfCredentialsFileContents(),
+            ],
         ];
     }
 
@@ -117,6 +150,68 @@ public function testAcsfAuthLoginCommand(bool $machineIsAuthenticated, array $in
         $this->assertEquals(self::$acsfCurrentFactoryUrl, $this->cloudCredentials->getBaseUri());
     }
 
+    /**
+     * @return string[][]
+     */
+    public static function providerTestAcsfAuthLoginInvalid(): array
+    {
+        return [
+            [
+                ['--factory-url' => 'example.com', '--key' => 'asdfasdfasdf','--username' => 'asdf'],
+                'This value is not a valid URL.',
+            ],
+            [
+                ['--factory-url' => 'https://example.com', '--key' => 'asdf','--username' => 'asdf'],
+                'This value is too short. It should have 10 characters or more.',
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider providerTestAcsfAuthLoginInvalid
+     * @throws \Exception
+     */
+    public function testAcsfAuthLoginInvalid(array $args, string $message): void
+    {
+        $this->clientServiceProphecy->isMachineAuthenticated()
+            ->willReturn(false);
+        $this->removeMockCloudConfigFile();
+        $this->createDataStores();
+        $this->command = $this->createCommand();
+        $this->expectException(ValidatorException::class);
+        $this->expectExceptionMessage($message);
+        $this->executeCommand($args);
+    }
+
+    /**
+     * @return string[][]
+     */
+    public static function providerTestAcsfAuthLoginInvalidInput(): array
+    {
+        return [
+            [
+                ['Enter a new factory URL', 'example.com'],
+            ],
+            [
+                ['Enter a new factory URL', ''],
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider providerTestAcsfAuthLoginInvalidInput
+     * @throws \JsonException
+     */
+    public function testAcsfAuthLoginInvalidInput(array $input): void
+    {
+        $this->removeMockCloudConfigFile();
+        $this->createMockCloudConfigFile(AcsfCommandTestBase::getAcsfCredentialsFileContents());
+        $this->createDataStores();
+        $this->command = $this->createCommand();
+        $this->expectException(MissingInputException::class);
+        $this->executeCommand([], $input);
+    }
+
     protected function assertKeySavedCorrectly(): void
     {
         $credsFile = $this->cloudConfigFilepath;

diff --git a/tests/phpunit/src/Commands/Acsf/AcsfCommandTestBase.php b/tests/phpunit/src/Commands/Acsf/AcsfCommandTestBase.php
@@ -22,7 +22,7 @@ abstract class AcsfCommandTestBase extends CommandTestBase
 
     protected static string $acsfUsername = 'tester';
 
-    protected static string $acsfKey = 'h@x0r';
+    protected static string $acsfKey = 'abcdefghijklmnop';
 
     protected string $apiCommandPrefix = 'acsf';