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

[SRA] Invalidate credentials caching when environment variables change #3590

Open
wants to merge 2 commits into
base: sra-identity-auth
Choose a base branch
from

Conversation

dscpinheiro
Copy link
Contributor

Description

This PR makes two changes to the DefaultAWSCredentialsIdentityResolver (which is the replacement for the FallbackCredentialsFactory in V4):

  • If there are changes to environment variables used by our credentials provider (such as AWS_PROFILE), do not return cached credentials
  • If a custom / non-default profile is specified and the profile doesn't exist, throw an error instead of going to the next provider in the credential chain (which at the moment is IMDS). With the previous behavior, customers running outside of EC2 would get a different error (Unable to get IAM security credentials from EC2 Instance Metadata Service) that wouldn't reference the invalid profile at all.

Motivation and Context

Internal issue: DOTNET-7855

Testing

  • Dry-run: DRY_RUN-cf127d60-49ba-48a7-b0e9-04eecaa186c3
  • Console app:
var s3Client = new AmazonS3Client();
var response1 = await s3Client.ListBucketsAsync();
Console.WriteLine($"Found {response1.Buckets.Count} buckets for default profile");

Environment.SetEnvironmentVariable("AWS_PROFILE", "test-runner");
var response2 = await s3Client.ListBucketsAsync();
Console.WriteLine($"Found {response2.Buckets.Count} buckets for test-runner profile");

try
{
    Environment.SetEnvironmentVariable("AWS_PROFILE", "non-existent");
    await s3Client.ListBucketsAsync();
}
catch (AmazonClientException)
{
    Console.WriteLine("Failed as expected locally since non-existent profile will cause the credential search to be short-circuited.");
}

Output:

Found 30 buckets for default profile
Found 16 buckets for test-runner profile
Failed as expected locally since non-existent profile will cause the credential search to be short-circuited.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • My code follows the code style of this project
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

() => new EnvironmentVariablesAWSCredentials(), // Look for credentials set in environment vars.
() => AssumeRoleWithWebIdentityCredentials.FromEnvironmentVariables(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Important: This change is intentional! The search order now matches the credentials-provider-chain-search-precedence SEP.

The order defined in the spec is: Environment variables: credentials -> Environment variables: web identity token / STS -> Profile: credentials.

@@ -93,7 +93,6 @@ public void StaticStabilityWhenIMDSExperiencesAnOutageScenarioTest()
Expiration = currentTime.AddHours(6)
};

using (new AWSConfigsDateFaker(() => currentTime.ToUniversalTime()))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line was causing the test to fail locally for me. currentTime is already UTC after the DateTime changes that were recently merged into v4-development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants