fix(amplify-provider-awscloudformation): mfa role assumption fails with cached credentials#14627
Open
danrivett wants to merge 2 commits intoaws-amplify:devfrom
Open
Conversation
…th cached credentials (aws-amplify#14626) Fixes three related bugs in credential caching for MFA-based role assumption introduced in commit 04f7bcf (PR aws-amplify#14315). Bug 1 - MFA prompt never appears: getCachedRoleCredentials() always returned { credentials: {} } even with no valid cache, so the STS AssumeRole call was never executed. Fix: return undefined when no valid cached credentials exist. Bug 2 - Cache validation always fails: credentials were cached in nested format { credentials: { accessKeyId, ... } } but validateCachedCredentials() expected flat format. Fix: cache the flat credentials object (roleCredentials.credentials). Bug 3 - expiration.getTime error: cached Date is deserialized as a string, but the AWS SDK calls expiration.getTime(). The fix in PR aws-amplify#14315 only addressed this in getConfiguredAWSClientConfig(), not in getProfiledAwsConfig(). Fix: convert expiration to Date when returning cached credentials.
Add tests for the credential caching logic used in MFA role assumption. These tests verify the bug fixes made to address three issues: 1. getCachedRoleCredentials returns undefined when no cache exists - Test: should return undefined when cache file does not exist - Test: should return undefined when roleArn not in cache 2. getCachedRoleCredentials returns undefined for expired credentials - Test: should return undefined when cached credentials are expired 3. Cached credentials use flat format and expiration is converted to Date - Test: should return valid cached credentials with Date expiration - Test: should cache credentials in flat format (not nested) The tests verify that: - STS is called when no valid cache exists - STS is NOT called when valid cached credentials exist - Cached expiration string is converted to Date object on read - Credentials are cached in flat format (accessKeyId at top level, not nested under credentials property)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes three cascading bugs in the MFA role assumption credential caching logic introduced by commit
04f7bcfc24(PR #14315, fixing #14290). These bugs cause MFA-based role assumption to fail withResolved credential object is not valid.Bug 1 — MFA prompt never appears:
getCachedRoleCredentials()always returned{ credentials: {} }even when no valid cached credentials existed, so the STSAssumeRolecall was never executed.Bug 2 — Cache validation always fails: Credentials were cached in nested format
{ credentials: { accessKeyId, ... } }butvalidateCachedCredentials()expected flat format{ accessKeyId, ... }.Bug 3 —
expiration.getTime is not a function: CachedDateis deserialized as a string from JSON, but the AWS SDK callsexpiration.getTime(). PR #14315 only fixed this ingetConfiguredAWSClientConfig(), not ingetProfiledAwsConfig().Issue
Closes #14626
How did you test these changes?
getCachedRoleCredentialsandcacheRoleCredentialscovering:undefinedwhen cache file does not existundefinedwhenroleArnnot in cacheundefinedwhen cached credentials are expiredexpirationconverted toDaterole_arn+source_profile+mfa_serialChecklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.