From dd5701dd6d7ebaf396e3fcc449e6f7a64a1a1f52 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 24 Oct 2023 12:46:51 -0700 Subject: [PATCH] pass in access token from cache when possible --- .../ExternalAccountCredentials.php | 16 +- src/FetchAuthTokenCache.php | 18 +- .../ExternalAccountCredentialsTest.php | 159 +++++++++++++++--- tests/FetchAuthTokenCacheTest.php | 72 -------- 4 files changed, 159 insertions(+), 106 deletions(-) diff --git a/src/Credentials/ExternalAccountCredentials.php b/src/Credentials/ExternalAccountCredentials.php index e8ab4f144..13d174e99 100644 --- a/src/Credentials/ExternalAccountCredentials.php +++ b/src/Credentials/ExternalAccountCredentials.php @@ -23,10 +23,10 @@ use Google\Auth\ExternalAccountCredentialSourceInterface; use Google\Auth\FetchAuthTokenInterface; use Google\Auth\GetQuotaProjectInterface; -use Google\Auth\ProjectIdProviderInterface; use Google\Auth\HttpHandler\HttpClientCache; use Google\Auth\HttpHandler\HttpHandlerFactory; use Google\Auth\OAuth2; +use Google\Auth\ProjectIdProviderInterface; use Google\Auth\UpdateMetadataInterface; use Google\Auth\UpdateMetadataTrait; use GuzzleHttp\Psr7\Request; @@ -252,9 +252,12 @@ public function getQuotaProject() * Get the project ID. * * @param callable $httpHandler Callback which delivers psr7 request + * @param string $accessToken The access token to use to sign the blob. If + * provided, saves a call to the metadata server for a new access + * token. **Defaults to** `null`. * @return string|null */ - public function getProjectId(callable $httpHandler = null) + public function getProjectId(callable $httpHandler = null, string $accessToken = null) { if (isset($this->projectId)) { return $this->projectId; @@ -270,10 +273,11 @@ public function getProjectId(callable $httpHandler = null) } $url = sprintf(self::CLOUD_RESOURCE_MANAGER_URL, $projectNumber); - // This is not ideal, as it does not take advantage of caching. - // @TOOD: find a way to fix this. - $token = $this->fetchAuthToken($httpHandler); - $request = new Request('GET', $url, ['authorization' => 'Bearer ' . $token['access_token']]); + + if (is_null($accessToken)) { + $accessToken = $this->fetchAuthToken($httpHandler)['access_token']; + } + $request = new Request('GET', $url, ['authorization' => 'Bearer ' . $accessToken]); $response = $httpHandler($request); $body = json_decode((string) $response->getBody(), true); diff --git a/src/FetchAuthTokenCache.php b/src/FetchAuthTokenCache.php index 47174a1b7..e99f71ae6 100644 --- a/src/FetchAuthTokenCache.php +++ b/src/FetchAuthTokenCache.php @@ -145,9 +145,12 @@ public function signBlob($stringToSign, $forceOpenSsl = false) ); } - // Pass the access token from cache to GCECredentials for signing a blob. - // This saves a call to the metadata server when a cached token exists. - if ($this->fetcher instanceof Credentials\GCECredentials) { + // Pass the access token from cache for credentials that sign blobs + // using the IAM API. This saves a call to fetch an access token when a + // cached token exists. + if ($this->fetcher instanceof Credentials\GCECredentials + || $this->fetcher instanceof Credentials\ImpersonatedServiceAccountCredentials + ) { $cached = $this->fetchAuthTokenFromCache(); $accessToken = $cached['access_token'] ?? null; return $this->fetcher->signBlob($stringToSign, $forceOpenSsl, $accessToken); @@ -188,6 +191,15 @@ public function getProjectId(callable $httpHandler = null) ); } + // Pass the access token from cache for credentials that reqiore an + // access token to fetch the project ID. This saves a call to fetch am + // access token when a cached token exists. + if ($this->fetcher instanceof Credentials\ExternalAccountCredentials) { + $cached = $this->fetchAuthTokenFromCache(); + $accessToken = $cached['access_token'] ?? null; + return $this->fetcher->getProjectId($httpHandler, $accessToken); + } + return $this->fetcher->getProjectId($httpHandler); } diff --git a/tests/Credentials/ExternalAccountCredentialsTest.php b/tests/Credentials/ExternalAccountCredentialsTest.php index 39fe46045..139e68bd7 100644 --- a/tests/Credentials/ExternalAccountCredentialsTest.php +++ b/tests/Credentials/ExternalAccountCredentialsTest.php @@ -21,9 +21,11 @@ use Google\Auth\CredentialSource\AwsNativeSource; use Google\Auth\CredentialSource\FileSource; use Google\Auth\CredentialSource\UrlSource; +use Google\Auth\FetchAuthTokenCache; use Google\Auth\OAuth2; use InvalidArgumentException; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; @@ -36,6 +38,12 @@ class ExternalAccountCredentialsTest extends TestCase { use ProphecyTrait; + private $baseCreds = [ + 'type' => 'external_account', + 'token_url' => 'token-url.com', + 'audience' => '', + 'subject_token_type' => '', + ]; /** * @dataProvider provideCredentialSourceFromCredentials @@ -45,11 +53,7 @@ public function testCredentialSourceFromCredentials( string $expectedSourceClass, array $expectedProperties = [] ) { - $jsonCreds = [ - 'type' => 'external_account', - 'token_url' => '', - 'audience' => '', - 'subject_token_type' => '', + $jsonCreds = $this->baseCreds + [ 'credential_source' => $credentialSource, ]; @@ -198,11 +202,7 @@ public function testFetchAuthTokenFileCredentials() $tmpFile = tempnam(sys_get_temp_dir(), 'test'); file_put_contents($tmpFile, 'abc'); - $jsonCreds = [ - 'type' => 'external_account', - 'token_url' => 'token-url.com', - 'audience' => '', - 'subject_token_type' => '', + $jsonCreds = $this->baseCreds + [ 'credential_source' => ['file' => $tmpFile], ]; @@ -230,11 +230,7 @@ public function testFetchAuthTokenFileCredentials() public function testFetchAuthTokenUrlCredentials() { - $jsonCreds = [ - 'type' => 'external_account', - 'token_url' => 'token-url.com', - 'audience' => '', - 'subject_token_type' => '', + $jsonCreds = $this->baseCreds + [ 'credential_source' => ['url' => 'sts-url.com'], ]; @@ -278,11 +274,7 @@ public function testFetchAuthTokenWithImpersonation() $tmpFile = tempnam(sys_get_temp_dir(), 'test'); file_put_contents($tmpFile, 'abc'); - $jsonCreds = [ - 'type' => 'external_account', - 'token_url' => 'token-url.com', - 'audience' => '', - 'subject_token_type' => '', + $jsonCreds = $this->baseCreds + [ 'credential_source' => ['file' => $tmpFile], 'service_account_impersonation_url' => 'service-account-impersonation-url.com', ]; @@ -325,11 +317,7 @@ public function testFetchAuthTokenWithImpersonation() public function testGetQuotaProject() { - $jsonCreds = [ - 'type' => 'external_account', - 'token_url' => 'token-url.com', - 'audience' => '', - 'subject_token_type' => '', + $jsonCreds = $this->baseCreds + [ 'credential_source' => ['url' => 'sts-url.com'], 'quota_project_id' => 'test_quota_project', ]; @@ -337,4 +325,125 @@ public function testGetQuotaProject() $creds = new ExternalAccountCredentials('a-scope', $jsonCreds); $this->assertEquals('test_quota_project', $creds->getQuotaProject()); } + + /** + * @dataProvider provideGetProjectId + */ + public function testGetProjectId(array $jsonCreds, string $expectedProjectNumber) + { + $requestCount = 0; + $httpHandler = function (RequestInterface $request) use (&$requestCount, $expectedProjectNumber) { + switch (++$requestCount) { + case 1: + $this->assertEquals('sts-url.com', (string) $request->getUri()); + $responseBody = 'abc'; + break; + case 2: + $this->assertEquals('token-url.com', (string) $request->getUri()); + $responseBody = '{"access_token": "def"}'; + break; + case 3: + $this->assertEquals( + 'https://cloudresourcemanager.googleapis.com/v1/projects/' . $expectedProjectNumber, + (string) $request->getUri() + ); + $responseBody = json_encode(['projectId' => 'test-project-id']); + break; + } + + $body = $this->prophesize(StreamInterface::class); + $body->__toString()->willReturn($responseBody); + + $response = $this->prophesize(ResponseInterface::class); + $response->getBody()->willReturn($body->reveal()); + $response->hasHeader('Content-Type')->willReturn(false); + + return $response->reveal(); + }; + + $creds = new ExternalAccountCredentials('a-scope', $jsonCreds); + $this->assertEquals('test-project-id', $creds->getProjectId($httpHandler)); + } + + public function provideGetProjectId() + { + return [ + // from audience + [ + [ + 'audience' => 'https://foo.com/projects/1234/locations/global/workloadIdentityPools/foo/providers/bar', + ] + $this->baseCreds + [ + 'credential_source' => ['url' => 'sts-url.com'], + ], + '1234' + ], + // from workforce_pool_user_project + [ + $this->baseCreds + [ + 'credential_source' => ['url' => 'sts-url.com'], + 'workforce_pool_user_project' => '4567', + ], + '4567' + ], + // when both are available, use the audience + [ + [ + 'audience' => 'https://foo.com/projects/1234/locations/global/workloadIdentityPools/foo/providers/bar', + ] + $this->baseCreds + [ + 'credential_source' => ['url' => 'sts-url.com'], + 'workforce_pool_user_project' => '4567', + ], + '1234' + ], + ]; + } + + public function testCacheIsCalledForGetProjectIdWithCache() + { + $jsonCreds = [ + 'audience' => 'https://foo.com/projects/1234/locations/global/workloadIdentityPools/foo/providers/bar', + ] + $this->baseCreds + [ + 'credential_source' => ['url' => 'sts-url.com'], + ]; + + $httpHandler = function (RequestInterface $request) { + $this->assertEquals( + 'https://cloudresourcemanager.googleapis.com/v1/projects/1234', + (string) $request->getUri() + ); + $this->assertEquals('Bearer some-token', $request->getHeaderLine('authorization')); + $body = $this->prophesize(StreamInterface::class); + $body->__toString()->willReturn(json_encode(['projectId' => 'test-project-id'])); + + $response = $this->prophesize(ResponseInterface::class); + $response->getBody()->willReturn($body->reveal()); + $response->hasHeader('Content-Type')->willReturn(false); + + return $response->reveal(); + }; + + $mockCacheItem = $this->prophesize('Psr\Cache\CacheItemInterface'); + $mockCacheItem->isHit() + ->shouldBeCalledTimes(1) + ->willReturn(true); + $mockCacheItem->get() + ->shouldBeCalledTimes(1) + ->willReturn(['access_token' => 'some-token']); + $mockCache = $this->prophesize('Psr\Cache\CacheItemPoolInterface'); + $mockCache->getItem(Argument::any()) + ->shouldBeCalledTimes(1) + ->willReturn($mockCacheItem->reveal()); + + // Run the test + $creds = new ExternalAccountCredentials('a-scope', $jsonCreds); + + // Verify the cache passed to the wrapping Fetcher is never called + $cachedFetcher = new FetchAuthTokenCache( + $creds, + [], + $mockCache->reveal() + ); + + $this->assertEquals('test-project-id', $cachedFetcher->getProjectId($httpHandler)); + } } diff --git a/tests/FetchAuthTokenCacheTest.php b/tests/FetchAuthTokenCacheTest.php index 2defbb877..f59c9295a 100644 --- a/tests/FetchAuthTokenCacheTest.php +++ b/tests/FetchAuthTokenCacheTest.php @@ -615,76 +615,4 @@ public function testGetFetcher() $this->assertSame($mockFetcher, $fetcher->getFetcher()); } - - public function testCacheIsOnlyCalledOnceWhenWrappedInMultipleFetchers() - { - $prefix = 'test_prefix_'; - $cacheKey = 'myKey'; - $token = '2/abcdef1234567890'; - $cachedValue = ['access_token' => $token]; - $this->mockCacheItem->isHit() - ->shouldBeCalledTimes(1) - ->willReturn(true); - $this->mockCacheItem->get() - ->shouldBeCalledTimes(1) - ->willReturn($cachedValue); - $this->mockCache->getItem($prefix . $cacheKey) - ->shouldBeCalledTimes(1) - ->willReturn($this->mockCacheItem->reveal()); - $this->mockFetcher->fetchAuthToken() - ->shouldNotBeCalled(); - $this->mockFetcher->getCacheKey() - ->shouldBeCalled() - ->willReturn($cacheKey); - - // Run the test - $cachedFetcher = new FetchAuthTokenCache( - $this->mockFetcher->reveal(), - ['prefix' => $prefix], - $this->mockCache->reveal() - ); - // Verify the cache passed to the wrapping Fetcher is never called - $mockCache2 = $this->prophesize('Psr\Cache\CacheItemPoolInterface'); - $mockCache2->getItem(Argument::any()) - ->shouldNotBeCalled(); - $cachedFetcher2 = new FetchAuthTokenCache( - $cachedFetcher, - ['prefix' => $prefix], - $mockCache2->reveal() - ); - $accessToken = $cachedFetcher2->fetchAuthToken(); - $this->assertEquals($accessToken, ['access_token' => $token]); - } - - public function testCacheIsCalledWhenWrappedInFetcherWithoutCache() - { - $prefix = 'test_prefix_'; - $cacheKey = 'myKey'; - $token = '2/abcdef1234567890'; - $cachedValue = ['access_token' => $token]; - $this->mockCacheItem->isHit() - ->shouldBeCalledTimes(1) - ->willReturn(true); - $this->mockCacheItem->get() - ->shouldBeCalledTimes(1) - ->willReturn($cachedValue); - $this->mockCache->getItem($prefix . $cacheKey) - ->shouldBeCalledTimes(1) - ->willReturn($this->mockCacheItem->reveal()); - - // Create a Fetcher without a cache - $authTokenCache = $this->prophesize('Google\Auth\Credentials\ExternalAccountCredentials'); - $authTokenCache->getCacheKey() - ->shouldBeCalled() - ->willReturn($cacheKey); - - // Verify the cache passed to the wrapping Fetcher is called instead - $cachedFetcher = new FetchAuthTokenCache( - $authTokenCache->reveal(), - ['prefix' => $prefix], - $this->mockCache->reveal() - ); - $accessToken = $cachedFetcher->fetchAuthToken(); - $this->assertEquals($accessToken, ['access_token' => $token]); - } }