Skip to content

Commit 9d5f0c8

Browse files
chore: address PR feedback and include tests
Addresses feedback for conditionally calling the method resolvingAccountId based on if the parameter is present in the rulesset of the endpoint provider. Include tests coverage for AssumeRoleCredentialProvider, AssumeRoleWithWebIdentityCredentialProvider, process, shared files, and credentials from environment.
1 parent afa8033 commit 9d5f0c8

13 files changed

+86
-42
lines changed

src/AwsClient.php

+1-2
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ private function setClientContextParams($args)
572572
private function setClientBuiltIns($args, $resolvedConfig)
573573
{
574574
$builtIns = [];
575-
$config = $this->getConfig();
575+
$config = $resolvedConfig['config'];
576576
$service = $args['service'];
577577

578578
$builtIns['SDK::Endpoint'] = null;
@@ -593,7 +593,6 @@ private function setClientBuiltIns($args, $resolvedConfig)
593593
$builtIns['AWS::S3::ForcePathStyle'] = $config['use_path_style_endpoint'];
594594
$builtIns['AWS::S3::DisableMultiRegionAccessPoints'] = $config['disable_multiregion_access_points'];
595595
}
596-
$builtIns['AWS::Auth::AccountId'] = null;
597596
$builtIns['AWS::Auth::AccountIdEndpointMode'] = $resolvedConfig['account_id_endpoint_mode'];
598597

599598
$this->clientBuiltIns += $builtIns;

src/ClientResolver.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ class ClientResolver
293293
'account_id_endpoint_mode' => [
294294
'type' => 'value',
295295
'valid' => ['string'],
296-
'doc' => 'To decide whether account_id must a be a required resolved credentials parameter. If this configuration is set to disabled, then account_id is not required. If set to preferred a warning will be logged when account_id is not resolved, and when set to required an exception will be thrown if account_id is not resolved.',
296+
'doc' => 'Decides whether account_id must a be a required resolved credentials property. If this configuration is set to disabled, then account_id is not required. If set to preferred a warning will be logged when account_id is not resolved, and when set to required an exception will be thrown if account_id is not resolved.',
297297
'default' => [__CLASS__, '_default_account_id_endpoint_mode'],
298298
'fn' => [__CLASS__, '_apply_account_id_endpoint_mode']
299299
],

src/Credentials/CredentialProvider.php

+4-11
Original file line numberDiff line numberDiff line change
@@ -292,17 +292,15 @@ public static function env()
292292
// Use credentials from environment variables, if available
293293
$key = getenv(self::ENV_KEY);
294294
$secret = getenv(self::ENV_SECRET);
295-
$accountId = getenv(self::ENV_ACCOUNT_ID);
296-
if (empty($accountId)) {
297-
$accountId = null;
298-
}
295+
$accountId = getenv(self::ENV_ACCOUNT_ID) ?: null;
296+
$token = getenv(self::ENV_SESSION) ?: null;
299297

300298
if ($key && $secret) {
301299
return Promise\Create::promiseFor(
302300
new Credentials(
303301
$key,
304302
$secret,
305-
getenv(self::ENV_SESSION) ?: NULL,
303+
$token,
306304
null,
307305
$accountId
308306
)
@@ -549,18 +547,13 @@ public static function ini($profile = null, $filename = null, array $config = []
549547
: null;
550548
}
551549

552-
$accountId = null;
553-
if (!empty($data[$profile]['aws_account_id'])) {
554-
$accountId = $data[$profile]['aws_account_id'];
555-
}
556-
557550
return Promise\Create::promiseFor(
558551
new Credentials(
559552
$data[$profile]['aws_access_key_id'],
560553
$data[$profile]['aws_secret_access_key'],
561554
$data[$profile]['aws_session_token'],
562555
null,
563-
$accountId
556+
$data[$profile]['aws_account_id'] ?? null
564557
)
565558
);
566559
};

src/Credentials/EcsCredentialProvider.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public function __invoke()
7676
$result['SecretAccessKey'],
7777
$result['Token'],
7878
strtotime($result['Expiration']),
79-
$result['AccountId']
79+
$result['AccountId'] ?? null
8080
);
8181
})->otherwise(function ($reason) {
8282
$reason = is_array($reason) ? $reason['exception'] : $reason;

src/Credentials/InstanceProfileProvider.php

+1-6
Original file line numberDiff line numberDiff line change
@@ -216,17 +216,12 @@ public function __invoke($previousCredentials = null)
216216
if (!isset($result)) {
217217
$credentials = $previousCredentials;
218218
} else {
219-
$accountId = null;
220-
if (!empty($result['AccountId'])) {
221-
$accountId = $result['AccountId'];
222-
}
223-
224219
$credentials = new Credentials(
225220
$result['AccessKeyId'],
226221
$result['SecretAccessKey'],
227222
$result['Token'],
228223
strtotime($result['Expiration']),
229-
$accountId
224+
$result['AccountId'] ?? null
230225
);
231226
}
232227

src/EndpointV2/EndpointV2Middleware.php

+13-13
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
class EndpointV2Middleware
2121
{
2222
const ACCOUNT_ID_PARAM = 'AccountId';
23+
const ACCOUNT_ID_ENDPOINT_MODE_PARAM = 'AccountIdEndpointMode';
2324
private static $validAuthSchemes = [
2425
'sigv4' => true,
2526
'sigv4a' => true,
@@ -39,6 +40,7 @@ class EndpointV2Middleware
3940

4041
/** @var array */
4142
private $clientArgs;
43+
4244
/** @var Closure */
4345
private $identityProvider;
4446

@@ -75,7 +77,7 @@ public function __construct(
7577
EndpointProviderV2 $endpointProvider,
7678
Service $api,
7779
array $args,
78-
callable $identityProvider
80+
callable $identityProvider = null
7981
)
8082
{
8183
$this->nextHandler = $nextHandler;
@@ -121,7 +123,10 @@ private function resolveArgs(array $commandArgs, Operation $operation) : array
121123
{
122124
$rulesetParams = $this->endpointProvider->getRuleset()->getParameters();
123125

124-
$this->resolveAccountId($commandArgs, $rulesetParams);
126+
if (isset($rulesetParams[self::ACCOUNT_ID_PARAM])
127+
&& isset($rulesetParams[self::ACCOUNT_ID_ENDPOINT_MODE_PARAM])) {
128+
$this->resolveAccountId($commandArgs);
129+
}
125130

126131
$endpointCommandArgs = $this->filterEndpointCommandArgs(
127132
$rulesetParams,
@@ -323,32 +328,27 @@ private function normalizeAuthScheme(array $authScheme) : array
323328
* and we will inject the resolved identity into the $command['@context'] property, so it can be reused along the way.
324329
*
325330
* @param array $commandArgs
326-
* @param array $rulesetParams
327331
* @return void
328332
*/
329-
private function resolveAccountId(array &$commandArgs, array $rulesetParams)
333+
private function resolveAccountId(array &$commandArgs)
330334
{
331335
$accountIdEndpointMode = $this->clientArgs['AccountIdEndpointMode'];
332-
if (!isset($rulesetParams[self::ACCOUNT_ID_PARAM]) || $accountIdEndpointMode === 'disabled') {
333-
return;
334-
}
335-
336336
$identity = null;
337337
if (isset($commandArgs['@context']['resolved_identity'])) {
338-
$identity = $commandArgs['@context']['resolved_identity']->wait();
338+
$identity = $commandArgs['@context']['resolved_identity'];
339339
} else {
340340
$identityProviderFn = $this->identityProvider;
341341
$identity = $identityProviderFn()->wait();
342-
$commandArgs['@context']['resolved_identity'] = Create::promiseFor($identity);
342+
$commandArgs['@context']['resolved_identity'] = $identity;
343343
}
344344

345345
if (empty($identity->getAccountId())) {
346346
switch ($accountIdEndpointMode) {
347347
case 'preferred':
348-
trigger_error($this->getAccountIdNotResolvedErrorMessage($accountIdEndpointMode), E_USER_WARNING);
348+
trigger_error($this->getAccountIdErrorMessage($accountIdEndpointMode), E_USER_WARNING);
349349
break;
350350
case 'required':
351-
throw new AccountIdNotFoundException($this->getAccountIdNotResolvedErrorMessage($accountIdEndpointMode));
351+
throw new AccountIdNotFoundException($this->getAccountIdErrorMessage($accountIdEndpointMode));
352352
default:
353353
throw new \InvalidArgumentException('account_id_endpoint_mode value not supported: '. $accountIdEndpointMode);
354354
}
@@ -359,7 +359,7 @@ private function resolveAccountId(array &$commandArgs, array $rulesetParams)
359359
$commandArgs[self::ACCOUNT_ID_PARAM] = $identity->getAccountId();
360360
}
361361

362-
private function getAccountIdNotResolvedErrorMessage($mode) : string
362+
private function getAccountIdErrorMessage($mode) : string
363363
{
364364
return "Unable to resolve an `accountId` as part of the credentials identity when `account_id_endpoint_mode` is set to {$mode}. "
365365
. "\nTo remedy this you can:"

src/Middleware.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ function (TokenInterface $token)
145145
if ($signer instanceof S3ExpressSignature) {
146146
$credentialPromise = $config['s3_express_identity_provider']($command);
147147
} elseif (isset($command['@context']['resolved_identity'])) {
148-
$credentialPromise = $command['@context']['resolved_identity'];
148+
$credentialPromise = Promise\Create::promiseFor($command['@context']['resolved_identity']);
149149
} else {
150150
$credentialPromise = $credProvider();
151151
}

src/Sts/StsClient.php

+3
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ public function createCredentials(Result $result)
8080
if ($result->hasKey('AssumedRoleUser')) {
8181
$parsedArn = ArnParser::parse($result->get('AssumedRoleUser')['Arn']);
8282
$accountId = $parsedArn->getAccountId();
83+
} elseif ($result->hasKey('FederatedUser')) {
84+
$parsedArn = ArnParser::parse($result->get('FederatedUser')['Arn']);
85+
$accountId = $parsedArn->getAccountId();
8386
}
8487

8588
$credentials = $result['Credentials'];

tests/AwsClientTest.php

+2
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,7 @@ public function testGetClientBuiltins()
504504
'AWS::UseFIPS' => false,
505505
'AWS::UseDualStack' => false,
506506
'AWS::STS::UseGlobalEndpoint' => true,
507+
'AWS::Auth::AccountIdEndpointMode' => 'preferred',
507508
];
508509
$builtIns = $client->getClientBuiltIns();
509510
$this->assertEquals(
@@ -524,6 +525,7 @@ public function testGetEndpointProviderArgs()
524525
'UseFIPS' => false,
525526
'UseDualStack' => false,
526527
'UseGlobalEndpoint' => true,
528+
'AccountIdEndpointMode' => 'preferred'
527529
];
528530
$providerArgs = $client->getEndpointProviderArgs();
529531
$this->assertEquals(

tests/Credentials/AssumeRoleCredentialProviderTest.php

+3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?php
22
namespace Aws\Test\Credentials;
33

4+
use Aws\Arn\ArnParser;
45
use Aws\Credentials\AssumeRoleCredentialProvider;
56
use Aws\Credentials\Credentials;
67
use Aws\Exception\AwsException;
@@ -92,6 +93,8 @@ function ($c, $r) use ($result) {
9293
$this->assertNull($creds->getSecurityToken());
9394
$this->assertIsInt($creds->getExpiration());
9495
$this->assertFalse($creds->isExpired());
96+
$expectedAccountId = ArnParser::parse(self::SAMPLE_ROLE_ARN)->getAccountId();
97+
$this->assertSame($expectedAccountId, $creds->getAccountId());
9598
}
9699

97100
public function testThrowsExceptionWhenRetrievingAssumeRoleCredentialFails()

tests/Credentials/AssumeRoleWithWebIdentityCredentialProviderTest.php

+7
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?php
22
namespace Aws\Test\Credentials;
33

4+
use Aws\Arn\ArnParser;
45
use Aws\Command;
56
use Aws\Credentials\AssumeRoleWithWebIdentityCredentialProvider;
67
use Aws\Credentials\Credentials;
@@ -76,6 +77,10 @@ public function testCanLoadAssumeRoleWithWebIdentityCredentials()
7677
'SessionToken' => 'baz',
7778
'Expiration' => DateTimeResult::fromEpoch(time() + 10)
7879
],
80+
'AssumedRoleUser' => [
81+
'AssumedRoleId' => 'test_user_621903f1f21f5.01530789',
82+
'Arn' => self::SAMPLE_ROLE_ARN
83+
]
7984
];
8085

8186
$tokenPath = $dir . '/my-token.jwt';
@@ -102,6 +107,8 @@ function ($c, $r) use ($result) {
102107
$this->assertSame('baz', $creds->getSecurityToken());
103108
$this->assertIsInt($creds->getExpiration());
104109
$this->assertFalse($creds->isExpired());
110+
$expectedAccountId = ArnParser::parse(self::SAMPLE_ROLE_ARN)->getAccountId();
111+
$this->assertSame($expectedAccountId, $creds->getAccountId());
105112
} catch (\Error $e) {
106113
throw $e;
107114
} finally {

0 commit comments

Comments
 (0)