feat: Change getCacheKey implementation for more unique keys (#560)

googleapis · Jul 10, 2024 · a35c4db · a35c4db
1 parent 9ebf46e
commit a35c4db
Show file tree

Hide file tree

Showing 16 changed files with 202 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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()
         );
     }