From a4af559ca8d249d629afc7d4111080bbedad0e75 Mon Sep 17 00:00:00 2001 From: Stefan Grootscholten Date: Tue, 27 Dec 2016 00:39:02 +0100 Subject: [PATCH] Store access-token for re-use Store the Bitbucket access-token (and the expiration time) so it can be re-used within the time it is valid. The Bitbucket::requestToken and Bitbucket::getToken now only return the access-token and not all other parameters it receives from the Bitbucket API. --- src/Composer/Config.php | 2 + src/Composer/Util/Bitbucket.php | 84 ++++++++++++--- src/Composer/Util/Git.php | 10 +- src/Composer/Util/RemoteFilesystem.php | 6 +- tests/Composer/Test/Util/BitbucketTest.php | 116 ++++++++++----------- 5 files changed, 134 insertions(+), 84 deletions(-) diff --git a/src/Composer/Config.php b/src/Composer/Config.php index 2888b8d29..fcf00c2f2 100644 --- a/src/Composer/Config.php +++ b/src/Composer/Config.php @@ -79,7 +79,9 @@ class Config private $config; private $baseDir; private $repositories; + /** @var ConfigSourceInterface */ private $configSource; + /** @var ConfigSourceInterface */ private $authConfigSource; private $useEnvironment; private $warnedHosts = array(); diff --git a/src/Composer/Util/Bitbucket.php b/src/Composer/Util/Bitbucket.php index f8b460103..152fe5fa0 100644 --- a/src/Composer/Util/Bitbucket.php +++ b/src/Composer/Util/Bitbucket.php @@ -27,6 +27,7 @@ class Bitbucket private $process; private $remoteFilesystem; private $token = array(); + private $time; const OAUTH2_ACCESS_TOKEN_URL = 'https://bitbucket.org/site/oauth2/access_token'; @@ -37,21 +38,26 @@ class Bitbucket * @param Config $config The composer configuration * @param ProcessExecutor $process Process instance, injectable for mocking * @param RemoteFilesystem $remoteFilesystem Remote Filesystem, injectable for mocking + * @param int $time Timestamp, injectable for mocking */ - public function __construct(IOInterface $io, Config $config, ProcessExecutor $process = null, RemoteFilesystem $remoteFilesystem = null) + public function __construct(IOInterface $io, Config $config, ProcessExecutor $process = null, RemoteFilesystem $remoteFilesystem = null, $time = null) { $this->io = $io; $this->config = $config; $this->process = $process ?: new ProcessExecutor; $this->remoteFilesystem = $remoteFilesystem ?: Factory::createRemoteFilesystem($this->io, $config); + $this->time = $time; } /** - * @return array + * @return string */ public function getToken() { - return $this->token; + if (! isset($this->token['access_token'])) { + return ''; + } + return $this->token['access_token']; } /** @@ -109,6 +115,8 @@ class Bitbucket throw $e; } + + return true; } /** @@ -151,16 +159,13 @@ class Bitbucket $this->io->setAuthentication($originUrl, $consumerKey, $consumerSecret); - $this->requestAccessToken($originUrl); + if (! $this->requestAccessToken($originUrl)) { + return false; + } // store value in user config - $this->config->getConfigSource()->removeConfigSetting('bitbucket-oauth.'.$originUrl); + $this->storeInAuthConfig($originUrl, $consumerKey, $consumerSecret); - $consumer = array( - "consumer-key" => $consumerKey, - "consumer-secret" => $consumerSecret, - ); - $this->config->getAuthConfigSource()->addConfigSetting('bitbucket-oauth.'.$originUrl, $consumer); // Remove conflicting basic auth credentials (if available) $this->config->getAuthConfigSource()->removeConfigSetting('http-basic.' . $originUrl); @@ -175,17 +180,66 @@ class Bitbucket * @param string $originUrl * @param string $consumerKey * @param string $consumerSecret - * @return array + * @return string */ public function requestToken($originUrl, $consumerKey, $consumerSecret) { - if (!empty($this->token)) { - return $this->token; + if (!empty($this->token) || $this->getTokenFromConfig($originUrl)) { + return $this->token['access_token']; } $this->io->setAuthentication($originUrl, $consumerKey, $consumerSecret); - $this->requestAccessToken($originUrl); + if (! $this->requestAccessToken($originUrl)) { + return ''; + } - return $this->token; + $this->storeInAuthConfig($originUrl, $consumerKey, $consumerSecret); + + return $this->token['access_token']; + } + + /** + * Store the new/updated credentials to the configuration + * @param string $originUrl + * @param string $consumerKey + * @param string $consumerSecret + */ + private function storeInAuthConfig($originUrl, $consumerKey, $consumerSecret) + { + $this->config->getConfigSource()->removeConfigSetting('bitbucket-oauth.'.$originUrl); + + $time = null === $this->time ? time() : $this->time; + $consumer = array( + "consumer-key" => $consumerKey, + "consumer-secret" => $consumerSecret, + "access-token" => $this->token['access_token'], + "access-token-expiration" => $time + $this->token['expires_in'] + ); + + $this->config->getAuthConfigSource()->addConfigSetting('bitbucket-oauth.'.$originUrl, $consumer); + } + + /** + * @param string $originUrl + * @return bool + */ + private function getTokenFromConfig($originUrl) + { + $authConfig = $this->config->get('bitbucket-oauth'); + + if (empty($authConfig) || + ! isset($authConfig[$originUrl]) || + ! isset($authConfig[$originUrl]['access-token']) || + ! isset($authConfig[$originUrl]['access-token-expiration']) || + time() > $authConfig[$originUrl]['access-token-expiration'] + ) { + return false; + } + + $this->token = array( + 'access_token' => $authConfig[$originUrl]['access-token'] + ); + + return true; } } diff --git a/src/Composer/Util/Git.php b/src/Composer/Util/Git.php index c7e892244..90722bc5a 100644 --- a/src/Composer/Util/Git.php +++ b/src/Composer/Util/Git.php @@ -122,17 +122,17 @@ class Git if (!$bitbucketUtil->authorizeOAuth($match[1]) && $this->io->isInteractive()) { $bitbucketUtil->authorizeOAuthInteractively($match[1], $message); - $token = $bitbucketUtil->getToken(); - $this->io->setAuthentication($match[1], 'x-token-auth', $token['access_token']); + $access_token = $bitbucketUtil->getToken(); + $this->io->setAuthentication($match[1], 'x-token-auth', $access_token); } } else { //We're authenticating with a locally stored consumer. $auth = $this->io->getAuthentication($match[1]); //We already have an access_token from a previous request. if ($auth['username'] !== 'x-token-auth') { - $token = $bitbucketUtil->requestToken($match[1], $auth['username'], $auth['password']); - if (!empty($token)) { - $this->io->setAuthentication($match[1], 'x-token-auth', $token['access_token']); + $access_token = $bitbucketUtil->requestToken($match[1], $auth['username'], $auth['password']); + if (! empty($access_token)) { + $this->io->setAuthentication($match[1], 'x-token-auth', $access_token); } } } diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index 5b137686d..4437f79ea 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -601,9 +601,9 @@ class RemoteFilesystem $auth = $this->io->getAuthentication($this->originUrl); if ($auth['username'] !== 'x-token-auth') { $bitbucketUtil = new Bitbucket($this->io, $this->config); - $token = $bitbucketUtil->requestToken($this->originUrl, $auth['username'], $auth['password']); - if (! empty($token)) { - $this->io->setAuthentication($this->originUrl, 'x-token-auth', $token['access_token']); + $access_token = $bitbucketUtil->requestToken($this->originUrl, $auth['username'], $auth['password']); + if (! empty($access_token)) { + $this->io->setAuthentication($this->originUrl, 'x-token-auth', $access_token); $askForOAuthToken = false; } } else { diff --git a/tests/Composer/Test/Util/BitbucketTest.php b/tests/Composer/Test/Util/BitbucketTest.php index 45b4ce752..e9ca9e269 100644 --- a/tests/Composer/Test/Util/BitbucketTest.php +++ b/tests/Composer/Test/Util/BitbucketTest.php @@ -35,6 +35,8 @@ class BitbucketTest extends \PHPUnit_Framework_TestCase private $config; /** @type Bitbucket */ private $bitbucket; + /** @var int */ + private $time; protected function setUp() { @@ -52,7 +54,9 @@ class BitbucketTest extends \PHPUnit_Framework_TestCase $this->config = $this->getMock('Composer\Config'); - $this->bitbucket = new Bitbucket($this->io, $this->config, null, $this->rfs); + $this->time = time(); + + $this->bitbucket = new Bitbucket($this->io, $this->config, null, $this->rfs, $this->time); } public function testRequestAccessTokenWithValidOAuthConsumer() @@ -82,14 +86,15 @@ class BitbucketTest extends \PHPUnit_Framework_TestCase ) ); + $this->config->expects($this->once()) + ->method('get') + ->with('bitbucket-oauth') + ->willReturn(null); + + $this->setExpectationsForStoringAccessToken(); + $this->assertEquals( - array( - 'access_token' => $this->token, - 'scopes' => 'repository', - 'expires_in' => 3600, - 'refresh_token' => 'refreshtoken', - 'token_type' => 'bearer' - ), + $this->token, $this->bitbucket->requestToken($this->origin, $this->consumer_key, $this->consumer_secret) ); } @@ -133,7 +138,12 @@ class BitbucketTest extends \PHPUnit_Framework_TestCase ) ); - $this->assertEquals(array(), $this->bitbucket->requestToken($this->origin, $this->username, $this->password)); + $this->config->expects($this->once()) + ->method('get') + ->with('bitbucket-oauth') + ->willReturn(null); + + $this->assertEquals('', $this->bitbucket->requestToken($this->origin, $this->username, $this->password)); } public function testUsernamePasswordAuthenticationFlow() @@ -161,67 +171,51 @@ class BitbucketTest extends \PHPUnit_Framework_TestCase $this->isFalse(), $this->anything() ) - ->willReturn(sprintf('{}', $this->token)) - ; - - $authJson = $this->getAuthJsonMock(); - $this->config - ->expects($this->exactly(3)) - ->method('getAuthConfigSource') - ->willReturn($authJson) - ; - $this->config - ->expects($this->once()) - ->method('getConfigSource') - ->willReturn($this->getConfJsonMock()) - ; - - $authJson->expects($this->once()) - ->method('addConfigSetting') - ->with( - 'bitbucket-oauth.'.$this->origin, - array( - 'consumer-key' => $this->consumer_key, - 'consumer-secret' => $this->consumer_secret + ->willReturn( + sprintf( + '{"access_token": "%s", "scopes": "repository", "expires_in": 3600, "refresh_token": "refresh_token", "token_type": "bearer"}', + $this->token ) - ); + ) + ; - $authJson->expects($this->once()) - ->method('removeConfigSetting') - ->with('http-basic.'.$this->origin); + $this->setExpectationsForStoringAccessToken(true); $this->assertTrue($this->bitbucket->authorizeOAuthInteractively($this->origin, $this->message)); } - private function getAuthJsonMock() + private function setExpectationsForStoringAccessToken($removeBasicAuth = false) { - $authjson = $this - ->getMockBuilder('Composer\Config\JsonConfigSource') - ->disableOriginalConstructor() - ->getMock() - ; - $authjson - ->expects($this->atLeastOnce()) - ->method('getName') - ->willReturn('auth.json') - ; + $configSourceMock = $this->getMock('Composer\Config\ConfigSourceInterface'); + $this->config->expects($this->once()) + ->method('getConfigSource') + ->willReturn($configSourceMock); - return $authjson; - } - - private function getConfJsonMock() - { - $confjson = $this - ->getMockBuilder('Composer\Config\JsonConfigSource') - ->disableOriginalConstructor() - ->getMock() - ; - $confjson - ->expects($this->atLeastOnce()) + $configSourceMock->expects($this->once()) ->method('removeConfigSetting') - ->with('bitbucket-oauth.'.$this->origin) - ; + ->with('bitbucket-oauth.' . $this->origin); - return $confjson; + $authConfigSourceMock = $this->getMock('Composer\Config\ConfigSourceInterface'); + $this->config->expects($this->atLeastOnce()) + ->method('getAuthConfigSource') + ->willReturn($authConfigSourceMock); + + $authConfigSourceMock->expects($this->once()) + ->method('addConfigSetting') + ->with( + 'bitbucket-oauth.' . $this->origin, + array( + "consumer-key" => $this->consumer_key, + "consumer-secret" => $this->consumer_secret, + "access-token" => $this->token, + "access-token-expiration" => $this->time + 3600 + ) + ); + + if ($removeBasicAuth) { + $authConfigSourceMock->expects($this->once()) + ->method('removeConfigSetting') + ->with('http-basic.' . $this->origin); + } } }