-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Update shared config logic to resolve sso section #4868
Changes from all commits
5099b1d
a30faf1
b09e692
6843104
675b487
1384e97
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,13 @@ const ( | |
roleSessionNameKey = `role_session_name` // optional | ||
roleDurationSecondsKey = "duration_seconds" // optional | ||
|
||
// Prefix to be used for SSO sections. These are supposed to only exist in | ||
// the shared config file, not the credentials file. | ||
ssoSectionPrefix = `sso-session ` | ||
|
||
// AWS Single Sign-On (AWS SSO) group | ||
ssoSessionNameKey = "sso_session" | ||
|
||
// AWS Single Sign-On (AWS SSO) group | ||
ssoAccountIDKey = "sso_account_id" | ||
ssoRegionKey = "sso_region" | ||
|
@@ -99,6 +106,10 @@ type sharedConfig struct { | |
CredentialProcess string | ||
WebIdentityTokenFile string | ||
|
||
// SSO session options | ||
SSOSessionName string | ||
SSOSession *ssoSession | ||
|
||
SSOAccountID string | ||
SSORegion string | ||
SSORoleName string | ||
|
@@ -186,6 +197,20 @@ type sharedConfigFile struct { | |
IniData ini.Sections | ||
} | ||
|
||
// SSOSession provides the shared configuration parameters of the sso-session | ||
// section. | ||
type ssoSession struct { | ||
Name string | ||
SSORegion string | ||
SSOStartURL string | ||
} | ||
|
||
func (s *ssoSession) setFromIniSection(section ini.Section) { | ||
updateString(&s.Name, section, ssoSessionNameKey) | ||
updateString(&s.SSORegion, section, ssoRegionKey) | ||
updateString(&s.SSOStartURL, section, ssoStartURL) | ||
} | ||
|
||
// loadSharedConfig retrieves the configuration from the list of files using | ||
// the profile provided. The order the files are listed will determine | ||
// precedence. Values in subsequent files will overwrite values defined in | ||
|
@@ -266,13 +291,13 @@ func (cfg *sharedConfig) setFromIniFiles(profiles map[string]struct{}, profile s | |
// profile only have credential provider options. | ||
cfg.clearAssumeRoleOptions() | ||
} else { | ||
// First time a profile has been seen, It must either be a assume role | ||
// credentials, or SSO. Assert if the credential type requires a role ARN, | ||
// the ARN is also set, or validate that the SSO configuration is complete. | ||
// First time a profile has been seen. Assert if the credential type | ||
// requires a role ARN, the ARN is also set | ||
if err := cfg.validateCredentialsConfig(profile); err != nil { | ||
return err | ||
} | ||
} | ||
|
||
profiles[profile] = struct{}{} | ||
|
||
if err := cfg.validateCredentialType(); err != nil { | ||
|
@@ -308,6 +333,30 @@ func (cfg *sharedConfig) setFromIniFiles(profiles map[string]struct{}, profile s | |
cfg.SourceProfile = srcCfg | ||
} | ||
|
||
// If the profile contains an SSO session parameter, the session MUST exist | ||
// as a section in the config file. Load the SSO session using the name | ||
// provided. If the session section is not found or incomplete an error | ||
// will be returned. | ||
if cfg.hasSSOTokenProviderConfiguration() { | ||
skippedFiles = 0 | ||
for _, f := range files { | ||
section, ok := f.IniData.GetSection(fmt.Sprintf(ssoSectionPrefix + strings.TrimSpace(cfg.SSOSessionName))) | ||
if ok { | ||
var ssoSession ssoSession | ||
ssoSession.setFromIniSection(section) | ||
ssoSession.Name = cfg.SSOSessionName | ||
cfg.SSOSession = &ssoSession | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: So this looks like we set a single e.g. with
[profile foo]
role_arn = role1
source_profile = bar
[profile bar]
role_arn = role2
source_profile = baz
[profile baz]
role_arn = role3
sso_account_id = 1234
sso_region = us-east-1
sso_session = session1
[sso-session session1]
...
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to v1's logic of resolving source profile , the current
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then during step of resolving user credentials from shared config, it will recursively detect source profile and resolve sso credential provider if present in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great thanks for clarifying this, that is helpful. |
||
break | ||
} | ||
skippedFiles++ | ||
} | ||
if skippedFiles == len(files) { | ||
// If all files were skipped because the sso session section is not found, return | ||
// the sso section not found error. | ||
return fmt.Errorf("failed to find SSO session section, %v", cfg.SSOSessionName) | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
|
@@ -363,6 +412,10 @@ func (cfg *sharedConfig) setFromIniFile(profile string, file sharedConfigFile, e | |
cfg.S3UsEast1RegionalEndpoint = sre | ||
} | ||
|
||
// AWS Single Sign-On (AWS SSO) | ||
// SSO session options | ||
updateString(&cfg.SSOSessionName, section, ssoSessionNameKey) | ||
|
||
// AWS Single Sign-On (AWS SSO) | ||
updateString(&cfg.SSOAccountID, section, ssoAccountIDKey) | ||
updateString(&cfg.SSORegion, section, ssoRegionKey) | ||
|
@@ -461,32 +514,20 @@ func (cfg *sharedConfig) validateCredentialType() error { | |
} | ||
|
||
func (cfg *sharedConfig) validateSSOConfiguration() error { | ||
if !cfg.hasSSOConfiguration() { | ||
if cfg.hasSSOTokenProviderConfiguration() { | ||
err := cfg.validateSSOTokenProviderConfiguration() | ||
if err != nil { | ||
return err | ||
} | ||
return nil | ||
} | ||
|
||
var missing []string | ||
if len(cfg.SSOAccountID) == 0 { | ||
missing = append(missing, ssoAccountIDKey) | ||
} | ||
|
||
if len(cfg.SSORegion) == 0 { | ||
missing = append(missing, ssoRegionKey) | ||
} | ||
|
||
if len(cfg.SSORoleName) == 0 { | ||
missing = append(missing, ssoRoleNameKey) | ||
} | ||
|
||
if len(cfg.SSOStartURL) == 0 { | ||
missing = append(missing, ssoStartURL) | ||
} | ||
|
||
if len(missing) > 0 { | ||
return fmt.Errorf("profile %q is configured to use SSO but is missing required configuration: %s", | ||
cfg.Profile, strings.Join(missing, ", ")) | ||
if cfg.hasLegacySSOConfiguration() { | ||
err := cfg.validateLegacySSOConfiguration() | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
|
@@ -525,15 +566,76 @@ func (cfg *sharedConfig) clearAssumeRoleOptions() { | |
} | ||
|
||
func (cfg *sharedConfig) hasSSOConfiguration() bool { | ||
switch { | ||
case len(cfg.SSOAccountID) != 0: | ||
case len(cfg.SSORegion) != 0: | ||
case len(cfg.SSORoleName) != 0: | ||
case len(cfg.SSOStartURL) != 0: | ||
default: | ||
return false | ||
return cfg.hasSSOTokenProviderConfiguration() || cfg.hasLegacySSOConfiguration() | ||
} | ||
|
||
func (c *sharedConfig) hasSSOTokenProviderConfiguration() bool { | ||
return len(c.SSOSessionName) > 0 | ||
} | ||
|
||
func (c *sharedConfig) hasLegacySSOConfiguration() bool { | ||
return len(c.SSORegion) > 0 || len(c.SSOAccountID) > 0 || len(c.SSOStartURL) > 0 || len(c.SSORoleName) > 0 | ||
} | ||
|
||
func (c *sharedConfig) validateSSOTokenProviderConfiguration() error { | ||
var missing []string | ||
|
||
if len(c.SSOSessionName) == 0 { | ||
missing = append(missing, ssoSessionNameKey) | ||
} | ||
return true | ||
|
||
if c.SSOSession == nil { | ||
missing = append(missing, ssoSectionPrefix) | ||
} else { | ||
if len(c.SSOSession.SSORegion) == 0 { | ||
missing = append(missing, ssoRegionKey) | ||
} | ||
|
||
if len(c.SSOSession.SSOStartURL) == 0 { | ||
missing = append(missing, ssoStartURL) | ||
} | ||
} | ||
|
||
if len(missing) > 0 { | ||
return fmt.Errorf("profile %q is configured to use SSO but is missing required configuration: %s", | ||
c.Profile, strings.Join(missing, ", ")) | ||
} | ||
|
||
if len(c.SSORegion) > 0 && c.SSORegion != c.SSOSession.SSORegion { | ||
return fmt.Errorf("%s in profile %q must match %s in %s", ssoRegionKey, c.Profile, ssoRegionKey, ssoSectionPrefix) | ||
} | ||
|
||
if len(c.SSOStartURL) > 0 && c.SSOStartURL != c.SSOSession.SSOStartURL { | ||
return fmt.Errorf("%s in profile %q must match %s in %s", ssoStartURL, c.Profile, ssoStartURL, ssoSectionPrefix) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func (c *sharedConfig) validateLegacySSOConfiguration() error { | ||
var missing []string | ||
|
||
if len(c.SSORegion) == 0 { | ||
missing = append(missing, ssoRegionKey) | ||
} | ||
|
||
if len(c.SSOStartURL) == 0 { | ||
missing = append(missing, ssoStartURL) | ||
} | ||
|
||
if len(c.SSOAccountID) == 0 { | ||
missing = append(missing, ssoAccountIDKey) | ||
} | ||
|
||
if len(c.SSORoleName) == 0 { | ||
missing = append(missing, ssoRoleNameKey) | ||
} | ||
|
||
if len(missing) > 0 { | ||
return fmt.Errorf("profile %q is configured to use SSO but is missing required configuration: %s", | ||
c.Profile, strings.Join(missing, ", ")) | ||
} | ||
return nil | ||
} | ||
|
||
func oneOrNone(bs ...bool) bool { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -390,6 +390,27 @@ func TestLoadSharedConfig(t *testing.T) { | |
UseFIPSEndpoint: endpoints.FIPSEndpointStateDisabled, | ||
}, | ||
}, | ||
{ | ||
Filenames: []string{testConfigFilename}, | ||
Profile: "sso-session-success", | ||
Expected: sharedConfig{ | ||
Profile: "sso-session-success", | ||
Region: "us-east-1", | ||
SSOAccountID: "123456789012", | ||
SSORoleName: "testRole", | ||
SSOSessionName: "sso-session-success-dev", | ||
SSOSession: &ssoSession{ | ||
Name: "sso-session-success-dev", | ||
SSORegion: "us-east-1", | ||
SSOStartURL: "https://d-123456789a.awsapps.com/start", | ||
}, | ||
}, | ||
}, | ||
{ | ||
Filenames: []string{testConfigFilename}, | ||
Profile: "sso-session-not-exist", | ||
Err: fmt.Errorf("failed to find SSO session section, sso-session-lost"), | ||
}, | ||
} | ||
|
||
for i, c := range cases { | ||
|
@@ -507,6 +528,15 @@ func TestLoadSharedConfigFromFile(t *testing.T) { | |
S3UseARNRegion: true, | ||
}, | ||
}, | ||
{ | ||
Profile: "sso-session-success", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Would be good to add some of the validation cases here as well, not just the happy path. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ^agreed |
||
Expected: sharedConfig{ | ||
Region: "us-east-1", | ||
SSOAccountID: "123456789012", | ||
SSORoleName: "testRole", | ||
SSOSessionName: "sso-session-success-dev", | ||
}, | ||
}, | ||
} | ||
|
||
for i, c := range cases { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does V1 only have a concept of multiple shared config files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, in v2's sharedConfig loading, sections will be extracted from config files first, but in v1 files will be used directly to load shared config