Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Change getCacheKey implementation for more unique keys #560

Merged
merged 17 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/CredentialSource/AwsNativeSource.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
bshaffer marked this conversation as resolved.
Show resolved Hide resolved
{
return ($this->imdsv2SessionTokenUrl ?? '') .
'.' . ($this->securityCredentialsUrl ?? '') .
'.' . $this->regionUrl .
'.' . $this->regionalCredVerificationUrl;
Copy link
Contributor

@bshaffer bshaffer Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just had a thought... one way you could get rid of the . without having a separate function is to do something like this:

return implode('.', array_filter([
    $this->imdsv2SessionTokenUrl,
    $this->securityCredentialsUrl,
    $this->regionUrl,
    $this->regionalCredVerificationUrl
]));

But I don't think that's necessary... what we have here looks LGTM to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brent, you a genius, I do like that better haha.

}

/**
* Return HMAC hash in binary string
*/
Expand Down
12 changes: 12 additions & 0 deletions src/CredentialSource/ExecutableSource.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions src/CredentialSource/FileSource.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
12 changes: 12 additions & 0 deletions src/CredentialSource/UrlSource.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
26 changes: 21 additions & 5 deletions src/Credentials/ExternalAccountCredentials.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 5 additions & 1 deletion src/Credentials/GCECredentials.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Hectorhammett marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
3 changes: 3 additions & 0 deletions src/Credentials/ImpersonatedServiceAccountCredentials.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
14 changes: 12 additions & 2 deletions src/Credentials/ServiceAccountCredentials.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Hectorhammett marked this conversation as resolved.
Show resolved Hide resolved
*
* @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;
Expand Down
12 changes: 11 additions & 1 deletion src/Credentials/ServiceAccountJwtAccessCredentials.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
12 changes: 11 additions & 1 deletion src/Credentials/UserRefreshCredentials.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/ExternalAccountCredentialSourceInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@
interface ExternalAccountCredentialSourceInterface
{
public function fetchSubjectToken(callable $httpHandler = null): string;
public function getCacheKey(): ?string;
}
22 changes: 22 additions & 0 deletions src/OAuth2.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
*
Expand Down Expand Up @@ -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.
Expand Down
57 changes: 57 additions & 0 deletions tests/Credentials/ExternalAccountCredentialsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
6 changes: 3 additions & 3 deletions tests/Credentials/ServiceAccountCredentialsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
}
Expand All @@ -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()
);
}
Expand All @@ -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()
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion tests/Credentials/UserRefreshCredentialsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
}
Expand Down