From c754f96677da6cc22ad0f07383ea26d47268b118 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sun, 6 May 2012 17:17:36 +0200 Subject: [PATCH 1/3] Removed last password/username from IOInterface --- src/Composer/IO/ConsoleIO.php | 28 +++------------- src/Composer/IO/IOInterface.php | 14 -------- src/Composer/IO/NullIO.php | 16 ---------- src/Composer/Util/RemoteFilesystem.php | 4 --- tests/Composer/Test/IO/ConsoleIOTest.php | 26 --------------- tests/Composer/Test/IO/NullIOTest.php | 14 -------- .../Test/Util/RemoteFilesystemTest.php | 32 ------------------- 7 files changed, 4 insertions(+), 130 deletions(-) diff --git a/src/Composer/IO/ConsoleIO.php b/src/Composer/IO/ConsoleIO.php index 95aed6a29..0865fab29 100644 --- a/src/Composer/IO/ConsoleIO.php +++ b/src/Composer/IO/ConsoleIO.php @@ -22,6 +22,7 @@ use Symfony\Component\Console\Helper\HelperSet; * The Input/Output helper. * * @author François Pluchino + * @author Jordi Boggiano */ class ConsoleIO implements IOInterface { @@ -29,8 +30,6 @@ class ConsoleIO implements IOInterface protected $output; protected $helperSet; protected $authorizations = array(); - protected $lastUsername; - protected $lastPassword; protected $lastMessage; /** @@ -179,22 +178,6 @@ class ConsoleIO implements IOInterface return $this->ask($question); } - /** - * {@inheritDoc} - */ - public function getLastUsername() - { - return $this->lastUsername; - } - - /** - * {@inheritDoc} - */ - public function getLastPassword() - { - return $this->lastPassword; - } - /** * {@inheritDoc} */ @@ -209,6 +192,7 @@ class ConsoleIO implements IOInterface public function hasAuthorization($repositoryName) { $auths = $this->getAuthorizations(); + return isset($auths[$repositoryName]); } @@ -218,6 +202,7 @@ class ConsoleIO implements IOInterface public function getAuthorization($repositoryName) { $auths = $this->getAuthorizations(); + return isset($auths[$repositoryName]) ? $auths[$repositoryName] : array('username' => null, 'password' => null); } @@ -226,11 +211,6 @@ class ConsoleIO implements IOInterface */ public function setAuthorization($repositoryName, $username, $password = null) { - $auths = $this->getAuthorizations(); - $auths[$repositoryName] = array('username' => $username, 'password' => $password); - - $this->authorizations = $auths; - $this->lastUsername = $username; - $this->lastPassword = $password; + $this->authorizations[$repositoryName] = array('username' => $username, 'password' => $password); } } diff --git a/src/Composer/IO/IOInterface.php b/src/Composer/IO/IOInterface.php index 583800424..79e0d7e8e 100644 --- a/src/Composer/IO/IOInterface.php +++ b/src/Composer/IO/IOInterface.php @@ -108,20 +108,6 @@ interface IOInterface */ function askAndHideAnswer($question); - /** - * Get the last username entered. - * - * @return string The username - */ - function getLastUsername(); - - /** - * Get the last password entered. - * - * @return string The password - */ - function getLastPassword(); - /** * Get all authorization informations entered. * diff --git a/src/Composer/IO/NullIO.php b/src/Composer/IO/NullIO.php index 78688f6d5..b5ee190b3 100644 --- a/src/Composer/IO/NullIO.php +++ b/src/Composer/IO/NullIO.php @@ -89,22 +89,6 @@ class NullIO implements IOInterface return null; } - /** - * {@inheritDoc} - */ - public function getLastUsername() - { - return null; - } - - /** - * {@inheritDoc} - */ - public function getLastPassword() - { - return null; - } - /** * {@inheritDoc} */ diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index f6cfa67da..695e765f3 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -223,10 +223,6 @@ class RemoteFilesystem $auth = $this->io->getAuthorization($originUrl); $authStr = base64_encode($auth['username'] . ':' . $auth['password']); $options['http']['header'] .= "Authorization: Basic $authStr\r\n"; - } elseif (null !== $this->io->getLastUsername()) { - $authStr = base64_encode($this->io->getLastUsername() . ':' . $this->io->getLastPassword()); - $options['http']['header'] .= "Authorization: Basic $authStr\r\n"; - $this->io->setAuthorization($originUrl, $this->io->getLastUsername(), $this->io->getLastPassword()); } return $options; diff --git a/tests/Composer/Test/IO/ConsoleIOTest.php b/tests/Composer/Test/IO/ConsoleIOTest.php index 73f0faedb..f3ea6b15d 100644 --- a/tests/Composer/Test/IO/ConsoleIOTest.php +++ b/tests/Composer/Test/IO/ConsoleIOTest.php @@ -190,30 +190,4 @@ class ConsoleIOTest extends TestCase $this->assertTrue($consoleIO->hasAuthorization('repoName')); $this->assertFalse($consoleIO->hasAuthorization('repoName2')); } - - public function testGetLastUsername() - { - $inputMock = $this->getMock('Symfony\Component\Console\Input\InputInterface'); - $outputMock = $this->getMock('Symfony\Component\Console\Output\OutputInterface'); - $helperMock = $this->getMock('Symfony\Component\Console\Helper\HelperSet'); - - $consoleIO = new ConsoleIO($inputMock, $outputMock, $helperMock); - $consoleIO->setAuthorization('repoName', 'l3l0', 'passwd'); - $consoleIO->setAuthorization('repoName2', 'l3l02', 'passwd2'); - - $this->assertEquals('l3l02', $consoleIO->getLastUsername()); - } - - public function testGetLastPassword() - { - $inputMock = $this->getMock('Symfony\Component\Console\Input\InputInterface'); - $outputMock = $this->getMock('Symfony\Component\Console\Output\OutputInterface'); - $helperMock = $this->getMock('Symfony\Component\Console\Helper\HelperSet'); - - $consoleIO = new ConsoleIO($inputMock, $outputMock, $helperMock); - $consoleIO->setAuthorization('repoName', 'l3l0', 'passwd'); - $consoleIO->setAuthorization('repoName2', 'l3l02', 'passwd2'); - - $this->assertEquals('passwd2', $consoleIO->getLastPassword()); - } } diff --git a/tests/Composer/Test/IO/NullIOTest.php b/tests/Composer/Test/IO/NullIOTest.php index 854f27862..e0e07dd76 100644 --- a/tests/Composer/Test/IO/NullIOTest.php +++ b/tests/Composer/Test/IO/NullIOTest.php @@ -31,20 +31,6 @@ class NullIOTest extends TestCase $this->assertFalse($io->hasAuthorization('foo')); } - public function testGetLastPassword() - { - $io = new NullIO(); - - $this->assertNull($io->getLastPassword()); - } - - public function testGetLastUsername() - { - $io = new NullIO(); - - $this->assertNull($io->getLastUsername()); - } - public function testAskAndHideAnswer() { $io = new NullIO(); diff --git a/tests/Composer/Test/Util/RemoteFilesystemTest.php b/tests/Composer/Test/Util/RemoteFilesystemTest.php index 6fdb565eb..93e42153d 100644 --- a/tests/Composer/Test/Util/RemoteFilesystemTest.php +++ b/tests/Composer/Test/Util/RemoteFilesystemTest.php @@ -25,11 +25,6 @@ class RemoteFilesystemTest extends \PHPUnit_Framework_TestCase ->method('hasAuthorization') ->will($this->returnValue(false)) ; - $io - ->expects($this->once()) - ->method('getLastUsername') - ->will($this->returnValue(null)) - ; $res = $this->callGetOptionsForUrl($io, array('http://example.org')); $this->assertTrue(isset($res['http']['header']) && false !== strpos($res['http']['header'], 'User-Agent'), 'getOptions must return an array with a header containing a User-Agent'); @@ -53,33 +48,6 @@ class RemoteFilesystemTest extends \PHPUnit_Framework_TestCase $this->assertContains('Authorization: Basic', $options['http']['header']); } - public function testGetOptionsForUrlWithLastUsername() - { - $io = $this->getMock('Composer\IO\IOInterface'); - $io - ->expects($this->once()) - ->method('hasAuthorization') - ->will($this->returnValue(false)) - ; - $io - ->expects($this->any()) - ->method('getLastUsername') - ->will($this->returnValue('login')) - ; - $io - ->expects($this->any()) - ->method('getLastPassword') - ->will($this->returnValue('password')) - ; - $io - ->expects($this->once()) - ->method('setAuthorization') - ; - - $options = $this->callGetOptionsForUrl($io, array('http://example.org')); - $this->assertContains('Authorization: Basic', $options['http']['header']); - } - public function testCallbackGetFileSize() { $fs = new RemoteFilesystem($this->getMock('Composer\IO\IOInterface')); From 7bfe0317689983d5775314f9b570a1597d64e19f Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sun, 6 May 2012 17:18:26 +0200 Subject: [PATCH 2/3] VcsDrivers now send proper originUrl for authentication --- src/Composer/Repository/Vcs/GitBitbucketDriver.php | 1 + src/Composer/Repository/Vcs/GitHubDriver.php | 3 ++- src/Composer/Repository/Vcs/HgBitbucketDriver.php | 1 + src/Composer/Repository/Vcs/VcsDriver.php | 4 +++- .../Composer/Test/Repository/Vcs/GitHubDriverTest.php | 10 +++++----- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/Composer/Repository/Vcs/GitBitbucketDriver.php b/src/Composer/Repository/Vcs/GitBitbucketDriver.php index b5cb8960d..28eb66631 100644 --- a/src/Composer/Repository/Vcs/GitBitbucketDriver.php +++ b/src/Composer/Repository/Vcs/GitBitbucketDriver.php @@ -35,6 +35,7 @@ class GitBitbucketDriver extends VcsDriver implements VcsDriverInterface preg_match('#^https://bitbucket\.org/([^/]+)/(.+?)\.git$#', $this->url, $match); $this->owner = $match[1]; $this->repository = $match[2]; + $this->originUrl = 'bitbucket.org'; } /** diff --git a/src/Composer/Repository/Vcs/GitHubDriver.php b/src/Composer/Repository/Vcs/GitHubDriver.php index 9091d0c6f..f1e507235 100644 --- a/src/Composer/Repository/Vcs/GitHubDriver.php +++ b/src/Composer/Repository/Vcs/GitHubDriver.php @@ -46,6 +46,7 @@ class GitHubDriver extends VcsDriver preg_match('#^(?:https?|git)://github\.com/([^/]+)/(.+?)(?:\.git)?$#', $this->url, $match); $this->owner = $match[1]; $this->repository = $match[2]; + $this->originUrl = 'github.com'; $this->fetchRootIdentifier(); } @@ -245,7 +246,7 @@ class GitHubDriver extends VcsDriver $this->io->write('Authentication required ('.$this->url.'):'); $username = $this->io->ask('Username: '); $password = $this->io->askAndHideAnswer('Password: '); - $this->io->setAuthorization($this->url, $username, $password); + $this->io->setAuthorization($this->originUrl, $username, $password); break; default: diff --git a/src/Composer/Repository/Vcs/HgBitbucketDriver.php b/src/Composer/Repository/Vcs/HgBitbucketDriver.php index 43fff9aba..dfdf033f5 100644 --- a/src/Composer/Repository/Vcs/HgBitbucketDriver.php +++ b/src/Composer/Repository/Vcs/HgBitbucketDriver.php @@ -35,6 +35,7 @@ class HgBitbucketDriver extends VcsDriver preg_match('#^https://bitbucket\.org/([^/]+)/([^/]+)/?$#', $this->url, $match); $this->owner = $match[1]; $this->repository = $match[2]; + $this->originUrl = 'bitbucket.org'; } /** diff --git a/src/Composer/Repository/Vcs/VcsDriver.php b/src/Composer/Repository/Vcs/VcsDriver.php index d13b4b4a1..785d53548 100644 --- a/src/Composer/Repository/Vcs/VcsDriver.php +++ b/src/Composer/Repository/Vcs/VcsDriver.php @@ -26,6 +26,7 @@ use Composer\Util\RemoteFilesystem; abstract class VcsDriver implements VcsDriverInterface { protected $url; + protected $originUrl; protected $io; protected $config; protected $process; @@ -43,6 +44,7 @@ abstract class VcsDriver implements VcsDriverInterface final public function __construct($url, IOInterface $io, Config $config, ProcessExecutor $process = null, $remoteFilesystem = null) { $this->url = $url; + $this->originUrl = $url; $this->io = $io; $this->config = $config; $this->process = $process ?: new ProcessExecutor; @@ -86,7 +88,7 @@ abstract class VcsDriver implements VcsDriverInterface */ protected function getContents($url) { - return $this->remoteFilesystem->getContents($this->url, $url, false); + return $this->remoteFilesystem->getContents($this->originUrl, $url, false); } protected static function isLocalUrl($url) diff --git a/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php b/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php index 064e870f9..bbd2d3896 100644 --- a/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php +++ b/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php @@ -40,7 +40,7 @@ class GitHubDriverTest extends \PHPUnit_Framework_TestCase $remoteFilesystem->expects($this->at(0)) ->method('getContents') - ->with($this->equalTo($repoUrl), $this->equalTo($repoApiUrl), $this->equalTo(false)) + ->with($this->equalTo('github.com'), $this->equalTo($repoApiUrl), $this->equalTo(false)) ->will($this->throwException(new TransportException('HTTP/1.1 404 Not Found', 404))); $io->expects($this->once()) @@ -55,11 +55,11 @@ class GitHubDriverTest extends \PHPUnit_Framework_TestCase $io->expects($this->once()) ->method('setAuthorization') - ->with($this->equalTo($repoUrl), 'someuser', 'somepassword'); + ->with($this->equalTo('github.com'), 'someuser', 'somepassword'); $remoteFilesystem->expects($this->at(1)) ->method('getContents') - ->with($this->equalTo($repoUrl), $this->equalTo($repoApiUrl), $this->equalTo(false)) + ->with($this->equalTo('github.com'), $this->equalTo($repoApiUrl), $this->equalTo(false)) ->will($this->returnValue('{"master_branch": "test_master"}')); $gitHubDriver = new GitHubDriver($repoUrl, $io, new Config(), null, $remoteFilesystem); @@ -109,7 +109,7 @@ class GitHubDriverTest extends \PHPUnit_Framework_TestCase $remoteFilesystem->expects($this->at(0)) ->method('getContents') - ->with($this->equalTo($repoUrl), $this->equalTo($repoApiUrl), $this->equalTo(false)) + ->with($this->equalTo('github.com'), $this->equalTo($repoApiUrl), $this->equalTo(false)) ->will($this->returnValue('{"master_branch": "test_master"}')); $gitHubDriver = new GitHubDriver($repoUrl, $io, new Config(), null, $remoteFilesystem); @@ -164,7 +164,7 @@ class GitHubDriverTest extends \PHPUnit_Framework_TestCase $remoteFilesystem->expects($this->at(0)) ->method('getContents') - ->with($this->equalTo($repoUrl), $this->equalTo($repoApiUrl), $this->equalTo(false)) + ->with($this->equalTo('github.com'), $this->equalTo($repoApiUrl), $this->equalTo(false)) ->will($this->throwException(new TransportException('HTTP/1.1 404 Not Found', 404))); // clean local clone if present From a9fe8838275c44dca06c1d1b6311e51cd8e612a4 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sun, 6 May 2012 17:19:30 +0200 Subject: [PATCH 3/3] Use https fallback for github private repos if ssh protocol fails and we can ask the user's password --- src/Composer/Downloader/GitDownloader.php | 30 +++++++++++++++++++ .../Test/Downloader/GitDownloaderTest.php | 12 ++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/Composer/Downloader/GitDownloader.php b/src/Composer/Downloader/GitDownloader.php index 6af142809..a529a9635 100644 --- a/src/Composer/Downloader/GitDownloader.php +++ b/src/Composer/Downloader/GitDownloader.php @@ -46,6 +46,12 @@ class GitDownloader extends VcsDownloader $this->io->write(" Checking out ".$target->getSourceReference()); $command = 'cd %s && git remote set-url composer %s && git fetch composer && git fetch --tags composer && git checkout %3$s && git reset --hard %3$s'; + // capture username/password from github URL if there is one + $this->process->execute(sprintf('cd %s && git remote -v', escapeshellarg($path)), $output); + if (preg_match('{^composer\s+https://(.+):(.+)@github.com/}im', $output, $match)) { + $this->io->setAuthorization('github.com', $match[1], $match[2]); + } + // TODO: BC for the composer remote that didn't exist, to be remove after May 18th. $this->process->execute(sprintf('cd %s && git remote add composer %s', escapeshellarg($path), escapeshellarg($initial->getSourceUrl())), $ignoredOutput); @@ -102,6 +108,30 @@ class GitDownloader extends VcsDownloader $command = call_user_func($commandCallable, $url); if (0 !== $this->process->execute($command, $handler)) { + if (preg_match('{^git@github.com:(.+?)\.git$}i', $url, $match) && $this->io->isInteractive()) { + // private repository without git access, try https with auth + $retries = 3; + $retrying = false; + do { + if ($retrying) { + $this->io->write('Invalid credentials'); + } + if (!$this->io->hasAuthorization('github.com') || $retrying) { + $username = $this->io->ask('Username: '); + $password = $this->io->askAndHideAnswer('Password: '); + $this->io->setAuthorization('github.com', $username, $password); + } + + $auth = $this->io->getAuthorization('github.com'); + $url = 'https://'.$auth['username'] . ':' . $auth['password'] . '@github.com/'.$match[1].'.git'; + + $command = call_user_func($commandCallable, $url); + if (0 === $this->process->execute($command, $handler)) { + return; + } + $retrying = true; + } while (--$retries); + } $this->throwException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput(), $url); } } diff --git a/tests/Composer/Test/Downloader/GitDownloaderTest.php b/tests/Composer/Test/Downloader/GitDownloaderTest.php index c23b95a7a..6d4953153 100644 --- a/tests/Composer/Test/Downloader/GitDownloaderTest.php +++ b/tests/Composer/Test/Downloader/GitDownloaderTest.php @@ -156,9 +156,13 @@ class GitDownloaderTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue(0)); $processExecutor->expects($this->at(1)) ->method('execute') - ->with($this->equalTo($this->getCmd("cd 'composerPath' && git remote add composer 'https://github.com/composer/composer'"))) + ->with($this->equalTo($this->getCmd("cd 'composerPath' && git remote -v"))) ->will($this->returnValue(0)); $processExecutor->expects($this->at(2)) + ->method('execute') + ->with($this->equalTo($this->getCmd("cd 'composerPath' && git remote add composer 'https://github.com/composer/composer'"))) + ->will($this->returnValue(0)); + $processExecutor->expects($this->at(3)) ->method('execute') ->with($this->equalTo($expectedGitUpdateCommand)) ->will($this->returnValue(0)); @@ -189,9 +193,13 @@ class GitDownloaderTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue(0)); $processExecutor->expects($this->at(1)) ->method('execute') - ->with($this->equalTo($this->getCmd("cd 'composerPath' && git remote add composer 'https://github.com/composer/composer'"))) + ->with($this->equalTo($this->getCmd("cd 'composerPath' && git remote -v"))) ->will($this->returnValue(0)); $processExecutor->expects($this->at(2)) + ->method('execute') + ->with($this->equalTo($this->getCmd("cd 'composerPath' && git remote add composer 'https://github.com/composer/composer'"))) + ->will($this->returnValue(0)); + $processExecutor->expects($this->at(3)) ->method('execute') ->with($this->equalTo($expectedGitUpdateCommand)) ->will($this->returnValue(1));