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

Cannot programatically assume role with web identity using config.WithWebIdentityRoleCredentialOptions #2412

Closed
gdavison opened this issue Dec 6, 2023 · 5 comments
Assignees

Comments

@gdavison
Copy link
Contributor

gdavison commented Dec 6, 2023

Describe the bug

When trying to assume a role with web identity, it is not possible to assume the role, even when all values are set by passing config. config.WithWebIdentityRoleCredentialOptions to config.LoadDefaultConfig, e.g.:

loadOptions := []func(*config.LoadOptions) error{
	// other options
	config.WithWebIdentityRoleCredentialOptions(func(opts *stscreds.WebIdentityRoleOptions) {
		opts.TokenRetriever = retriever
		opts.RoleARN = roleARN
		opts.RoleSessionName = sessionName
		opts.Duration = duration

		if ar.Policy != "" {
			opts.Policy = aws.String(policy)
		}

		if len(ar.PolicyARNs) > 0 {
			opts.PolicyARNs = getPolicyDescriptorTypes(policyARNs)
		}
	}
}

awsConfig, err := config.LoadDefaultConfig(ctx, loadOptions...)

This will not work, and fall back to trying IMDS.

Looking through config.resolveCredentialChain, config.assumeWebIdentity is only called if WebIdentityTokenFilePath is set on the EnvConfig or WebIdentityTokenFile is set on the SharedConfig.

Manually creating an sts.Client and stscreds.WebIdentityRoleProvider is an option, but if there are other configurations set, we have to:

  1. either manually resolve the configurations or call config.LoadDefaultConfig
  2. set the Config.Credentials to nil
  3. create the StsClient using the resolved Config

Expected Behavior

config.LoadDefaultConfig should be able to directly assume the role with web identity if all of the parameters are set

Current Behavior

config.LoadDefaultConfig does not assume the role, and requires manual configuration

Reproduction Steps

Add the following test case to TestResolveWebIdentityWithOptions in config/resolve_web_identity_test.go

t.Run("token supplied directly", func(t *testing.T) {
	restoreEnv := initConfigTestEnv()
	defer awstesting.PopEnv(restoreEnv)

	var tokenFile = filepath.Join("testdata", "wit.txt")
	os.Setenv("AWS_REGION", "us-east-1")

	config, err := LoadDefaultConfig(context.Background(),
		WithEC2IMDSClientEnableState(imds.ClientDisabled),
		WithWebIdentityRoleCredentialOptions(func(options *stscreds.WebIdentityRoleOptions) {
			options.TokenRetriever = stscreds.IdentityTokenFile(tokenFile)
			options.RoleARN = "test-arn"
		}),
	)

	if err != nil {
		t.Fatalf("expect no error, got %v", err)
	}

	target := stscreds.WebIdentityRoleProvider{}
	if !aws.IsCredentialsProvider(config.Credentials, &target) {
		t.Fatalf("expected type %T", target)
	}
})

Possible Solution

No response

Additional Information/Context

Related to #2015

AWS Go SDK V2 Module Versions Used

github.com/aws/aws-sdk-go-v2 v1.23.4
github.com/aws/aws-sdk-go-v2/config v1.25.10
github.com/aws/aws-sdk-go-v2/credentials v1.16.8
github.com/aws/aws-sdk-go-v2/service/sts v1.26.1

Compiler and Version used

go version go1.21.0 darwin/arm64

Operating System and version

macOS 13.4.1

@gdavison gdavison added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 6, 2023
@lucix-aws
Copy link
Contributor

@gdavison Can you try the latest set of modules? (mainly the sts client). I believe this is related to a fix we just pushed out today--

**Bug Fix**: STS `AssumeRoleWithSAML` and `AssumeRoleWithWebIdentity` would incorrectly attempt to use SigV4 authentication.

@lucix-aws lucix-aws added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Dec 6, 2023
@gdavison
Copy link
Contributor Author

gdavison commented Dec 7, 2023

Hi @lucix-aws, that doesn't fix it. The issue is in config.resolveCredentialChain and config.resolveCredsFromProfile:

Looking through config.resolveCredentialChain, config.assumeWebIdentity is only called if WebIdentityTokenFilePath is set on the EnvConfig or WebIdentityTokenFile is set on the SharedConfig.

In my use case, since I'm trying to set it programatically, neither the EnvConfig nor the SharedConfig are in use. Resolution never tries the web credentials, but instead falls through to trying IMDS

@lucix-aws lucix-aws removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Dec 7, 2023
@lucix-aws lucix-aws assigned lucix-aws and wty-Bryant and unassigned lucix-aws Dec 7, 2023
@RanVaknin RanVaknin added p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Dec 7, 2023
@wty-Bryant
Copy link
Contributor

@gdavison Hi we have made sure the issue is caused by resolveCredentialChain. It allows LoadOptions to config web identity role provider but does not use it as a trigger within resolveCredentialChain to call assumeWebIdentity. Therefore you can not simply use WithWebIdentityRoleCredentialOptions to assume role without setting envConfig/sharedConfig. We have posted a draft PR for that as linked.
However, we actually could achieve your expectation ahead of the cred chain. Within resolveCredentialProvider, all configs including LoadOptions would be iterated to find the first credentialsProviderProvider. So you could config your WebIdentityProvider directly like this:

loadOptions := []func(*config.LoadOptions) error{
	// other options
	WithCredentialsProvider(stscreds.NewWebIdentityRoleProvider(client, roleARN, IdentityTokenFilePath, optFns...))
}

awsConfig, err := config.LoadDefaultConfig(ctx, loadOptions...)

@wty-Bryant wty-Bryant added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Dec 12, 2023
Copy link

This issue has not received a response in 1 week. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Dec 15, 2023
@lucix-aws lucix-aws added closed-for-staleness and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. closing-soon This issue will automatically close in 4 days unless further comments are made. bug This issue is a bug. p2 This is a standard priority issue labels Dec 18, 2023
@lucix-aws lucix-aws closed this as not planned Won't fix, can't repro, duplicate, stale Dec 18, 2023
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

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