Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions legacy/src/Command/Auth/BrowserLoginCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
use Platformsh\Cli\Service\Api;
use Platformsh\Cli\Service\Config;
use Platformsh\Cli\Service\QuestionHelper;
use GuzzleHttp\Client;
use GuzzleHttp\Exception\BadResponseException;
use GuzzleHttp\Psr7\Request;
use GuzzleHttp\Utils;
Expand Down Expand Up @@ -141,14 +140,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int
chmod($responseFile, 0o600);

// Start the local server.
$process = new Process([
(new PhpExecutableFinder())->find() ?: PHP_BINARY,
'-dvariables_order=egps',
'-S',
$localAddress,
'-t',
$listenerDir,
]);
$phpCommand = [(new PhpExecutableFinder())->find() ?: PHP_BINARY];
if (!php_ini_loaded_file() && !php_ini_scanned_files()) {
// This process parsed no php.ini file, so tell the server not to
// parse one either. Otherwise it may load a php.ini which does not
// suit this PHP build, for example one that enables extensions
// which are already built in.
$phpCommand[] = '-n';
}
$phpCommand[] = '-dvariables_order=egps';
$process = new Process(array_merge($phpCommand, ['-S', $localAddress, '-t', $listenerDir]));
$codeVerifier = $this->generateCodeVerifier();
$process->setEnv([
'CLI_OAUTH_APP_NAME' => $this->config->getStr('application.name'),
Expand Down Expand Up @@ -313,7 +314,9 @@ private function createDocumentRoot(string $dir): void
*/
private function getAccessToken(string $authCode, string $codeVerifier, string $redirectUri): array
{
$client = new Client(['verify' => !$this->config->getBool('api.skip_ssl')]);
// Use the shared HTTP client, so that the request uses the detected CA
// bundle and any configured proxy.
$client = $this->api->getExternalHttpClient();
$request = new Request('POST', $this->config->getStr('api.oauth2_token_url'), body: http_build_query([
'grant_type' => 'authorization_code',
'code' => $authCode,
Expand Down
12 changes: 10 additions & 2 deletions legacy/src/Service/Api.php
Original file line number Diff line number Diff line change
Expand Up @@ -485,18 +485,26 @@ private function tokenFromSession(SessionInterface $session): ?AccessToken
/**
* Returns proxy config in the format expected by Guzzle.
*
* @return string[]
* @return array<string, string|string[]>
*/
private function guzzleProxyConfig(): array
{
return array_map(function ($proxyUrl) {
$config = array_map(function ($proxyUrl) {
// If Guzzle is going to use PHP's built-in HTTP streams,
// rather than curl, then transform the proxy scheme.
if (!\extension_loaded('curl') && \ini_get('allow_url_fopen')) {
return \str_replace(['http://', 'https://'], ['tcp://', 'tcp://'], $proxyUrl);
}
return $proxyUrl;
}, $this->config->getProxies());

// Guzzle only reads the no_proxy environment variable itself when no
// proxy has been configured explicitly, so pass the hosts on.
if ($noProxy = $this->config->getNoProxy()) {
$config['no'] = $noProxy;
}

return $config;
}

/**
Expand Down
18 changes: 18 additions & 0 deletions legacy/src/Service/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,24 @@ public function getProxies(): array
return $proxies;
}

/**
* Finds the hosts which should not be proxied, from the no_proxy environment variable.
*
* @return string[]
*/
public function getNoProxy(): array
{
$value = \getenv('no_proxy');
if ($value === false) {
$value = \getenv('NO_PROXY');
}
if ($value === false) {
return [];
}
$hosts = \array_map('trim', \explode(',', $value));
return \array_values(\array_filter($hosts, fn(string $host): bool => $host !== ''));
}

/**
* Returns an array of context options for HTTP/HTTPS streams.
*
Expand Down
75 changes: 75 additions & 0 deletions legacy/tests/Command/Auth/BrowserLoginCommandTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

declare(strict_types=1);

namespace Platformsh\Cli\Tests\Command\Auth;

use GuzzleHttp\ClientInterface;
use GuzzleHttp\Psr7\Response;
use PHPUnit\Framework\Attributes\Group;
use PHPUnit\Framework\TestCase;
use Platformsh\Cli\Command\Auth\BrowserLoginCommand;
use Platformsh\Cli\Service\Api;
use Platformsh\Cli\Service\Config;
use Platformsh\Cli\Service\Io;
use Platformsh\Cli\Service\Login;
use Platformsh\Cli\Service\QuestionHelper;
use Platformsh\Cli\Service\Url;
use Psr\Http\Message\RequestInterface;

#[Group('commands')]
class BrowserLoginCommandTest extends TestCase
{
/**
* Tests that the access token request goes through the shared HTTP client.
*
* The shared client applies the detected CA bundle and the configured
* proxy. A client built here instead would use the default certificate
* verification, which fails behind a TLS-inspecting proxy.
*/
public function testAccessTokenRequestUsesSharedHttpClient(): void
{
$tokenUrl = 'https://auth.example.com/oauth2/token';

$sentRequest = null;
$sentOptions = [];
$httpClient = $this->createMock(ClientInterface::class);
$httpClient->expects($this->once())
->method('send')
->willReturnCallback(function (RequestInterface $request, array $options = []) use (&$sentRequest, &$sentOptions): Response {
$sentRequest = $request;
$sentOptions = $options;
return new Response(200, [], '{"access_token": "test-token", "token_type": "bearer"}');
});

$api = $this->createMock(Api::class);
$api->expects($this->once())
->method('getExternalHttpClient')
->willReturn($httpClient);

$values = [
'api.oauth2_token_url' => $tokenUrl,
'api.oauth2_client_id' => 'test-client-id',
];
$config = $this->createMock(Config::class);
$config->method('getStr')->willReturnCallback(fn(string $key): string => $values[$key] ?? '');
$config->method('get')->willReturnCallback(fn(string $key): string => $values[$key] ?? '');

$command = new BrowserLoginCommand(
$api,
$config,
$this->createMock(Io::class),
$this->createMock(Login::class),
$this->createMock(QuestionHelper::class),
$this->createMock(Url::class),
);

$method = new \ReflectionMethod($command, 'getAccessToken');
$token = $method->invoke($command, 'test-code', 'test-verifier', 'http://127.0.0.1:5000');

$this->assertSame('test-token', $token['access_token']);
$this->assertInstanceOf(RequestInterface::class, $sentRequest);
$this->assertSame($tokenUrl, (string) $sentRequest->getUri());
$this->assertSame(['test-client-id', ''], $sentOptions['auth'] ?? null);
}
}
25 changes: 25 additions & 0 deletions legacy/tests/ConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,29 @@ public function testGetWritableUserDir(): void
$home = $config->getHomeDirectory();
$this->assertEquals($home . DIRECTORY_SEPARATOR . 'mock-cli-user-config', $config->getWritableUserDir());
}

/**
* Test finding the hosts which should not be proxied.
*/
public function testGetNoProxy(): void
{
$config = new Config([], $this->configFile);
$original = ['no_proxy' => getenv('no_proxy'), 'NO_PROXY' => getenv('NO_PROXY')];
try {
putenv('no_proxy');
putenv('NO_PROXY');
$this->assertEquals([], $config->getNoProxy());

putenv('no_proxy=example.com, .example.net,');
$this->assertEquals(['example.com', '.example.net'], $config->getNoProxy());
putenv('no_proxy');

putenv('NO_PROXY=example.org');
$this->assertEquals(['example.org'], $config->getNoProxy());
} finally {
foreach ($original as $name => $value) {
putenv($value === false ? $name : $name . '=' . $value);
}
}
}
}