From a35c4dbb52e01faedacd09d23634939ced4a8a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Mendoza?= Date: Wed, 10 Jul 2024 10:49:03 -0400 Subject: [PATCH] feat: Change getCacheKey implementation for more unique keys (#560) --- src/CredentialSource/AwsNativeSource.php | 15 +++++ src/CredentialSource/ExecutableSource.php | 12 ++++ src/CredentialSource/FileSource.php | 12 ++++ src/CredentialSource/UrlSource.php | 12 ++++ .../ExternalAccountCredentials.php | 26 +++++++-- src/Credentials/GCECredentials.php | 6 +- .../ImpersonatedServiceAccountCredentials.php | 3 + src/Credentials/ServiceAccountCredentials.php | 14 ++++- .../ServiceAccountJwtAccessCredentials.php | 12 +++- src/Credentials/UserRefreshCredentials.php | 12 +++- ...ternalAccountCredentialSourceInterface.php | 1 + src/OAuth2.php | 22 +++++++ .../ExternalAccountCredentialsTest.php | 57 +++++++++++++++++++ .../ServiceAccountCredentialsTest.php | 6 +- ...ServiceAccountJwtAccessCredentialsTest.php | 6 +- .../UserRefreshCredentialsTest.php | 2 +- 16 files changed, 202 insertions(+), 16 deletions(-) diff --git a/src/CredentialSource/AwsNativeSource.php b/src/CredentialSource/AwsNativeSource.php index 460d9e5ea..6d9244ba2 100644 --- a/src/CredentialSource/AwsNativeSource.php +++ b/src/CredentialSource/AwsNativeSource.php @@ -328,6 +328,21 @@ public static function getSigningVarsFromEnv(): ?array return null; } + /** + * Gets the unique key for caching + * For AwsNativeSource the values are: + * Imdsv2SessionTokenUrl.SecurityCredentialsUrl.RegionUrl.RegionalCredVerificationUrl + * + * @return string + */ + public function getCacheKey(): string + { + return ($this->imdsv2SessionTokenUrl ?? '') . + '.' . ($this->securityCredentialsUrl ?? '') . + '.' . $this->regionUrl . + '.' . $this->regionalCredVerificationUrl; + } + /** * Return HMAC hash in binary string */ diff --git a/src/CredentialSource/ExecutableSource.php b/src/CredentialSource/ExecutableSource.php index 7661fc9cc..ce3bd9fda 100644 --- a/src/CredentialSource/ExecutableSource.php +++ b/src/CredentialSource/ExecutableSource.php @@ -100,6 +100,18 @@ public function __construct( $this->executableHandler = $executableHandler ?: new ExecutableHandler(); } + /** + * Gets the unique key for caching + * The format for the cache key is: + * Command.OutputFile + * + * @return ?string + */ + public function getCacheKey(): ?string + { + return $this->command . '.' . $this->outputFile; + } + /** * @param callable $httpHandler unused. * @return string diff --git a/src/CredentialSource/FileSource.php b/src/CredentialSource/FileSource.php index e2afc6c58..00ac835a8 100644 --- a/src/CredentialSource/FileSource.php +++ b/src/CredentialSource/FileSource.php @@ -72,4 +72,16 @@ public function fetchSubjectToken(callable $httpHandler = null): string return $contents; } + + /** + * Gets the unique key for caching. + * The format for the cache key one of the following: + * Filename + * + * @return string + */ + public function getCacheKey(): ?string + { + return $this->file; + } } diff --git a/src/CredentialSource/UrlSource.php b/src/CredentialSource/UrlSource.php index 0acb3c6ef..6046d52fa 100644 --- a/src/CredentialSource/UrlSource.php +++ b/src/CredentialSource/UrlSource.php @@ -94,4 +94,16 @@ public function fetchSubjectToken(callable $httpHandler = null): string return $body; } + + /** + * Get the cache key for the credentials. + * The format for the cache key is: + * URL + * + * @return ?string + */ + public function getCacheKey(): ?string + { + return $this->url; + } } diff --git a/src/Credentials/ExternalAccountCredentials.php b/src/Credentials/ExternalAccountCredentials.php index 98f427a33..3614d24d0 100644 --- a/src/Credentials/ExternalAccountCredentials.php +++ b/src/Credentials/ExternalAccountCredentials.php @@ -98,9 +98,7 @@ public function __construct( ); } - if (array_key_exists('service_account_impersonation_url', $jsonKey)) { - $this->serviceAccountImpersonationUrl = $jsonKey['service_account_impersonation_url']; - } + $this->serviceAccountImpersonationUrl = $jsonKey['service_account_impersonation_url'] ?? null; $this->quotaProject = $jsonKey['quota_project_id'] ?? null; $this->workforcePoolUserProject = $jsonKey['workforce_pool_user_project'] ?? null; @@ -276,9 +274,27 @@ public function fetchAuthToken(callable $httpHandler = null) return $stsToken; } - public function getCacheKey() + /** + * Get the cache token key for the credentials. + * The cache token key format depends on the type of source + * The format for the cache key one of the following: + * FetcherCacheKey.Scope.[ServiceAccount].[TokenType].[WorkforcePoolUserProject] + * FetcherCacheKey.Audience.[ServiceAccount].[TokenType].[WorkforcePoolUserProject] + * + * @return ?string; + */ + public function getCacheKey(): ?string { - return $this->auth->getCacheKey(); + $scopeOrAudience = $this->auth->getAudience(); + if (!$scopeOrAudience) { + $scopeOrAudience = $this->auth->getScope(); + } + + return $this->auth->getSubjectTokenFetcher()->getCacheKey() . + '.' . $scopeOrAudience . + '.' . ($this->serviceAccountImpersonationUrl ?? '') . + '.' . ($this->auth->getSubjectTokenType() ?? '') . + '.' . ($this->workforcePoolUserProject ?? ''); } public function getLastReceivedToken() diff --git a/src/Credentials/GCECredentials.php b/src/Credentials/GCECredentials.php index 5fed54763..8b7547816 100644 --- a/src/Credentials/GCECredentials.php +++ b/src/Credentials/GCECredentials.php @@ -489,11 +489,15 @@ public function fetchAuthToken(callable $httpHandler = null) } /** + * Returns the Cache Key for the credential token. + * The format for the cache key is: + * TokenURI + * * @return string */ public function getCacheKey() { - return self::cacheKey; + return $this->tokenUri; } /** diff --git a/src/Credentials/ImpersonatedServiceAccountCredentials.php b/src/Credentials/ImpersonatedServiceAccountCredentials.php index 791fe985a..5d3522827 100644 --- a/src/Credentials/ImpersonatedServiceAccountCredentials.php +++ b/src/Credentials/ImpersonatedServiceAccountCredentials.php @@ -131,6 +131,9 @@ public function fetchAuthToken(callable $httpHandler = null) } /** + * Returns the Cache Key for the credentials + * The cache key is the same as the UserRefreshCredentials class + * * @return string */ public function getCacheKey() diff --git a/src/Credentials/ServiceAccountCredentials.php b/src/Credentials/ServiceAccountCredentials.php index 91238029d..4090b8931 100644 --- a/src/Credentials/ServiceAccountCredentials.php +++ b/src/Credentials/ServiceAccountCredentials.php @@ -219,13 +219,23 @@ public function fetchAuthToken(callable $httpHandler = null) } /** + * Return the Cache Key for the credentials. + * For the cache key format is one of the following: + * ClientEmail.Scope[.Sub] + * ClientEmail.Audience[.Sub] + * * @return string */ public function getCacheKey() { - $key = $this->auth->getIssuer() . ':' . $this->auth->getCacheKey(); + $scopeOrAudience = $this->auth->getScope(); + if (!$scopeOrAudience) { + $scopeOrAudience = $this->auth->getAudience(); + } + + $key = $this->auth->getIssuer() . '.' . $scopeOrAudience; if ($sub = $this->auth->getSub()) { - $key .= ':' . $sub; + $key .= '.' . $sub; } return $key; diff --git a/src/Credentials/ServiceAccountJwtAccessCredentials.php b/src/Credentials/ServiceAccountJwtAccessCredentials.php index 87baa7500..6c582a830 100644 --- a/src/Credentials/ServiceAccountJwtAccessCredentials.php +++ b/src/Credentials/ServiceAccountJwtAccessCredentials.php @@ -166,11 +166,21 @@ public function fetchAuthToken(callable $httpHandler = null) } /** + * Return the cache key for the credentials. + * The format for the Cache Key one of the following: + * ClientEmail.Scope + * ClientEmail.Audience + * * @return string */ public function getCacheKey() { - return $this->auth->getCacheKey(); + $scopeOrAudience = $this->auth->getScope(); + if (!$scopeOrAudience) { + $scopeOrAudience = $this->auth->getAudience(); + } + + return $this->auth->getIssuer() . '.' . $scopeOrAudience; } /** diff --git a/src/Credentials/UserRefreshCredentials.php b/src/Credentials/UserRefreshCredentials.php index 69778f7c8..d40055562 100644 --- a/src/Credentials/UserRefreshCredentials.php +++ b/src/Credentials/UserRefreshCredentials.php @@ -130,11 +130,21 @@ public function fetchAuthToken(callable $httpHandler = null, array $metricsHeade } /** + * Return the Cache Key for the credentials. + * The format for the Cache key is one of the following: + * ClientId.Scope + * ClientId.Audience + * * @return string */ public function getCacheKey() { - return $this->auth->getClientId() . ':' . $this->auth->getCacheKey(); + $scopeOrAudience = $this->auth->getScope(); + if (!$scopeOrAudience) { + $scopeOrAudience = $this->auth->getAudience(); + } + + return $this->auth->getClientId() . '.' . $scopeOrAudience; } /** diff --git a/src/ExternalAccountCredentialSourceInterface.php b/src/ExternalAccountCredentialSourceInterface.php index b4d00f8b4..041b18d51 100644 --- a/src/ExternalAccountCredentialSourceInterface.php +++ b/src/ExternalAccountCredentialSourceInterface.php @@ -20,4 +20,5 @@ interface ExternalAccountCredentialSourceInterface { public function fetchSubjectToken(callable $httpHandler = null): string; + public function getCacheKey(): ?string; } diff --git a/src/OAuth2.php b/src/OAuth2.php index b1f9ae26d..4019e258a 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -683,6 +683,8 @@ public function fetchAuthToken(callable $httpHandler = null, $headers = []) } /** + * @deprecated + * * Obtains a key that can used to cache the results of #fetchAuthToken. * * The key is derived from the scopes. @@ -703,6 +705,16 @@ public function getCacheKey() return null; } + /** + * Gets this instance's SubjectTokenFetcher + * + * @return null|ExternalAccountCredentialSourceInterface + */ + public function getSubjectTokenFetcher(): ?ExternalAccountCredentialSourceInterface + { + return $this->subjectTokenFetcher; + } + /** * Parses the fetched tokens. * @@ -1020,6 +1032,16 @@ public function getScope() return implode(' ', $this->scope); } + /** + * Gets the subject token type + * + * @return ?string + */ + public function getSubjectTokenType(): ?string + { + return $this->subjectTokenType; + } + /** * Sets the scope of the access request, expressed either as an Array or as * a space-delimited String. diff --git a/tests/Credentials/ExternalAccountCredentialsTest.php b/tests/Credentials/ExternalAccountCredentialsTest.php index c658054ec..09cac05db 100644 --- a/tests/Credentials/ExternalAccountCredentialsTest.php +++ b/tests/Credentials/ExternalAccountCredentialsTest.php @@ -521,6 +521,63 @@ public function testFetchAuthTokenWithWorkforcePoolCredentials() $this->assertEquals(strtotime($expiry), $authToken['expires_at']); } + public function testFileSourceCacheKey() + { + $this->baseCreds['credential_source'] = ['file' => 'fakeFile']; + $credentials = new ExternalAccountCredentials('scope1', $this->baseCreds); + $cacheKey = $credentials->getCacheKey(); + $expectedKey = 'fakeFile.scope1...'; + $this->assertEquals($expectedKey, $cacheKey); + } + + public function testAWSSourceCacheKey() + { + $this->baseCreds['credential_source'] = [ + 'environment_id' => 'aws1', + 'regional_cred_verification_url' => 'us-east', + 'region_url' => 'aws.us-east.com', + 'url' => 'aws.us-east.token.com', + 'imdsv2_session_token_url' => '12345' + ]; + $this->baseCreds['audience'] = 'audience1'; + $credentials = new ExternalAccountCredentials('scope1', $this->baseCreds); + $cacheKey = $credentials->getCacheKey(); + $expectedKey = '12345.aws.us-east.token.com.aws.us-east.com.us-east.audience1...'; + $this->assertEquals($expectedKey, $cacheKey); + } + + public function testUrlSourceCacheKey() + { + $this->baseCreds['credential_source'] = [ + 'url' => 'fakeUrl', + 'format' => [ + 'type' => 'json', + 'subject_token_field_name' => 'keyShouldBeHere' + ] + ]; + + $credentials = new ExternalAccountCredentials('scope1', $this->baseCreds); + $cacheKey = $credentials->getCacheKey(); + $expectedKey = 'fakeUrl.scope1...'; + $this->assertEquals($expectedKey, $cacheKey); + } + + public function testExecutableSourceCacheKey() + { + $this->baseCreds['credential_source'] = [ + 'executable' => [ + 'command' => 'ls -al', + 'output_file' => './output.txt' + ] + ]; + + $credentials = new ExternalAccountCredentials('scope1', $this->baseCreds); + $cacheKey = $credentials->getCacheKey(); + + $expectedCacheKey = 'ls -al../output.txt.scope1...'; + $this->assertEquals($cacheKey, $expectedCacheKey); + } + /** * @runInSeparateProcess */ diff --git a/tests/Credentials/ServiceAccountCredentialsTest.php b/tests/Credentials/ServiceAccountCredentialsTest.php index a53f55158..818f543ef 100644 --- a/tests/Credentials/ServiceAccountCredentialsTest.php +++ b/tests/Credentials/ServiceAccountCredentialsTest.php @@ -54,7 +54,7 @@ public function testShouldBeTheSameAsOAuth2WithTheSameScope() ); $o = new OAuth2(['scope' => $scope]); $this->assertSame( - $testJson['client_email'] . ':' . $o->getCacheKey(), + $testJson['client_email'] . '.' . implode(' ', $scope), $sa->getCacheKey() ); } @@ -71,7 +71,7 @@ public function testShouldBeTheSameAsOAuth2WithTheSameScopeWithSub() ); $o = new OAuth2(['scope' => $scope]); $this->assertSame( - $testJson['client_email'] . ':' . $o->getCacheKey() . ':' . $sub, + $testJson['client_email'] . '.' . implode(' ', $scope) . '.' . $sub, $sa->getCacheKey() ); } @@ -90,7 +90,7 @@ public function testShouldBeTheSameAsOAuth2WithTheSameScopeWithSubAddedLater() $o = new OAuth2(['scope' => $scope]); $this->assertSame( - $testJson['client_email'] . ':' . $o->getCacheKey() . ':' . $sub, + $testJson['client_email'] . '.' . implode(' ', $scope) . '.' . $sub, $sa->getCacheKey() ); } diff --git a/tests/Credentials/ServiceAccountJwtAccessCredentialsTest.php b/tests/Credentials/ServiceAccountJwtAccessCredentialsTest.php index 510225dd7..2cac3dac1 100644 --- a/tests/Credentials/ServiceAccountJwtAccessCredentialsTest.php +++ b/tests/Credentials/ServiceAccountJwtAccessCredentialsTest.php @@ -480,8 +480,10 @@ public function testShouldBeTheSameAsOAuth2WithTheSameScope() { $testJson = $this->createTestJson(); $scope = ['scope/1', 'scope/2']; - $sa = new ServiceAccountJwtAccessCredentials($testJson); - $this->assertNull($sa->getCacheKey()); + $sa = new ServiceAccountJwtAccessCredentials($testJson, $scope); + + $expectedKey = $testJson['client_email'] . '.' . implode(' ', $scope); + $this->assertEquals($expectedKey, $sa->getCacheKey()); } public function testReturnsClientEmail() diff --git a/tests/Credentials/UserRefreshCredentialsTest.php b/tests/Credentials/UserRefreshCredentialsTest.php index 420790a6f..b944dd40e 100644 --- a/tests/Credentials/UserRefreshCredentialsTest.php +++ b/tests/Credentials/UserRefreshCredentialsTest.php @@ -50,7 +50,7 @@ public function testShouldBeTheSameAsOAuth2WithTheSameScope() ); $o = new OAuth2(['scope' => $scope]); $this->assertSame( - $testJson['client_id'] . ':' . $o->getCacheKey(), + $testJson['client_id'] . '.' . implode(' ', $scope), $sa->getCacheKey() ); }