From 489a8b45664dc0242fd9155660f11c87d0a3537b Mon Sep 17 00:00:00 2001 From: Dane Powell Date: Thu, 14 Nov 2024 14:41:43 -0800 Subject: [PATCH] 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 --- src/Command/Auth/AuthAcsfLoginCommand.php | 10 +- src/Command/CommandBase.php | 25 ++++- .../Email/ConfigurePlatformEmailCommand.php | 2 +- src/Command/Remote/DrushCommand.php | 3 + .../Acsf/AcsfAuthLoginCommandTest.php | 95 +++++++++++++++++++ .../src/Commands/Acsf/AcsfCommandTestBase.php | 2 +- 6 files changed, 131 insertions(+), 6 deletions(-) diff --git a/src/Command/Auth/AuthAcsfLoginCommand.php b/src/Command/Auth/AuthAcsfLoginCommand.php index 3011e9b34..9f32ab102 100644 --- 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("Saved credentials"); diff --git a/src/Command/CommandBase.php b/src/Command/CommandBase.php index 6b4632a60..fb1583bd9 100644 --- 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 index 368c60707..85556aa59 100644 --- 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 index 36e7c52fa..2bc1f906c 100644 --- 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 index 050f6ae81..de6a706ec 100644 --- 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 index 4265046ec..745619ca3 100644 --- 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';