refactor based on pr feedback

CHANGELOG_PENDING.md

@@ -1,4 +1,6 @@
 ### SDK Features
+* `aws/session`: Add support for assuming role via Web Identity Token ([#2667](https://github.com/aws/aws-sdk-go/pull/2667))
+  * Adds support for assuming an role via the Web Identity Token. Allows for OIDC token files to be used by specifying the token path through the AWS_WEB_IDENTITY_TOKEN_FILE, and AWS_ROLE_ARN environment variables.
 
 ### SDK Enhancements
 

aws/credentials/credentials.go

@@ -84,6 +84,12 @@ type Value struct {
 	ProviderName string
 }
 
+// HasKeys returns if the credentials Value has both AccessKeyID and
+// SecretAccessKey value set.
+func (v Value) HasKeys() bool {
+	return len(v.AccessKeyID) != 0 && len(v.SecretAccessKey) != 0
+}
+
 // A Provider is the interface for any component which will provide credentials
 // Value. A provider is required to manage its own Expired state, and what to
 // be expired means.

aws/session/credentials.go

@@ -25,11 +25,9 @@ func resolveCredentials(cfg *aws.Config,
 
 	switch {
 	case len(sharedCfg.RoleARN) != 0 && len(sharedCfg.CredentialSource) != 0:
-		// Customer explicitly provided an Profile, so load from shared config
-		// first.
 		return resolveCredsFromProfile(cfg, envCfg, sharedCfg, handlers, sessOpts)
 
-	case len(envCfg.Creds.AccessKeyID) != 0:
+	case envCfg.Creds.HasKeys():
 		// Environment credentials
 		return credentials.NewStaticCredentialsFromCreds(envCfg.Creds), nil
 
@@ -48,6 +46,14 @@ func resolveCredentials(cfg *aws.Config,
 	}
 }
 
+// WebIdentityEmptyRoleARNErr will occur if 'AWS_WEB_IDENTITY_TOKEN_FILE' was set but
+// 'AWS_IAM_ROLE_ARN' was not set.
+var WebIdentityEmptyRoleARNErr = awserr.New(stscreds.ErrCodeWebIdentity, "role ARN is not set", nil)
+
+// WebIdentityEmptyTokenFilePathErr will occur if 'AWS_IAM_ROLE_ARN' was set but
+// 'AWS_WEB_IDENTITY_TOKEN_FILE' was not set.
+var WebIdentityEmptyTokenFilePathErr = awserr.New(stscreds.ErrCodeWebIdentity, "token file path is not set", nil)
+
 func assumeWebIdentity(cfg *aws.Config, handlers request.Handlers,
 	filepath string,
 	roleARN, sessionName string,
@@ -87,7 +93,7 @@ func resolveCredsFromProfile(cfg *aws.Config,
 			*sharedCfg.SourceProfile, handlers, sessOpts,
 		)
 
-	case len(sharedCfg.Creds.AccessKeyID) != 0:
+	case sharedCfg.Creds.HasKeys():
 		// Static Credentials from Shared Config/Credentials file.
 		creds = credentials.NewStaticCredentialsFromCreds(
 			sharedCfg.Creds,

aws/session/env_config.go

@@ -208,9 +208,15 @@ func envConfigLoad(enableSharedConfig bool) envConfig {
 	cfg.EnableSharedConfig = enableSharedConfig
 
 	// Static environment credentials
-	setFromEnvVal(&cfg.Creds.AccessKeyID, credAccessEnvKey)
-	setFromEnvVal(&cfg.Creds.SecretAccessKey, credSecretEnvKey)
-	setFromEnvVal(&cfg.Creds.SessionToken, credSessionEnvKey)
+	var creds credentials.Value
+	setFromEnvVal(&creds.AccessKeyID, credAccessEnvKey)
+	setFromEnvVal(&creds.SecretAccessKey, credSecretEnvKey)
+	setFromEnvVal(&creds.SessionToken, credSessionEnvKey)
+	if creds.HasKeys() {
+		// Require logical grouping of credentials
+		creds.ProviderName = EnvProviderName
+		cfg.Creds = creds
+	}
 
 	// Role Metadata
 	setFromEnvVal(&cfg.RoleARN, roleARNEnvKey)
@@ -226,13 +232,6 @@ func envConfigLoad(enableSharedConfig bool) envConfig {
 	setFromEnvVal(&cfg.CSMClientID, csmClientIDEnvKey)
 	cfg.CSMEnabled = len(cfg.csmEnabled) > 0
 
-	// Require logical grouping of credentials
-	if len(cfg.Creds.AccessKeyID) == 0 || len(cfg.Creds.SecretAccessKey) == 0 {
-		cfg.Creds = credentials.Value{}
-	} else {
-		cfg.Creds.ProviderName = EnvProviderName
-	}
-
 	regionKeys := regionEnvKeys
 	profileKeys := profileEnvKeys
 	if !cfg.EnableSharedConfig {

aws/session/session.go

@@ -15,7 +15,6 @@ import (
 	"github.com/aws/aws-sdk-go/aws/client"
 	"github.com/aws/aws-sdk-go/aws/corehandlers"
 	"github.com/aws/aws-sdk-go/aws/credentials"
-	"github.com/aws/aws-sdk-go/aws/credentials/stscreds"
 	"github.com/aws/aws-sdk-go/aws/csm"
 	"github.com/aws/aws-sdk-go/aws/defaults"
 	"github.com/aws/aws-sdk-go/aws/endpoints"
@@ -322,14 +321,6 @@ func Must(sess *Session, err error) *Session {
 	return sess
 }
 
-// WebIdentityEmptyRoleARNErr will occur if 'AWS_WEB_IDENTITY_TOKEN_FILE' was set but
-// 'AWS_IAM_ROLE_ARN' was not set.
-var WebIdentityEmptyRoleARNErr = awserr.New(stscreds.ErrCodeWebIdentity, "role ARN is not set", nil)
-
-// WebIdentityEmptyTokenFilePathErr will occur if 'AWS_IAM_ROLE_ARN' was set but
-// 'AWS_WEB_IDENTITY_TOKEN_FILE' was not set.
-var WebIdentityEmptyTokenFilePathErr = awserr.New(stscreds.ErrCodeWebIdentity, "token file path is not set", nil)
-
 func deprecatedNewSession(cfgs ...*aws.Config) *Session {
 	cfg := defaults.Config()
 	handlers := defaults.Handlers()

aws/session/shared_config.go

@@ -164,7 +164,7 @@ func (cfg *sharedConfig) setFromIniFiles(profiles map[string]struct{}, profile s
 		// First time a profile has been seen, It must either be a assume role
 		// or credentials. Assert if the credential type requires a role ARN,
 		// the ARN is also set.
-		if err := cfg.validateRoleARNWithCredentials(profile); err != nil {
+		if err := cfg.validateCredentialsRequireARN(profile); err != nil {
 			return err
 		}
 	}
@@ -175,7 +175,7 @@ func (cfg *sharedConfig) setFromIniFiles(profiles map[string]struct{}, profile s
 	}
 
 	// Link source profiles for assume roles
-	if len(cfg.RoleARN) != 0 && len(cfg.SourceProfileName) != 0 {
+	if len(cfg.SourceProfileName) != 0 {
 		// Linked profile via source_profile ignore credential provider
 		// options, the source profile must provide the credentials.
 		cfg.clearCredentialOptions()
@@ -240,15 +240,14 @@ func (cfg *sharedConfig) setFromIniFile(profile string, file sharedConfigFile, e
 	updateString(&cfg.WebIdentityTokenFile, section, webIdentityTokenFileKey)
 
 	// Shared Credentials
-	akid := section.String(accessKeyIDKey)
-	secret := section.String(secretAccessKey)
-	if len(akid) > 0 && len(secret) > 0 {
-		cfg.Creds = credentials.Value{
-			AccessKeyID:     akid,
-			SecretAccessKey: secret,
-			SessionToken:    section.String(sessionTokenKey),
-			ProviderName:    fmt.Sprintf("SharedConfigCredentials: %s", file.Filename),
-		}
+	creds := credentials.Value{
+		AccessKeyID:     section.String(accessKeyIDKey),
+		SecretAccessKey: section.String(secretAccessKey),
+		SessionToken:    section.String(sessionTokenKey),
+		ProviderName:    fmt.Sprintf("SharedConfigCredentials: %s", file.Filename),
+	}
+	if creds.HasKeys() {
+		cfg.Creds = creds
 	}
 
 	// Endpoint discovery
@@ -260,7 +259,7 @@ func (cfg *sharedConfig) setFromIniFile(profile string, file sharedConfigFile, e
 	return nil
 }
 
-func (cfg *sharedConfig) validateRoleARNWithCredentials(profile string) error {
+func (cfg *sharedConfig) validateCredentialsRequireARN(profile string) error {
 	var credSource string
 
 	switch {