Fix SSO Validation of Incomplete Profiles (#1103)

aws · Feb 3, 2021 · db0cc11 · db0cc11
1 parent e5e313f
commit db0cc11
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 17 deletions.
diff --git a/.changes/next-release/config-bugfix-1612292652982606000.json b/.changes/next-release/config-bugfix-1612292652982606000.json
@@ -0,0 +1,9 @@
+{
+ "ID": "config-bugfix-1612292652982606000",
+ "SchemaVersion": 1,
+ "Module": "config",
+ "Type": "bugfix",
+ "Description": "Only Validate SSO profile configuration when attempting to use SSO credentials",
+ "MinVersion": "",
+ "AffectedModules": null
+}
diff --git a/.changes/next-release/config-bugfix-1612293019330574000.json b/.changes/next-release/config-bugfix-1612293019330574000.json
@@ -0,0 +1,9 @@
+{
+ "ID": "config-bugfix-1612293019330574000",
+ "SchemaVersion": 1,
+ "Module": "config",
+ "Type": "bugfix",
+ "Description": "Environment credentials were not taking precedence over AWS_PROFILE",
+ "MinVersion": "",
+ "AffectedModules": null
+}
diff --git a/config/resolve_credentials.go b/config/resolve_credentials.go
@@ -80,13 +80,16 @@ func resolveCredentialProvider(ctx context.Context, cfg *aws.Config, cfgs config
 // credentials are only refreshed when needed. This also protects the
 // credential provider to be used concurrently.
 func resolveCredentialChain(ctx context.Context, cfg *aws.Config, configs configs) (err error) {
-	_, sharedProfileSet, err := getSharedConfigProfile(ctx, configs)
+	envConfig, sharedConfig, other := getAWSConfigSources(configs)
+
+	// When checking if a profile was specified programmatically we should only consider the "other"
+	// configuration sources that have been provided. This ensures we correctly honor the expected credential
+	// hierarchy.
+	_, sharedProfileSet, err := getSharedConfigProfile(ctx, other)
 	if err != nil {
 		return err
 	}
 
-	envConfig, sharedConfig, other := getAWSConfigSources(configs)
-
 	switch {
 	case sharedProfileSet:
 		err = resolveCredsFromProfile(ctx, cfg, envConfig, sharedConfig, other)
@@ -157,6 +160,10 @@ func resolveCredsFromProfile(ctx context.Context, cfg *aws.Config, envConfig *En
 }
 
 func resolveSSOCredentials(ctx context.Context, cfg *aws.Config, sharedConfig *SharedConfig, configs configs) error {
+	if err := sharedConfig.validateSSOConfiguration(); err != nil {
+		return err
+	}
+
 	var options []func(*ssocreds.Options)
 	v, found, err := getSSOProviderOptions(ctx, configs)
 	if err != nil {

diff --git a/config/resolve_credentials_test.go b/config/resolve_credentials_test.go
@@ -296,6 +296,20 @@ func TestSharedConfigCredentialSource(t *testing.T) {
 				"assume_sso_and_static_arn",
 			},
 		},
+		"invalid sso configuration": {
+			envProfile:    "sso_invalid",
+			expectedError: "profile \"sso_invalid\" is configured to use SSO but is missing required configuration: sso_region, sso_start_url",
+		},
+		"environment credentials with invalid sso": {
+			envProfile:        "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 name, c := range cases {

diff --git a/config/shared_config.go b/config/shared_config.go
@@ -883,10 +883,6 @@ func (c *SharedConfig) validateCredentialsConfig(profile string) error {
 		return err
 	}
 
-	if err := c.validateSSOConfiguration(profile); err != nil {
-		return err
-	}
-
 	return nil
 }
 
@@ -927,7 +923,7 @@ func (c *SharedConfig) validateCredentialType() error {
 	return nil
 }
 
-func (c *SharedConfig) validateSSOConfiguration(profile string) error {
+func (c *SharedConfig) validateSSOConfiguration() error {
 	if !c.hasSSOConfiguration() {
 		return nil
 	}
@@ -951,7 +947,7 @@ func (c *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, ", "))
+			c.Profile, strings.Join(missing, ", "))
 	}
 
 	return nil

diff --git a/config/shared_config_test.go b/config/shared_config_test.go
@@ -246,11 +246,6 @@ func TestNewSharedConfig(t *testing.T) {
 				},
 			},
 		},
-		"AWS SSO Invalid Profile": {
-			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"),
-		},
 		"AWS SSO Profile and Static Credentials": {
 			Filenames: []string{testConfigFilename},
 			Profile:   "sso_and_static",

diff --git a/config/testdata/config_source_shared b/config/testdata/config_source_shared
@@ -57,3 +57,6 @@ 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
diff --git a/config/testdata/shared_config b/config/testdata/shared_config
@@ -115,9 +115,6 @@ sso_start_url = https://127.0.0.1/start
 role_arn = source_sso_creds_arn
 source_profile = sso_creds
 
-[profile invalid_sso_creds]
-sso_account_id = 012345678901
-
 [profile sso_and_static]
 aws_access_key_id = sso_and_static_akid
 aws_secret_access_key = sso_and_static_secret