From 08407f1f8467c4608ab0f9075ac7862ad8912cbe Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Wed, 16 Sep 2026 16:36:10 +0530 Subject: [PATCH 01/16] enabled device-code for authentication --- config/prod/services.yml | 11 + src/AcsfApi/AcsfCredentials.php | 5 + src/ApiCredentialsInterface.php | 2 + src/CloudApi/ClientService.php | 3 +- src/CloudApi/CloudCredentials.php | 22 +- src/CloudApi/ConnectorFactory.php | 15 + src/CloudApi/DeviceTokenRefresher.php | 81 ++++++ src/Command/Auth/AuthLoginCommand.php | 219 ++++++++++++++- src/Command/Auth/AuthLogoutCommand.php | 38 ++- src/Config/CloudDataConfig.php | 8 + src/Helpers/TelemetryHelper.php | 3 + .../src/CloudApi/DeviceTokenRefresherTest.php | 192 +++++++++++++ .../Commands/Auth/AuthLoginCommandTest.php | 259 +++++++++++++++++- 13 files changed, 837 insertions(+), 21 deletions(-) create mode 100644 src/CloudApi/DeviceTokenRefresher.php create mode 100644 tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php diff --git a/config/prod/services.yml b/config/prod/services.yml index 5e5c41921..5eac97e6c 100644 --- a/config/prod/services.yml +++ b/config/prod/services.yml @@ -73,14 +73,21 @@ services: Acquia\Cli\ApiCredentialsInterface: alias: Acquia\Cli\CloudApi\CloudCredentials + Acquia\Cli\CloudApi\DeviceTokenRefresher: ~ + cloud.credentials: class: Acquia\Cli\CloudApi\CloudCredentials + arguments: + $deviceTokenRefresher: '@Acquia\Cli\CloudApi\DeviceTokenRefresher' acsf.credentials: class: Acquia\Cli\AcsfApi\AcsfCredentials # AcquiaCloudApi services. + Acquia\Cli\Command\Api\ApiCommandHelper: ~ + Acquia\Cli\Command\Api\ApiV3CommandHelper: ~ Acquia\Cli\Command\Api\ApiCommandFactory: ~ + Acquia\Cli\Command\Api\ApiV3CommandFactory: ~ Acquia\Cli\Command\Api\ApiBaseCommand: class: Acquia\Cli\Command\Api\ApiBaseCommand factory: ['@Acquia\Cli\Command\Api\ApiCommandFactory', 'createCommand'] @@ -100,6 +107,8 @@ services: secret: '@=service("cloud.credentials").getCloudSecret()' accessToken: '@=service("cloud.credentials").getCloudAccessToken()' accessTokenExpiry: '@=service("cloud.credentials").getCloudAccessTokenExpiry()' + deviceAccessToken: '@=service("cloud.credentials").getCloudDeviceAccessToken()' + deviceAccessTokenExpiry: '@=service("cloud.credentials").getCloudDeviceTokenExpiry()' $baseUri: '@=service("cloud.credentials").getBaseUri()' $accountsUri: '@=service("cloud.credentials").getAccountsUri()' @@ -113,6 +122,8 @@ services: secret: '@=service("cloud.credentials").getCloudSecret()' accessToken: '@=service("cloud.credentials").getCloudAccessToken()' accessTokenExpiry: '@=service("cloud.credentials").getCloudAccessTokenExpiry()' + deviceAccessToken: '@=service("cloud.credentials").getCloudDeviceAccessToken()' + deviceAccessTokenExpiry: '@=service("cloud.credentials").getCloudDeviceTokenExpiry()' $baseUri: '@=service("cloud.credentials").getV3BaseUri()' $accountsUri: '@=service("cloud.credentials").getAccountsUri()' diff --git a/src/AcsfApi/AcsfCredentials.php b/src/AcsfApi/AcsfCredentials.php index 971e98bdf..21ab5df72 100644 --- a/src/AcsfApi/AcsfCredentials.php +++ b/src/AcsfApi/AcsfCredentials.php @@ -76,4 +76,9 @@ public function getBaseUri(): ?string return null; } + + public function getCloudDeviceAccessToken(): ?string + { + return null; + } } diff --git a/src/ApiCredentialsInterface.php b/src/ApiCredentialsInterface.php index 1224e1db0..e0e1c3866 100644 --- a/src/ApiCredentialsInterface.php +++ b/src/ApiCredentialsInterface.php @@ -11,4 +11,6 @@ public function getBaseUri(): ?string; public function getCloudKey(): ?string; public function getCloudSecret(): ?string; + + public function getCloudDeviceAccessToken(): ?string; } diff --git a/src/CloudApi/ClientService.php b/src/CloudApi/ClientService.php index 357dda551..03eca4406 100644 --- a/src/CloudApi/ClientService.php +++ b/src/CloudApi/ClientService.php @@ -77,7 +77,8 @@ protected function checkAuthentication(): bool { return ( $this->credentials->getCloudAccessToken() || - ($this->credentials->getCloudKey() && $this->credentials->getCloudSecret()) + ($this->credentials->getCloudKey() && $this->credentials->getCloudSecret()) || + ($this->credentials->getCloudDeviceAccessToken() !== null) ); } } diff --git a/src/CloudApi/CloudCredentials.php b/src/CloudApi/CloudCredentials.php index 79c906354..757bfe73f 100644 --- a/src/CloudApi/CloudCredentials.php +++ b/src/CloudApi/CloudCredentials.php @@ -10,11 +10,25 @@ class CloudCredentials implements ApiCredentialsInterface { - /** - * CloudCredentials constructor. - */ - public function __construct(private CloudDataStore $datastoreCloud) + public function __construct( + private CloudDataStore $datastoreCloud, + private ?DeviceTokenRefresher $deviceTokenRefresher = null, + ) { + } + + public function getCloudDeviceAccessToken(): ?string + { + if ($this->deviceTokenRefresher !== null) { + return $this->deviceTokenRefresher->getValidAccessToken(); + } + $stored = $this->datastoreCloud->get('device_token'); + return $stored['access_token'] ?? null; + } + + public function getCloudDeviceTokenExpiry(): ?int { + $stored = $this->datastoreCloud->get('device_token'); + return isset($stored['expiry']) ? (int) $stored['expiry'] : null; } public function getCloudAccessToken(): ?string diff --git a/src/CloudApi/ConnectorFactory.php b/src/CloudApi/ConnectorFactory.php index 74cd33fe5..66b4f8044 100644 --- a/src/CloudApi/ConnectorFactory.php +++ b/src/CloudApi/ConnectorFactory.php @@ -40,6 +40,21 @@ private function buildConnector(): ConnectorInterface return new Connector($this->config, $this->baseUri, $this->accountsUri); } + // Device code token path. + if (!empty($this->config['deviceAccessToken'])) { + $accessToken = new AccessToken([ + 'access_token' => $this->config['deviceAccessToken'], + 'expires' => $this->config['deviceAccessTokenExpiry'] ?? 0, + ]); + if (!$accessToken->hasExpired()) { + return new AccessTokenConnector([ + 'access_token' => $accessToken, + 'key' => null, + 'secret' => null, + ], $this->baseUri, $this->accountsUri); + } + } + // Fall back to a valid access token. if ($this->config['accessToken']) { $accessToken = $this->createAccessToken(); diff --git a/src/CloudApi/DeviceTokenRefresher.php b/src/CloudApi/DeviceTokenRefresher.php new file mode 100644 index 000000000..90f895440 --- /dev/null +++ b/src/CloudApi/DeviceTokenRefresher.php @@ -0,0 +1,81 @@ + 15]), + ) { + } + + /** + * Returns a valid device access token, refreshing silently if expired. + * + * Returns null when no token is stored, the refresh token is missing, or + * the refresh request fails — callers should fall back to other auth methods. + */ + public function getValidAccessToken(): ?string + { + $stored = $this->datastore->get('device_token'); + if (!$stored || empty($stored['access_token'])) { + return null; + } + + $token = new AccessToken([ + 'access_token' => $stored['access_token'], + 'expires' => $stored['expiry'] ?? 0, + ]); + + if (!$token->hasExpired()) { + return $stored['access_token']; + } + + // Access token expired — attempt a silent refresh. + $refreshToken = $stored['refresh_token'] ?? null; + $clientId = $stored['client_id'] ?? null; + $domain = getenv('ACLI_OKTA_DOMAIN'); + $authServer = getenv('ACLI_OKTA_AUTH_SERVER_ID'); + + if (!$refreshToken || !$clientId || !$domain || !$authServer) { + return null; + } + + try { + $response = $this->httpClient->post( + sprintf('https://%s/oauth2/%s/v1/token', $domain, $authServer), + [ + 'form_params' => [ + 'client_id' => $clientId, + 'grant_type' => 'refresh_token', + 'refresh_token' => $refreshToken, + ], + ] + ); + $new = json_decode((string) $response->getBody(), true); + } catch (GuzzleException) { + return null; + } + + if (empty($new['access_token'])) { + return null; + } + + $this->datastore->set('device_token', [ + 'access_token' => $new['access_token'], + 'client_id' => $clientId, + 'expiry' => isset($new['expires_in']) ? time() + (int) $new['expires_in'] : 0, + 'refresh_token' => $new['refresh_token'] ?? $refreshToken, + ]); + + return $new['access_token']; + } +} diff --git a/src/Command/Auth/AuthLoginCommand.php b/src/Command/Auth/AuthLoginCommand.php index 28118ff62..fdba7fef3 100644 --- a/src/Command/Auth/AuthLoginCommand.php +++ b/src/Command/Auth/AuthLoginCommand.php @@ -4,9 +4,21 @@ namespace Acquia\Cli\Command\Auth; +use Acquia\Cli\ApiCredentialsInterface; +use Acquia\Cli\CloudApi\ClientService; use Acquia\Cli\Command\CommandBase; +use Acquia\Cli\DataStore\AcquiaCliDatastore; +use Acquia\Cli\DataStore\CloudDataStore; use Acquia\Cli\Exception\AcquiaCliException; +use Acquia\Cli\Helpers\LocalMachineHelper; +use Acquia\Cli\Helpers\SshHelper; +use Acquia\Cli\Helpers\TelemetryHelper; +use Acquia\DrupalEnvironmentDetector\AcquiaDrupalEnvironmentDetector; use AcquiaCloudApi\Endpoints\Account; +use GuzzleHttp\Client as GuzzleClient; +use GuzzleHttp\Exception\ClientException; +use Psr\Log\LoggerInterface; +use SelfUpdate\SelfUpdateManager; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -16,18 +28,221 @@ #[AsCommand(name: 'auth:login', description: 'Register Cloud Platform API credentials', aliases: ['login'])] final class AuthLoginCommand extends CommandBase { + private const DEVICE_CLIENT_ID = ''; + private const OKTA_DOMAIN = ''; + private const OKTA_AUTH_SERVER_ID = ''; + + public function __construct( + public LocalMachineHelper $localMachineHelper, + protected CloudDataStore $datastoreCloud, + protected AcquiaCliDatastore $datastoreAcli, + protected ApiCredentialsInterface $cloudCredentials, + protected TelemetryHelper $telemetryHelper, + protected string $projectDir, + protected ClientService $cloudApiClientService, + public SshHelper $sshHelper, + protected string $sshDir, + LoggerInterface $logger, + public SelfUpdateManager $selfUpdateManager, + private GuzzleClient $httpClient = new GuzzleClient(['timeout' => 30]), + ) { + parent::__construct($this->localMachineHelper, $this->datastoreCloud, $this->datastoreAcli, $this->cloudCredentials, $this->telemetryHelper, $this->projectDir, $this->cloudApiClientService, $this->sshHelper, $this->sshDir, $logger, $this->selfUpdateManager); + } + protected function configure(): void { $this ->addOption('key', 'k', InputOption::VALUE_REQUIRED, 'Your Cloud Platform API key') ->addOption('secret', 's', InputOption::VALUE_REQUIRED, 'Your Cloud Platform API secret') + ->addOption('use-legacy-auth', null, InputOption::VALUE_NONE, 'Force API key/secret authentication instead of device code flow') ->addOption('environment', null, InputOption::VALUE_REQUIRED, 'Cloud Platform API environment', 'prod') - ->setHelp('Acquia CLI can store multiple sets of credentials in case you have multiple Cloud Platform accounts. However, only a single account can be active at a time. This command allows you to activate a new or existing set of credentials.'); + ->setHelp('Authenticates ACLI with Acquia Cloud Platform. Uses API key/secret when credentials are already stored or passed via --key/--secret. Otherwise uses device code flow (no API key required).'); } protected function execute(InputInterface $input, OutputInterface $output): int { - $env = $input->getOption('environment'); + // Explicit legacy flag or key/secret passed on the command line — always legacy. + if ($input->getOption('use-legacy-auth') || $input->getOption('key') || $input->getOption('secret')) { + return $this->executeLegacyAuth($input, $output); + } + + $keys = $this->datastoreCloud->get('keys'); + $activeKey = $this->datastoreCloud->get('acli_key'); + $deviceToken = $this->datastoreCloud->get('device_token'); + + // Smart routing: existing API key → legacy, device token → device code re-auth. + if ($activeKey && $keys) { + $label = $keys[$activeKey]['label'] ?? $activeKey; + $output->writeln("Already authenticated as $label (API key)"); + + if ($input->isInteractive()) { + $reauth = $this->io->confirm('Re-authenticate?', false); + if (!$reauth) { + return Command::SUCCESS; + } + } + return $this->executeLegacyAuth($input, $output); + } + + if ($deviceToken) { + $output->writeln('Already authenticated via device code'); + + if ($input->isInteractive()) { + $reauth = $this->io->confirm('Re-authenticate?', false); + if (!$reauth) { + return Command::SUCCESS; + } + } + return $this->executeDeviceCodeFlowWithFallback($input, $output); + } + + // No stored credentials — try device code, + // otherwise fall back to legacy key/secret prompt. + if ($this->isDeviceCodeConfigured()) { + return $this->executeDeviceCodeFlowWithFallback($input, $output); + } + + return $this->executeLegacyAuth($input, $output); + } + + private function executeDeviceCodeFlowWithFallback(InputInterface $input, OutputInterface $output): int + { + $result = $this->executeDeviceCodeFlow($output); + if ( + $result !== Command::SUCCESS + && $input->isInteractive() + && $this->io->confirm('Fall back to API key authentication?', false) + ) { + return $this->executeLegacyAuth($input, $output); + } + return $result; + } + + private function isDeviceCodeConfigured(): bool + { + return (bool) (getenv('ACLI_DEVICE_CLIENT_ID') ?: self::DEVICE_CLIENT_ID) + && (bool) (getenv('ACLI_OKTA_DOMAIN') ?: self::OKTA_DOMAIN) + && (bool) (getenv('ACLI_OKTA_AUTH_SERVER_ID') ?: self::OKTA_AUTH_SERVER_ID); + } + + private function executeDeviceCodeFlow(OutputInterface $output): int + { + $clientId = getenv('ACLI_DEVICE_CLIENT_ID') ?: self::DEVICE_CLIENT_ID; + $domain = getenv('ACLI_OKTA_DOMAIN') ?: self::OKTA_DOMAIN; + $authServer = getenv('ACLI_OKTA_AUTH_SERVER_ID') ?: self::OKTA_AUTH_SERVER_ID; + + $baseUrl = sprintf('https://%s/oauth2/%s/v1', $domain, $authServer); + + // Step 1 — initiate: get device_code + user_code. + try { + $response = $this->httpClient->post("$baseUrl/device/authorize", [ + 'form_params' => [ + 'client_id' => $clientId, + 'scope' => 'openid profile email offline_access', + ], + ]); + } catch (ClientException $e) { + $output->writeln('Failed to initiate device code flow: ' . $e->getMessage() . ''); + return Command::FAILURE; + } + + $data = json_decode((string) $response->getBody(), true); + + $deviceCode = $data['device_code']; + $userCode = $data['user_code']; + $verifyUrl = $data['verification_uri']; + $expiresIn = $data['expires_in'] ?? 600; + $interval = $data['interval'] ?? 5; + + // Step 2 — surface the code and URL for the human. + $output->writeln(''); + $output->writeln('Sign in to Acquia ID in your browser:'); + $output->writeln(''); + $output->writeln(" $verifyUrl"); + $output->writeln(''); + $output->writeln('Then enter this code when prompted:'); + $output->writeln(''); + $output->writeln(" $userCode"); + $output->writeln(''); + + if (!AcquiaDrupalEnvironmentDetector::isAhIdeEnv() && $this->io->confirm('Do you want to open this page to sign in now?')) { + $this->localMachineHelper->startBrowser($verifyUrl); + } + + $output->writeln(sprintf('Waiting for authorization... (code expires in %d minutes)', (int) ceil($expiresIn / 60))); + + // Step 3 — poll until approved, denied, or expired. + $deadline = time() + $expiresIn; + while (time() < $deadline) { + sleep($interval); + + try { + $tokenResponse = $this->httpClient->post("$baseUrl/token", [ + 'form_params' => [ + 'client_id' => $clientId, + 'device_code' => $deviceCode, + 'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code', + ], + ]); + $token = json_decode((string) $tokenResponse->getBody(), true); + } catch (ClientException $e) { + $token = json_decode((string) $e->getResponse()->getBody(), true); + } + + $error = $token['error'] ?? ''; + + switch ($error) { + case '': + // Success — store token and exit. + $this->storeDeviceToken($token, $clientId); + $output->writeln(''); + $output->writeln('✓ Authenticated successfully.'); + return Command::SUCCESS; + + case 'authorization_pending': + // Normal — human hasn't approved yet. Keep polling. + break; + + case 'slow_down': + // Okta asked us to back off. + $interval += 5; + break; + + case 'access_denied': + $output->writeln('Authorization denied.'); + return Command::FAILURE; + + case 'expired_token': + $output->writeln('Code expired. Run acli login again.'); + return Command::FAILURE; + + default: + $output->writeln("Unexpected error: $error"); + return Command::FAILURE; + } + } + + $output->writeln('Timed out waiting for authorization.'); + return Command::FAILURE; + } + + private function storeDeviceToken(array $token, string $clientId): void + { + $expiry = isset($token['expires_in']) + ? time() + (int) $token['expires_in'] + : 0; + + $this->datastoreCloud->set('device_token', [ + 'access_token' => $token['access_token'], + 'client_id' => $clientId, + 'expiry' => $expiry, + 'refresh_token' => $token['refresh_token'] ?? null, + ]); + } + + private function executeLegacyAuth(InputInterface $input, OutputInterface $output): int + { + $env = $input->getOption('environment') ?? 'prod'; [$baseUri, $accountsUri] = $this->getUrisForEnvironment($env); $keys = $this->datastoreCloud->get('keys'); diff --git a/src/Command/Auth/AuthLogoutCommand.php b/src/Command/Auth/AuthLogoutCommand.php index 35e48f431..04a30855e 100644 --- a/src/Command/Auth/AuthLogoutCommand.php +++ b/src/Command/Auth/AuthLogoutCommand.php @@ -22,22 +22,34 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { - $keys = $this->datastoreCloud->get('keys'); $activeKey = $this->datastoreCloud->get('acli_key'); - if (!$activeKey) { - throw new AcquiaCliException('There is no active Cloud Platform API key'); + $deviceToken = $this->datastoreCloud->get('device_token'); + + if (!$activeKey && !$deviceToken) { + throw new AcquiaCliException('There are no active Cloud Platform credentials'); + } + + // Remove device token session if present. + if ($deviceToken) { + $this->datastoreCloud->remove('device_token'); + $output->writeln('Device code session removed.'); } - $activeKeyLabel = $keys[$activeKey]['label']; - $output->writeln("The key $activeKeyLabel will be deactivated on this machine. However, the credentials will remain on disk and can be reactivated by running acli auth:login unless you also choose to delete them."); - $delete = $this->determineOption('delete', false, null, null, false); - $this->datastoreCloud->remove('acli_key'); - $action = 'deactivated'; - if ($delete) { - $this->datastoreCloud->remove("keys.$activeKey"); - $action = 'deleted'; + + // Remove legacy API key if active. + if ($activeKey) { + $keys = $this->datastoreCloud->get('keys'); + $activeKeyLabel = $keys[$activeKey]['label']; + $output->writeln("The key $activeKeyLabel will be deactivated on this machine. However, the credentials will remain on disk and can be reactivated by running acli auth:login unless you also choose to delete them."); + $delete = $this->determineOption('delete', false, null, null, false); + $this->datastoreCloud->remove('acli_key'); + $action = 'deactivated'; + if ($delete) { + $this->datastoreCloud->remove("keys.$activeKey"); + $action = 'deleted'; + } + $output->writeln("The active Cloud Platform API credentials were $action"); } - $output->writeln("The active Cloud Platform API credentials were $action"); - $output->writeln('No Cloud Platform API key is active. Run acli auth:login to continue using the Cloud Platform API.'); + $output->writeln('No Cloud Platform credentials are active. Run acli auth:login to authenticate.'); return Command::SUCCESS; } diff --git a/src/Config/CloudDataConfig.php b/src/Config/CloudDataConfig.php index c4eef2c40..9d259fd9e 100644 --- a/src/Config/CloudDataConfig.php +++ b/src/Config/CloudDataConfig.php @@ -66,6 +66,14 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() ->scalarNode('acsf_active_factory')->end() + ->arrayNode('device_token') + ->children() + ->scalarNode('access_token')->end() + ->scalarNode('refresh_token')->end() + ->scalarNode('expiry')->end() + ->scalarNode('client_id')->end() + ->end() + ->end() ->end() ->validate() ->ifTrue(function ($config) { diff --git a/src/Helpers/TelemetryHelper.php b/src/Helpers/TelemetryHelper.php index d3b84df10..ad0c572bd 100644 --- a/src/Helpers/TelemetryHelper.php +++ b/src/Helpers/TelemetryHelper.php @@ -338,6 +338,9 @@ public static function getProviders(): array 'platformsh' => ['PLATFORM_ENVIRONMENT'], 'teamcity' => ['TEAMCITY_VERSION'], 'travis' => ['TRAVIS'], + 'claude-code' => ['CLAUDE_CODE'], + 'cursor' => ['CURSOR_TRACE_ID'], + 'copilot' => ['GITHUB_COPILOT_LANGUAGE_SERVER_VERSION'], ]; } } diff --git a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php new file mode 100644 index 000000000..55ec94445 --- /dev/null +++ b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php @@ -0,0 +1,192 @@ + */ + private array $savedEnvVars = []; + + private ObjectProphecy $datastoreProphecy; + + protected function setUp(): void + { + parent::setUp(); + foreach (['ACLI_OKTA_DOMAIN', 'ACLI_OKTA_AUTH_SERVER_ID'] as $var) { + $this->savedEnvVars[$var] = getenv($var); + putenv($var); + } + } + + protected function tearDown(): void + { + parent::tearDown(); + foreach ($this->savedEnvVars as $var => $value) { + putenv($value === false ? $var : $var . '=' . $value); + } + } + + private function createRefresher(array $responses = []): DeviceTokenRefresher + { + $mock = new MockHandler($responses); + $client = new GuzzleClient(['handler' => HandlerStack::create($mock)]); + $datastoreProphecy = $this->prophet->prophesize(CloudDataStore::class); + $this->datastoreProphecy = $datastoreProphecy; + + return new DeviceTokenRefresher($datastoreProphecy->reveal(), $client); + } + + public function testReturnsNullWhenNoTokenStored(): void + { + $refresher = $this->createRefresher(); + $this->datastoreProphecy->get('device_token')->willReturn(null); + + $this->assertNull($refresher->getValidAccessToken()); + } + + public function testReturnsNullWhenStoredTokenHasNoAccessToken(): void + { + $refresher = $this->createRefresher(); + $this->datastoreProphecy->get('device_token')->willReturn(['access_token' => '']); + + $this->assertNull($refresher->getValidAccessToken()); + } + + public function testReturnsStoredTokenWhenNotExpired(): void + { + $refresher = $this->createRefresher(); + $this->datastoreProphecy->get('device_token')->willReturn([ + 'access_token' => 'valid-token', + 'expiry' => time() + 300, + ]); + + $this->assertEquals('valid-token', $refresher->getValidAccessToken()); + } + + public function testReturnsNullWhenExpiredAndRefreshTokenMissing(): void + { + putenv('ACLI_OKTA_DOMAIN=example.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $refresher = $this->createRefresher(); + $this->datastoreProphecy->get('device_token')->willReturn([ + 'access_token' => 'expired-token', + 'client_id' => 'client-123', + 'expiry' => time() - 300, + ]); + + $this->assertNull($refresher->getValidAccessToken()); + } + + public function testReturnsNullWhenExpiredAndEnvVarsMissing(): void + { + // ACLI_OKTA_DOMAIN / ACLI_OKTA_AUTH_SERVER_ID are unset per setUp(). + $refresher = $this->createRefresher(); + $this->datastoreProphecy->get('device_token')->willReturn([ + 'access_token' => 'expired-token', + 'client_id' => 'client-123', + 'expiry' => time() - 300, + 'refresh_token' => 'refresh-123', + ]); + + $this->assertNull($refresher->getValidAccessToken()); + } + + public function testRefreshesExpiredTokenSuccessfully(): void + { + putenv('ACLI_OKTA_DOMAIN=example.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $refresher = $this->createRefresher([ + new Response(200, [], json_encode([ + 'access_token' => 'new-access-token', + 'expires_in' => 3600, + 'refresh_token' => 'new-refresh-token', + ])), + ]); + $this->datastoreProphecy->get('device_token')->willReturn([ + 'access_token' => 'expired-token', + 'client_id' => 'client-123', + 'expiry' => time() - 300, + 'refresh_token' => 'old-refresh-token', + ]); + $this->datastoreProphecy->set('device_token', Argument::that( + fn (array $token): bool => $token['access_token'] === 'new-access-token' + && $token['client_id'] === 'client-123' + && $token['refresh_token'] === 'new-refresh-token' + && is_int($token['expiry']) + ))->shouldBeCalled(); + + $this->assertEquals('new-access-token', $refresher->getValidAccessToken()); + } + + public function testKeepsOldRefreshTokenWhenResponseOmitsNewOne(): void + { + putenv('ACLI_OKTA_DOMAIN=example.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $refresher = $this->createRefresher([ + new Response(200, [], json_encode([ + 'access_token' => 'new-access-token', + 'expires_in' => 3600, + ])), + ]); + $this->datastoreProphecy->get('device_token')->willReturn([ + 'access_token' => 'expired-token', + 'client_id' => 'client-123', + 'expiry' => time() - 300, + 'refresh_token' => 'old-refresh-token', + ]); + $this->datastoreProphecy->set('device_token', Argument::that( + fn (array $token): bool => $token['access_token'] === 'new-access-token' + && $token['client_id'] === 'client-123' + && $token['refresh_token'] === 'old-refresh-token' + && is_int($token['expiry']) + ))->shouldBeCalled(); + + $this->assertEquals('new-access-token', $refresher->getValidAccessToken()); + } + + public function testReturnsNullWhenRefreshResponseHasNoAccessToken(): void + { + putenv('ACLI_OKTA_DOMAIN=example.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $refresher = $this->createRefresher([ + new Response(200, [], json_encode(['error' => 'invalid_grant'])), + ]); + $this->datastoreProphecy->get('device_token')->willReturn([ + 'access_token' => 'expired-token', + 'client_id' => 'client-123', + 'expiry' => time() - 300, + 'refresh_token' => 'old-refresh-token', + ]); + + $this->assertNull($refresher->getValidAccessToken()); + } + + public function testReturnsNullOnRefreshRequestFailure(): void + { + putenv('ACLI_OKTA_DOMAIN=example.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $refresher = $this->createRefresher([ + new Response(400, [], json_encode(['error' => 'invalid_grant'])), + ]); + $this->datastoreProphecy->get('device_token')->willReturn([ + 'access_token' => 'expired-token', + 'client_id' => 'client-123', + 'expiry' => time() - 300, + 'refresh_token' => 'old-refresh-token', + ]); + + $this->assertNull($refresher->getValidAccessToken()); + } +} diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php index 13ae91c0e..65df9ac7f 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php @@ -10,11 +10,17 @@ use Acquia\Cli\Config\CloudDataConfig; use Acquia\Cli\DataStore\CloudDataStore; use Acquia\Cli\Exception\AcquiaCliException; +use Acquia\Cli\Helpers\LocalMachineHelper; use Acquia\Cli\Tests\CommandTestBase; use AcquiaCloudApi\Connector\Connector; use Generator; +use GuzzleHttp\Client as GuzzleClient; +use GuzzleHttp\Handler\MockHandler; +use GuzzleHttp\HandlerStack; +use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\Attributes\DataProvider; use Prophecy\Argument; +use Prophecy\Prophecy\ObjectProphecy; use Symfony\Component\Validator\Exception\ValidatorException; /** @@ -22,11 +28,262 @@ */ class AuthLoginCommandTest extends CommandTestBase { + /** @var array */ + private array $savedDeviceCodeEnvVars = []; + + protected function setUp(): void + { + parent::setUp(); + foreach (['ACLI_DEVICE_CLIENT_ID', 'ACLI_OKTA_DOMAIN', 'ACLI_OKTA_AUTH_SERVER_ID'] as $var) { + $this->savedDeviceCodeEnvVars[$var] = getenv($var); + } + } + + protected function tearDown(): void + { + parent::tearDown(); + foreach ($this->savedDeviceCodeEnvVars as $var => $value) { + putenv($value === false ? $var : $var . '=' . $value); + } + } + protected function createCommand(): CommandBase { return $this->injectCommand(AuthLoginCommand::class); } + private function enableDeviceCodeConfig(): void + { + putenv('ACLI_DEVICE_CLIENT_ID=test-client-id'); + putenv('ACLI_OKTA_DOMAIN=example.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + } + + /** + * @param array<\GuzzleHttp\Psr7\Response> $responses + */ + private function createDeviceCodeCommand(array $responses, ?ObjectProphecy $localMachineHelperProphecy = null): AuthLoginCommand + { + $mock = new MockHandler($responses); + $client = new GuzzleClient(['handler' => HandlerStack::create($mock)]); + $localMachineHelper = $localMachineHelperProphecy?->reveal() + ?? $this->prophet->prophesize(LocalMachineHelper::class)->reveal(); + + return new AuthLoginCommand( + $localMachineHelper, + $this->datastoreCloud, + $this->datastoreAcli, + $this->cloudCredentials, + $this->telemetryHelper, + $this->acliRepoRoot, + $this->clientServiceProphecy->reveal(), + $this->sshHelper, + $this->sshDir, + $this->logger, + $this->selfUpdateManager, + $client, + ); + } + + private function givenFreshCloudConfigWithTelemetryDisabled(): void + { + $this->removeMockCloudConfigFile(); + $this->fs->dumpFile($this->cloudConfigFilepath, json_encode(['send_telemetry' => false])); + $this->createDataStores(); + } + + private function deviceAuthorizeResponse(): Response + { + return new Response(200, [], json_encode([ + 'device_code' => 'test-device-code', + 'expires_in' => 600, + 'interval' => 0, + 'user_code' => 'ABCD1234', + 'verification_uri' => 'https://example.okta.com/activate', + ])); + } + + // ------------------------------------------------------------------------- + // Device code flow tests + // ------------------------------------------------------------------------- + public function testDeviceCodeFlowSuccess(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode([ + 'access_token' => 'test-access-token', + 'expires_in' => 300, + 'refresh_token' => 'test-refresh-token', + ])), + ]); + + $this->executeCommand([], ['no']); + $output = $this->getDisplay(); + + $this->assertStringContainsString('https://example.okta.com/activate', $output); + $this->assertStringContainsString('ABCD1234', $output); + $this->assertStringContainsString('Authenticated successfully', $output); + + $config = new CloudDataStore($this->localMachineHelper, new CloudDataConfig(), $this->cloudConfigFilepath); + $deviceToken = $config->get('device_token'); + $this->assertEquals('test-access-token', $deviceToken['access_token']); + $this->assertEquals('test-refresh-token', $deviceToken['refresh_token']); + $this->assertEquals('test-client-id', $deviceToken['client_id']); + } + + public function testDeviceCodeFlowAuthorizationPendingThenSuccess(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode(['error' => 'authorization_pending'])), + new Response(200, [], json_encode([ + 'access_token' => 'test-access-token', + 'expires_in' => 300, + 'refresh_token' => 'test-refresh-token', + ])), + ]); + + $this->executeCommand([], ['no']); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Authenticated successfully', $output); + } + + public function testDeviceCodeFlowAccessDenied(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode(['error' => 'access_denied'])), + ]); + + $this->executeCommand([], ['no', 'no']); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Authorization denied', $output); + } + + public function testDeviceCodeFlowExpiredToken(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode(['error' => 'expired_token'])), + ]); + + $this->executeCommand([], ['no', 'no']); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Code expired', $output); + } + + public function testDeviceCodeFlowInitiateFailure(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->command = $this->createDeviceCodeCommand([ + new Response(400, [], json_encode(['error' => 'invalid_client'])), + ]); + + $this->executeCommand([], ['no']); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Failed to initiate device code flow', $output); + } + + public function testDeviceCodeFlowFallsBackToLegacyOnFailure(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->mockRequest('getAccount'); + $this->clientServiceProphecy->setConnector(Argument::type(Connector::class)) + ->shouldBeCalled(); + $this->clientServiceProphecy->isMachineAuthenticated() + ->willReturn(false); + + $localMachineHelperProphecy = $this->prophet->prophesize(LocalMachineHelper::class); + $localMachineHelperProphecy->useTty()->willReturn(false); + $this->command = $this->createDeviceCodeCommand([ + new Response(400, [], json_encode(['error' => 'invalid_client'])), + ], $localMachineHelperProphecy); + + // inputs: 'yes' = fallback confirm, 'no' = open browser to create token, then key + secret. + $this->executeCommand([], ['yes', 'no', self::$key, self::$secret]); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Failed to initiate device code flow', $output); + $this->assertStringContainsString('Saved credentials', $output); + } + + public function testDeviceCodeFlowOpensBrowserWhenConfirmed(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $localMachineHelperProphecy = $this->prophet->prophesize(LocalMachineHelper::class); + $localMachineHelperProphecy->startBrowser('https://example.okta.com/activate') + ->shouldBeCalled() + ->willReturn(true); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode([ + 'access_token' => 'test-access-token', + 'expires_in' => 300, + ])), + ], $localMachineHelperProphecy); + + $this->executeCommand([], ['yes']); + } + + public function testDeviceCodeFlowDoesNotOpenBrowserWhenDeclined(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $localMachineHelperProphecy = $this->prophet->prophesize(LocalMachineHelper::class); + $localMachineHelperProphecy->startBrowser(Argument::any()) + ->shouldNotBeCalled(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode([ + 'access_token' => 'test-access-token', + 'expires_in' => 300, + ])), + ], $localMachineHelperProphecy); + + $this->executeCommand([], ['no']); + } + + public function testSmartRoutingDeviceTokenReauthDeclined(): void + { + $this->enableDeviceCodeConfig(); + $this->removeMockCloudConfigFile(); + $this->fs->dumpFile($this->cloudConfigFilepath, json_encode([ + 'device_token' => [ + 'access_token' => 'existing-token', + 'client_id' => 'test-client-id', + 'expiry' => time() + 300, + 'refresh_token' => 'existing-refresh-token', + ], + 'send_telemetry' => false, + ])); + $this->createDataStores(); + $this->command = $this->createDeviceCodeCommand([]); + + $this->executeCommand([], ['no']); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Already authenticated via', $output); + $this->assertStringNotContainsString('Authenticated successfully', $output); + } + + // ------------------------------------------------------------------------- + // Legacy auth tests + // ------------------------------------------------------------------------- public function testAuthLoginCommand(): void { $this->mockRequest('getAccount'); @@ -60,7 +317,7 @@ public static function providerTestAuthLoginCommandWithStagingEnvironment(): arr ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('providerTestAuthLoginCommandWithStagingEnvironment')] + #[DataProvider('providerTestAuthLoginCommandWithStagingEnvironment')] public function testAuthLoginCommandWithStagingEnvironment(string $environment): void { $this->mockRequest('getAccount'); From 45f06cfd9c178069e8c5bb8ca4eed5754f84152d Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Mon, 21 Sep 2026 20:29:05 +0530 Subject: [PATCH 02/16] work on token refresh --- config/prod/services.yml | 2 ++ src/CloudApi/AccessTokenConnector.php | 16 ++++++++++++-- src/CloudApi/ClientService.php | 7 +++++- src/CloudApi/ConnectorFactory.php | 22 +++++++++---------- src/CloudApi/DeviceTokenRefresher.php | 12 +++++++++- src/Command/Auth/AuthLoginCommand.php | 21 +++++++++++++----- .../src/CloudApi/AccessTokenConnectorTest.php | 2 +- .../Commands/Auth/AuthLoginCommandTest.php | 2 +- 8 files changed, 60 insertions(+), 24 deletions(-) diff --git a/config/prod/services.yml b/config/prod/services.yml index 5eac97e6c..0c2c65a39 100644 --- a/config/prod/services.yml +++ b/config/prod/services.yml @@ -111,6 +111,7 @@ services: deviceAccessTokenExpiry: '@=service("cloud.credentials").getCloudDeviceTokenExpiry()' $baseUri: '@=service("cloud.credentials").getBaseUri()' $accountsUri: '@=service("cloud.credentials").getAccountsUri()' + $deviceTokenRefresher: '@Acquia\Cli\CloudApi\DeviceTokenRefresher' # v3 (MEO) connector factory — same credentials as v2, prod gateway at # https://api.acquia.com/v3. Override with ACLI_CLOUD_API_V3_BASE_URI for dev/stage. @@ -126,6 +127,7 @@ services: deviceAccessTokenExpiry: '@=service("cloud.credentials").getCloudDeviceTokenExpiry()' $baseUri: '@=service("cloud.credentials").getV3BaseUri()' $accountsUri: '@=service("cloud.credentials").getAccountsUri()' + $deviceTokenRefresher: '@Acquia\Cli\CloudApi\DeviceTokenRefresher' Acquia\Cli\CloudApi\V3ClientService: arguments: diff --git a/src/CloudApi/AccessTokenConnector.php b/src/CloudApi/AccessTokenConnector.php index 54a343c64..81d0c8c15 100644 --- a/src/CloudApi/AccessTokenConnector.php +++ b/src/CloudApi/AccessTokenConnector.php @@ -21,8 +21,12 @@ class AccessTokenConnector extends Connector /** * @param array $config */ - public function __construct(array $config, ?string $baseUri = null, ?string $urlAccessToken = null) - { + public function __construct( + array $config, + ?string $baseUri = null, + ?string $urlAccessToken = null, + private ?DeviceTokenRefresher $deviceTokenRefresher = null, + ) { $this->accessToken = new AccessToken(['access_token' => $config['access_token']]); parent::__construct($config, $baseUri, $urlAccessToken); } @@ -30,10 +34,18 @@ public function __construct(array $config, ?string $baseUri = null, ?string $url public function createRequest(string $verb, string $path): RequestInterface { if ($file = getenv('ACLI_ACCESS_TOKEN_FILE')) { + // File token takes priority — skip device token refresh to avoid + // burning refresh token rotations for a token nothing uses. if (!file_exists($file)) { throw new AcquiaCliException('Access token file not found at {file}', ['file' => $file]); } $this->accessToken = new AccessToken(['access_token' => trim(file_get_contents($file), "\"\n")]); + } elseif ($this->deviceTokenRefresher !== null) { + $freshToken = $this->deviceTokenRefresher->getValidAccessToken(); + if ($freshToken === null) { + throw new AcquiaCliException('Device token session expired. Run acli auth:login to re-authenticate.'); + } + $this->accessToken = new AccessToken(['access_token' => $freshToken]); } return $this->provider->getAuthenticatedRequest( $verb, diff --git a/src/CloudApi/ClientService.php b/src/CloudApi/ClientService.php index 03eca4406..4a820e10c 100644 --- a/src/CloudApi/ClientService.php +++ b/src/CloudApi/ClientService.php @@ -7,6 +7,7 @@ use Acquia\Cli\ApiCredentialsInterface; use Acquia\Cli\Application; use Acquia\Cli\ConnectorFactoryInterface; +use Acquia\Cli\Helpers\TelemetryHelper; use AcquiaCloudApi\Connector\Client; use AcquiaCloudApi\Connector\ConnectorInterface; @@ -54,7 +55,11 @@ public function getClient(): Client protected function configureClient(Client $client): void { - $userAgent = sprintf("acli/%s", $this->application->getVersion()); + $userAgent = sprintf('acli/%s', $this->application->getVersion()); + $provider = TelemetryHelper::getEnvironmentProvider(); + if ($provider !== null) { + $userAgent .= sprintf(' (agent:%s)', $provider); + } $customHeaders = [ 'User-Agent' => [$userAgent], ]; diff --git a/src/CloudApi/ConnectorFactory.php b/src/CloudApi/ConnectorFactory.php index 66b4f8044..3f3f60c2d 100644 --- a/src/CloudApi/ConnectorFactory.php +++ b/src/CloudApi/ConnectorFactory.php @@ -14,8 +14,12 @@ class ConnectorFactory implements ConnectorFactoryInterface /** * @param array $config */ - public function __construct(protected array $config, protected ?string $baseUri = null, protected ?string $accountsUri = null) - { + public function __construct( + protected array $config, + protected ?string $baseUri = null, + protected ?string $accountsUri = null, + private ?DeviceTokenRefresher $deviceTokenRefresher = null, + ) { } public function createConnector(): ConnectorInterface @@ -42,17 +46,11 @@ private function buildConnector(): ConnectorInterface // Device code token path. if (!empty($this->config['deviceAccessToken'])) { - $accessToken = new AccessToken([ + return new AccessTokenConnector([ 'access_token' => $this->config['deviceAccessToken'], - 'expires' => $this->config['deviceAccessTokenExpiry'] ?? 0, - ]); - if (!$accessToken->hasExpired()) { - return new AccessTokenConnector([ - 'access_token' => $accessToken, - 'key' => null, - 'secret' => null, - ], $this->baseUri, $this->accountsUri); - } + 'key' => null, + 'secret' => null, + ], $this->baseUri, $this->accountsUri, $this->deviceTokenRefresher); } // Fall back to a valid access token. diff --git a/src/CloudApi/DeviceTokenRefresher.php b/src/CloudApi/DeviceTokenRefresher.php index 90f895440..a80d6ec46 100644 --- a/src/CloudApi/DeviceTokenRefresher.php +++ b/src/CloudApi/DeviceTokenRefresher.php @@ -6,6 +6,7 @@ use Acquia\Cli\DataStore\CloudDataStore; use GuzzleHttp\Client as GuzzleClient; +use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\GuzzleException; use League\OAuth2\Client\Token\AccessToken; @@ -35,7 +36,8 @@ public function getValidAccessToken(): ?string 'expires' => $stored['expiry'] ?? 0, ]); - if (!$token->hasExpired()) { + // Refresh 60 s before actual expiry so requests in-flight don't hit a 401. + if (($token->getExpires() - time()) > 60) { return $stored['access_token']; } @@ -61,7 +63,15 @@ public function getValidAccessToken(): ?string ] ); $new = json_decode((string) $response->getBody(), true); + } catch (ClientException $e) { + // HTTP 4xx: refresh token is likely expired or revoked. + $status = $e->getResponse()->getStatusCode(); + if ($status === 400 || $status === 401) { + error_log('[acli] Device token refresh failed (HTTP ' . $status . '). Run `acli auth:login` to re-authenticate.'); + } + return null; } catch (GuzzleException) { + // Network / transport error — fail silently; caller falls back. return null; } diff --git a/src/Command/Auth/AuthLoginCommand.php b/src/Command/Auth/AuthLoginCommand.php index fdba7fef3..305adba83 100644 --- a/src/Command/Auth/AuthLoginCommand.php +++ b/src/Command/Auth/AuthLoginCommand.php @@ -17,6 +17,8 @@ use AcquiaCloudApi\Endpoints\Account; use GuzzleHttp\Client as GuzzleClient; use GuzzleHttp\Exception\ClientException; +use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Exception\RequestException; use Psr\Log\LoggerInterface; use SelfUpdate\SelfUpdateManager; use Symfony\Component\Console\Attribute\AsCommand; @@ -148,11 +150,12 @@ private function executeDeviceCodeFlow(OutputInterface $output): int $data = json_decode((string) $response->getBody(), true); - $deviceCode = $data['device_code']; - $userCode = $data['user_code']; - $verifyUrl = $data['verification_uri']; - $expiresIn = $data['expires_in'] ?? 600; - $interval = $data['interval'] ?? 5; + $deviceCode = $data['device_code']; + $userCode = $data['user_code']; + $verifyUrl = $data['verification_uri']; + $verifyUrlComplete = $data['verification_uri_complete'] ?? $verifyUrl; + $expiresIn = $data['expires_in'] ?? 600; + $interval = $data['interval'] ?? 5; // Step 2 — surface the code and URL for the human. $output->writeln(''); @@ -166,7 +169,7 @@ private function executeDeviceCodeFlow(OutputInterface $output): int $output->writeln(''); if (!AcquiaDrupalEnvironmentDetector::isAhIdeEnv() && $this->io->confirm('Do you want to open this page to sign in now?')) { - $this->localMachineHelper->startBrowser($verifyUrl); + $this->localMachineHelper->startBrowser($verifyUrlComplete); } $output->writeln(sprintf('Waiting for authorization... (code expires in %d minutes)', (int) ceil($expiresIn / 60))); @@ -186,7 +189,13 @@ private function executeDeviceCodeFlow(OutputInterface $output): int ]); $token = json_decode((string) $tokenResponse->getBody(), true); } catch (ClientException $e) { + // HTTP 4xx — parse the structured error body. $token = json_decode((string) $e->getResponse()->getBody(), true); + } catch (ConnectException | RequestException $e) { + // Transport error (DNS, timeout, no response). Back off exponentially + // and retry; only abort when the device code itself has expired. + $interval = min($interval * 2, 30); + continue; } $error = $token['error'] ?? ''; diff --git a/tests/phpunit/src/CloudApi/AccessTokenConnectorTest.php b/tests/phpunit/src/CloudApi/AccessTokenConnectorTest.php index 3dba738ae..dabf3363a 100644 --- a/tests/phpunit/src/CloudApi/AccessTokenConnectorTest.php +++ b/tests/phpunit/src/CloudApi/AccessTokenConnectorTest.php @@ -197,7 +197,7 @@ public function testIdeHeader(): void $client = $clientService->getClient(); $options = $client->getOptions(); $this->assertEquals([ - 'User-Agent' => [0 => 'acli/UNKNOWN'], + 'User-Agent' => [0 => 'acli/UNKNOWN (agent:acquia)'], 'X-Cloud-IDE-UUID' => IdeHelper::$remoteIdeUuid, ], $options['headers']); diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php index 65df9ac7f..fae669383 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php @@ -419,7 +419,7 @@ public function testAuthLoginInteractiveSelectsExistingEnvironmentKey(): void $this->executeCommand( ['--environment' => 'staging'], - ['Staging Key'], + ['yes', 'Staging Key'], ); $output = $this->getDisplay(); From 076929caf29225431c23001ead9a2ebfaafec50e Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Tue, 22 Sep 2026 16:45:36 +0530 Subject: [PATCH 03/16] error handling added --- config/prod/services.yml | 3 --- src/Command/Auth/AuthLoginCommand.php | 4 ++++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/config/prod/services.yml b/config/prod/services.yml index 0c2c65a39..8a286d21a 100644 --- a/config/prod/services.yml +++ b/config/prod/services.yml @@ -84,10 +84,7 @@ services: class: Acquia\Cli\AcsfApi\AcsfCredentials # AcquiaCloudApi services. - Acquia\Cli\Command\Api\ApiCommandHelper: ~ - Acquia\Cli\Command\Api\ApiV3CommandHelper: ~ Acquia\Cli\Command\Api\ApiCommandFactory: ~ - Acquia\Cli\Command\Api\ApiV3CommandFactory: ~ Acquia\Cli\Command\Api\ApiBaseCommand: class: Acquia\Cli\Command\Api\ApiBaseCommand factory: ['@Acquia\Cli\Command\Api\ApiCommandFactory', 'createCommand'] diff --git a/src/Command/Auth/AuthLoginCommand.php b/src/Command/Auth/AuthLoginCommand.php index 305adba83..82190ee4a 100644 --- a/src/Command/Auth/AuthLoginCommand.php +++ b/src/Command/Auth/AuthLoginCommand.php @@ -149,6 +149,10 @@ private function executeDeviceCodeFlow(OutputInterface $output): int } $data = json_decode((string) $response->getBody(), true); + if (!is_array($data) || !isset($data['device_code'], $data['user_code'], $data['verification_uri'])) { + $output->writeln('Unexpected response from device authorization endpoint.'); + return Command::FAILURE; + } $deviceCode = $data['device_code']; $userCode = $data['user_code']; From ef131ee278737ed58de97831e23bd8910c8c3460 Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Tue, 22 Sep 2026 16:58:06 +0530 Subject: [PATCH 04/16] log for error handling --- src/CloudApi/DeviceTokenRefresher.php | 4 +++- tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/CloudApi/DeviceTokenRefresher.php b/src/CloudApi/DeviceTokenRefresher.php index a80d6ec46..f8c66cf1d 100644 --- a/src/CloudApi/DeviceTokenRefresher.php +++ b/src/CloudApi/DeviceTokenRefresher.php @@ -9,11 +9,13 @@ use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\GuzzleException; use League\OAuth2\Client\Token\AccessToken; +use Psr\Log\LoggerInterface; class DeviceTokenRefresher { public function __construct( private CloudDataStore $datastore, + private LoggerInterface $logger, private GuzzleClient $httpClient = new GuzzleClient(['timeout' => 15]), ) { } @@ -67,7 +69,7 @@ public function getValidAccessToken(): ?string // HTTP 4xx: refresh token is likely expired or revoked. $status = $e->getResponse()->getStatusCode(); if ($status === 400 || $status === 401) { - error_log('[acli] Device token refresh failed (HTTP ' . $status . '). Run `acli auth:login` to re-authenticate.'); + $this->logger->warning('Device token refresh failed (HTTP {status}). Run `acli auth:login` to re-authenticate.', ['status' => $status]); } return null; } catch (GuzzleException) { diff --git a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php index 55ec94445..10a2708e3 100644 --- a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php +++ b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php @@ -45,7 +45,7 @@ private function createRefresher(array $responses = []): DeviceTokenRefresher $datastoreProphecy = $this->prophet->prophesize(CloudDataStore::class); $this->datastoreProphecy = $datastoreProphecy; - return new DeviceTokenRefresher($datastoreProphecy->reveal(), $client); + return new DeviceTokenRefresher($datastoreProphecy->reveal(), $this->logger, $client); } public function testReturnsNullWhenNoTokenStored(): void From 71c88135c8e6b18c417ebbbcaa11c66f09d652a3 Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Wed, 23 Sep 2026 16:34:33 +0530 Subject: [PATCH 05/16] addressed browser auto-open gap --- config/prod/services.yml | 2 - src/CloudApi/CloudCredentials.php | 4 -- src/Command/Auth/AuthLoginCommand.php | 5 +- .../src/CloudApi/AccessTokenConnectorTest.php | 42 +++++++++++++++ .../Commands/Auth/AuthLoginCommandTest.php | 51 +++++++++++++++---- 5 files changed, 86 insertions(+), 18 deletions(-) diff --git a/config/prod/services.yml b/config/prod/services.yml index 8a286d21a..95e984e36 100644 --- a/config/prod/services.yml +++ b/config/prod/services.yml @@ -77,8 +77,6 @@ services: cloud.credentials: class: Acquia\Cli\CloudApi\CloudCredentials - arguments: - $deviceTokenRefresher: '@Acquia\Cli\CloudApi\DeviceTokenRefresher' acsf.credentials: class: Acquia\Cli\AcsfApi\AcsfCredentials diff --git a/src/CloudApi/CloudCredentials.php b/src/CloudApi/CloudCredentials.php index 757bfe73f..a7be2cee4 100644 --- a/src/CloudApi/CloudCredentials.php +++ b/src/CloudApi/CloudCredentials.php @@ -12,15 +12,11 @@ class CloudCredentials implements ApiCredentialsInterface { public function __construct( private CloudDataStore $datastoreCloud, - private ?DeviceTokenRefresher $deviceTokenRefresher = null, ) { } public function getCloudDeviceAccessToken(): ?string { - if ($this->deviceTokenRefresher !== null) { - return $this->deviceTokenRefresher->getValidAccessToken(); - } $stored = $this->datastoreCloud->get('device_token'); return $stored['access_token'] ?? null; } diff --git a/src/Command/Auth/AuthLoginCommand.php b/src/Command/Auth/AuthLoginCommand.php index 82190ee4a..49984fbc0 100644 --- a/src/Command/Auth/AuthLoginCommand.php +++ b/src/Command/Auth/AuthLoginCommand.php @@ -13,7 +13,6 @@ use Acquia\Cli\Helpers\LocalMachineHelper; use Acquia\Cli\Helpers\SshHelper; use Acquia\Cli\Helpers\TelemetryHelper; -use Acquia\DrupalEnvironmentDetector\AcquiaDrupalEnvironmentDetector; use AcquiaCloudApi\Endpoints\Account; use GuzzleHttp\Client as GuzzleClient; use GuzzleHttp\Exception\ClientException; @@ -172,8 +171,10 @@ private function executeDeviceCodeFlow(OutputInterface $output): int $output->writeln(" $userCode"); $output->writeln(''); - if (!AcquiaDrupalEnvironmentDetector::isAhIdeEnv() && $this->io->confirm('Do you want to open this page to sign in now?')) { + if ($this->localMachineHelper->isBrowserAvailable() && !getenv('SSH_CONNECTION')) { $this->localMachineHelper->startBrowser($verifyUrlComplete); + $output->writeln('Confirm the code above matches what appears in your browser before approving.'); + $output->writeln(''); } $output->writeln(sprintf('Waiting for authorization... (code expires in %d minutes)', (int) ceil($expiresIn / 60))); diff --git a/tests/phpunit/src/CloudApi/AccessTokenConnectorTest.php b/tests/phpunit/src/CloudApi/AccessTokenConnectorTest.php index dabf3363a..107d584b8 100644 --- a/tests/phpunit/src/CloudApi/AccessTokenConnectorTest.php +++ b/tests/phpunit/src/CloudApi/AccessTokenConnectorTest.php @@ -7,6 +7,7 @@ use Acquia\Cli\CloudApi\AccessTokenConnector; use Acquia\Cli\CloudApi\ClientService; use Acquia\Cli\CloudApi\ConnectorFactory; +use Acquia\Cli\CloudApi\DeviceTokenRefresher; use Acquia\Cli\Exception\AcquiaCliException; use Acquia\Cli\Tests\Commands\Ide\IdeHelper; use Acquia\Cli\Tests\TestBase; @@ -182,6 +183,47 @@ public function testConnectorConfig(): void $this->prophet->checkPredictions(); } + public function testDeviceTokenRefreshOnRequest(): void + { + $refresherProphecy = $this->prophet->prophesize(DeviceTokenRefresher::class); + $refresherProphecy->getValidAccessToken()->willReturn('fresh-device-token')->shouldBeCalled(); + + $connector = new AccessTokenConnector([ + 'access_token' => 'stale-token', + 'key' => null, + 'secret' => null, + ], null, null, $refresherProphecy->reveal()); + + $mockProvider = $this->prophet->prophesize(GenericProvider::class); + $mockProvider->getAuthenticatedRequest('get', ConnectorInterface::BASE_URI . 'api', Argument::that( + fn (AccessTokenInterface $t) => $t->getToken() === 'fresh-device-token' + ))->willReturn($this->prophet->prophesize(RequestInterface::class)->reveal())->shouldBeCalled(); + $connector->setProvider($mockProvider->reveal()); + $request = $connector->createRequest('get', 'api'); + + $this->assertInstanceOf(RequestInterface::class, $request); + $this->prophet->checkPredictions(); + } + + public function testDeviceTokenExpiredThrowsOnRequest(): void + { + $refresherProphecy = $this->prophet->prophesize(DeviceTokenRefresher::class); + $refresherProphecy->getValidAccessToken()->willReturn(null); + + $connector = new AccessTokenConnector([ + 'access_token' => 'stale-token', + 'key' => null, + 'secret' => null, + ], null, null, $refresherProphecy->reveal()); + + $mockProvider = $this->prophet->prophesize(GenericProvider::class); + $connector->setProvider($mockProvider->reveal()); + + $this->expectException(AcquiaCliException::class); + $this->expectExceptionMessage('Device token session expired'); + $connector->createRequest('get', 'api'); + } + public function testIdeHeader(): void { self::setAccessTokenEnvVars(); diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php index fae669383..5f0d8f9f3 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php @@ -66,8 +66,11 @@ private function createDeviceCodeCommand(array $responses, ?ObjectProphecy $loca { $mock = new MockHandler($responses); $client = new GuzzleClient(['handler' => HandlerStack::create($mock)]); - $localMachineHelper = $localMachineHelperProphecy?->reveal() - ?? $this->prophet->prophesize(LocalMachineHelper::class)->reveal(); + if ($localMachineHelperProphecy === null) { + $localMachineHelperProphecy = $this->prophet->prophesize(LocalMachineHelper::class); + $localMachineHelperProphecy->isBrowserAvailable()->willReturn(false); + } + $localMachineHelper = $localMachineHelperProphecy->reveal(); return new AuthLoginCommand( $localMachineHelper, @@ -119,7 +122,7 @@ public function testDeviceCodeFlowSuccess(): void ])), ]); - $this->executeCommand([], ['no']); + $this->executeCommand([], []); $output = $this->getDisplay(); $this->assertStringContainsString('https://example.okta.com/activate', $output); @@ -147,7 +150,7 @@ public function testDeviceCodeFlowAuthorizationPendingThenSuccess(): void ])), ]); - $this->executeCommand([], ['no']); + $this->executeCommand([], []); $output = $this->getDisplay(); $this->assertStringContainsString('Authenticated successfully', $output); @@ -162,7 +165,7 @@ public function testDeviceCodeFlowAccessDenied(): void new Response(200, [], json_encode(['error' => 'access_denied'])), ]); - $this->executeCommand([], ['no', 'no']); + $this->executeCommand([], ['no']); $output = $this->getDisplay(); $this->assertStringContainsString('Authorization denied', $output); @@ -177,7 +180,7 @@ public function testDeviceCodeFlowExpiredToken(): void new Response(200, [], json_encode(['error' => 'expired_token'])), ]); - $this->executeCommand([], ['no', 'no']); + $this->executeCommand([], ['no']); $output = $this->getDisplay(); $this->assertStringContainsString('Code expired', $output); @@ -221,11 +224,12 @@ public function testDeviceCodeFlowFallsBackToLegacyOnFailure(): void $this->assertStringContainsString('Saved credentials', $output); } - public function testDeviceCodeFlowOpensBrowserWhenConfirmed(): void + public function testDeviceCodeFlowOpensBrowserWhenAvailable(): void { $this->enableDeviceCodeConfig(); $this->givenFreshCloudConfigWithTelemetryDisabled(); $localMachineHelperProphecy = $this->prophet->prophesize(LocalMachineHelper::class); + $localMachineHelperProphecy->isBrowserAvailable()->willReturn(true); $localMachineHelperProphecy->startBrowser('https://example.okta.com/activate') ->shouldBeCalled() ->willReturn(true); @@ -237,14 +241,16 @@ public function testDeviceCodeFlowOpensBrowserWhenConfirmed(): void ])), ], $localMachineHelperProphecy); - $this->executeCommand([], ['yes']); + $this->executeCommand([], []); + $this->assertStringContainsString('Confirm the code above matches', $this->getDisplay()); } - public function testDeviceCodeFlowDoesNotOpenBrowserWhenDeclined(): void + public function testDeviceCodeFlowDoesNotOpenBrowserWhenUnavailable(): void { $this->enableDeviceCodeConfig(); $this->givenFreshCloudConfigWithTelemetryDisabled(); $localMachineHelperProphecy = $this->prophet->prophesize(LocalMachineHelper::class); + $localMachineHelperProphecy->isBrowserAvailable()->willReturn(false); $localMachineHelperProphecy->startBrowser(Argument::any()) ->shouldNotBeCalled(); $this->command = $this->createDeviceCodeCommand([ @@ -255,7 +261,32 @@ public function testDeviceCodeFlowDoesNotOpenBrowserWhenDeclined(): void ])), ], $localMachineHelperProphecy); - $this->executeCommand([], ['no']); + $this->executeCommand([], []); + } + + public function testDeviceCodeFlowDoesNotOpenBrowserOverSsh(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $savedSsh = getenv('SSH_CONNECTION'); + putenv('SSH_CONNECTION=127.0.0.1 12345'); + $localMachineHelperProphecy = $this->prophet->prophesize(LocalMachineHelper::class); + $localMachineHelperProphecy->isBrowserAvailable()->willReturn(true); + $localMachineHelperProphecy->startBrowser(Argument::any()) + ->shouldNotBeCalled(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode([ + 'access_token' => 'test-access-token', + 'expires_in' => 300, + ])), + ], $localMachineHelperProphecy); + + try { + $this->executeCommand([], []); + } finally { + putenv($savedSsh === false ? 'SSH_CONNECTION' : 'SSH_CONNECTION=' . $savedSsh); + } } public function testSmartRoutingDeviceTokenReauthDeclined(): void From 678be26f966575008a16bd9d54ef785e8eb8670f Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Fri, 25 Sep 2026 18:22:57 +0530 Subject: [PATCH 06/16] Addresses review findings 1-5 and 7-10 on this PR --- .env.example | 3 + .github/workflows/ci.yml | 3 + config/prod/services.yml | 2 - src/CloudApi/ClientService.php | 11 +- src/CloudApi/CloudCredentials.php | 6 - src/CloudApi/DeviceTokenRefresher.php | 45 ++++- src/CloudApi/OktaConfig.php | 33 ++++ src/Command/Auth/AuthLoginCommand.php | 31 +-- .../src/CloudApi/AccessTokenConnectorTest.php | 8 +- .../src/CloudApi/ClientServiceTest.php | 23 +++ .../src/CloudApi/ConnectorFactoryTest.php | 52 +++++ .../src/CloudApi/DeviceTokenRefresherTest.php | 116 ++++++++++- tests/phpunit/src/CloudApi/OktaConfigTest.php | 85 ++++++++ .../Commands/Auth/AuthLoginCommandTest.php | 183 +++++++++++++++++- .../Commands/Auth/AuthLogoutCommandTest.php | 67 +++++++ 15 files changed, 633 insertions(+), 35 deletions(-) create mode 100644 src/CloudApi/OktaConfig.php create mode 100644 tests/phpunit/src/CloudApi/OktaConfigTest.php diff --git a/.env.example b/.env.example index c9e997a9b..626fdf8a7 100644 --- a/.env.example +++ b/.env.example @@ -1,3 +1,6 @@ BUGSNAG_KEY=abcd1234 AMPLITUDE_KEY=acbd1234 ACLI_VERSION=1.0.0 +ACLI_DEVICE_CLIENT_ID=0oa1example2client3id +ACLI_OKTA_DOMAIN=example.okta.com +ACLI_OKTA_AUTH_SERVER_ID=ausExample1234 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bb13c8b63..04c76e2cc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -94,6 +94,9 @@ jobs: touch .env echo BUGSNAG_KEY=${{ secrets.BUGSNAG_KEY }} >> .env echo AMPLITUDE_KEY=${{ secrets.AMPLITUDE_KEY }} >> .env + echo ACLI_DEVICE_CLIENT_ID=${{ secrets.ACLI_DEVICE_CLIENT_ID }} >> .env + echo ACLI_OKTA_DOMAIN=${{ secrets.ACLI_OKTA_DOMAIN }} >> .env + echo ACLI_OKTA_AUTH_SERVER_ID=${{ secrets.ACLI_OKTA_AUTH_SERVER_ID }} >> .env echo ACLI_VERSION=${{ steps.acli-version.outputs.ACLI_VERSION }} >> .env echo ACLI_BUILD_DATE=$(date -u +"%Y-%m-%dT%H:%M:%SZ") >> .env - name: 'Extract additional API specs from secrets' diff --git a/config/prod/services.yml b/config/prod/services.yml index 95e984e36..c9f0cc920 100644 --- a/config/prod/services.yml +++ b/config/prod/services.yml @@ -103,7 +103,6 @@ services: accessToken: '@=service("cloud.credentials").getCloudAccessToken()' accessTokenExpiry: '@=service("cloud.credentials").getCloudAccessTokenExpiry()' deviceAccessToken: '@=service("cloud.credentials").getCloudDeviceAccessToken()' - deviceAccessTokenExpiry: '@=service("cloud.credentials").getCloudDeviceTokenExpiry()' $baseUri: '@=service("cloud.credentials").getBaseUri()' $accountsUri: '@=service("cloud.credentials").getAccountsUri()' $deviceTokenRefresher: '@Acquia\Cli\CloudApi\DeviceTokenRefresher' @@ -119,7 +118,6 @@ services: accessToken: '@=service("cloud.credentials").getCloudAccessToken()' accessTokenExpiry: '@=service("cloud.credentials").getCloudAccessTokenExpiry()' deviceAccessToken: '@=service("cloud.credentials").getCloudDeviceAccessToken()' - deviceAccessTokenExpiry: '@=service("cloud.credentials").getCloudDeviceTokenExpiry()' $baseUri: '@=service("cloud.credentials").getV3BaseUri()' $accountsUri: '@=service("cloud.credentials").getAccountsUri()' $deviceTokenRefresher: '@Acquia\Cli\CloudApi\DeviceTokenRefresher' diff --git a/src/CloudApi/ClientService.php b/src/CloudApi/ClientService.php index 4a820e10c..6fb8f2c0d 100644 --- a/src/CloudApi/ClientService.php +++ b/src/CloudApi/ClientService.php @@ -10,6 +10,7 @@ use Acquia\Cli\Helpers\TelemetryHelper; use AcquiaCloudApi\Connector\Client; use AcquiaCloudApi\Connector\ConnectorInterface; +use loophp\phposinfo\OsInfo; /** * Factory producing Acquia Cloud Api clients. @@ -55,11 +56,13 @@ public function getClient(): Client protected function configureClient(Client $client): void { - $userAgent = sprintf('acli/%s', $this->application->getVersion()); $provider = TelemetryHelper::getEnvironmentProvider(); - if ($provider !== null) { - $userAgent .= sprintf(' (agent:%s)', $provider); - } + $userAgent = sprintf( + 'Acquia CLI (%s, %s%s)', + $this->application->getVersion(), + OsInfo::uuid(), + $provider !== null ? ', ' . $provider : '', + ); $customHeaders = [ 'User-Agent' => [$userAgent], ]; diff --git a/src/CloudApi/CloudCredentials.php b/src/CloudApi/CloudCredentials.php index a7be2cee4..8120d9f3d 100644 --- a/src/CloudApi/CloudCredentials.php +++ b/src/CloudApi/CloudCredentials.php @@ -21,12 +21,6 @@ public function getCloudDeviceAccessToken(): ?string return $stored['access_token'] ?? null; } - public function getCloudDeviceTokenExpiry(): ?int - { - $stored = $this->datastoreCloud->get('device_token'); - return isset($stored['expiry']) ? (int) $stored['expiry'] : null; - } - public function getCloudAccessToken(): ?string { if ($token = getenv('ACLI_ACCESS_TOKEN')) { diff --git a/src/CloudApi/DeviceTokenRefresher.php b/src/CloudApi/DeviceTokenRefresher.php index f8c66cf1d..37bb5aa17 100644 --- a/src/CloudApi/DeviceTokenRefresher.php +++ b/src/CloudApi/DeviceTokenRefresher.php @@ -13,10 +13,13 @@ class DeviceTokenRefresher { + private const REQUEST_TIMEOUT_SECONDS = 15; + public function __construct( private CloudDataStore $datastore, private LoggerInterface $logger, - private GuzzleClient $httpClient = new GuzzleClient(['timeout' => 15]), + private GuzzleClient $httpClient = new GuzzleClient(), + private OktaConfig $oktaConfig = new OktaConfig(), ) { } @@ -46,8 +49,8 @@ public function getValidAccessToken(): ?string // Access token expired — attempt a silent refresh. $refreshToken = $stored['refresh_token'] ?? null; $clientId = $stored['client_id'] ?? null; - $domain = getenv('ACLI_OKTA_DOMAIN'); - $authServer = getenv('ACLI_OKTA_AUTH_SERVER_ID'); + $domain = $this->oktaConfig->domain(); + $authServer = $this->oktaConfig->authServerId(); if (!$refreshToken || !$clientId || !$domain || !$authServer) { return null; @@ -62,12 +65,23 @@ public function getValidAccessToken(): ?string 'grant_type' => 'refresh_token', 'refresh_token' => $refreshToken, ], + 'timeout' => self::REQUEST_TIMEOUT_SECONDS, ] ); $new = json_decode((string) $response->getBody(), true); } catch (ClientException $e) { - // HTTP 4xx: refresh token is likely expired or revoked. $status = $e->getResponse()->getStatusCode(); + $body = json_decode((string) $e->getResponse()->getBody(), true); + $error = is_array($body) ? ($body['error'] ?? '') : ''; + + if ($error === 'invalid_grant') { + if ($winnersToken = $this->tokenWrittenByAnotherProcess($stored)) { + return $winnersToken; + } + $this->logger->warning('Device token refresh failed: the refresh token is expired or revoked. Run `acli auth:login` to re-authenticate.'); + return null; + } + if ($status === 400 || $status === 401) { $this->logger->warning('Device token refresh failed (HTTP {status}). Run `acli auth:login` to re-authenticate.', ['status' => $status]); } @@ -90,4 +104,27 @@ public function getValidAccessToken(): ?string return $new['access_token']; } + + /** + * Returns a usable access token another process wrote while we were refreshing. + * + * Null means nothing changed, so the refresh token really is expired or revoked. + * + * @param array $stored the token this call started with + */ + private function tokenWrittenByAnotherProcess(array $stored): ?string + { + $current = $this->datastore->get('device_token'); + if (!is_array($current) || empty($current['access_token'])) { + return null; + } + if ($current['access_token'] === ($stored['access_token'] ?? null)) { + return null; + } + if ((($current['expiry'] ?? 0) - time()) <= 60) { + return null; + } + + return $current['access_token']; + } } diff --git a/src/CloudApi/OktaConfig.php b/src/CloudApi/OktaConfig.php new file mode 100644 index 000000000..5da64f5cc --- /dev/null +++ b/src/CloudApi/OktaConfig.php @@ -0,0 +1,33 @@ +clientId() !== '' + && $this->domain() !== '' + && $this->authServerId() !== ''; + } +} diff --git a/src/Command/Auth/AuthLoginCommand.php b/src/Command/Auth/AuthLoginCommand.php index 49984fbc0..fe98edc89 100644 --- a/src/Command/Auth/AuthLoginCommand.php +++ b/src/Command/Auth/AuthLoginCommand.php @@ -6,6 +6,7 @@ use Acquia\Cli\ApiCredentialsInterface; use Acquia\Cli\CloudApi\ClientService; +use Acquia\Cli\CloudApi\OktaConfig; use Acquia\Cli\Command\CommandBase; use Acquia\Cli\DataStore\AcquiaCliDatastore; use Acquia\Cli\DataStore\CloudDataStore; @@ -17,6 +18,7 @@ use GuzzleHttp\Client as GuzzleClient; use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Exception\GuzzleException; use GuzzleHttp\Exception\RequestException; use Psr\Log\LoggerInterface; use SelfUpdate\SelfUpdateManager; @@ -29,9 +31,7 @@ #[AsCommand(name: 'auth:login', description: 'Register Cloud Platform API credentials', aliases: ['login'])] final class AuthLoginCommand extends CommandBase { - private const DEVICE_CLIENT_ID = ''; - private const OKTA_DOMAIN = ''; - private const OKTA_AUTH_SERVER_ID = ''; + private const REQUEST_TIMEOUT_SECONDS = 30; public function __construct( public LocalMachineHelper $localMachineHelper, @@ -45,7 +45,8 @@ public function __construct( protected string $sshDir, LoggerInterface $logger, public SelfUpdateManager $selfUpdateManager, - private GuzzleClient $httpClient = new GuzzleClient(['timeout' => 30]), + private GuzzleClient $httpClient = new GuzzleClient(), + private OktaConfig $oktaConfig = new OktaConfig(), ) { parent::__construct($this->localMachineHelper, $this->datastoreCloud, $this->datastoreAcli, $this->cloudCredentials, $this->telemetryHelper, $this->projectDir, $this->cloudApiClientService, $this->sshHelper, $this->sshDir, $logger, $this->selfUpdateManager); } @@ -103,6 +104,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int return $this->executeDeviceCodeFlowWithFallback($input, $output); } + $output->writeln('Device code sign-in is not configured in this build; falling back to API key authentication.'); + return $this->executeLegacyAuth($input, $output); } @@ -121,16 +124,14 @@ private function executeDeviceCodeFlowWithFallback(InputInterface $input, Output private function isDeviceCodeConfigured(): bool { - return (bool) (getenv('ACLI_DEVICE_CLIENT_ID') ?: self::DEVICE_CLIENT_ID) - && (bool) (getenv('ACLI_OKTA_DOMAIN') ?: self::OKTA_DOMAIN) - && (bool) (getenv('ACLI_OKTA_AUTH_SERVER_ID') ?: self::OKTA_AUTH_SERVER_ID); + return $this->oktaConfig->isConfigured(); } private function executeDeviceCodeFlow(OutputInterface $output): int { - $clientId = getenv('ACLI_DEVICE_CLIENT_ID') ?: self::DEVICE_CLIENT_ID; - $domain = getenv('ACLI_OKTA_DOMAIN') ?: self::OKTA_DOMAIN; - $authServer = getenv('ACLI_OKTA_AUTH_SERVER_ID') ?: self::OKTA_AUTH_SERVER_ID; + $clientId = $this->oktaConfig->clientId(); + $domain = $this->oktaConfig->domain(); + $authServer = $this->oktaConfig->authServerId(); $baseUrl = sprintf('https://%s/oauth2/%s/v1', $domain, $authServer); @@ -141,8 +142,9 @@ private function executeDeviceCodeFlow(OutputInterface $output): int 'client_id' => $clientId, 'scope' => 'openid profile email offline_access', ], + 'timeout' => self::REQUEST_TIMEOUT_SECONDS, ]); - } catch (ClientException $e) { + } catch (GuzzleException $e) { $output->writeln('Failed to initiate device code flow: ' . $e->getMessage() . ''); return Command::FAILURE; } @@ -191,6 +193,7 @@ private function executeDeviceCodeFlow(OutputInterface $output): int 'device_code' => $deviceCode, 'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code', ], + 'timeout' => self::REQUEST_TIMEOUT_SECONDS, ]); $token = json_decode((string) $tokenResponse->getBody(), true); } catch (ClientException $e) { @@ -203,10 +206,14 @@ private function executeDeviceCodeFlow(OutputInterface $output): int continue; } - $error = $token['error'] ?? ''; + $error = is_array($token) ? ($token['error'] ?? '') : ''; switch ($error) { case '': + if (!is_array($token) || empty($token['access_token'])) { + $output->writeln('Unexpected response from the token endpoint: no access token returned.'); + return Command::FAILURE; + } // Success — store token and exit. $this->storeDeviceToken($token, $clientId); $output->writeln(''); diff --git a/tests/phpunit/src/CloudApi/AccessTokenConnectorTest.php b/tests/phpunit/src/CloudApi/AccessTokenConnectorTest.php index 107d584b8..fe59eebab 100644 --- a/tests/phpunit/src/CloudApi/AccessTokenConnectorTest.php +++ b/tests/phpunit/src/CloudApi/AccessTokenConnectorTest.php @@ -15,6 +15,7 @@ use AcquiaCloudApi\Connector\ConnectorInterface; use League\OAuth2\Client\Provider\GenericProvider; use League\OAuth2\Client\Token\AccessTokenInterface; +use loophp\phposinfo\OsInfo; use org\bovigo\vfs\vfsStream; use Prophecy\Argument; use Psr\Http\Message\RequestInterface; @@ -178,7 +179,10 @@ public function testConnectorConfig(): void $clientService = new ClientService($connectorFactory, $this->application, $this->cloudCredentials); $client = $clientService->getClient(); $options = $client->getOptions(); - $this->assertEquals(['User-Agent' => [0 => 'acli/UNKNOWN']], $options['headers']); + $this->assertStringStartsWith( + sprintf('Acquia CLI (UNKNOWN, %s', OsInfo::uuid()), + $options['headers']['User-Agent'][0], + ); $this->prophet->checkPredictions(); } @@ -239,7 +243,7 @@ public function testIdeHeader(): void $client = $clientService->getClient(); $options = $client->getOptions(); $this->assertEquals([ - 'User-Agent' => [0 => 'acli/UNKNOWN (agent:acquia)'], + 'User-Agent' => [0 => sprintf('Acquia CLI (UNKNOWN, %s, acquia)', OsInfo::uuid())], 'X-Cloud-IDE-UUID' => IdeHelper::$remoteIdeUuid, ], $options['headers']); diff --git a/tests/phpunit/src/CloudApi/ClientServiceTest.php b/tests/phpunit/src/CloudApi/ClientServiceTest.php index 3851449de..3fa5206e6 100644 --- a/tests/phpunit/src/CloudApi/ClientServiceTest.php +++ b/tests/phpunit/src/CloudApi/ClientServiceTest.php @@ -10,10 +10,33 @@ use Acquia\Cli\CloudApi\V3ClientService; use Acquia\Cli\DataStore\CloudDataStore; use Acquia\Cli\Tests\TestBase; +use loophp\phposinfo\OsInfo; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Group; class ClientServiceTest extends TestBase { + #[Group('serial')] + public function testUserAgentCarriesVersionTelemetryIdAndProvider(): void + { + putenv('LANDO=1'); + try { + $clientService = new ClientService( + new ConnectorFactory(['accessToken' => null, 'key' => null, 'secret' => null]), + $this->application, + $this->cloudCredentials, + ); + $options = $clientService->getClient()->getOptions(); + + $this->assertSame( + sprintf('Acquia CLI (UNKNOWN, %s, lando)', OsInfo::uuid()), + $options['headers']['User-Agent'][0], + ); + } finally { + putenv('LANDO'); + } + } + /** * @return array */ diff --git a/tests/phpunit/src/CloudApi/ConnectorFactoryTest.php b/tests/phpunit/src/CloudApi/ConnectorFactoryTest.php index 1af9873ba..d32e7e4df 100644 --- a/tests/phpunit/src/CloudApi/ConnectorFactoryTest.php +++ b/tests/phpunit/src/CloudApi/ConnectorFactoryTest.php @@ -4,6 +4,7 @@ namespace Acquia\Cli\Tests\CloudApi; +use Acquia\Cli\CloudApi\AccessTokenConnector; use Acquia\Cli\CloudApi\ConnectorFactory; use Acquia\Cli\CloudApi\PathRewriteConnector; use AcquiaCloudApi\Connector\Connector; @@ -46,6 +47,57 @@ public function testCreateConnectorFactoryBehavior(?string $envValue, string $ex $this->assertInstanceOf($expectedClass, $connector); } + public function testBuildsAccessTokenConnectorForAStaleDeviceTokenWithARefreshToken(): void + { + putenv('AH_CODEBASE_UUID'); + $factory = new ConnectorFactory([ + 'accessToken' => null, + 'deviceAccessToken' => 'expired-but-present', + 'key' => null, + 'secret' => null, + ], 'https://api.example.com'); + + $this->assertInstanceOf(AccessTokenConnector::class, $factory->createConnector()); + } + + public function testStoredKeyAndSecretOutrankADeviceToken(): void + { + putenv('AH_CODEBASE_UUID'); + $factory = new ConnectorFactory([ + 'accessToken' => null, + 'deviceAccessToken' => 'device-token', + 'key' => 'k', + 'secret' => 's', + ], 'https://api.example.com'); + + $connector = $factory->createConnector(); + + $this->assertInstanceOf(Connector::class, $connector); + $this->assertNotInstanceOf(AccessTokenConnector::class, $connector); + } + + public function testDeviceTokenOutranksTheAccessTokenEnvVar(): void + { + putenv('AH_CODEBASE_UUID'); + // An expired env token makes the branches distinguishable: alone it fails the + // expiry check, so an AccessTokenConnector here can only be the device path. + $expiredEnvToken = [ + 'accessToken' => 'env-access-token', + 'accessTokenExpiry' => time() - 300, + 'key' => null, + 'secret' => null, + ]; + + $withoutDeviceToken = (new ConnectorFactory($expiredEnvToken, 'https://api.example.com'))->createConnector(); + $this->assertNotInstanceOf(AccessTokenConnector::class, $withoutDeviceToken); + + $withDeviceToken = (new ConnectorFactory( + $expiredEnvToken + ['deviceAccessToken' => 'device-token'], + 'https://api.example.com', + ))->createConnector(); + $this->assertInstanceOf(AccessTokenConnector::class, $withDeviceToken); + } + /** * Data provider for testCreateConnectorFactoryBehavior() test. * diff --git a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php index 10a2708e3..038ec666f 100644 --- a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php +++ b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php @@ -5,15 +5,19 @@ namespace Acquia\Cli\Tests\CloudApi; use Acquia\Cli\CloudApi\DeviceTokenRefresher; +use Acquia\Cli\CloudApi\OktaConfig; use Acquia\Cli\DataStore\CloudDataStore; use Acquia\Cli\Tests\TestBase; use GuzzleHttp\Client as GuzzleClient; use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; +use GuzzleHttp\Promise\Create; use GuzzleHttp\Psr7\Response; +use PHPUnit\Framework\Attributes\Group; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; +#[Group('serial')] class DeviceTokenRefresherTest extends TestBase { /** @var array */ @@ -45,7 +49,47 @@ private function createRefresher(array $responses = []): DeviceTokenRefresher $datastoreProphecy = $this->prophet->prophesize(CloudDataStore::class); $this->datastoreProphecy = $datastoreProphecy; - return new DeviceTokenRefresher($datastoreProphecy->reveal(), $this->logger, $client); + return new DeviceTokenRefresher( + $datastoreProphecy->reveal(), + $this->logger, + $client, + new OktaConfig(), + ); + } + + public function testRefreshRequestCarriesAnExplicitTimeout(): void + { + // A hung /token would otherwise block every API request indefinitely. + putenv('ACLI_OKTA_DOMAIN=example.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $captured = []; + $handler = function ($request, array $options) use (&$captured) { + $captured[] = $options; + return Create::promiseFor(new Response(200, [], json_encode([ + 'access_token' => 'new-access-token', + 'expires_in' => 3600, + ]))); + }; + $datastoreProphecy = $this->prophet->prophesize(CloudDataStore::class); + $datastoreProphecy->get('device_token')->willReturn([ + 'access_token' => 'expired-token', + 'client_id' => 'client-123', + 'expiry' => time() - 300, + 'refresh_token' => 'old-refresh-token', + ]); + $datastoreProphecy->set('device_token', Argument::any())->shouldBeCalled(); + $refresher = new DeviceTokenRefresher( + $datastoreProphecy->reveal(), + $this->logger, + new GuzzleClient(['handler' => $handler]), + new OktaConfig(), + ); + + $refresher->getValidAccessToken(); + + $this->assertNotEmpty($captured, 'Expected a refresh request to be made.'); + $this->assertArrayHasKey('timeout', $captured[0]); + $this->assertGreaterThan(0, $captured[0]['timeout']); } public function testReturnsNullWhenNoTokenStored(): void @@ -89,9 +133,8 @@ public function testReturnsNullWhenExpiredAndRefreshTokenMissing(): void $this->assertNull($refresher->getValidAccessToken()); } - public function testReturnsNullWhenExpiredAndEnvVarsMissing(): void + public function testReturnsNullWhenExpiredAndOktaConfigMissing(): void { - // ACLI_OKTA_DOMAIN / ACLI_OKTA_AUTH_SERVER_ID are unset per setUp(). $refresher = $this->createRefresher(); $this->datastoreProphecy->get('device_token')->willReturn([ 'access_token' => 'expired-token', @@ -103,6 +146,28 @@ public function testReturnsNullWhenExpiredAndEnvVarsMissing(): void $this->assertNull($refresher->getValidAccessToken()); } + public function testResolvesOktaConfigThroughTheSharedResolver(): void + { + putenv('ACLI_OKTA_DOMAIN=shared.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausShared'); + $refresher = $this->createRefresher([ + new Response(200, [], json_encode([ + 'access_token' => 'new-access-token', + 'expires_in' => 3600, + 'refresh_token' => 'new-refresh-token', + ])), + ]); + $this->datastoreProphecy->get('device_token')->willReturn([ + 'access_token' => 'expired-token', + 'client_id' => 'client-123', + 'expiry' => time() - 300, + 'refresh_token' => 'old-refresh-token', + ]); + $this->datastoreProphecy->set('device_token', Argument::any())->shouldBeCalled(); + + $this->assertEquals('new-access-token', $refresher->getValidAccessToken()); + } + public function testRefreshesExpiredTokenSuccessfully(): void { putenv('ACLI_OKTA_DOMAIN=example.okta.com'); @@ -173,6 +238,51 @@ public function testReturnsNullWhenRefreshResponseHasNoAccessToken(): void $this->assertNull($refresher->getValidAccessToken()); } + public function testUsesTheWinnersTokenWhenAConcurrentInvocationRotatedFirst(): void + { + // Under ROTATE this call loses the race: invalid_grant, but the winner has + // already persisted a fresh token, so the session is fine. + putenv('ACLI_OKTA_DOMAIN=example.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $refresher = $this->createRefresher([ + new Response(400, [], json_encode(['error' => 'invalid_grant'])), + ]); + $this->datastoreProphecy->get('device_token')->willReturn( + [ + 'access_token' => 'stale-token', + 'client_id' => 'client-123', + 'expiry' => time() - 300, + 'refresh_token' => 'rotated-away-refresh-token', + ], + [ + 'access_token' => 'winners-fresh-token', + 'client_id' => 'client-123', + 'expiry' => time() + 300, + 'refresh_token' => 'winners-refresh-token', + ], + ); + + $this->assertEquals('winners-fresh-token', $refresher->getValidAccessToken()); + } + + public function testReturnsNullWhenInvalidGrantAndNoConcurrentRefreshHappened(): void + { + putenv('ACLI_OKTA_DOMAIN=example.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $refresher = $this->createRefresher([ + new Response(400, [], json_encode(['error' => 'invalid_grant'])), + ]); + $stored = [ + 'access_token' => 'stale-token', + 'client_id' => 'client-123', + 'expiry' => time() - 300, + 'refresh_token' => 'dead-refresh-token', + ]; + $this->datastoreProphecy->get('device_token')->willReturn($stored, $stored); + + $this->assertNull($refresher->getValidAccessToken()); + } + public function testReturnsNullOnRefreshRequestFailure(): void { putenv('ACLI_OKTA_DOMAIN=example.okta.com'); diff --git a/tests/phpunit/src/CloudApi/OktaConfigTest.php b/tests/phpunit/src/CloudApi/OktaConfigTest.php new file mode 100644 index 000000000..9125b734e --- /dev/null +++ b/tests/phpunit/src/CloudApi/OktaConfigTest.php @@ -0,0 +1,85 @@ + */ + private array $savedEnvVars = []; + + protected function setUp(): void + { + parent::setUp(); + foreach (['ACLI_DEVICE_CLIENT_ID', 'ACLI_OKTA_DOMAIN', 'ACLI_OKTA_AUTH_SERVER_ID'] as $var) { + $this->savedEnvVars[$var] = getenv($var); + putenv($var); + } + } + + protected function tearDown(): void + { + foreach ($this->savedEnvVars as $var => $value) { + putenv($value === false ? $var : $var . '=' . $value); + } + parent::tearDown(); + } + + public function testResolvesValuesFromTheEnvironment(): void + { + putenv('ACLI_DEVICE_CLIENT_ID=env-client'); + putenv('ACLI_OKTA_DOMAIN=env.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausEnv'); + $config = new OktaConfig(); + + $this->assertSame('env-client', $config->clientId()); + $this->assertSame('env.okta.com', $config->domain()); + $this->assertSame('ausEnv', $config->authServerId()); + } + + public function testReturnsEmptyStringsWhenNothingIsSet(): void + { + $config = new OktaConfig(); + + $this->assertSame('', $config->clientId()); + $this->assertSame('', $config->domain()); + $this->assertSame('', $config->authServerId()); + } + + public function testIsConfiguredIsFalseWhenNothingIsSet(): void + { + $this->assertFalse((new OktaConfig())->isConfigured()); + } + + public function testIsConfiguredIsFalseWhenOnlySomeValuesAreSet(): void + { + putenv('ACLI_DEVICE_CLIENT_ID=env-client'); + putenv('ACLI_OKTA_DOMAIN=env.okta.com'); + + $this->assertFalse((new OktaConfig())->isConfigured()); + } + + public function testIsConfiguredIsTrueWhenAllValuesResolve(): void + { + putenv('ACLI_DEVICE_CLIENT_ID=env-client'); + putenv('ACLI_OKTA_DOMAIN=env.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausEnv'); + + $this->assertTrue((new OktaConfig())->isConfigured()); + } + + public function testIsConfiguredIsFalseWhenAValueIsSetButEmpty(): void + { + putenv('ACLI_DEVICE_CLIENT_ID=env-client'); + putenv('ACLI_OKTA_DOMAIN=env.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID='); + + $this->assertFalse((new OktaConfig())->isConfigured()); + } +} diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php index 5f0d8f9f3..ee472acb8 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php @@ -15,17 +15,22 @@ use AcquiaCloudApi\Connector\Connector; use Generator; use GuzzleHttp\Client as GuzzleClient; +use GuzzleHttp\Exception\ConnectException; use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; +use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Group; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; +use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Validator\Exception\ValidatorException; /** * @property AuthLoginCommand $command */ +#[Group('serial')] class AuthLoginCommandTest extends CommandTestBase { /** @var array */ @@ -95,6 +100,18 @@ private function givenFreshCloudConfigWithTelemetryDisabled(): void $this->createDataStores(); } + /** + * A LocalMachineHelper double for tests that do not exercise browser opening. + */ + private function headlessLocalMachineHelper(): ObjectProphecy + { + $prophecy = $this->prophet->prophesize(LocalMachineHelper::class); + $prophecy->useTty()->willReturn(false); + $prophecy->isBrowserAvailable()->willReturn(false); + + return $prophecy; + } + private function deviceAuthorizeResponse(): Response { return new Response(200, [], json_encode([ @@ -210,8 +227,7 @@ public function testDeviceCodeFlowFallsBackToLegacyOnFailure(): void $this->clientServiceProphecy->isMachineAuthenticated() ->willReturn(false); - $localMachineHelperProphecy = $this->prophet->prophesize(LocalMachineHelper::class); - $localMachineHelperProphecy->useTty()->willReturn(false); + $localMachineHelperProphecy = $this->headlessLocalMachineHelper(); $this->command = $this->createDeviceCodeCommand([ new Response(400, [], json_encode(['error' => 'invalid_client'])), ], $localMachineHelperProphecy); @@ -312,6 +328,169 @@ public function testSmartRoutingDeviceTokenReauthDeclined(): void $this->assertStringNotContainsString('Authenticated successfully', $output); } + public function testNoInteractionReauthenticatesAnExistingDeviceSessionWithoutPrompting(): void + { + $this->enableDeviceCodeConfig(); + $this->removeMockCloudConfigFile(); + $this->fs->dumpFile($this->cloudConfigFilepath, json_encode([ + 'device_token' => [ + 'access_token' => 'existing-token', + 'client_id' => 'test-client-id', + 'expiry' => time() + 300, + 'refresh_token' => 'existing-refresh-token', + ], + 'send_telemetry' => false, + ])); + $this->createDataStores(); + $localMachineHelperProphecy = $this->headlessLocalMachineHelper(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode([ + 'access_token' => 'new-access-token', + 'expires_in' => 300, + 'refresh_token' => 'new-refresh-token', + ])), + ], $localMachineHelperProphecy); + + $this->executeCommand([], [], OutputInterface::VERBOSITY_VERY_VERBOSE, false); + $output = $this->getDisplay(); + + $this->assertStringNotContainsString('Re-authenticate?', $output); + $this->assertStringContainsString('Authenticated successfully', $output); + } + + public function testNoInteractionWithAnActiveApiKeyRoutesToLegacyWithoutPrompting(): void + { + $this->enableDeviceCodeConfig(); + $localMachineHelperProphecy = $this->headlessLocalMachineHelper(); + $localMachineHelperProphecy->startBrowser(Argument::any())->willReturn(true); + $this->command = $this->createDeviceCodeCommand([], $localMachineHelperProphecy); + + $this->expectException(AcquiaCliException::class); + $this->expectExceptionMessage('Enter your Cloud Platform API key'); + + $this->executeCommand([], [], OutputInterface::VERBOSITY_VERY_VERBOSE, false); + } + + public function testNoInteractionDoesNotOfferLegacyFallbackOnDeviceCodeFailure(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $localMachineHelperProphecy = $this->headlessLocalMachineHelper(); + $this->command = $this->createDeviceCodeCommand([ + new Response(400, [], json_encode(['error' => 'invalid_client'])), + ], $localMachineHelperProphecy); + + $this->executeCommand([], [], OutputInterface::VERBOSITY_VERY_VERBOSE, false); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Failed to initiate device code flow', $output); + $this->assertStringNotContainsString('Fall back to API key authentication?', $output); + $this->assertStringNotContainsString('Saved credentials', $output); + } + + public function testDeviceCodeFlowBacksOffOnSlowDownThenSucceeds(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $localMachineHelperProphecy = $this->headlessLocalMachineHelper(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode(['error' => 'slow_down'])), + new Response(200, [], json_encode([ + 'access_token' => 'access-token-123', + 'expires_in' => 300, + 'refresh_token' => 'refresh-token-123', + ])), + ], $localMachineHelperProphecy); + + $this->executeCommand([], []); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Authenticated successfully', $output); + $this->assertStringNotContainsString('slow_down', $output); + $this->assertStringNotContainsString('Unexpected error', $output); + } + + public function testDeviceCodeFlowRetriesAfterTransportFailureInPollLoop(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $localMachineHelperProphecy = $this->headlessLocalMachineHelper(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new ConnectException('cURL error 28: Operation timed out', new Request('POST', 'https://example.okta.com')), + new Response(200, [], json_encode([ + 'access_token' => 'access-token-123', + 'expires_in' => 300, + 'refresh_token' => 'refresh-token-123', + ])), + ], $localMachineHelperProphecy); + + $this->executeCommand([], []); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Authenticated successfully', $output); + $this->assertStringNotContainsString('Timed out waiting for authorization', $output); + } + + public function testDeviceCodeFlowRejectsTokenResponseWithoutAccessToken(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $localMachineHelperProphecy = $this->headlessLocalMachineHelper(); + $this->command = $this->createDeviceCodeCommand([ + new Response(200, [], json_encode([ + 'device_code' => 'device-123', + 'expires_in' => 600, + 'interval' => 0, + 'user_code' => 'ABCD-EFGH', + 'verification_uri' => 'https://example.okta.com/activate', + ])), + new Response(200, [], json_encode(['token_type' => 'Bearer'])), + ], $localMachineHelperProphecy); + + $this->executeCommand([], ['no']); + $output = $this->getDisplay(); + + $this->assertStringNotContainsString('Authenticated successfully', $output); + $this->assertStringContainsString('Unexpected response', $output); + } + + public function testDeviceCodeFlowSurvivesTransportFailureOnInitiation(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $localMachineHelperProphecy = $this->headlessLocalMachineHelper(); + $this->command = $this->createDeviceCodeCommand([ + new ConnectException('cURL error 6: Could not resolve host', new Request('POST', 'https://example.okta.com')), + ], $localMachineHelperProphecy); + + $this->executeCommand([], ['no']); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Failed to initiate device code flow', $output); + } + + public function testReportsWhyItFallsBackWhenDeviceCodeIsNotConfigured(): void + { + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->mockRequest('getAccount'); + $this->clientServiceProphecy->setConnector(Argument::type(Connector::class)) + ->shouldBeCalled(); + $this->clientServiceProphecy->isMachineAuthenticated() + ->willReturn(false); + + $localMachineHelperProphecy = $this->headlessLocalMachineHelper(); + $this->command = $this->createDeviceCodeCommand([], $localMachineHelperProphecy); + + $this->executeCommand([], ['no', self::$key, self::$secret]); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Device code sign-in is not configured', $output); + $this->assertStringContainsString('Saved credentials', $output); + } + // ------------------------------------------------------------------------- // Legacy auth tests // ------------------------------------------------------------------------- diff --git a/tests/phpunit/src/Commands/Auth/AuthLogoutCommandTest.php b/tests/phpunit/src/Commands/Auth/AuthLogoutCommandTest.php index a915c6a4c..17f1998d2 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLogoutCommandTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLogoutCommandTest.php @@ -29,6 +29,73 @@ public function testAuthLogoutCommand(): void $this->assertStringContainsString('The active Cloud Platform API credentials were deactivated', $output); } + public function testAuthLogoutWithDeviceTokenOnly(): void + { + $this->removeMockCloudConfigFile(); + $this->fs->dumpFile($this->cloudConfigFilepath, json_encode([ + 'device_token' => [ + 'access_token' => 'existing-token', + 'client_id' => 'client-123', + 'expiry' => time() + 300, + 'refresh_token' => 'existing-refresh-token', + ], + 'send_telemetry' => false, + ])); + $this->createDataStores(); + $this->command = $this->createCommand(); + + $this->executeCommand(); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Device code session removed', $output); + $this->assertStringContainsString('No Cloud Platform credentials are active', $output); + $this->assertStringNotContainsString('will be deactivated on this machine', $output); + $this->assertArrayNotHasKey('device_token', json_decode(file_get_contents($this->cloudConfigFilepath), true)); + } + + public function testAuthLogoutRemovesBothCredentialTypes(): void + { + $this->executeCommandWithDeviceTokenAlongsideKey(); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Device code session removed', $output); + $this->assertStringContainsString('The key Test Key will be deactivated on this machine.', $output); + $this->assertStringContainsString('The active Cloud Platform API credentials were deactivated', $output); + + $config = json_decode(file_get_contents($this->cloudConfigFilepath), true); + $this->assertArrayNotHasKey('device_token', $config); + $this->assertArrayNotHasKey('acli_key', $config); + } + + public function testAuthLogoutWithNoCredentialsAtAll(): void + { + $this->removeMockCloudConfigFile(); + $this->fs->dumpFile($this->cloudConfigFilepath, json_encode(['send_telemetry' => false])); + $this->createDataStores(); + $this->command = $this->createCommand(); + + $this->expectException(AcquiaCliException::class); + $this->expectExceptionMessage('There are no active Cloud Platform credentials'); + + $this->executeCommand(); + } + + private function executeCommandWithDeviceTokenAlongsideKey(): void + { + $config = json_decode(file_get_contents($this->cloudConfigFilepath), true); + $config['device_token'] = [ + 'access_token' => 'existing-token', + 'client_id' => 'client-123', + 'expiry' => time() + 300, + 'refresh_token' => 'existing-refresh-token', + ]; + $this->fs->dumpFile($this->cloudConfigFilepath, json_encode($config)); + $this->createDataStores(); + $this->command = $this->createCommand(); + + $this->executeCommand(); + } + public function testAuthLogoutInvalidDatastore(): void { $this->clientServiceProphecy->isMachineAuthenticated() From 8f775213d58ceff6ba934b510fee9cab2ffa2ae2 Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Mon, 28 Sep 2026 17:20:37 +0530 Subject: [PATCH 07/16] addressed the CI patchcov --- src/CloudApi/DeviceTokenRefresher.php | 122 +++++-- src/Command/Auth/AuthLoginCommand.php | 34 +- .../src/AcsfApi/AcsfCredentialsTest.php | 20 ++ .../src/CloudApi/DeviceTokenRefresherTest.php | 336 +++++++++--------- .../Commands/Auth/AuthLoginCommandTest.php | 163 +++++++++ .../Commands/Auth/AuthLogoutCommandTest.php | 11 + 6 files changed, 474 insertions(+), 212 deletions(-) create mode 100644 tests/phpunit/src/AcsfApi/AcsfCredentialsTest.php diff --git a/src/CloudApi/DeviceTokenRefresher.php b/src/CloudApi/DeviceTokenRefresher.php index 37bb5aa17..b99a2fea9 100644 --- a/src/CloudApi/DeviceTokenRefresher.php +++ b/src/CloudApi/DeviceTokenRefresher.php @@ -13,6 +13,11 @@ class DeviceTokenRefresher { + /** + * Refresh this long before expiry, so in-flight requests don't 401. + */ + private const PRE_EXPIRY_WINDOW_SECONDS = 60; + private const REQUEST_TIMEOUT_SECONDS = 15; public function __construct( @@ -26,8 +31,8 @@ public function __construct( /** * Returns a valid device access token, refreshing silently if expired. * - * Returns null when no token is stored, the refresh token is missing, or - * the refresh request fails — callers should fall back to other auth methods. + * Returns null when no token is stored, the refresh token is missing, or the + * refresh request fails — callers should surface a re-login prompt. */ public function getValidAccessToken(): ?string { @@ -36,17 +41,36 @@ public function getValidAccessToken(): ?string return null; } - $token = new AccessToken([ - 'access_token' => $stored['access_token'], - 'expires' => $stored['expiry'] ?? 0, - ]); + if ($this->hasLifeLeft($stored)) { + return $stored['access_token']; + } - // Refresh 60 s before actual expiry so requests in-flight don't hit a 401. - if (($token->getExpires() - time()) > 60) { + // Presenting a rotated refresh token invalidates the whole session, so only + // one invocation may refresh a given token. + $lock = $this->acquireLock(); + try { + return $this->refresh(); + } finally { + if ($lock !== null) { + flock($lock, LOCK_UN); + fclose($lock); + } + } + } + + /** + * Performs the refresh, assuming the caller holds the lock. + */ + private function refresh(): ?string + { + $stored = $this->readTokenFromDisk(); + if (!$stored || empty($stored['access_token'])) { + return null; + } + if ($this->hasLifeLeft($stored)) { return $stored['access_token']; } - // Access token expired — attempt a silent refresh. $refreshToken = $stored['refresh_token'] ?? null; $clientId = $stored['client_id'] ?? null; $domain = $this->oktaConfig->domain(); @@ -75,19 +99,14 @@ public function getValidAccessToken(): ?string $error = is_array($body) ? ($body['error'] ?? '') : ''; if ($error === 'invalid_grant') { - if ($winnersToken = $this->tokenWrittenByAnotherProcess($stored)) { - return $winnersToken; - } $this->logger->warning('Device token refresh failed: the refresh token is expired or revoked. Run `acli auth:login` to re-authenticate.'); return null; } - if ($status === 400 || $status === 401) { $this->logger->warning('Device token refresh failed (HTTP {status}). Run `acli auth:login` to re-authenticate.', ['status' => $status]); } return null; } catch (GuzzleException) { - // Network / transport error — fail silently; caller falls back. return null; } @@ -95,7 +114,7 @@ public function getValidAccessToken(): ?string return null; } - $this->datastore->set('device_token', [ + $this->persistToken([ 'access_token' => $new['access_token'], 'client_id' => $clientId, 'expiry' => isset($new['expires_in']) ? time() + (int) $new['expires_in'] : 0, @@ -106,25 +125,78 @@ public function getValidAccessToken(): ?string } /** - * Returns a usable access token another process wrote while we were refreshing. - * - * Null means nothing changed, so the refresh token really is expired or revoked. + * @param array $token + */ + private function hasLifeLeft(array $token): bool + { + $accessToken = new AccessToken([ + 'access_token' => $token['access_token'], + 'expires' => $token['expiry'] ?? 0, + ]); + + return ($accessToken->getExpires() - time()) > self::PRE_EXPIRY_WINDOW_SECONDS; + } + + /** + * Reads `device_token` from disk, bypassing the in-memory datastore. * - * @param array $stored the token this call started with + * @return array|null */ - private function tokenWrittenByAnotherProcess(array $stored): ?string + private function readTokenFromDisk(): ?array { - $current = $this->datastore->get('device_token'); - if (!is_array($current) || empty($current['access_token'])) { + $contents = @file_get_contents($this->datastore->filepath); + if ($contents === false) { return null; } - if ($current['access_token'] === ($stored['access_token'] ?? null)) { + + $decoded = json_decode($contents, true); + if (!is_array($decoded) || !isset($decoded['device_token']) || !is_array($decoded['device_token'])) { return null; } - if ((($current['expiry'] ?? 0) - time()) <= 60) { + + return $decoded['device_token']; + } + + /** + * Writes `device_token` without disturbing anything else in the file. + * + * @param array $token + */ + private function persistToken(array $token): void + { + $path = $this->datastore->filepath; + $contents = @file_get_contents($path); + $data = $contents === false ? null : json_decode($contents, true); + + if (!is_array($data)) { + $this->datastore->set('device_token', $token); + return; + } + + $data['device_token'] = $token; + file_put_contents($path, json_encode($data, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT)); + chmod($path, 0600); + } + + /** + * Takes an exclusive lock on a sidecar file beside the datastore. + * + * @return resource|null + */ + private function acquireLock() + { + $handle = @fopen($this->datastore->filepath . '.lock', 'c'); + if ($handle === false) { + $this->logger->warning('Could not open the device token lock file; refreshing without serialisation.'); + return null; + } + + if (!flock($handle, LOCK_EX)) { + fclose($handle); + $this->logger->warning('Could not lock the device token lock file; refreshing without serialisation.'); return null; } - return $current['access_token']; + return $handle; } } diff --git a/src/Command/Auth/AuthLoginCommand.php b/src/Command/Auth/AuthLoginCommand.php index fe98edc89..ad64534db 100644 --- a/src/Command/Auth/AuthLoginCommand.php +++ b/src/Command/Auth/AuthLoginCommand.php @@ -72,16 +72,17 @@ protected function execute(InputInterface $input, OutputInterface $output): int $activeKey = $this->datastoreCloud->get('acli_key'); $deviceToken = $this->datastoreCloud->get('device_token'); - // Smart routing: existing API key → legacy, device token → device code re-auth. + if (!$this->oktaConfig->isConfigured()) { + $output->writeln('Device code sign-in is not configured; falling back to API key authentication.'); + return $this->executeLegacyAuth($input, $output); + } + if ($activeKey && $keys) { $label = $keys[$activeKey]['label'] ?? $activeKey; $output->writeln("Already authenticated as $label (API key)"); - if ($input->isInteractive()) { - $reauth = $this->io->confirm('Re-authenticate?', false); - if (!$reauth) { - return Command::SUCCESS; - } + if ($input->isInteractive() && $this->io->confirm('Sign in with device code instead?', false)) { + return $this->executeDeviceCodeFlowWithFallback($input, $output); } return $this->executeLegacyAuth($input, $output); } @@ -98,15 +99,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int return $this->executeDeviceCodeFlowWithFallback($input, $output); } - // No stored credentials — try device code, - // otherwise fall back to legacy key/secret prompt. - if ($this->isDeviceCodeConfigured()) { - return $this->executeDeviceCodeFlowWithFallback($input, $output); - } - - $output->writeln('Device code sign-in is not configured in this build; falling back to API key authentication.'); - - return $this->executeLegacyAuth($input, $output); + return $this->executeDeviceCodeFlowWithFallback($input, $output); } private function executeDeviceCodeFlowWithFallback(InputInterface $input, OutputInterface $output): int @@ -122,10 +115,7 @@ private function executeDeviceCodeFlowWithFallback(InputInterface $input, Output return $result; } - private function isDeviceCodeConfigured(): bool - { - return $this->oktaConfig->isConfigured(); - } + private function executeDeviceCodeFlow(OutputInterface $output): int { @@ -259,6 +249,12 @@ private function storeDeviceToken(array $token, string $clientId): void 'expiry' => $expiry, 'refresh_token' => $token['refresh_token'] ?? null, ]); + + // The key itself stays on + // disk and can be reactivated with --use-legacy-auth. + if ($this->datastoreCloud->get('acli_key')) { + $this->datastoreCloud->remove('acli_key'); + } } private function executeLegacyAuth(InputInterface $input, OutputInterface $output): int diff --git a/tests/phpunit/src/AcsfApi/AcsfCredentialsTest.php b/tests/phpunit/src/AcsfApi/AcsfCredentialsTest.php new file mode 100644 index 000000000..e02cf3095 --- /dev/null +++ b/tests/phpunit/src/AcsfApi/AcsfCredentialsTest.php @@ -0,0 +1,20 @@ +datastoreCloud); + + $this->assertNull($credentials->getCloudDeviceAccessToken()); + } +} diff --git a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php index 038ec666f..06a3d541f 100644 --- a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php +++ b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php @@ -6,25 +6,27 @@ use Acquia\Cli\CloudApi\DeviceTokenRefresher; use Acquia\Cli\CloudApi\OktaConfig; +use Acquia\Cli\Config\CloudDataConfig; use Acquia\Cli\DataStore\CloudDataStore; use Acquia\Cli\Tests\TestBase; use GuzzleHttp\Client as GuzzleClient; +use GuzzleHttp\Exception\ConnectException; use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; use GuzzleHttp\Promise\Create; +use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\Attributes\Group; -use Prophecy\Argument; -use Prophecy\Prophecy\ObjectProphecy; +/** + * Uses a real on-disk CloudDataStore rather than a double. + */ #[Group('serial')] class DeviceTokenRefresherTest extends TestBase { /** @var array */ private array $savedEnvVars = []; - private ObjectProphecy $datastoreProphecy; - protected function setUp(): void { parent::setUp(); @@ -32,124 +34,119 @@ protected function setUp(): void $this->savedEnvVars[$var] = getenv($var); putenv($var); } + $this->removeMockCloudConfigFile(); } protected function tearDown(): void { - parent::tearDown(); foreach ($this->savedEnvVars as $var => $value) { putenv($value === false ? $var : $var . '=' . $value); } + parent::tearDown(); + } + + private function givenOktaIsConfigured(): void + { + putenv('ACLI_OKTA_DOMAIN=example.okta.com'); + putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + } + + /** + * Writes the credential file. + * + * Call before createRefresher() to seed what its datastore loads; call after + * to simulate another invocation writing while this one holds a stale + * in-memory snapshot. + * + * @param array $config + */ + private function writeCloudConfig(array $config): void + { + $this->fs->dumpFile($this->cloudConfigFilepath, json_encode($config + ['send_telemetry' => false])); } - private function createRefresher(array $responses = []): DeviceTokenRefresher + /** + * @return array + */ + private function readCloudConfig(): array { - $mock = new MockHandler($responses); - $client = new GuzzleClient(['handler' => HandlerStack::create($mock)]); - $datastoreProphecy = $this->prophet->prophesize(CloudDataStore::class); - $this->datastoreProphecy = $datastoreProphecy; + return json_decode(file_get_contents($this->cloudConfigFilepath), true); + } + + /** + * @param array $responses + */ + private function createRefresher(array $responses = [], ?callable $handler = null): DeviceTokenRefresher + { + $client = new GuzzleClient([ + 'handler' => $handler ?? HandlerStack::create(new MockHandler($responses)), + ]); return new DeviceTokenRefresher( - $datastoreProphecy->reveal(), + new CloudDataStore($this->localMachineHelper, new CloudDataConfig(), $this->cloudConfigFilepath), $this->logger, $client, new OktaConfig(), ); } - public function testRefreshRequestCarriesAnExplicitTimeout(): void + /** + * @return array + */ + private function expiredToken(): array { - // A hung /token would otherwise block every API request indefinitely. - putenv('ACLI_OKTA_DOMAIN=example.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); - $captured = []; - $handler = function ($request, array $options) use (&$captured) { - $captured[] = $options; - return Create::promiseFor(new Response(200, [], json_encode([ - 'access_token' => 'new-access-token', - 'expires_in' => 3600, - ]))); - }; - $datastoreProphecy = $this->prophet->prophesize(CloudDataStore::class); - $datastoreProphecy->get('device_token')->willReturn([ + return [ 'access_token' => 'expired-token', 'client_id' => 'client-123', 'expiry' => time() - 300, 'refresh_token' => 'old-refresh-token', - ]); - $datastoreProphecy->set('device_token', Argument::any())->shouldBeCalled(); - $refresher = new DeviceTokenRefresher( - $datastoreProphecy->reveal(), - $this->logger, - new GuzzleClient(['handler' => $handler]), - new OktaConfig(), - ); - - $refresher->getValidAccessToken(); - - $this->assertNotEmpty($captured, 'Expected a refresh request to be made.'); - $this->assertArrayHasKey('timeout', $captured[0]); - $this->assertGreaterThan(0, $captured[0]['timeout']); + ]; } public function testReturnsNullWhenNoTokenStored(): void { - $refresher = $this->createRefresher(); - $this->datastoreProphecy->get('device_token')->willReturn(null); - - $this->assertNull($refresher->getValidAccessToken()); - } - - public function testReturnsNullWhenStoredTokenHasNoAccessToken(): void - { - $refresher = $this->createRefresher(); - $this->datastoreProphecy->get('device_token')->willReturn(['access_token' => '']); + $this->writeCloudConfig([]); - $this->assertNull($refresher->getValidAccessToken()); + $this->assertNull($this->createRefresher()->getValidAccessToken()); } public function testReturnsStoredTokenWhenNotExpired(): void { - $refresher = $this->createRefresher(); - $this->datastoreProphecy->get('device_token')->willReturn([ + $this->writeCloudConfig(['device_token' => [ 'access_token' => 'valid-token', + 'client_id' => 'client-123', 'expiry' => time() + 300, + 'refresh_token' => 'refresh-123', + ], ]); - $this->assertEquals('valid-token', $refresher->getValidAccessToken()); + $this->assertSame('valid-token', $this->createRefresher()->getValidAccessToken()); } public function testReturnsNullWhenExpiredAndRefreshTokenMissing(): void { - putenv('ACLI_OKTA_DOMAIN=example.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); - $refresher = $this->createRefresher(); - $this->datastoreProphecy->get('device_token')->willReturn([ + $this->givenOktaIsConfigured(); + $this->writeCloudConfig(['device_token' => [ 'access_token' => 'expired-token', 'client_id' => 'client-123', 'expiry' => time() - 300, + ], ]); - $this->assertNull($refresher->getValidAccessToken()); + $this->assertNull($this->createRefresher()->getValidAccessToken()); } public function testReturnsNullWhenExpiredAndOktaConfigMissing(): void { - $refresher = $this->createRefresher(); - $this->datastoreProphecy->get('device_token')->willReturn([ - 'access_token' => 'expired-token', - 'client_id' => 'client-123', - 'expiry' => time() - 300, - 'refresh_token' => 'refresh-123', - ]); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); - $this->assertNull($refresher->getValidAccessToken()); + $this->assertNull($this->createRefresher()->getValidAccessToken()); } - public function testResolvesOktaConfigThroughTheSharedResolver(): void + public function testRefreshesExpiredTokenAndPersistsIt(): void { - putenv('ACLI_OKTA_DOMAIN=shared.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausShared'); + $this->givenOktaIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ new Response(200, [], json_encode([ 'access_token' => 'new-access-token', @@ -157,146 +154,149 @@ public function testResolvesOktaConfigThroughTheSharedResolver(): void 'refresh_token' => 'new-refresh-token', ])), ]); - $this->datastoreProphecy->get('device_token')->willReturn([ - 'access_token' => 'expired-token', - 'client_id' => 'client-123', - 'expiry' => time() - 300, - 'refresh_token' => 'old-refresh-token', - ]); - $this->datastoreProphecy->set('device_token', Argument::any())->shouldBeCalled(); - $this->assertEquals('new-access-token', $refresher->getValidAccessToken()); + $this->assertSame('new-access-token', $refresher->getValidAccessToken()); + + $stored = $this->readCloudConfig()['device_token']; + $this->assertSame('new-access-token', $stored['access_token']); + $this->assertSame('new-refresh-token', $stored['refresh_token']); + $this->assertSame('client-123', $stored['client_id']); + $this->assertGreaterThan(time(), $stored['expiry']); } - public function testRefreshesExpiredTokenSuccessfully(): void + public function testKeepsOldRefreshTokenWhenResponseOmitsNewOne(): void { - putenv('ACLI_OKTA_DOMAIN=example.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $this->givenOktaIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ - new Response(200, [], json_encode([ - 'access_token' => 'new-access-token', - 'expires_in' => 3600, - 'refresh_token' => 'new-refresh-token', - ])), + new Response(200, [], json_encode(['access_token' => 'new-access-token', 'expires_in' => 3600])), ]); - $this->datastoreProphecy->get('device_token')->willReturn([ - 'access_token' => 'expired-token', - 'client_id' => 'client-123', - 'expiry' => time() - 300, - 'refresh_token' => 'old-refresh-token', - ]); - $this->datastoreProphecy->set('device_token', Argument::that( - fn (array $token): bool => $token['access_token'] === 'new-access-token' - && $token['client_id'] === 'client-123' - && $token['refresh_token'] === 'new-refresh-token' - && is_int($token['expiry']) - ))->shouldBeCalled(); - - $this->assertEquals('new-access-token', $refresher->getValidAccessToken()); + + $refresher->getValidAccessToken(); + + $this->assertSame('old-refresh-token', $this->readCloudConfig()['device_token']['refresh_token']); } - public function testKeepsOldRefreshTokenWhenResponseOmitsNewOne(): void + public function testUsesATokenAnotherInvocationWroteWhileThisOneWasStale(): void { - putenv('ACLI_OKTA_DOMAIN=example.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + // The datastore loads an expired token; another invocation then writes a + // fresh one. Spending a rotation here would kill the session for both. + $this->givenOktaIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ - new Response(200, [], json_encode([ - 'access_token' => 'new-access-token', - 'expires_in' => 3600, - ])), + new Response(500, [], 'the refresher must not reach Okta'), ]); - $this->datastoreProphecy->get('device_token')->willReturn([ - 'access_token' => 'expired-token', + + $this->writeCloudConfig(['device_token' => [ + 'access_token' => 'written-by-another-process', 'client_id' => 'client-123', - 'expiry' => time() - 300, - 'refresh_token' => 'old-refresh-token', + 'expiry' => time() + 3600, + 'refresh_token' => 'their-refresh-token', + ], ]); - $this->datastoreProphecy->set('device_token', Argument::that( - fn (array $token): bool => $token['access_token'] === 'new-access-token' - && $token['client_id'] === 'client-123' - && $token['refresh_token'] === 'old-refresh-token' - && is_int($token['expiry']) - ))->shouldBeCalled(); - - $this->assertEquals('new-access-token', $refresher->getValidAccessToken()); + + $this->assertSame('written-by-another-process', $refresher->getValidAccessToken()); } - public function testReturnsNullWhenRefreshResponseHasNoAccessToken(): void + public function testDoesNotClobberCredentialsWrittenSinceTheProcessStarted(): void { - putenv('ACLI_OKTA_DOMAIN=example.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $this->givenOktaIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ - new Response(200, [], json_encode(['error' => 'invalid_grant'])), + new Response(200, [], json_encode(['access_token' => 'new-access-token', 'expires_in' => 3600])), ]); - $this->datastoreProphecy->get('device_token')->willReturn([ - 'access_token' => 'expired-token', - 'client_id' => 'client-123', - 'expiry' => time() - 300, - 'refresh_token' => 'old-refresh-token', + + $this->writeCloudConfig([ + 'acli_key' => 'added-later', + 'device_token' => $this->expiredToken(), + 'keys' => ['added-later' => ['label' => 'Later Key', 'secret' => 's', 'uuid' => 'added-later']], ]); - $this->assertNull($refresher->getValidAccessToken()); + $refresher->getValidAccessToken(); + + $config = $this->readCloudConfig(); + $this->assertSame('added-later', $config['acli_key']); + $this->assertArrayHasKey('added-later', $config['keys']); + $this->assertSame('new-access-token', $config['device_token']['access_token']); } - public function testUsesTheWinnersTokenWhenAConcurrentInvocationRotatedFirst(): void + public function testReturnsNullOnInvalidGrant(): void { - // Under ROTATE this call loses the race: invalid_grant, but the winner has - // already persisted a fresh token, so the session is fine. - putenv('ACLI_OKTA_DOMAIN=example.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $this->givenOktaIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ new Response(400, [], json_encode(['error' => 'invalid_grant'])), ]); - $this->datastoreProphecy->get('device_token')->willReturn( - [ - 'access_token' => 'stale-token', - 'client_id' => 'client-123', - 'expiry' => time() - 300, - 'refresh_token' => 'rotated-away-refresh-token', - ], - [ - 'access_token' => 'winners-fresh-token', - 'client_id' => 'client-123', - 'expiry' => time() + 300, - 'refresh_token' => 'winners-refresh-token', - ], - ); - $this->assertEquals('winners-fresh-token', $refresher->getValidAccessToken()); + $this->assertNull($refresher->getValidAccessToken()); + $this->assertSame('expired-token', $this->readCloudConfig()['device_token']['access_token']); } - public function testReturnsNullWhenInvalidGrantAndNoConcurrentRefreshHappened(): void + public function testReturnsNullOnA400ThatIsNotInvalidGrant(): void { - putenv('ACLI_OKTA_DOMAIN=example.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $this->givenOktaIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ - new Response(400, [], json_encode(['error' => 'invalid_grant'])), + new Response(400, [], json_encode(['error' => 'invalid_client'])), ]); - $stored = [ - 'access_token' => 'stale-token', - 'client_id' => 'client-123', - 'expiry' => time() - 300, - 'refresh_token' => 'dead-refresh-token', - ]; - $this->datastoreProphecy->get('device_token')->willReturn($stored, $stored); $this->assertNull($refresher->getValidAccessToken()); } - public function testReturnsNullOnRefreshRequestFailure(): void + public function testReturnsNullOnATransportFailure(): void { - putenv('ACLI_OKTA_DOMAIN=example.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + $this->givenOktaIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ - new Response(400, [], json_encode(['error' => 'invalid_grant'])), + new ConnectException('cURL error 6: Could not resolve host', new Request('POST', 'https://example.okta.com')), ]); - $this->datastoreProphecy->get('device_token')->willReturn([ - 'access_token' => 'expired-token', - 'client_id' => 'client-123', - 'expiry' => time() - 300, - 'refresh_token' => 'old-refresh-token', + + $this->assertNull($refresher->getValidAccessToken()); + } + + public function testReturnsNullWhenRefreshResponseHasNoAccessToken(): void + { + $this->givenOktaIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); + $refresher = $this->createRefresher([ + new Response(200, [], json_encode(['token_type' => 'Bearer'])), ]); $this->assertNull($refresher->getValidAccessToken()); } + + public function testRefreshRequestCarriesAnExplicitTimeout(): void + { + // GuzzleHttp\Client is autowired, so the container injects one built + // without options; a hung /token would block every API request. + $this->givenOktaIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); + $captured = []; + $refresher = $this->createRefresher([], function ($request, array $options) use (&$captured) { + $captured[] = $options; + return Create::promiseFor(new Response(200, [], json_encode([ + 'access_token' => 'new-access-token', + 'expires_in' => 3600, + ]))); + }); + + $refresher->getValidAccessToken(); + + $this->assertNotEmpty($captured); + $this->assertArrayHasKey('timeout', $captured[0]); + $this->assertGreaterThan(0, $captured[0]['timeout']); + } + + public function testTakesTheLockBesideTheCredentialFile(): void + { + $this->givenOktaIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); + $refresher = $this->createRefresher([ + new Response(200, [], json_encode(['access_token' => 'new-access-token', 'expires_in' => 3600])), + ]); + + $refresher->getValidAccessToken(); + + $this->assertFileExists($this->cloudConfigFilepath . '.lock'); + } } diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php index ee472acb8..5df7b23d3 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php @@ -41,6 +41,7 @@ protected function setUp(): void parent::setUp(); foreach (['ACLI_DEVICE_CLIENT_ID', 'ACLI_OKTA_DOMAIN', 'ACLI_OKTA_AUTH_SERVER_ID'] as $var) { $this->savedDeviceCodeEnvVars[$var] = getenv($var); + putenv($var); } } @@ -434,6 +435,168 @@ public function testDeviceCodeFlowRetriesAfterTransportFailureInPollLoop(): void $this->assertStringNotContainsString('Timed out waiting for authorization', $output); } + public function testUsesDeviceCodeAfterLogoutEvenWhenKeysRemainOnDisk(): void + { + // Login routes on the *active* key, which auth:logout clears while leaving + // `keys` on disk. This is the migration path off API keys. + $this->enableDeviceCodeConfig(); + $this->removeMockCloudConfigFile(); + $this->fs->dumpFile($this->cloudConfigFilepath, json_encode([ + 'keys' => [ + 'key1' => ['label' => 'Test Key', 'secret' => 'secret', 'uuid' => 'key1'], + ], + 'send_telemetry' => false, + ])); + $this->createDataStores(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode([ + 'access_token' => 'access-token-123', + 'expires_in' => 300, + 'refresh_token' => 'refresh-token-123', + ])), + ], $this->headlessLocalMachineHelper()); + + $this->executeCommand([], []); + $output = $this->getDisplay(); + + $this->assertStringNotContainsString('Already authenticated as', $output); + $this->assertStringContainsString('Sign in to Acquia ID in your browser', $output); + $this->assertStringContainsString('Authenticated successfully', $output); + $this->assertArrayHasKey('device_token', json_decode(file_get_contents($this->cloudConfigFilepath), true)); + } + + + public function testDeviceTokenBranchRequiresOktaConfig(): void + { + $this->removeMockCloudConfigFile(); + $this->fs->dumpFile($this->cloudConfigFilepath, json_encode([ + 'device_token' => [ + 'access_token' => 'existing-token', + 'client_id' => 'client-123', + 'expiry' => time() + 300, + 'refresh_token' => 'existing-refresh-token', + ], + 'send_telemetry' => false, + ])); + $this->createDataStores(); + $this->mockRequest('getAccount'); + $this->clientServiceProphecy->setConnector(Argument::type(Connector::class))->shouldBeCalled(); + $this->clientServiceProphecy->isMachineAuthenticated()->willReturn(false); + $this->command = $this->createDeviceCodeCommand([], $this->headlessLocalMachineHelper()); + + $this->executeCommand([], ['no', self::$key, self::$secret]); + $output = $this->getDisplay(); + + $this->assertStringContainsString('not configured', $output); + $this->assertStringNotContainsString('Unable to parse URI', $output); + } + + public function testOffersDeviceCodeToAKeyHolderAndDeactivatesTheKeyOnSuccess(): void + { + $this->enableDeviceCodeConfig(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode([ + 'access_token' => 'access-token-123', + 'expires_in' => 300, + 'refresh_token' => 'refresh-token-123', + ])), + ], $this->headlessLocalMachineHelper()); + + $this->executeCommand([], ['yes']); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Sign in with device code instead?', $output); + $this->assertStringContainsString('Authenticated successfully', $output); + + // Without clearing acli_key the token sits unreachable behind + // ConnectorFactory priority (2) and the login changes nothing. + $config = json_decode(file_get_contents($this->cloudConfigFilepath), true); + $this->assertArrayNotHasKey('acli_key', $config); + $this->assertArrayHasKey('device_token', $config); + $this->assertArrayHasKey('keys', $config); + } + + public function testDecliningTheDeviceCodeOfferReachesTheKeyChooser(): void + { + // Declining must not exit: auth:login's documented purpose is switching + // between stored accounts, which is what the chooser is for. + $this->enableDeviceCodeConfig(); + $this->clientServiceProphecy->setConnector(Argument::type(Connector::class))->shouldBeCalled(); + $this->clientServiceProphecy->isMachineAuthenticated()->willReturn(false); + $this->command = $this->createDeviceCodeCommand([], $this->headlessLocalMachineHelper()); + + $this->executeCommand([], ['no', 'Test Key']); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Activate a Cloud Platform API key', $output); + $this->assertStringNotContainsString('Sign in to Acquia ID in your browser', $output); + } + + public function testDeviceCodeFlowRejectsMalformedAuthorizeResponse(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->command = $this->createDeviceCodeCommand([ + new Response(200, [], json_encode(['user_code' => 'ABCD-EFGH'])), + ], $this->headlessLocalMachineHelper()); + + $this->executeCommand([], ['no']); + + $this->assertStringContainsString('Unexpected response from device authorization endpoint', $this->getDisplay()); + } + + public function testDeviceCodeFlowStopsOnAnUnrecognizedTokenError(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode(['error' => 'invalid_scope'])), + ], $this->headlessLocalMachineHelper()); + + $this->executeCommand([], ['no']); + + $this->assertStringContainsString('Unexpected error: invalid_scope', $this->getDisplay()); + } + + public function testDeviceCodeFlowTimesOutWhenTheWindowCloses(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->command = $this->createDeviceCodeCommand([ + new Response(200, [], json_encode([ + 'device_code' => 'device-123', + 'expires_in' => 0, + 'interval' => 0, + 'user_code' => 'ABCD-EFGH', + 'verification_uri' => 'https://example.okta.com/activate', + ])), + ], $this->headlessLocalMachineHelper()); + + $this->executeCommand([], ['no']); + + $this->assertStringContainsString('Timed out waiting for authorization', $this->getDisplay()); + } + + public function testDeviceCodeFlowStoresAZeroExpiryWhenOktaOmitsExpiresIn(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(200, [], json_encode(['access_token' => 'access-token-123'])), + ], $this->headlessLocalMachineHelper()); + + $this->executeCommand([], []); + + $this->assertStringContainsString('Authenticated successfully', $this->getDisplay()); + $stored = json_decode(file_get_contents($this->cloudConfigFilepath), true)['device_token']; + $this->assertSame(0, $stored['expiry']); + $this->assertNull($stored['refresh_token']); + } + public function testDeviceCodeFlowRejectsTokenResponseWithoutAccessToken(): void { $this->enableDeviceCodeConfig(); diff --git a/tests/phpunit/src/Commands/Auth/AuthLogoutCommandTest.php b/tests/phpunit/src/Commands/Auth/AuthLogoutCommandTest.php index 17f1998d2..1e1a71292 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLogoutCommandTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLogoutCommandTest.php @@ -96,6 +96,17 @@ private function executeCommandWithDeviceTokenAlongsideKey(): void $this->executeCommand(); } + public function testAuthLogoutDeletesCredentialsWhenAsked(): void + { + $this->executeCommand(['--delete' => true]); + $output = $this->getDisplay(); + + $this->assertStringContainsString('The active Cloud Platform API credentials were deleted', $output); + $config = json_decode(file_get_contents($this->cloudConfigFilepath), true); + $this->assertArrayNotHasKey('acli_key', $config); + $this->assertSame([], $config['keys'] ?? []); + } + public function testAuthLogoutInvalidDatastore(): void { $this->clientServiceProphecy->isMachineAuthenticated() From 5bdcefb692f522f24450c4d8d2fe68dc1ee94125 Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Mon, 28 Sep 2026 18:03:00 +0530 Subject: [PATCH 08/16] resolve mutation testing --- src/Command/Auth/AuthLoginCommand.php | 2 +- tests/phpunit/src/AcsfApi/AcsfServiceTest.php | 22 +++++++++++++ .../Commands/Auth/AuthLoginCommandTest.php | 33 +++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/Command/Auth/AuthLoginCommand.php b/src/Command/Auth/AuthLoginCommand.php index ad64534db..58afff6af 100644 --- a/src/Command/Auth/AuthLoginCommand.php +++ b/src/Command/Auth/AuthLoginCommand.php @@ -259,7 +259,7 @@ private function storeDeviceToken(array $token, string $clientId): void private function executeLegacyAuth(InputInterface $input, OutputInterface $output): int { - $env = $input->getOption('environment') ?? 'prod'; + $env = $input->getOption('environment'); [$baseUri, $accountsUri] = $this->getUrisForEnvironment($env); $keys = $this->datastoreCloud->get('keys'); diff --git a/tests/phpunit/src/AcsfApi/AcsfServiceTest.php b/tests/phpunit/src/AcsfApi/AcsfServiceTest.php index 363bf3402..b2891e5c7 100644 --- a/tests/phpunit/src/AcsfApi/AcsfServiceTest.php +++ b/tests/phpunit/src/AcsfApi/AcsfServiceTest.php @@ -56,4 +56,26 @@ public function testIsMachineAuthenticated(array $envVars, bool $isAuthenticated $this->assertEquals($isAuthenticated, $clientService->isMachineAuthenticated()); self::unsetEnvVars($envVars); } + + public function testACloudPlatformDeviceTokenDoesNotAuthenticateAcsf(): void + { + $this->removeMockCloudConfigFile(); + $this->fs->dumpFile($this->cloudConfigFilepath, json_encode([ + 'device_token' => [ + 'access_token' => 'cloud-platform-token', + 'client_id' => 'client-123', + 'expiry' => time() + 300, + 'refresh_token' => 'cloud-platform-refresh-token', + ], + 'send_telemetry' => false, + ])); + $this->createDataStores(); + $clientService = new AcsfClientService(new AcsfConnectorFactory([ + 'key' => null, + 'secret' => null, + ]), $this->prophet->prophesize(Application::class) + ->reveal(), new AcsfCredentials($this->datastoreCloud)); + + $this->assertFalse($clientService->isMachineAuthenticated()); + } } diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php index 5df7b23d3..9dcdb9842 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php @@ -534,6 +534,39 @@ public function testDecliningTheDeviceCodeOfferReachesTheKeyChooser(): void $this->assertStringNotContainsString('Sign in to Acquia ID in your browser', $output); } + /** + * @return array, array}> + */ + public static function providerLegacyRoutingOptions(): array + { + return [ + 'key alone' => [['--key' => self::$key], ['no', self::$secret]], + 'secret alone' => [['--secret' => self::$secret], ['no', self::$key]], + 'use-legacy-auth alone' => [['--use-legacy-auth' => true], ['no', self::$key, self::$secret]], + ]; + } + + /** + * @param array $args + * @param array $inputs + */ + #[DataProvider('providerLegacyRoutingOptions')] + public function testCommandLineOptionsRouteToLegacyEvenWhenDeviceCodeIsConfigured(array $args, array $inputs): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->mockRequest('getAccount'); + $this->clientServiceProphecy->setConnector(Argument::type(Connector::class))->shouldBeCalled(); + $this->clientServiceProphecy->isMachineAuthenticated()->willReturn(false); + $this->command = $this->createDeviceCodeCommand([], $this->headlessLocalMachineHelper()); + + $this->executeCommand($args, $inputs); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Saved credentials', $output); + $this->assertStringNotContainsString('Sign in to Acquia ID in your browser', $output); + } + public function testDeviceCodeFlowRejectsMalformedAuthorizeResponse(): void { $this->enableDeviceCodeConfig(); From 16fd69c533820b07e779f618534df218f1a32fd8 Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Mon, 28 Sep 2026 18:29:26 +0530 Subject: [PATCH 09/16] added mutation testing scenario --- .../Commands/Auth/AuthLoginCommandTest.php | 32 ------- .../Commands/Auth/AuthLoginRoutingTest.php | 86 +++++++++++++++++++ 2 files changed, 86 insertions(+), 32 deletions(-) create mode 100644 tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php index 9dcdb9842..6ed79bab8 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php @@ -534,38 +534,6 @@ public function testDecliningTheDeviceCodeOfferReachesTheKeyChooser(): void $this->assertStringNotContainsString('Sign in to Acquia ID in your browser', $output); } - /** - * @return array, array}> - */ - public static function providerLegacyRoutingOptions(): array - { - return [ - 'key alone' => [['--key' => self::$key], ['no', self::$secret]], - 'secret alone' => [['--secret' => self::$secret], ['no', self::$key]], - 'use-legacy-auth alone' => [['--use-legacy-auth' => true], ['no', self::$key, self::$secret]], - ]; - } - - /** - * @param array $args - * @param array $inputs - */ - #[DataProvider('providerLegacyRoutingOptions')] - public function testCommandLineOptionsRouteToLegacyEvenWhenDeviceCodeIsConfigured(array $args, array $inputs): void - { - $this->enableDeviceCodeConfig(); - $this->givenFreshCloudConfigWithTelemetryDisabled(); - $this->mockRequest('getAccount'); - $this->clientServiceProphecy->setConnector(Argument::type(Connector::class))->shouldBeCalled(); - $this->clientServiceProphecy->isMachineAuthenticated()->willReturn(false); - $this->command = $this->createDeviceCodeCommand([], $this->headlessLocalMachineHelper()); - - $this->executeCommand($args, $inputs); - $output = $this->getDisplay(); - - $this->assertStringContainsString('Saved credentials', $output); - $this->assertStringNotContainsString('Sign in to Acquia ID in your browser', $output); - } public function testDeviceCodeFlowRejectsMalformedAuthorizeResponse(): void { diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php new file mode 100644 index 000000000..058bb66dc --- /dev/null +++ b/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php @@ -0,0 +1,86 @@ +prophet->prophesize(LocalMachineHelper::class); + $localMachineHelper->useTty()->willReturn(false); + $localMachineHelper->isBrowserAvailable()->willReturn(false); + + $oktaConfig = $this->prophet->prophesize(OktaConfig::class); + $oktaConfig->isConfigured()->willReturn(true); + + return new AuthLoginCommand( + $localMachineHelper->reveal(), + $this->datastoreCloud, + $this->datastoreAcli, + $this->cloudCredentials, + $this->telemetryHelper, + $this->acliRepoRoot, + $this->clientServiceProphecy->reveal(), + $this->sshHelper, + $this->sshDir, + $this->logger, + $this->selfUpdateManager, + new GuzzleClient(['handler' => HandlerStack::create(new MockHandler([]))]), + $oktaConfig->reveal(), + ); + } + + /** + * @return array, array}> + */ + public static function providerLegacyRoutingOptions(): array + { + return [ + 'key alone' => [['--key' => self::$key], ['no', self::$secret]], + 'secret alone' => [['--secret' => self::$secret], ['no', self::$key]], + + 'use-legacy-auth alone' => [['--use-legacy-auth' => true], ['no', self::$key, self::$secret]], + ]; + } + + /** + * @param array $args + * @param array $inputs + */ + #[DataProvider('providerLegacyRoutingOptions')] + public function testCommandLineOptionsRouteToLegacyEvenWhenDeviceCodeIsConfigured(array $args, array $inputs): void + { + $this->mockRequest('getAccount'); + $this->clientServiceProphecy->setConnector(Argument::type(Connector::class))->shouldBeCalled(); + $this->clientServiceProphecy->isMachineAuthenticated()->willReturn(false); + $this->removeMockCloudConfigFile(); + $this->fs->dumpFile($this->cloudConfigFilepath, json_encode(['send_telemetry' => false])); + $this->createDataStores(); + $this->command = $this->createCommand(); + + $this->executeCommand($args, $inputs); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Saved credentials', $output); + $this->assertStringNotContainsString('Sign in to Acquia ID in your browser', $output); + } +} From 3e526222ae55d7462407a426bb87fe0220d379ca Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Mon, 28 Sep 2026 18:40:02 +0530 Subject: [PATCH 10/16] added mutation testing scenario --- .../Commands/Auth/AuthLoginRoutingTest.php | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php index 058bb66dc..7c0c4d5be 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php @@ -13,6 +13,7 @@ use GuzzleHttp\Client as GuzzleClient; use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; +use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\Attributes\DataProvider; use Prophecy\Argument; @@ -23,6 +24,14 @@ */ class AuthLoginRoutingTest extends CommandTestBase { + /** + * Empty means any Okta request + * errors, which is what the legacy-routing cases rely on. + * + * @var array + */ + private array $oktaResponses = []; + protected function createCommand(): CommandBase { $localMachineHelper = $this->prophet->prophesize(LocalMachineHelper::class); @@ -31,6 +40,9 @@ protected function createCommand(): CommandBase $oktaConfig = $this->prophet->prophesize(OktaConfig::class); $oktaConfig->isConfigured()->willReturn(true); + $oktaConfig->clientId()->willReturn('client-123'); + $oktaConfig->domain()->willReturn('example.okta.com'); + $oktaConfig->authServerId()->willReturn('ausTest'); return new AuthLoginCommand( $localMachineHelper->reveal(), @@ -44,7 +56,7 @@ protected function createCommand(): CommandBase $this->sshDir, $this->logger, $this->selfUpdateManager, - new GuzzleClient(['handler' => HandlerStack::create(new MockHandler([]))]), + new GuzzleClient(['handler' => HandlerStack::create(new MockHandler($this->oktaResponses))]), $oktaConfig->reveal(), ); } @@ -83,4 +95,33 @@ public function testCommandLineOptionsRouteToLegacyEvenWhenDeviceCodeIsConfigure $this->assertStringContainsString('Saved credentials', $output); $this->assertStringNotContainsString('Sign in to Acquia ID in your browser', $output); } + + public function testNoCommandLineOptionsReachesDeviceCode(): void + { + $this->oktaResponses = [ + new Response(200, [], json_encode([ + 'device_code' => 'device-123', + 'expires_in' => 600, + 'interval' => 0, + 'user_code' => 'ABCD-EFGH', + 'verification_uri' => 'https://example.okta.com/activate', + ])), + new Response(200, [], json_encode([ + 'access_token' => 'access-token-123', + 'expires_in' => 300, + 'refresh_token' => 'refresh-token-123', + ])), + ]; + $this->removeMockCloudConfigFile(); + $this->fs->dumpFile($this->cloudConfigFilepath, json_encode(['send_telemetry' => false])); + $this->createDataStores(); + $this->command = $this->createCommand(); + + $this->executeCommand([], []); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Sign in to Acquia ID in your browser', $output); + $this->assertStringContainsString('Authenticated successfully', $output); + $this->assertStringNotContainsString('Saved credentials', $output); + } } From 68eeacd0fc375ad66197a7d051fb18891d0130b8 Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Mon, 28 Sep 2026 18:55:04 +0530 Subject: [PATCH 11/16] mutation testing scenario --- .../Commands/Auth/AuthLoginRoutingTest.php | 35 +++++++------------ 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php index 7c0c4d5be..dc14b0b2b 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php @@ -13,7 +13,6 @@ use GuzzleHttp\Client as GuzzleClient; use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; -use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\Attributes\DataProvider; use Prophecy\Argument; @@ -32,6 +31,11 @@ class AuthLoginRoutingTest extends CommandTestBase */ private array $oktaResponses = []; + /** + * Whether the stubbed OktaConfig reports device code as configured. + */ + private bool $oktaConfigured = true; + protected function createCommand(): CommandBase { $localMachineHelper = $this->prophet->prophesize(LocalMachineHelper::class); @@ -39,7 +43,7 @@ protected function createCommand(): CommandBase $localMachineHelper->isBrowserAvailable()->willReturn(false); $oktaConfig = $this->prophet->prophesize(OktaConfig::class); - $oktaConfig->isConfigured()->willReturn(true); + $oktaConfig->isConfigured()->willReturn($this->oktaConfigured); $oktaConfig->clientId()->willReturn('client-123'); $oktaConfig->domain()->willReturn('example.okta.com'); $oktaConfig->authServerId()->willReturn('ausTest'); @@ -96,32 +100,19 @@ public function testCommandLineOptionsRouteToLegacyEvenWhenDeviceCodeIsConfigure $this->assertStringNotContainsString('Sign in to Acquia ID in your browser', $output); } - public function testNoCommandLineOptionsReachesDeviceCode(): void + public function testNoCommandLineOptionsSkipsTheLegacyGuard(): void { - $this->oktaResponses = [ - new Response(200, [], json_encode([ - 'device_code' => 'device-123', - 'expires_in' => 600, - 'interval' => 0, - 'user_code' => 'ABCD-EFGH', - 'verification_uri' => 'https://example.okta.com/activate', - ])), - new Response(200, [], json_encode([ - 'access_token' => 'access-token-123', - 'expires_in' => 300, - 'refresh_token' => 'refresh-token-123', - ])), - ]; + $this->oktaConfigured = false; $this->removeMockCloudConfigFile(); $this->fs->dumpFile($this->cloudConfigFilepath, json_encode(['send_telemetry' => false])); $this->createDataStores(); + $this->mockRequest('getAccount'); + $this->clientServiceProphecy->setConnector(Argument::type(Connector::class))->shouldBeCalled(); + $this->clientServiceProphecy->isMachineAuthenticated()->willReturn(false); $this->command = $this->createCommand(); - $this->executeCommand([], []); - $output = $this->getDisplay(); + $this->executeCommand([], ['no', self::$key, self::$secret]); - $this->assertStringContainsString('Sign in to Acquia ID in your browser', $output); - $this->assertStringContainsString('Authenticated successfully', $output); - $this->assertStringNotContainsString('Saved credentials', $output); + $this->assertStringContainsString('Device code sign-in is not configured', $this->getDisplay()); } } From 84f525916c93993d68ce035fd6cc48a655cdceec Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Mon, 28 Sep 2026 19:49:05 +0530 Subject: [PATCH 12/16] mutation testing scenario for client service --- .../src/CloudApi/ClientServiceTest.php | 20 +++++++++++++++++ .../CloudApi/TestOverridingClientService.php | 22 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 tests/phpunit/src/CloudApi/TestOverridingClientService.php diff --git a/tests/phpunit/src/CloudApi/ClientServiceTest.php b/tests/phpunit/src/CloudApi/ClientServiceTest.php index 3fa5206e6..52d3e60e4 100644 --- a/tests/phpunit/src/CloudApi/ClientServiceTest.php +++ b/tests/phpunit/src/CloudApi/ClientServiceTest.php @@ -92,6 +92,26 @@ public function testIsMachineAuthenticated(array $envVars, bool $isAuthenticated self::unsetEnvVars($envVars); } + public function testASubclassOverridesTheAuthenticationCheck(): void + { + $envVars = [ + 'ACLI_ACCESS_TOKEN' => null, + 'ACLI_KEY' => 'key', + 'ACLI_SECRET' => 'secret', + ]; + self::setEnvVars($envVars); + $cloudDatastore = $this->prophet->prophesize(CloudDataStore::class); + $clientService = new TestOverridingClientService(new ConnectorFactory([ + 'accessToken' => null, + 'key' => null, + 'secret' => null, + ]), $this->application, new CloudCredentials($cloudDatastore->reveal())); + + $this->assertTrue($clientService->isMachineAuthenticated()); + $this->assertTrue($clientService->overrideWasCalled); + self::unsetEnvVars($envVars); + } + public function testV3ClientServiceConstructs(): void { $cloudDatastore = $this->prophet->prophesize(CloudDataStore::class); diff --git a/tests/phpunit/src/CloudApi/TestOverridingClientService.php b/tests/phpunit/src/CloudApi/TestOverridingClientService.php new file mode 100644 index 000000000..e179aa356 --- /dev/null +++ b/tests/phpunit/src/CloudApi/TestOverridingClientService.php @@ -0,0 +1,22 @@ +overrideWasCalled = true; + return parent::checkAuthentication(); + } +} From 3609e6730ce2718dae2d612f2e5c9cdb9373c644 Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Thu, 8 Oct 2026 11:36:57 +0530 Subject: [PATCH 13/16] revised convention --- .env.example | 4 +- .github/workflows/ci.yml | 4 +- .../{OktaConfig.php => AuthConfig.php} | 9 +++-- src/CloudApi/DeviceTokenRefresher.php | 6 +-- src/Command/Auth/AuthLoginCommand.php | 12 +++--- ...{OktaConfigTest.php => AuthConfigTest.php} | 34 ++++++++--------- .../src/CloudApi/DeviceTokenRefresherTest.php | 38 +++++++++---------- .../Commands/Auth/AuthLoginCommandTest.php | 24 ++++++------ .../Commands/Auth/AuthLoginRoutingTest.php | 22 +++++------ 9 files changed, 77 insertions(+), 76 deletions(-) rename src/CloudApi/{OktaConfig.php => AuthConfig.php} (67%) rename tests/phpunit/src/CloudApi/{OktaConfigTest.php => AuthConfigTest.php} (64%) diff --git a/.env.example b/.env.example index 626fdf8a7..b014ce0e6 100644 --- a/.env.example +++ b/.env.example @@ -2,5 +2,5 @@ BUGSNAG_KEY=abcd1234 AMPLITUDE_KEY=acbd1234 ACLI_VERSION=1.0.0 ACLI_DEVICE_CLIENT_ID=0oa1example2client3id -ACLI_OKTA_DOMAIN=example.okta.com -ACLI_OKTA_AUTH_SERVER_ID=ausExample1234 +ACLI_AUTH_DOMAIN=example.acquia.com +ACLI_AUTH_SERVER_ID=ausExample1234 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 04c76e2cc..13c6d474d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -95,8 +95,8 @@ jobs: echo BUGSNAG_KEY=${{ secrets.BUGSNAG_KEY }} >> .env echo AMPLITUDE_KEY=${{ secrets.AMPLITUDE_KEY }} >> .env echo ACLI_DEVICE_CLIENT_ID=${{ secrets.ACLI_DEVICE_CLIENT_ID }} >> .env - echo ACLI_OKTA_DOMAIN=${{ secrets.ACLI_OKTA_DOMAIN }} >> .env - echo ACLI_OKTA_AUTH_SERVER_ID=${{ secrets.ACLI_OKTA_AUTH_SERVER_ID }} >> .env + echo ACLI_AUTH_DOMAIN=${{ secrets.ACLI_AUTH_DOMAIN }} >> .env + echo ACLI_AUTH_SERVER_ID=${{ secrets.ACLI_AUTH_SERVER_ID }} >> .env echo ACLI_VERSION=${{ steps.acli-version.outputs.ACLI_VERSION }} >> .env echo ACLI_BUILD_DATE=$(date -u +"%Y-%m-%dT%H:%M:%SZ") >> .env - name: 'Extract additional API specs from secrets' diff --git a/src/CloudApi/OktaConfig.php b/src/CloudApi/AuthConfig.php similarity index 67% rename from src/CloudApi/OktaConfig.php rename to src/CloudApi/AuthConfig.php index 5da64f5cc..22dd18422 100644 --- a/src/CloudApi/OktaConfig.php +++ b/src/CloudApi/AuthConfig.php @@ -5,13 +5,14 @@ namespace Acquia\Cli\CloudApi; /** - * Okta device code configuration, shared by login and token refresh. + * Authorization-server configuration for the device code flow , + * shared by login and token refresh. */ -class OktaConfig +class AuthConfig { public function authServerId(): string { - return getenv('ACLI_OKTA_AUTH_SERVER_ID') ?: ''; + return getenv('ACLI_AUTH_SERVER_ID') ?: ''; } public function clientId(): string @@ -21,7 +22,7 @@ public function clientId(): string public function domain(): string { - return getenv('ACLI_OKTA_DOMAIN') ?: ''; + return getenv('ACLI_AUTH_DOMAIN') ?: ''; } public function isConfigured(): bool diff --git a/src/CloudApi/DeviceTokenRefresher.php b/src/CloudApi/DeviceTokenRefresher.php index b99a2fea9..5dc7afe4a 100644 --- a/src/CloudApi/DeviceTokenRefresher.php +++ b/src/CloudApi/DeviceTokenRefresher.php @@ -24,7 +24,7 @@ public function __construct( private CloudDataStore $datastore, private LoggerInterface $logger, private GuzzleClient $httpClient = new GuzzleClient(), - private OktaConfig $oktaConfig = new OktaConfig(), + private AuthConfig $authConfig = new AuthConfig(), ) { } @@ -73,8 +73,8 @@ private function refresh(): ?string $refreshToken = $stored['refresh_token'] ?? null; $clientId = $stored['client_id'] ?? null; - $domain = $this->oktaConfig->domain(); - $authServer = $this->oktaConfig->authServerId(); + $domain = $this->authConfig->domain(); + $authServer = $this->authConfig->authServerId(); if (!$refreshToken || !$clientId || !$domain || !$authServer) { return null; diff --git a/src/Command/Auth/AuthLoginCommand.php b/src/Command/Auth/AuthLoginCommand.php index 58afff6af..9896c9a71 100644 --- a/src/Command/Auth/AuthLoginCommand.php +++ b/src/Command/Auth/AuthLoginCommand.php @@ -5,8 +5,8 @@ namespace Acquia\Cli\Command\Auth; use Acquia\Cli\ApiCredentialsInterface; +use Acquia\Cli\CloudApi\AuthConfig; use Acquia\Cli\CloudApi\ClientService; -use Acquia\Cli\CloudApi\OktaConfig; use Acquia\Cli\Command\CommandBase; use Acquia\Cli\DataStore\AcquiaCliDatastore; use Acquia\Cli\DataStore\CloudDataStore; @@ -46,7 +46,7 @@ public function __construct( LoggerInterface $logger, public SelfUpdateManager $selfUpdateManager, private GuzzleClient $httpClient = new GuzzleClient(), - private OktaConfig $oktaConfig = new OktaConfig(), + private AuthConfig $authConfig = new AuthConfig(), ) { parent::__construct($this->localMachineHelper, $this->datastoreCloud, $this->datastoreAcli, $this->cloudCredentials, $this->telemetryHelper, $this->projectDir, $this->cloudApiClientService, $this->sshHelper, $this->sshDir, $logger, $this->selfUpdateManager); } @@ -72,7 +72,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $activeKey = $this->datastoreCloud->get('acli_key'); $deviceToken = $this->datastoreCloud->get('device_token'); - if (!$this->oktaConfig->isConfigured()) { + if (!$this->authConfig->isConfigured()) { $output->writeln('Device code sign-in is not configured; falling back to API key authentication.'); return $this->executeLegacyAuth($input, $output); } @@ -119,9 +119,9 @@ private function executeDeviceCodeFlowWithFallback(InputInterface $input, Output private function executeDeviceCodeFlow(OutputInterface $output): int { - $clientId = $this->oktaConfig->clientId(); - $domain = $this->oktaConfig->domain(); - $authServer = $this->oktaConfig->authServerId(); + $clientId = $this->authConfig->clientId(); + $domain = $this->authConfig->domain(); + $authServer = $this->authConfig->authServerId(); $baseUrl = sprintf('https://%s/oauth2/%s/v1', $domain, $authServer); diff --git a/tests/phpunit/src/CloudApi/OktaConfigTest.php b/tests/phpunit/src/CloudApi/AuthConfigTest.php similarity index 64% rename from tests/phpunit/src/CloudApi/OktaConfigTest.php rename to tests/phpunit/src/CloudApi/AuthConfigTest.php index 9125b734e..510264957 100644 --- a/tests/phpunit/src/CloudApi/OktaConfigTest.php +++ b/tests/phpunit/src/CloudApi/AuthConfigTest.php @@ -4,12 +4,12 @@ namespace Acquia\Cli\Tests\CloudApi; -use Acquia\Cli\CloudApi\OktaConfig; +use Acquia\Cli\CloudApi\AuthConfig; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; #[Group('serial')] -class OktaConfigTest extends TestCase +class AuthConfigTest extends TestCase { /** @var array */ private array $savedEnvVars = []; @@ -17,7 +17,7 @@ class OktaConfigTest extends TestCase protected function setUp(): void { parent::setUp(); - foreach (['ACLI_DEVICE_CLIENT_ID', 'ACLI_OKTA_DOMAIN', 'ACLI_OKTA_AUTH_SERVER_ID'] as $var) { + foreach (['ACLI_DEVICE_CLIENT_ID', 'ACLI_AUTH_DOMAIN', 'ACLI_AUTH_SERVER_ID'] as $var) { $this->savedEnvVars[$var] = getenv($var); putenv($var); } @@ -34,18 +34,18 @@ protected function tearDown(): void public function testResolvesValuesFromTheEnvironment(): void { putenv('ACLI_DEVICE_CLIENT_ID=env-client'); - putenv('ACLI_OKTA_DOMAIN=env.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausEnv'); - $config = new OktaConfig(); + putenv('ACLI_AUTH_DOMAIN=env.acquia.com'); + putenv('ACLI_AUTH_SERVER_ID=ausEnv'); + $config = new AuthConfig(); $this->assertSame('env-client', $config->clientId()); - $this->assertSame('env.okta.com', $config->domain()); + $this->assertSame('env.acquia.com', $config->domain()); $this->assertSame('ausEnv', $config->authServerId()); } public function testReturnsEmptyStringsWhenNothingIsSet(): void { - $config = new OktaConfig(); + $config = new AuthConfig(); $this->assertSame('', $config->clientId()); $this->assertSame('', $config->domain()); @@ -54,32 +54,32 @@ public function testReturnsEmptyStringsWhenNothingIsSet(): void public function testIsConfiguredIsFalseWhenNothingIsSet(): void { - $this->assertFalse((new OktaConfig())->isConfigured()); + $this->assertFalse((new AuthConfig())->isConfigured()); } public function testIsConfiguredIsFalseWhenOnlySomeValuesAreSet(): void { putenv('ACLI_DEVICE_CLIENT_ID=env-client'); - putenv('ACLI_OKTA_DOMAIN=env.okta.com'); + putenv('ACLI_AUTH_DOMAIN=env.acquia.com'); - $this->assertFalse((new OktaConfig())->isConfigured()); + $this->assertFalse((new AuthConfig())->isConfigured()); } public function testIsConfiguredIsTrueWhenAllValuesResolve(): void { putenv('ACLI_DEVICE_CLIENT_ID=env-client'); - putenv('ACLI_OKTA_DOMAIN=env.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausEnv'); + putenv('ACLI_AUTH_DOMAIN=env.acquia.com'); + putenv('ACLI_AUTH_SERVER_ID=ausEnv'); - $this->assertTrue((new OktaConfig())->isConfigured()); + $this->assertTrue((new AuthConfig())->isConfigured()); } public function testIsConfiguredIsFalseWhenAValueIsSetButEmpty(): void { putenv('ACLI_DEVICE_CLIENT_ID=env-client'); - putenv('ACLI_OKTA_DOMAIN=env.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID='); + putenv('ACLI_AUTH_DOMAIN=env.acquia.com'); + putenv('ACLI_AUTH_SERVER_ID='); - $this->assertFalse((new OktaConfig())->isConfigured()); + $this->assertFalse((new AuthConfig())->isConfigured()); } } diff --git a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php index 06a3d541f..fc291d7d1 100644 --- a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php +++ b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php @@ -4,8 +4,8 @@ namespace Acquia\Cli\Tests\CloudApi; +use Acquia\Cli\CloudApi\AuthConfig; use Acquia\Cli\CloudApi\DeviceTokenRefresher; -use Acquia\Cli\CloudApi\OktaConfig; use Acquia\Cli\Config\CloudDataConfig; use Acquia\Cli\DataStore\CloudDataStore; use Acquia\Cli\Tests\TestBase; @@ -30,7 +30,7 @@ class DeviceTokenRefresherTest extends TestBase protected function setUp(): void { parent::setUp(); - foreach (['ACLI_OKTA_DOMAIN', 'ACLI_OKTA_AUTH_SERVER_ID'] as $var) { + foreach (['ACLI_AUTH_DOMAIN', 'ACLI_AUTH_SERVER_ID'] as $var) { $this->savedEnvVars[$var] = getenv($var); putenv($var); } @@ -45,10 +45,10 @@ protected function tearDown(): void parent::tearDown(); } - private function givenOktaIsConfigured(): void + private function givenAuthIsConfigured(): void { - putenv('ACLI_OKTA_DOMAIN=example.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + putenv('ACLI_AUTH_DOMAIN=example.acquia.com'); + putenv('ACLI_AUTH_SERVER_ID=ausTest'); } /** @@ -86,7 +86,7 @@ private function createRefresher(array $responses = [], ?callable $handler = nul new CloudDataStore($this->localMachineHelper, new CloudDataConfig(), $this->cloudConfigFilepath), $this->logger, $client, - new OktaConfig(), + new AuthConfig(), ); } @@ -125,7 +125,7 @@ public function testReturnsStoredTokenWhenNotExpired(): void public function testReturnsNullWhenExpiredAndRefreshTokenMissing(): void { - $this->givenOktaIsConfigured(); + $this->givenAuthIsConfigured(); $this->writeCloudConfig(['device_token' => [ 'access_token' => 'expired-token', 'client_id' => 'client-123', @@ -136,7 +136,7 @@ public function testReturnsNullWhenExpiredAndRefreshTokenMissing(): void $this->assertNull($this->createRefresher()->getValidAccessToken()); } - public function testReturnsNullWhenExpiredAndOktaConfigMissing(): void + public function testReturnsNullWhenExpiredAndAuthConfigMissing(): void { $this->writeCloudConfig(['device_token' => $this->expiredToken()]); @@ -145,7 +145,7 @@ public function testReturnsNullWhenExpiredAndOktaConfigMissing(): void public function testRefreshesExpiredTokenAndPersistsIt(): void { - $this->givenOktaIsConfigured(); + $this->givenAuthIsConfigured(); $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ new Response(200, [], json_encode([ @@ -166,7 +166,7 @@ public function testRefreshesExpiredTokenAndPersistsIt(): void public function testKeepsOldRefreshTokenWhenResponseOmitsNewOne(): void { - $this->givenOktaIsConfigured(); + $this->givenAuthIsConfigured(); $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ new Response(200, [], json_encode(['access_token' => 'new-access-token', 'expires_in' => 3600])), @@ -181,7 +181,7 @@ public function testUsesATokenAnotherInvocationWroteWhileThisOneWasStale(): void { // The datastore loads an expired token; another invocation then writes a // fresh one. Spending a rotation here would kill the session for both. - $this->givenOktaIsConfigured(); + $this->givenAuthIsConfigured(); $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ new Response(500, [], 'the refresher must not reach Okta'), @@ -200,7 +200,7 @@ public function testUsesATokenAnotherInvocationWroteWhileThisOneWasStale(): void public function testDoesNotClobberCredentialsWrittenSinceTheProcessStarted(): void { - $this->givenOktaIsConfigured(); + $this->givenAuthIsConfigured(); $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ new Response(200, [], json_encode(['access_token' => 'new-access-token', 'expires_in' => 3600])), @@ -222,7 +222,7 @@ public function testDoesNotClobberCredentialsWrittenSinceTheProcessStarted(): vo public function testReturnsNullOnInvalidGrant(): void { - $this->givenOktaIsConfigured(); + $this->givenAuthIsConfigured(); $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ new Response(400, [], json_encode(['error' => 'invalid_grant'])), @@ -234,7 +234,7 @@ public function testReturnsNullOnInvalidGrant(): void public function testReturnsNullOnA400ThatIsNotInvalidGrant(): void { - $this->givenOktaIsConfigured(); + $this->givenAuthIsConfigured(); $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ new Response(400, [], json_encode(['error' => 'invalid_client'])), @@ -245,10 +245,10 @@ public function testReturnsNullOnA400ThatIsNotInvalidGrant(): void public function testReturnsNullOnATransportFailure(): void { - $this->givenOktaIsConfigured(); + $this->givenAuthIsConfigured(); $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ - new ConnectException('cURL error 6: Could not resolve host', new Request('POST', 'https://example.okta.com')), + new ConnectException('cURL error 6: Could not resolve host', new Request('POST', 'https://example.acquia.com')), ]); $this->assertNull($refresher->getValidAccessToken()); @@ -256,7 +256,7 @@ public function testReturnsNullOnATransportFailure(): void public function testReturnsNullWhenRefreshResponseHasNoAccessToken(): void { - $this->givenOktaIsConfigured(); + $this->givenAuthIsConfigured(); $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ new Response(200, [], json_encode(['token_type' => 'Bearer'])), @@ -269,7 +269,7 @@ public function testRefreshRequestCarriesAnExplicitTimeout(): void { // GuzzleHttp\Client is autowired, so the container injects one built // without options; a hung /token would block every API request. - $this->givenOktaIsConfigured(); + $this->givenAuthIsConfigured(); $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $captured = []; $refresher = $this->createRefresher([], function ($request, array $options) use (&$captured) { @@ -289,7 +289,7 @@ public function testRefreshRequestCarriesAnExplicitTimeout(): void public function testTakesTheLockBesideTheCredentialFile(): void { - $this->givenOktaIsConfigured(); + $this->givenAuthIsConfigured(); $this->writeCloudConfig(['device_token' => $this->expiredToken()]); $refresher = $this->createRefresher([ new Response(200, [], json_encode(['access_token' => 'new-access-token', 'expires_in' => 3600])), diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php index 6ed79bab8..2fd15122f 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php @@ -39,7 +39,7 @@ class AuthLoginCommandTest extends CommandTestBase protected function setUp(): void { parent::setUp(); - foreach (['ACLI_DEVICE_CLIENT_ID', 'ACLI_OKTA_DOMAIN', 'ACLI_OKTA_AUTH_SERVER_ID'] as $var) { + foreach (['ACLI_DEVICE_CLIENT_ID', 'ACLI_AUTH_DOMAIN', 'ACLI_AUTH_SERVER_ID'] as $var) { $this->savedDeviceCodeEnvVars[$var] = getenv($var); putenv($var); } @@ -61,8 +61,8 @@ protected function createCommand(): CommandBase private function enableDeviceCodeConfig(): void { putenv('ACLI_DEVICE_CLIENT_ID=test-client-id'); - putenv('ACLI_OKTA_DOMAIN=example.okta.com'); - putenv('ACLI_OKTA_AUTH_SERVER_ID=ausTest'); + putenv('ACLI_AUTH_DOMAIN=example.acquia.com'); + putenv('ACLI_AUTH_SERVER_ID=ausTest'); } /** @@ -120,7 +120,7 @@ private function deviceAuthorizeResponse(): Response 'expires_in' => 600, 'interval' => 0, 'user_code' => 'ABCD1234', - 'verification_uri' => 'https://example.okta.com/activate', + 'verification_uri' => 'https://example.acquia.com/activate', ])); } @@ -143,7 +143,7 @@ public function testDeviceCodeFlowSuccess(): void $this->executeCommand([], []); $output = $this->getDisplay(); - $this->assertStringContainsString('https://example.okta.com/activate', $output); + $this->assertStringContainsString('https://example.acquia.com/activate', $output); $this->assertStringContainsString('ABCD1234', $output); $this->assertStringContainsString('Authenticated successfully', $output); @@ -247,7 +247,7 @@ public function testDeviceCodeFlowOpensBrowserWhenAvailable(): void $this->givenFreshCloudConfigWithTelemetryDisabled(); $localMachineHelperProphecy = $this->prophet->prophesize(LocalMachineHelper::class); $localMachineHelperProphecy->isBrowserAvailable()->willReturn(true); - $localMachineHelperProphecy->startBrowser('https://example.okta.com/activate') + $localMachineHelperProphecy->startBrowser('https://example.acquia.com/activate') ->shouldBeCalled() ->willReturn(true); $this->command = $this->createDeviceCodeCommand([ @@ -420,7 +420,7 @@ public function testDeviceCodeFlowRetriesAfterTransportFailureInPollLoop(): void $localMachineHelperProphecy = $this->headlessLocalMachineHelper(); $this->command = $this->createDeviceCodeCommand([ $this->deviceAuthorizeResponse(), - new ConnectException('cURL error 28: Operation timed out', new Request('POST', 'https://example.okta.com')), + new ConnectException('cURL error 28: Operation timed out', new Request('POST', 'https://example.acquia.com')), new Response(200, [], json_encode([ 'access_token' => 'access-token-123', 'expires_in' => 300, @@ -467,7 +467,7 @@ public function testUsesDeviceCodeAfterLogoutEvenWhenKeysRemainOnDisk(): void } - public function testDeviceTokenBranchRequiresOktaConfig(): void + public function testDeviceTokenBranchRequiresAuthConfig(): void { $this->removeMockCloudConfigFile(); $this->fs->dumpFile($this->cloudConfigFilepath, json_encode([ @@ -572,7 +572,7 @@ public function testDeviceCodeFlowTimesOutWhenTheWindowCloses(): void 'expires_in' => 0, 'interval' => 0, 'user_code' => 'ABCD-EFGH', - 'verification_uri' => 'https://example.okta.com/activate', + 'verification_uri' => 'https://example.acquia.com/activate', ])), ], $this->headlessLocalMachineHelper()); @@ -581,7 +581,7 @@ public function testDeviceCodeFlowTimesOutWhenTheWindowCloses(): void $this->assertStringContainsString('Timed out waiting for authorization', $this->getDisplay()); } - public function testDeviceCodeFlowStoresAZeroExpiryWhenOktaOmitsExpiresIn(): void + public function testDeviceCodeFlowStoresAZeroExpiryWhenTokenResponseOmitsExpiresIn(): void { $this->enableDeviceCodeConfig(); $this->givenFreshCloudConfigWithTelemetryDisabled(); @@ -609,7 +609,7 @@ public function testDeviceCodeFlowRejectsTokenResponseWithoutAccessToken(): void 'expires_in' => 600, 'interval' => 0, 'user_code' => 'ABCD-EFGH', - 'verification_uri' => 'https://example.okta.com/activate', + 'verification_uri' => 'https://example.acquia.com/activate', ])), new Response(200, [], json_encode(['token_type' => 'Bearer'])), ], $localMachineHelperProphecy); @@ -627,7 +627,7 @@ public function testDeviceCodeFlowSurvivesTransportFailureOnInitiation(): void $this->givenFreshCloudConfigWithTelemetryDisabled(); $localMachineHelperProphecy = $this->headlessLocalMachineHelper(); $this->command = $this->createDeviceCodeCommand([ - new ConnectException('cURL error 6: Could not resolve host', new Request('POST', 'https://example.okta.com')), + new ConnectException('cURL error 6: Could not resolve host', new Request('POST', 'https://example.acquia.com')), ], $localMachineHelperProphecy); $this->executeCommand([], ['no']); diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php index dc14b0b2b..147087405 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLoginRoutingTest.php @@ -4,7 +4,7 @@ namespace Acquia\Cli\Tests\Commands\Auth; -use Acquia\Cli\CloudApi\OktaConfig; +use Acquia\Cli\CloudApi\AuthConfig; use Acquia\Cli\Command\Auth\AuthLoginCommand; use Acquia\Cli\Command\CommandBase; use Acquia\Cli\Helpers\LocalMachineHelper; @@ -17,7 +17,7 @@ use Prophecy\Argument; /** - * Routing tests that stub OktaConfig rather than exporting its variables. + * Routing tests that stub AuthConfig rather than exporting its variables. * * @property AuthLoginCommand $command */ @@ -32,9 +32,9 @@ class AuthLoginRoutingTest extends CommandTestBase private array $oktaResponses = []; /** - * Whether the stubbed OktaConfig reports device code as configured. + * Whether the stubbed AuthConfig reports device code as configured. */ - private bool $oktaConfigured = true; + private bool $authConfigured = true; protected function createCommand(): CommandBase { @@ -42,11 +42,11 @@ protected function createCommand(): CommandBase $localMachineHelper->useTty()->willReturn(false); $localMachineHelper->isBrowserAvailable()->willReturn(false); - $oktaConfig = $this->prophet->prophesize(OktaConfig::class); - $oktaConfig->isConfigured()->willReturn($this->oktaConfigured); - $oktaConfig->clientId()->willReturn('client-123'); - $oktaConfig->domain()->willReturn('example.okta.com'); - $oktaConfig->authServerId()->willReturn('ausTest'); + $authConfig = $this->prophet->prophesize(AuthConfig::class); + $authConfig->isConfigured()->willReturn($this->authConfigured); + $authConfig->clientId()->willReturn('client-123'); + $authConfig->domain()->willReturn('example.acquia.com'); + $authConfig->authServerId()->willReturn('ausTest'); return new AuthLoginCommand( $localMachineHelper->reveal(), @@ -61,7 +61,7 @@ protected function createCommand(): CommandBase $this->logger, $this->selfUpdateManager, new GuzzleClient(['handler' => HandlerStack::create(new MockHandler($this->oktaResponses))]), - $oktaConfig->reveal(), + $authConfig->reveal(), ); } @@ -102,7 +102,7 @@ public function testCommandLineOptionsRouteToLegacyEvenWhenDeviceCodeIsConfigure public function testNoCommandLineOptionsSkipsTheLegacyGuard(): void { - $this->oktaConfigured = false; + $this->authConfigured = false; $this->removeMockCloudConfigFile(); $this->fs->dumpFile($this->cloudConfigFilepath, json_encode(['send_telemetry' => false])); $this->createDataStores(); From 9ef74fdd0ec03895db97cfffdb923990df8d3c49 Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Fri, 9 Oct 2026 13:01:16 +0530 Subject: [PATCH 14/16] addressed in-memory datastore --- src/CloudApi/DeviceTokenRefresher.php | 1 + src/DataStore/Datastore.php | 11 ++++++++ .../src/CloudApi/DeviceTokenRefresherTest.php | 25 +++++++++++++++++++ 3 files changed, 37 insertions(+) diff --git a/src/CloudApi/DeviceTokenRefresher.php b/src/CloudApi/DeviceTokenRefresher.php index 5dc7afe4a..07868f958 100644 --- a/src/CloudApi/DeviceTokenRefresher.php +++ b/src/CloudApi/DeviceTokenRefresher.php @@ -176,6 +176,7 @@ private function persistToken(array $token): void $data['device_token'] = $token; file_put_contents($path, json_encode($data, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT)); chmod($path, 0600); + $this->datastore->syncInMemory('device_token', $token); } /** diff --git a/src/DataStore/Datastore.php b/src/DataStore/Datastore.php index de8fa0bba..6deb4af93 100644 --- a/src/DataStore/Datastore.php +++ b/src/DataStore/Datastore.php @@ -54,6 +54,17 @@ public function remove(string $key): void $this->dump(); } + /** + * Updates the in-memory copy of a value without writing to disk. + * + * @param string $key The key to update in the in-memory datastore. + * @param mixed $value The value to set for the given key in memory. + */ + public function syncInMemory(string $key, mixed $value): void + { + $this->data->set($key, $value); + } + public function exists(string $key): bool { return $this->data->has($key); diff --git a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php index fc291d7d1..072bbfe47 100644 --- a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php +++ b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php @@ -287,6 +287,31 @@ public function testRefreshRequestCarriesAnExplicitTimeout(): void $this->assertGreaterThan(0, $captured[0]['timeout']); } + public function testDoesNotClobberTheRefreshWithALaterUnrelatedSet(): void + { + $this->givenAuthIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); + $datastore = new CloudDataStore($this->localMachineHelper, new CloudDataConfig(), $this->cloudConfigFilepath); + $client = new GuzzleClient([ + 'handler' => HandlerStack::create(new MockHandler([ + new Response(200, [], json_encode([ + 'access_token' => 'new-access-token', + 'expires_in' => 3600, + 'refresh_token' => 'new-refresh-token', + ])), + ])), + ]); + $refresher = new DeviceTokenRefresher($datastore, $this->logger, $client, new AuthConfig()); + + $this->assertSame('new-access-token', $refresher->getValidAccessToken()); + + $datastore->set('user', ['uuid' => 'test-uuid']); + + $stored = $this->readCloudConfig()['device_token']; + $this->assertSame('new-access-token', $stored['access_token']); + $this->assertSame('new-refresh-token', $stored['refresh_token']); + } + public function testTakesTheLockBesideTheCredentialFile(): void { $this->givenAuthIsConfigured(); From 6473621aaeb029eff5b123ee8939197650923384 Mon Sep 17 00:00:00 2001 From: Joyce Bansode Date: Wed, 14 Oct 2026 23:13:34 +0530 Subject: [PATCH 15/16] fix stale in-memory device token and make refresh writes atomic --- src/CloudApi/DeviceTokenRefresher.php | 43 ++++----- .../src/CloudApi/DeviceTokenRefresherTest.php | 94 +++++++++++++++++++ 2 files changed, 112 insertions(+), 25 deletions(-) diff --git a/src/CloudApi/DeviceTokenRefresher.php b/src/CloudApi/DeviceTokenRefresher.php index 07868f958..0aa886eef 100644 --- a/src/CloudApi/DeviceTokenRefresher.php +++ b/src/CloudApi/DeviceTokenRefresher.php @@ -10,6 +10,7 @@ use GuzzleHttp\Exception\GuzzleException; use League\OAuth2\Client\Token\AccessToken; use Psr\Log\LoggerInterface; +use Symfony\Component\Filesystem\Filesystem; class DeviceTokenRefresher { @@ -63,11 +64,13 @@ public function getValidAccessToken(): ?string */ private function refresh(): ?string { - $stored = $this->readTokenFromDisk(); - if (!$stored || empty($stored['access_token'])) { + $config = $this->readConfigFromDisk(); + $stored = $config['device_token'] ?? null; + if (!is_array($stored) || empty($stored['access_token'])) { return null; } if ($this->hasLifeLeft($stored)) { + $this->datastore->syncInMemory('device_token', $stored); return $stored['access_token']; } @@ -114,7 +117,7 @@ private function refresh(): ?string return null; } - $this->persistToken([ + $this->persistToken($config, [ 'access_token' => $new['access_token'], 'client_id' => $clientId, 'expiry' => isset($new['expires_in']) ? time() + (int) $new['expires_in'] : 0, @@ -138,44 +141,34 @@ private function hasLifeLeft(array $token): bool } /** - * Reads `device_token` from disk, bypassing the in-memory datastore. + * Reads the whole credential file from disk, bypassing the in-memory datastore. * * @return array|null */ - private function readTokenFromDisk(): ?array + private function readConfigFromDisk(): ?array { $contents = @file_get_contents($this->datastore->filepath); - if ($contents === false) { - return null; - } - - $decoded = json_decode($contents, true); - if (!is_array($decoded) || !isset($decoded['device_token']) || !is_array($decoded['device_token'])) { - return null; - } + $decoded = $contents === false ? null : json_decode($contents, true); - return $decoded['device_token']; + return is_array($decoded) ? $decoded : null; } /** * Writes `device_token` without disturbing anything else in the file. * + * @param array $config The file contents read inside the lock. * @param array $token */ - private function persistToken(array $token): void + private function persistToken(array $config, array $token): void { $path = $this->datastore->filepath; - $contents = @file_get_contents($path); - $data = $contents === false ? null : json_decode($contents, true); - - if (!is_array($data)) { - $this->datastore->set('device_token', $token); - return; - } + $config['device_token'] = $token; - $data['device_token'] = $token; - file_put_contents($path, json_encode($data, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT)); - chmod($path, 0600); + // Same write as JsonDataStore::dump(): atomic, and it follows a symlinked config. + $fileSystem = new Filesystem(); + $fileSystem->dumpFile($path, json_encode($config, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT)); + // Restrict access. + $fileSystem->chmod($path, 0600); $this->datastore->syncInMemory('device_token', $token); } diff --git a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php index 072bbfe47..6bf9c47e9 100644 --- a/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php +++ b/tests/phpunit/src/CloudApi/DeviceTokenRefresherTest.php @@ -17,6 +17,8 @@ use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\Attributes\Group; +use Symfony\Component\Filesystem\Exception\IOException; +use Symfony\Component\Filesystem\Path; /** * Uses a real on-disk CloudDataStore rather than a double. @@ -164,6 +166,69 @@ public function testRefreshesExpiredTokenAndPersistsIt(): void $this->assertGreaterThan(time(), $stored['expiry']); } + public function testRestrictsTheCredentialFileToTheOwner(): void + { + $this->givenAuthIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); + chmod($this->cloudConfigFilepath, 0644); + $refresher = $this->createRefresher([ + new Response(200, [], json_encode(['access_token' => 'new-access-token', 'expires_in' => 3600])), + ]); + + $refresher->getValidAccessToken(); + + clearstatcache(); + $this->assertSame('0600', substr(sprintf('%o', fileperms($this->cloudConfigFilepath)), -4)); + $this->assertSame('new-access-token', $this->readCloudConfig()['device_token']['access_token']); + } + + public function testThrowsAndKeepsTheOldFileWhenTheRefreshCannotBeSaved(): void + { + $this->givenAuthIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); + $refresher = $this->createRefresher([ + new Response(200, [], json_encode(['access_token' => 'new-access-token', 'expires_in' => 3600])), + ]); + chmod(dirname($this->cloudConfigFilepath), 0555); + + try { + $refresher->getValidAccessToken(); + $this->fail('Expected the unsaveable refresh to throw.'); + } catch (IOException) { + // Expected: the datastore lets filesystem errors propagate too. + } finally { + chmod(dirname($this->cloudConfigFilepath), 0755); + } + + $this->assertSame('expired-token', $this->readCloudConfig()['device_token']['access_token']); + } + + public function testWritesThroughASymlinkedCredentialFile(): void + { + $this->givenAuthIsConfigured(); + $dir = $this->getTempDir(); + $target = Path::join($dir, 'dotfiles', 'cloud_api.conf'); + $link = Path::join($dir, 'config', 'cloud_api.conf'); + $this->fs->dumpFile($target, json_encode(['device_token' => $this->expiredToken(), 'send_telemetry' => false])); + $this->fs->mkdir(dirname($link)); + $this->fs->symlink($target, $link); + + try { + $client = new GuzzleClient([ + 'handler' => HandlerStack::create(new MockHandler([ + new Response(200, [], json_encode(['access_token' => 'new-access-token', 'expires_in' => 3600])), + ])), + ]); + $datastore = new CloudDataStore($this->localMachineHelper, new CloudDataConfig(), $link); + (new DeviceTokenRefresher($datastore, $this->logger, $client, new AuthConfig()))->getValidAccessToken(); + + $this->assertTrue(is_link($link)); + $this->assertSame('new-access-token', json_decode(file_get_contents($target), true)['device_token']['access_token']); + } finally { + $this->fs->remove($dir); + } + } + public function testKeepsOldRefreshTokenWhenResponseOmitsNewOne(): void { $this->givenAuthIsConfigured(); @@ -198,6 +263,35 @@ public function testUsesATokenAnotherInvocationWroteWhileThisOneWasStale(): void $this->assertSame('written-by-another-process', $refresher->getValidAccessToken()); } + public function testDoesNotClobberAnotherProcessesRefreshWithALaterUnrelatedSet(): void + { + $this->givenAuthIsConfigured(); + $this->writeCloudConfig(['device_token' => $this->expiredToken()]); + $datastore = new CloudDataStore($this->localMachineHelper, new CloudDataConfig(), $this->cloudConfigFilepath); + $client = new GuzzleClient([ + 'handler' => HandlerStack::create(new MockHandler([ + new Response(500, [], 'the refresher must not reach Okta'), + ])), + ]); + $refresher = new DeviceTokenRefresher($datastore, $this->logger, $client, new AuthConfig()); + + $this->writeCloudConfig(['device_token' => [ + 'access_token' => 'written-by-another-process', + 'client_id' => 'client-123', + 'expiry' => time() + 3600, + 'refresh_token' => 'their-refresh-token', + ], + ]); + + $this->assertSame('written-by-another-process', $refresher->getValidAccessToken()); + + $datastore->set('user', ['uuid' => 'test-uuid']); + + $stored = $this->readCloudConfig()['device_token']; + $this->assertSame('written-by-another-process', $stored['access_token']); + $this->assertSame('their-refresh-token', $stored['refresh_token']); + } + public function testDoesNotClobberCredentialsWrittenSinceTheProcessStarted(): void { $this->givenAuthIsConfigured(); From 749228359d3cddb7c6502f7185ae895ab0de9a59 Mon Sep 17 00:00:00 2001 From: Nick DelRossi Date: Thu, 15 Oct 2026 10:42:35 -0400 Subject: [PATCH 16/16] Handle jit_failure from the device code token poll When a Google sign-in can't create the Okta user (e.g. the email already has a password Acquia ID), the token poll returns jit_failure. Show a clear message instead of "Unexpected error: jit_failure". Pairs with acquia/idm-identity-service#979. --- src/Command/Auth/AuthLoginCommand.php | 6 ++++++ .../Commands/Auth/AuthLoginCommandTest.php | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/Command/Auth/AuthLoginCommand.php b/src/Command/Auth/AuthLoginCommand.php index 9896c9a71..4cf27f8f7 100644 --- a/src/Command/Auth/AuthLoginCommand.php +++ b/src/Command/Auth/AuthLoginCommand.php @@ -227,6 +227,12 @@ private function executeDeviceCodeFlow(OutputInterface $output): int $output->writeln('Code expired. Run acli login again.'); return Command::FAILURE; + case 'jit_failure': + // Okta couldn't create the user from the Google sign-in, e.g. because + // the email already has a password Acquia ID. + $output->writeln('Google sign-in failed: an Acquia ID may already exist for this email. Run acli auth:login again and sign in with your email address and password.'); + return Command::FAILURE; + default: $output->writeln("Unexpected error: $error"); return Command::FAILURE; diff --git a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php index 2fd15122f..97e67a9e9 100644 --- a/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php +++ b/tests/phpunit/src/Commands/Auth/AuthLoginCommandTest.php @@ -204,6 +204,25 @@ public function testDeviceCodeFlowExpiredToken(): void $this->assertStringContainsString('Code expired', $output); } + public function testDeviceCodeFlowJitFailure(): void + { + $this->enableDeviceCodeConfig(); + $this->givenFreshCloudConfigWithTelemetryDisabled(); + $this->command = $this->createDeviceCodeCommand([ + $this->deviceAuthorizeResponse(), + new Response(400, [], json_encode([ + 'error' => 'jit_failure', + 'error_description' => 'Unable to create the user.', + ])), + ]); + + $this->executeCommand([], ['no']); + $output = $this->getDisplay(); + + $this->assertStringContainsString('Google sign-in failed', $output); + $this->assertStringNotContainsString('Unexpected error', $output); + } + public function testDeviceCodeFlowInitiateFailure(): void { $this->enableDeviceCodeConfig();