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

@@ -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 '<comment>The oauth token for '.$domain.' seems invalid, run "composer config --global --unset github-oauth.'.$domain.'" to remove it</comment>';

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;
+                $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))) {
+                    // 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->setAuthentication($originUrl, $username, $password);
-                }
+                    if (401 === $e->getCode()) {
+                        $this->io->write('Bad credentials.');
+                    } else {
+                        $this->io->write('Maximum number of login attempts exceeded. Please try again later.');
+                    }
 
-                // 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') . ']';
-                }
+                    $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 <token>"');
 
-                $headers = array();
-                if ($otp) {
-                    $headers = array('X-GitHub-OTP: ' . $otp);
+                    continue;
                 }
 
-                // 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;
-                    }
-                }
+                throw $e;
+            }
 
-                // 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');
-                }
-            } catch (TransportException $e) {
-                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());
+            $this->io->setAuthentication($originUrl, $response['token'], 'x-oauth-basic');
+            $this->config->getConfigSource()->removeConfigSetting('github-oauth.'.$originUrl);
+            // store value in user config
+            $this->config->getAuthConfigSource()->addConfigSetting('github-oauth.'.$originUrl, $response['token']);
+
+            return true;
+        }
 
-                        if ($key = array_search('x-github-otp', $headerNames)) {
-                            $headers = $e->getHeaders();
-                            list($required, $method) = array_map('trim', explode(';', substr(strstr($headers[$key], ':'), 1)));
+        throw new \RuntimeException("Invalid GitHub credentials 5 times in a row, aborting.");
+    }
 
-                            if ('required' === $required) {
-                                $this->io->write('Two-factor Authentication');
+    private function createToken($originUrl, $otp = null)
+    {
+        if (null === $otp || !$this->io->hasAuthentication($originUrl)) {
+            $username = $this->io->ask('Username: ');
+            $password = $this->io->askAndHideAnswer('Password: ');
 
-                                if ('app' === $method) {
-                                    $this->io->write('Open the two-factor authentication app on your device to view your authentication code and verify your identity.');
-                                }
+            $this->io->setAuthentication($originUrl, $username, $password);
+        }
 
-                                if ('sms' === $method) {
-                                    $this->io->write('You have been sent an SMS message with an authentication code to verify your identity.');
-                                }
+        $headers = array('Content-Type: application/json');
+        if ($otp) {
+            $headers[] = 'X-GitHub-OTP: ' . $otp;
+        }
 
-                                $otp = $this->io->ask('Authentication Code: ');
+        $note = 'Composer';
+        if ($this->config->get('github-expose-hostname') === true && 0 === $this->process->execute('hostname', $output)) {
+            $note .= ' on ' . trim($output);
+        }
+        $note .= ' [' . date('YmdHis') . ']';
 
-                                continue;
-                            }
-                        }
-                    }
+        $apiUrl = ('github.com' === $originUrl) ? 'api.github.com' : $originUrl . '/api/v3';
 
-                    $this->io->write('Invalid credentials.');
-                    continue;
-                }
+        $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);
+    }
 
-                throw $e;
-            }
+    private function checkTwoFactorAuthentication(array $headers)
+    {
+        $headerNames = array_map(
+            function ($header) {
+                return strtolower(strstr($header, ':', true));
+            },
+            $headers
+        );
 
-            $this->io->setAuthentication($originUrl, $contents['token'], 'x-oauth-basic');
+        if (false !== ($key = array_search('x-github-otp', $headerNames))) {
+            list($required, $method) = array_map('trim', explode(';', substr(strstr($headers[$key], ':'), 1)));
 
-            // store value in user config
-            $this->config->getConfigSource()->removeConfigSetting('github-oauth.'.$originUrl);
-            $this->config->getAuthConfigSource()->addConfigSetting('github-oauth.'.$originUrl, $contents['token']);
+            if ('required' === $required) {
+                $this->io->write('Two-factor Authentication');
 
-            return true;
+                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: ');
+            }
         }
 
-        throw new \RuntimeException("Invalid GitHub credentials 5 times in a row, aborting.");
+        return null;
     }
 }

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"}'));

tests/Composer/Test/Util/GitHubTest.php

@@ -0,0 +1,264 @@
+<?php
+
+/*
+* This file is part of Composer.
+*
+* (c) Nils Adermann <naderman@naderman.de>
+*     Jordi Boggiano <j.boggiano@seld.be>
+*
+* 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 <rob.bast@gmail.com>
+*/
+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;
+            }
+        }
+    }
+}