From a34335a9bb3bb258d31f9967da4823163a481755 Mon Sep 17 00:00:00 2001 From: Rob Bast Date: Thu, 5 Feb 2015 14:46:14 +0100 Subject: [PATCH] github deprecation changes - added some tests - minor bug fixes discovered during testing - resolved two deprecations (rate limit api and authorizations api) - added some more comments to make the flow more understandable --- src/Composer/Command/DiagnoseCommand.php | 2 +- src/Composer/Util/GitHub.php | 196 ++++++------- .../Test/Repository/Vcs/GitHubDriverTest.php | 7 +- tests/Composer/Test/Util/GitHubTest.php | 264 ++++++++++++++++++ 4 files changed, 366 insertions(+), 103 deletions(-) create mode 100644 tests/Composer/Test/Util/GitHubTest.php diff --git a/src/Composer/Command/DiagnoseCommand.php b/src/Composer/Command/DiagnoseCommand.php index 0d24d74e6..b1fcc8dbe 100644 --- a/src/Composer/Command/DiagnoseCommand.php +++ b/src/Composer/Command/DiagnoseCommand.php @@ -274,7 +274,7 @@ EOT $json = $this->rfs->getContents($domain, $url, false, array('retry-auth-failure' => false)); $data = json_decode($json, true); - return $data['rate']; + return $data['resources']['core']; } catch (\Exception $e) { if ($e instanceof TransportException && $e->getCode() === 401) { return 'The oauth token for '.$domain.' seems invalid, run "composer config --global --unset github-oauth.'.$domain.'" to remove it'; diff --git a/src/Composer/Util/GitHub.php b/src/Composer/Util/GitHub.php index 8624edc04..6008dc062 100644 --- a/src/Composer/Util/GitHub.php +++ b/src/Composer/Util/GitHub.php @@ -76,124 +76,128 @@ class GitHub */ public function authorizeOAuthInteractively($originUrl, $message = null) { - $attemptCounter = 0; - - $apiUrl = ('github.com' === $originUrl) ? 'api.github.com' : $originUrl . '/api/v3'; - if ($message) { $this->io->write($message); } - $this->io->write('The credentials will be swapped for an OAuth token stored in '.$this->config->getAuthConfigSource()->getName().', your password will not be stored'); + + $this->io->write(sprintf('A token will be created and stored in "%s", your password will never be stored', $this->config->getAuthConfigSource()->getName())); $this->io->write('To revoke access to this token you can visit https://github.com/settings/applications'); + + $otp = null; + $attemptCounter = 0; + while ($attemptCounter++ < 5) { try { - if (empty($otp) || !$this->io->hasAuthentication($originUrl)) { - $username = $this->io->ask('Username: '); - $password = $this->io->askAndHideAnswer('Password: '); - $otp = null; - - $this->io->setAuthentication($originUrl, $username, $password); - } - - // build up OAuth app name - $appName = 'Composer'; - if ($this->config->get('github-expose-hostname') === true && 0 === $this->process->execute('hostname', $output)) { - $appName .= ' on ' . trim($output); - } else { - $appName .= ' [' . date('YmdHis') . ']'; - } - - $headers = array(); - if ($otp) { - $headers = array('X-GitHub-OTP: ' . $otp); - } - - // try retrieving an existing token with the same name - $contents = null; - $auths = JsonFile::parseJson($this->remoteFilesystem->getContents($originUrl, 'https://'. $apiUrl . '/authorizations', false, array( - 'retry-auth-failure' => false, - 'http' => array( - 'header' => $headers - ) - ))); - foreach ($auths as $auth) { - if ( - isset($auth['app']['name']) - && 0 === strpos($auth['app']['name'], $appName) - && $auth['app']['url'] === 'https://getcomposer.org/' - ) { - $this->io->write('An existing OAuth token for Composer is present and will be reused'); - - $contents['token'] = $auth['token']; - break; - } - } - - // no existing token, create one - if (empty($contents['token'])) { - $headers[] = 'Content-Type: application/json'; - - $contents = JsonFile::parseJson($this->remoteFilesystem->getContents($originUrl, 'https://'. $apiUrl . '/authorizations', false, array( - 'retry-auth-failure' => false, - 'http' => array( - 'method' => 'POST', - 'follow_location' => false, - 'header' => $headers, - 'content' => json_encode(array( - 'scopes' => array('repo'), - 'note' => $appName, - 'note_url' => 'https://getcomposer.org/', - )), - ) - ))); - $this->io->write('Token successfully created'); - } + $response = $this->createToken($originUrl, $otp); } catch (TransportException $e) { + // https://developer.github.com/v3/#authentication && https://developer.github.com/v3/auth/#working-with-two-factor-authentication + // 401 is bad credentials, or missing otp code + // 403 is max login attempts exceeded if (in_array($e->getCode(), array(403, 401))) { - // 401 when authentication was supplied, handle 2FA if required. - if ($this->io->hasAuthentication($originUrl)) { - $headerNames = array_map(function ($header) { - return strtolower(strstr($header, ':', true)); - }, $e->getHeaders()); - - if ($key = array_search('x-github-otp', $headerNames)) { - $headers = $e->getHeaders(); - list($required, $method) = array_map('trim', explode(';', substr(strstr($headers[$key], ':'), 1))); - - if ('required' === $required) { - $this->io->write('Two-factor Authentication'); - - if ('app' === $method) { - $this->io->write('Open the two-factor authentication app on your device to view your authentication code and verify your identity.'); - } - - if ('sms' === $method) { - $this->io->write('You have been sent an SMS message with an authentication code to verify your identity.'); - } - - $otp = $this->io->ask('Authentication Code: '); - - continue; - } + // in case of a 401, and authentication was previously provided + if (401 === $e->getCode() && $this->io->hasAuthentication($originUrl)) { + // check for the presence of otp headers and get otp code from user + $otp = $this->checkTwoFactorAuthentication($e->getHeaders()); + // if given, retry creating a token using the user provided code + if (null !== $otp) { + continue; } } - $this->io->write('Invalid credentials.'); + if (401 === $e->getCode()) { + $this->io->write('Bad credentials.'); + } else { + $this->io->write('Maximum number of login attempts exceeded. Please try again later.'); + } + + $this->io->write('You can also manually create a personal token at https://github.com/settings/applications'); + $this->io->write('Add it using "composer config github-oauth.github.com "'); + continue; } throw $e; } - $this->io->setAuthentication($originUrl, $contents['token'], 'x-oauth-basic'); - - // store value in user config + $this->io->setAuthentication($originUrl, $response['token'], 'x-oauth-basic'); $this->config->getConfigSource()->removeConfigSetting('github-oauth.'.$originUrl); - $this->config->getAuthConfigSource()->addConfigSetting('github-oauth.'.$originUrl, $contents['token']); + // store value in user config + $this->config->getAuthConfigSource()->addConfigSetting('github-oauth.'.$originUrl, $response['token']); return true; } throw new \RuntimeException("Invalid GitHub credentials 5 times in a row, aborting."); } + + private function createToken($originUrl, $otp = null) + { + if (null === $otp || !$this->io->hasAuthentication($originUrl)) { + $username = $this->io->ask('Username: '); + $password = $this->io->askAndHideAnswer('Password: '); + + $this->io->setAuthentication($originUrl, $username, $password); + } + + $headers = array('Content-Type: application/json'); + if ($otp) { + $headers[] = 'X-GitHub-OTP: ' . $otp; + } + + $note = 'Composer'; + if ($this->config->get('github-expose-hostname') === true && 0 === $this->process->execute('hostname', $output)) { + $note .= ' on ' . trim($output); + } + $note .= ' [' . date('YmdHis') . ']'; + + $apiUrl = ('github.com' === $originUrl) ? 'api.github.com' : $originUrl . '/api/v3'; + + $json = $this->remoteFilesystem->getContents($originUrl, 'https://'. $apiUrl . '/authorizations', false, array( + 'retry-auth-failure' => false, + 'http' => array( + 'method' => 'POST', + 'follow_location' => false, + 'header' => $headers, + 'content' => json_encode(array( + 'scopes' => array('repo'), + 'note' => $note, + 'note_url' => 'https://getcomposer.org/', + )), + ) + )); + + $this->io->write('Token successfully created'); + + return JsonFile::parseJson($json); + } + + private function checkTwoFactorAuthentication(array $headers) + { + $headerNames = array_map( + function ($header) { + return strtolower(strstr($header, ':', true)); + }, + $headers + ); + + if (false !== ($key = array_search('x-github-otp', $headerNames))) { + list($required, $method) = array_map('trim', explode(';', substr(strstr($headers[$key], ':'), 1))); + + if ('required' === $required) { + $this->io->write('Two-factor Authentication'); + + if ('app' === $method) { + $this->io->write('Open the two-factor authentication app on your device to view your authentication code and verify your identity.'); + } + + if ('sms' === $method) { + $this->io->write('You have been sent an SMS message with an authentication code to verify your identity.'); + } + + return $this->io->ask('Authentication Code: '); + } + } + + return null; + } } diff --git a/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php b/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php index d20da87dc..e180c5eec 100644 --- a/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php +++ b/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php @@ -79,16 +79,11 @@ class GitHubDriverTest extends \PHPUnit_Framework_TestCase ->with($this->equalTo('github.com'), $this->matchesRegularExpression('{someuser|abcdef}'), $this->matchesRegularExpression('{somepassword|x-oauth-basic}')); $remoteFilesystem->expects($this->at(1)) - ->method('getContents') - ->with($this->equalTo('github.com'), $this->equalTo('https://api.github.com/authorizations'), $this->equalTo(false)) - ->will($this->returnValue('[]')); - - $remoteFilesystem->expects($this->at(2)) ->method('getContents') ->with($this->equalTo('github.com'), $this->equalTo('https://api.github.com/authorizations'), $this->equalTo(false)) ->will($this->returnValue('{"token": "abcdef"}')); - $remoteFilesystem->expects($this->at(3)) + $remoteFilesystem->expects($this->at(2)) ->method('getContents') ->with($this->equalTo('github.com'), $this->equalTo($repoApiUrl), $this->equalTo(false)) ->will($this->returnValue('{"master_branch": "test_master", "private": true, "owner": {"login": "composer"}, "name": "packagist"}')); diff --git a/tests/Composer/Test/Util/GitHubTest.php b/tests/Composer/Test/Util/GitHubTest.php new file mode 100644 index 000000000..4a1f3a35a --- /dev/null +++ b/tests/Composer/Test/Util/GitHubTest.php @@ -0,0 +1,264 @@ + +* Jordi Boggiano +* +* For the full copyright and license information, please view the LICENSE +* file that was distributed with this source code. +*/ + +namespace Composer\Test\Util; + +use Composer\Downloader\TransportException; +use Composer\Util\GitHub; +use RecursiveArrayIterator; +use RecursiveIteratorIterator; + +/** +* @author Rob Bast +*/ +class GitHubTest extends \PHPUnit_Framework_TestCase +{ + private $username = 'username'; + private $password = 'password'; + private $authcode = 'authcode'; + private $message = 'mymessage'; + private $origin = 'github.com'; + private $token = 'githubtoken'; + + public function testUsernamePasswordAuthenticationFlow() + { + $io = $this->getIOMock(); + $io + ->expects($this->at(0)) + ->method('write') + ->with($this->message) + ; + $io + ->expects($this->once()) + ->method('ask') + ->with('Username: ') + ->willReturn($this->username) + ; + $io + ->expects($this->once()) + ->method('askAndHideAnswer') + ->with('Password: ') + ->willReturn($this->password) + ; + + $rfs = $this->getRemoteFilesystemMock(); + $rfs + ->expects($this->once()) + ->method('getContents') + ->with( + $this->equalTo($this->origin), + $this->equalTo(sprintf('https://api.%s/authorizations', $this->origin)), + $this->isFalse(), + $this->anything() + ) + ->willReturn(sprintf('{"token": "%s"}', $this->token)) + ; + + $config = $this->getConfigMock(); + $config + ->expects($this->exactly(2)) + ->method('getAuthConfigSource') + ->willReturn($this->getAuthJsonMock()) + ; + $config + ->expects($this->once()) + ->method('getConfigSource') + ->willReturn($this->getConfJsonMock()) + ; + + $github = new GitHub($io, $config, null, $rfs); + + $this->assertTrue($github->authorizeOAuthInteractively($this->origin, $this->message)); + } + + /** + * @expectedException \RuntimeException + * @expectedExceptionMessage Invalid GitHub credentials 5 times in a row, aborting. + */ + public function testUsernamePasswordFailure() + { + $io = $this->getIOMock(); + $io + ->expects($this->exactly(5)) + ->method('ask') + ->with('Username: ') + ->willReturn($this->username) + ; + $io + ->expects($this->exactly(5)) + ->method('askAndHideAnswer') + ->with('Password: ') + ->willReturn($this->password) + ; + + $rfs = $this->getRemoteFilesystemMock(); + $rfs + ->expects($this->exactly(5)) + ->method('getContents') + ->will($this->throwException(new TransportException('', 401))) + ; + + $config = $this->getConfigMock(); + $config + ->expects($this->exactly(1)) + ->method('getAuthConfigSource') + ->willReturn($this->getAuthJsonMock()) + ; + + $github = new GitHub($io, $config, null, $rfs); + + $github->authorizeOAuthInteractively($this->origin); + } + + public function testTwoFactorAuthentication() + { + $io = $this->getIOMock(); + $io + ->expects($this->exactly(2)) + ->method('hasAuthentication') + ->will($this->onConsecutiveCalls(true, true)) + ; + $io + ->expects($this->exactly(2)) + ->method('ask') + ->withConsecutive( + array('Username: '), + array('Authentication Code: ') + ) + ->will($this->onConsecutiveCalls($this->username, $this->authcode)) + ; + $io + ->expects($this->once()) + ->method('askAndHideAnswer') + ->with('Password: ') + ->willReturn($this->password) + ; + + $exception = new TransportException('', 401); + $exception->setHeaders(array('X-GitHub-OTP: required; app')); + + $rfs = $this->getRemoteFilesystemMock(); + $rfs + ->expects($this->at(0)) + ->method('getContents') + ->will($this->throwException($exception)) + ; + $rfs + ->expects($this->at(1)) + ->method('getContents') + ->with( + $this->equalTo($this->origin), + $this->equalTo(sprintf('https://api.%s/authorizations', $this->origin)), + $this->isFalse(), + $this->callback(function ($array) { + $headers = GitHubTest::recursiveFind($array, 'header'); + foreach ($headers as $string) { + if ('X-GitHub-OTP: authcode' === $string) { + return true; + } + } + return false; + }) + ) + ->willReturn(sprintf('{"token": "%s"}', $this->token)) + ; + + $config = $this->getConfigMock(); + $config + ->expects($this->atLeastOnce()) + ->method('getAuthConfigSource') + ->willReturn($this->getAuthJsonMock()) + ; + $config + ->expects($this->atLeastOnce()) + ->method('getConfigSource') + ->willReturn($this->getConfJsonMock()) + ; + + $github = new GitHub($io, $config, null, $rfs); + + $this->assertTrue($github->authorizeOAuthInteractively($this->origin)); + } + + private function getIOMock() + { + $io = $this + ->getMockBuilder('Composer\IO\ConsoleIO') + ->disableOriginalConstructor() + ->getMock() + ; + + return $io; + } + + private function getConfigMock() + { + $config = $this->getMock('Composer\Config'); + + return $config; + } + + private function getRemoteFilesystemMock() + { + $rfs = $this + ->getMockBuilder('Composer\Util\RemoteFilesystem') + ->disableOriginalConstructor() + ->getMock() + ; + + return $rfs; + } + + private function getAuthJsonMock() + { + $authjson = $this + ->getMockBuilder('Composer\Config\JsonConfigSource') + ->disableOriginalConstructor() + ->getMock() + ; + $authjson + ->expects($this->atLeastOnce()) + ->method('getName') + ->willReturn('auth.json') + ; + + return $authjson; + } + + private function getConfJsonMock() + { + $confjson = $this + ->getMockBuilder('Composer\Config\JsonConfigSource') + ->disableOriginalConstructor() + ->getMock() + ; + $confjson + ->expects($this->atLeastOnce()) + ->method('removeConfigSetting') + ->with('github-oauth.'.$this->origin) + ; + + return $confjson; + } + + public static function recursiveFind($array, $needle) + { + $iterator = new RecursiveArrayIterator($array); + $recursive = new RecursiveIteratorIterator($iterator, RecursiveIteratorIterator::SELF_FIRST); + + foreach ($recursive as $key => $value) { + if ($key === $needle) { + return $value; + } + } + } +}