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

Fix SSO Validation of Incomplete Profiles #1103

Merged
merged 1 commit into from
Feb 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -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
}
13 changes: 10 additions & 3 deletions config/resolve_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
skotambkar marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}

envConfig, sharedConfig, other := getAWSConfigSources(configs)

switch {
case sharedProfileSet:
err = resolveCredsFromProfile(ctx, cfg, envConfig, sharedConfig, other)
Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions config/resolve_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 2 additions & 6 deletions config/shared_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -883,10 +883,6 @@ func (c *SharedConfig) validateCredentialsConfig(profile string) error {
return err
}

if err := c.validateSSOConfiguration(profile); err != nil {
return err
}

return nil
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down
5 changes: 0 additions & 5 deletions config/shared_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 3 additions & 0 deletions config/testdata/config_source_shared
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 0 additions & 3 deletions config/testdata/shared_config
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down