Fix: Only Validate SSO profile configuration when attempting to use S…

…SO credentials. (#3769)
aws · Feb 1, 2021 · 4a3fa39 · 4a3fa39
1 parent 3974dd0
commit 4a3fa39
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 20 deletions.
diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md
@@ -3,3 +3,5 @@
 ### SDK Enhancements
 
 ### SDK Bugs
+* `aws/session`: Fixed a bug that prevented credentials from being sourced from the environment if the loaded shared config profile contained partial SSO configuration. ([#3769](https://github.com/aws/aws-sdk-go/pull/3769))
+  * Fixes ([#3768](https://github.com/aws/aws-sdk-go/issues/3768))
diff --git a/aws/session/credentials.go b/aws/session/credentials.go
@@ -102,7 +102,7 @@ func resolveCredsFromProfile(cfg *aws.Config,
 		)
 
 	case sharedCfg.hasSSOConfiguration():
-		creds = resolveSSOCredentials(cfg, sharedCfg, handlers)
+		creds, err = resolveSSOCredentials(cfg, sharedCfg, handlers)
 
 	case len(sharedCfg.CredentialProcess) != 0:
 		// Get credentials from CredentialProcess
@@ -155,7 +155,11 @@ func resolveCredsFromProfile(cfg *aws.Config,
 	return creds, nil
 }
 
-func resolveSSOCredentials(cfg *aws.Config, sharedCfg sharedConfig, handlers request.Handlers) *credentials.Credentials {
+func resolveSSOCredentials(cfg *aws.Config, sharedCfg sharedConfig, handlers request.Handlers) (*credentials.Credentials, error) {
+	if err := sharedCfg.validateSSOConfiguration(); err != nil {
+		return nil, err
+	}
+
 	cfgCopy := cfg.Copy()
 	cfgCopy.Region = &sharedCfg.SSORegion
 
@@ -167,7 +171,7 @@ func resolveSSOCredentials(cfg *aws.Config, sharedCfg sharedConfig, handlers req
 		sharedCfg.SSOAccountID,
 		sharedCfg.SSORoleName,
 		sharedCfg.SSOStartURL,
-	)
+	), nil
 }
 
 // valid credential source values

diff --git a/aws/session/credentials_test.go b/aws/session/credentials_test.go
@@ -259,6 +259,22 @@ func TestSharedConfigCredentialSource(t *testing.T) {
 				"assume_sso_and_static_arn",
 			},
 		},
+		{
+			name:          "invalid sso configuration",
+			profile:       "sso_invalid",
+			expectedError: fmt.Errorf("profile \"sso_invalid\" is configured to use SSO but is missing required configuration: sso_region, sso_start_url"),
+		},
+		{
+			name:              "environment credentials with invalid sso",
+			profile:           "sso_invalid",
+			expectedAccessKey: "access_key",
+			expectedSecretKey: "secret_key",
+			init: func() (func(), error) {
+				os.Setenv("AWS_ACCESS_KEY", "access_key")
+				os.Setenv("AWS_SECRET_KEY", "secret_key")
+				return func() {}, nil
+			},
+		},
 	}
 
 	for i, c := range cases {
@@ -308,8 +324,15 @@ func TestSharedConfigCredentialSource(t *testing.T) {
 				Handlers:        handlers,
 				EC2IMDSEndpoint: c.sessOptEC2IMDSEndpoint,
 			})
-			if e, a := c.expectedError, err; e != a {
-				t.Fatalf("expected %v, but received %v", e, a)
+
+			if c.expectedError != nil {
+				var errStr string
+				if err != nil {
+					errStr = err.Error()
+				}
+				if e, a := c.expectedError.Error(), errStr; !strings.Contains(a, e) {
+					t.Fatalf("expected %v, but received %v", e, a)
+				}
 			}
 
 			if c.expectedError != nil {

diff --git a/aws/session/shared_config.go b/aws/session/shared_config.go
@@ -70,6 +70,8 @@ const (
 
 // sharedConfig represents the configuration fields of the SDK config files.
 type sharedConfig struct {
+	Profile string
+
 	// Credentials values from the config file. Both aws_access_key_id and
 	// aws_secret_access_key must be provided together in the same file to be
 	// considered valid. The values will be ignored if not a complete group.
@@ -201,6 +203,8 @@ func loadSharedConfigIniFiles(filenames []string) ([]sharedConfigFile, error) {
 }
 
 func (cfg *sharedConfig) setFromIniFiles(profiles map[string]struct{}, profile string, files []sharedConfigFile, exOpts bool) error {
+	cfg.Profile = profile
+
 	// Trim files from the list that don't exist.
 	var skippedFiles int
 	var profileNotFoundErr error
@@ -365,10 +369,6 @@ func (cfg *sharedConfig) validateCredentialsConfig(profile string) error {
 		return err
 	}
 
-	if err := cfg.validateSSOConfiguration(profile); err != nil {
-		return err
-	}
-
 	return nil
 }
 
@@ -409,7 +409,7 @@ func (cfg *sharedConfig) validateCredentialType() error {
 	return nil
 }
 
-func (cfg *sharedConfig) validateSSOConfiguration(profile string) error {
+func (cfg *sharedConfig) validateSSOConfiguration() error {
 	if !cfg.hasSSOConfiguration() {
 		return nil
 	}
@@ -433,7 +433,7 @@ func (cfg *sharedConfig) validateSSOConfiguration(profile string) error {
 
 	if len(missing) > 0 {
 		return fmt.Errorf("profile %q is configured to use SSO but is missing required configuration: %s",
-			profile, strings.Join(missing, ", "))
+			cfg.Profile, strings.Join(missing, ", "))
 	}
 
 	return nil

diff --git a/aws/session/shared_config_test.go b/aws/session/shared_config_test.go
@@ -30,19 +30,23 @@ func TestLoadSharedConfig(t *testing.T) {
 		{
 			Filenames: []string{"file_not_exists"},
 			Profile:   "default",
-			Expected:  sharedConfig{},
+			Expected: sharedConfig{
+				Profile: "default",
+			},
 		},
 		{
 			Filenames: []string{testConfigFilename},
 			Expected: sharedConfig{
-				Region: "default_region",
+				Profile: "default",
+				Region:  "default_region",
 			},
 		},
 		{
 			Filenames: []string{testConfigOtherFilename, testConfigFilename},
 			Profile:   "config_file_load_order",
 			Expected: sharedConfig{
-				Region: "shared_config_region",
+				Profile: "config_file_load_order",
+				Region:  "shared_config_region",
 				Creds: credentials.Value{
 					AccessKeyID:     "shared_config_akid",
 					SecretAccessKey: "shared_config_secret",
@@ -54,7 +58,8 @@ func TestLoadSharedConfig(t *testing.T) {
 			Filenames: []string{testConfigFilename, testConfigOtherFilename},
 			Profile:   "config_file_load_order",
 			Expected: sharedConfig{
-				Region: "shared_config_other_region",
+				Profile: "config_file_load_order",
+				Region:  "shared_config_other_region",
 				Creds: credentials.Value{
 					AccessKeyID:     "shared_config_other_akid",
 					SecretAccessKey: "shared_config_other_secret",
@@ -66,9 +71,11 @@ func TestLoadSharedConfig(t *testing.T) {
 			Filenames: []string{testConfigOtherFilename, testConfigFilename},
 			Profile:   "assume_role",
 			Expected: sharedConfig{
+				Profile:           "assume_role",
 				RoleARN:           "assume_role_role_arn",
 				SourceProfileName: "complete_creds",
 				SourceProfile: &sharedConfig{
+					Profile: "complete_creds",
 					Creds: credentials.Value{
 						AccessKeyID:     "complete_creds_akid",
 						SecretAccessKey: "complete_creds_secret",
@@ -81,6 +88,7 @@ func TestLoadSharedConfig(t *testing.T) {
 			Filenames: []string{testConfigOtherFilename, testConfigFilename},
 			Profile:   "assume_role_invalid_source_profile",
 			Expected: sharedConfig{
+				Profile:           "assume_role_invalid_source_profile",
 				RoleARN:           "assume_role_invalid_source_profile_role_arn",
 				SourceProfileName: "profile_not_exists",
 			},
@@ -93,11 +101,13 @@ func TestLoadSharedConfig(t *testing.T) {
 			Filenames: []string{testConfigOtherFilename, testConfigFilename},
 			Profile:   "assume_role_w_creds",
 			Expected: sharedConfig{
+				Profile:           "assume_role_w_creds",
 				RoleARN:           "assume_role_w_creds_role_arn",
 				ExternalID:        "1234",
 				RoleSessionName:   "assume_role_w_creds_session_name",
 				SourceProfileName: "assume_role_w_creds",
 				SourceProfile: &sharedConfig{
+					Profile: "assume_role_w_creds",
 					Creds: credentials.Value{
 						AccessKeyID:     "assume_role_w_creds_akid",
 						SecretAccessKey: "assume_role_w_creds_secret",
@@ -110,6 +120,7 @@ func TestLoadSharedConfig(t *testing.T) {
 			Filenames: []string{testConfigOtherFilename, testConfigFilename},
 			Profile:   "assume_role_wo_creds",
 			Expected: sharedConfig{
+				Profile:           "assume_role_wo_creds",
 				RoleARN:           "assume_role_wo_creds_role_arn",
 				SourceProfileName: "assume_role_wo_creds",
 			},
@@ -127,6 +138,7 @@ func TestLoadSharedConfig(t *testing.T) {
 			Filenames: []string{testConfigOtherFilename, testConfigFilename},
 			Profile:   "assume_role_with_credential_source",
 			Expected: sharedConfig{
+				Profile:          "assume_role_with_credential_source",
 				RoleARN:          "assume_role_with_credential_source_role_arn",
 				CredentialSource: credSourceEc2Metadata,
 			},
@@ -135,12 +147,15 @@ func TestLoadSharedConfig(t *testing.T) {
 			Filenames: []string{testConfigOtherFilename, testConfigFilename},
 			Profile:   "multiple_assume_role",
 			Expected: sharedConfig{
+				Profile:           "multiple_assume_role",
 				RoleARN:           "multiple_assume_role_role_arn",
 				SourceProfileName: "assume_role",
 				SourceProfile: &sharedConfig{
+					Profile:           "assume_role",
 					RoleARN:           "assume_role_role_arn",
 					SourceProfileName: "complete_creds",
 					SourceProfile: &sharedConfig{
+						Profile: "complete_creds",
 						Creds: credentials.Value{
 							AccessKeyID:     "complete_creds_akid",
 							SecretAccessKey: "complete_creds_secret",
@@ -154,9 +169,11 @@ func TestLoadSharedConfig(t *testing.T) {
 			Filenames: []string{testConfigOtherFilename, testConfigFilename},
 			Profile:   "multiple_assume_role_with_credential_source",
 			Expected: sharedConfig{
+				Profile:           "multiple_assume_role_with_credential_source",
 				RoleARN:           "multiple_assume_role_with_credential_source_role_arn",
 				SourceProfileName: "assume_role_with_credential_source",
 				SourceProfile: &sharedConfig{
+					Profile:          "assume_role_with_credential_source",
 					RoleARN:          "assume_role_with_credential_source_role_arn",
 					CredentialSource: credSourceEc2Metadata,
 				},
@@ -166,12 +183,15 @@ func TestLoadSharedConfig(t *testing.T) {
 			Filenames: []string{testConfigOtherFilename, testConfigFilename},
 			Profile:   "multiple_assume_role_with_credential_source2",
 			Expected: sharedConfig{
+				Profile:           "multiple_assume_role_with_credential_source2",
 				RoleARN:           "multiple_assume_role_with_credential_source2_role_arn",
 				SourceProfileName: "multiple_assume_role_with_credential_source",
 				SourceProfile: &sharedConfig{
+					Profile:           "multiple_assume_role_with_credential_source",
 					RoleARN:           "multiple_assume_role_with_credential_source_role_arn",
 					SourceProfileName: "assume_role_with_credential_source",
 					SourceProfile: &sharedConfig{
+						Profile:          "assume_role_with_credential_source",
 						RoleARN:          "assume_role_with_credential_source_role_arn",
 						CredentialSource: credSourceEc2Metadata,
 					},
@@ -182,20 +202,23 @@ func TestLoadSharedConfig(t *testing.T) {
 			Filenames: []string{testConfigFilename},
 			Profile:   "with_sts_regional",
 			Expected: sharedConfig{
+				Profile:             "with_sts_regional",
 				STSRegionalEndpoint: endpoints.RegionalSTSEndpoint,
 			},
 		},
 		{
 			Filenames: []string{testConfigFilename},
 			Profile:   "with_s3_us_east_1_regional",
 			Expected: sharedConfig{
+				Profile:                   "with_s3_us_east_1_regional",
 				S3UsEast1RegionalEndpoint: endpoints.RegionalS3UsEast1Endpoint,
 			},
 		},
 		{
 			Filenames: []string{testConfigFilename},
 			Profile:   "sso_creds",
 			Expected: sharedConfig{
+				Profile:      "sso_creds",
 				SSOAccountID: "012345678901",
 				SSORegion:    "us-west-2",
 				SSORoleName:  "TestRole",
@@ -206,25 +229,23 @@ func TestLoadSharedConfig(t *testing.T) {
 			Filenames: []string{testConfigFilename},
 			Profile:   "source_sso_creds",
 			Expected: sharedConfig{
+				Profile:           "source_sso_creds",
 				RoleARN:           "source_sso_creds_arn",
 				SourceProfileName: "sso_creds",
 				SourceProfile: &sharedConfig{
+					Profile:      "sso_creds",
 					SSOAccountID: "012345678901",
 					SSORegion:    "us-west-2",
 					SSORoleName:  "TestRole",
 					SSOStartURL:  "https://127.0.0.1/start",
 				},
 			},
 		},
-		{
-			Filenames: []string{testConfigFilename},
-			Profile:   "invalid_sso_creds",
-			Err:       fmt.Errorf("profile \"invalid_sso_creds\" is configured to use SSO but is missing required configuration: sso_region, sso_role_name, sso_start_url"),
-		},
 		{
 			Filenames: []string{testConfigFilename},
 			Profile:   "sso_and_static",
 			Expected: sharedConfig{
+				Profile: "sso_and_static",
 				Creds: credentials.Value{
 					AccessKeyID:     "sso_and_static_akid",
 					SecretAccessKey: "sso_and_static_secret",

diff --git a/aws/session/testdata/credential_source_config b/aws/session/testdata/credential_source_config
@@ -52,3 +52,8 @@ sso_account_id = 012345678901
 sso_region = us-west-2
 sso_role_name = TestRole
 sso_start_url = https://THIS_SHOULD_NOT_BE_IN_TESTDATA_CACHE/start
+
+[profile sso_invalid]
+sso_account_id = 012345678901
+sso_role_name = TestRole
+