Skip to content

Commit

Permalink
config: Enable SSO provider to be mixed with other credential providers
Browse files Browse the repository at this point in the history
- Fixes GitHub Issue 1204
- Fixes incorrect assume role call with web identity token provider
  • Loading branch information
skmcgrail committed May 14, 2021
1 parent d05dd9b commit c47c648
Show file tree
Hide file tree
Showing 16 changed files with 218 additions and 23 deletions.
8 changes: 8 additions & 0 deletions .changelog/016af652b95e472f960ded9a2e976e11.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "016af652-b95e-472f-960d-ed9a2e976e11",
"type": "bugfix",
"description": "`internal/ini`: Disable noramalization of config profile names",
"modules": [
"."
]
}
8 changes: 8 additions & 0 deletions .changelog/9eda03d68f1849a981362947a1a2941f.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "9eda03d6-8f18-49a9-8136-2947a1a2941f",
"type": "feature",
"description": "SSO credentials can now be defined alongside other credential providers within the same configuration profile.",
"modules": [
"config"
]
}
8 changes: 8 additions & 0 deletions .changelog/f2c891806ed64485bdbee78723db3a88.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "f2c89180-6ed6-4485-bdbe-e78723db3a88",
"type": "bugfix",
"description": "Fixed a bug that caused configuration profile names to be incorrectly normalized, which could cause incorrect profile loading in certain cases. ([#1204](https://github.com/aws/aws-sdk-go-v2/issues/1204))",
"modules": [
"config"
]
}
16 changes: 8 additions & 8 deletions config/resolve_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,21 +123,21 @@ func resolveCredsFromProfile(ctx context.Context, cfg *aws.Config, envConfig *En
Value: sharedConfig.Credentials,
}

case sharedConfig.hasSSOConfiguration():
err = resolveSSOCredentials(ctx, cfg, sharedConfig, configs)

case len(sharedConfig.CredentialProcess) != 0:
// Get credentials from CredentialProcess
err = processCredentials(ctx, cfg, sharedConfig, configs)

case len(sharedConfig.CredentialSource) != 0:
err = resolveCredsFromSource(ctx, cfg, envConfig, sharedConfig, configs)

case len(sharedConfig.WebIdentityTokenFile) != 0:
// Credentials from Assume Web Identity token require an IAM Role, and
// that roll will be assumed. May be wrapped with another assume role
// via SourceProfile.
err = assumeWebIdentity(ctx, cfg, sharedConfig.WebIdentityTokenFile, sharedConfig.RoleARN, sharedConfig.RoleSessionName, configs)
return assumeWebIdentity(ctx, cfg, sharedConfig.WebIdentityTokenFile, sharedConfig.RoleARN, sharedConfig.RoleSessionName, configs)

case sharedConfig.hasSSOConfiguration():
err = resolveSSOCredentials(ctx, cfg, sharedConfig, configs)

case len(sharedConfig.CredentialProcess) != 0:
// Get credentials from CredentialProcess
err = processCredentials(ctx, cfg, sharedConfig, configs)

case len(envConfig.ContainerCredentialsEndpoint) != 0:
err = resolveLocalHTTPCredProvider(ctx, cfg, envConfig.ContainerCredentialsEndpoint, envConfig.ContainerAuthorizationToken, configs)
Expand Down
51 changes: 46 additions & 5 deletions config/resolve_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,31 @@ func setupCredentialsEndpoints(t *testing.T) (aws.EndpointResolver, func()) {

stsServer := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte(fmt.Sprintf(
assumeRoleRespMsg,
time.Now().
Add(15*time.Minute).
Format("2006-01-02T15:04:05Z"))))
if err := r.ParseForm(); err != nil {
w.WriteHeader(500)
return
}

form := r.Form

switch form.Get("Action") {
case "AssumeRole":
w.Write([]byte(fmt.Sprintf(
assumeRoleRespMsg,
time.Now().
Add(15*time.Minute).
Format("2006-01-02T15:04:05Z"))))
return
case "AssumeRoleWithWebIdentity":
w.Write([]byte(fmt.Sprintf(assumeRoleWithWebIdentityResponse,
time.Now().
Add(15*time.Minute).
Format("2006-01-02T15:04:05Z"))))
return
default:
w.WriteHeader(404)
return
}
}))

ssoServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -310,6 +330,27 @@ func TestSharedConfigCredentialSource(t *testing.T) {
return func() {}, nil
},
},
"sso mixed with credential process provider": {
envProfile: "sso_mixed_credproc",
expectedAccessKey: "SSO_AKID",
expectedSecretKey: "SSO_SECRET_KEY",
expectedSessionToken: "SSO_SESSION_TOKEN",
init: func() (func(), error) {
return ssoTestSetup()
},
},
"sso mixed with web identity token provider": {
envProfile: "sso_mixed_webident",
expectedAccessKey: "WEB_IDENTITY_AKID",
expectedSecretKey: "WEB_IDENTITY_SECRET",
expectedSessionToken: "WEB_IDENTITY_SESSION_TOKEN",
},
"web identity": {
envProfile: "webident",
expectedAccessKey: "WEB_IDENTITY_AKID",
expectedSecretKey: "WEB_IDENTITY_SECRET",
expectedSessionToken: "WEB_IDENTITY_SESSION_TOKEN",
},
}

for name, c := range cases {
Expand Down
8 changes: 4 additions & 4 deletions config/shared_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,6 @@ func LoadSharedConfigProfile(ctx context.Context, profile string, optFns ...func
return SharedConfig{}, err
}

// profile should be lower-cased to standardize
profile = strings.ToLower(profile)

cfg := SharedConfig{}
profiles := map[string]struct{}{}
if err = cfg.setFromIniSections(profiles, profile, configSections, option.Logger); err != nil {
Expand Down Expand Up @@ -915,7 +912,6 @@ func (c *SharedConfig) validateCredentialType() error {
len(c.CredentialSource) != 0,
len(c.CredentialProcess) != 0,
len(c.WebIdentityTokenFile) != 0,
c.hasSSOConfiguration(),
) {
return fmt.Errorf("only one credential type may be specified per profile: source profile, credential source, credential process, web identity token, or sso")
}
Expand Down Expand Up @@ -993,6 +989,10 @@ func (c *SharedConfig) clearCredentialOptions() {
c.CredentialProcess = ""
c.WebIdentityTokenFile = ""
c.Credentials = aws.Credentials{}
c.SSOAccountID = ""
c.SSORegion = ""
c.SSORoleName = ""
c.SSOStartURL = ""
}

// SharedConfigLoadError is an error for the shared config file failed to load.
Expand Down
46 changes: 45 additions & 1 deletion config/shared_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,38 @@ func TestNewSharedConfig(t *testing.T) {
"Assume Role with AWS SSO Configuration and Source Profile": {
Filenames: []string{testConfigFilename},
Profile: "source_sso_and_assume",
Err: fmt.Errorf("only one credential type may be specified per profile"),
Expected: SharedConfig{
Profile: "source_sso_and_assume",
RoleARN: "source_sso_and_assume_arn",
SourceProfileName: "sso_and_assume",
Source: &SharedConfig{
Profile: "sso_and_assume",
RoleARN: "sso_with_assume_role_arn",
SourceProfileName: "multiple_assume_role_with_credential_source",
Source: &SharedConfig{
Profile: "multiple_assume_role_with_credential_source",
RoleARN: "multiple_assume_role_with_credential_source_role_arn",
SourceProfileName: "assume_role_with_credential_source",
Source: &SharedConfig{
Profile: "assume_role_with_credential_source",
RoleARN: "assume_role_with_credential_source_role_arn",
CredentialSource: credSourceEc2Metadata,
},
},
},
},
},
"SSO Mixed with Additional Credential Providrer": {
Filenames: []string{testConfigFilename},
Profile: "sso_mixed_credproc",
Expected: SharedConfig{
Profile: "sso_mixed_credproc",
SSOAccountID: "012345678901",
SSORegion: "us-west-2",
SSORoleName: "TestRole",
SSOStartURL: "https://127.0.0.1/start",
CredentialProcess: "/path/to/process",
},
},
}

Expand Down Expand Up @@ -1011,3 +1042,16 @@ func TestSharedConfigLoading(t *testing.T) {
})
}
}

// TestGitHubIssue1204 https://github.com/aws/aws-sdk-go-v2/issues/1204
func TestGitHubIssue1204(t *testing.T) {
_, err := LoadDefaultConfig(
context.TODO(),
WithSharedConfigProfile("default"),
WithSharedConfigFiles([]string{filepath.Join("testdata", "gh1204_config")}),
WithSharedCredentialsFiles([]string{filepath.Join("testdata", "gh1204_credentials")}),
)
if err != nil {
t.Errorf("expect no error, got %v", err)
}
}
22 changes: 22 additions & 0 deletions config/shared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,28 @@ const assumeRoleRespMsg = `
</AssumeRoleResponse>
`

var assumeRoleWithWebIdentityResponse = `<AssumeRoleWithWebIdentityResponse xmlns="https://sts.amazonaws.com/doc/2011-06-15/">
<AssumeRoleWithWebIdentityResult>
<SubjectFromWebIdentityToken>amzn1.account.AF6RHO7KZU5XRVQJGXK6HB56KR2A</SubjectFromWebIdentityToken>
<Audience>client.5498841531868486423.1548@apps.example.com</Audience>
<AssumedRoleUser>
<Arn>arn:aws:sts::123456789012:assumed-role/FederatedWebIdentityRole/app1</Arn>
<AssumedRoleId>AROACLKWSDQRAOEXAMPLE:app1</AssumedRoleId>
</AssumedRoleUser>
<Credentials>
<AccessKeyId>WEB_IDENTITY_AKID</AccessKeyId>
<SecretAccessKey>WEB_IDENTITY_SECRET</SecretAccessKey>
<SessionToken>WEB_IDENTITY_SESSION_TOKEN</SessionToken>
<Expiration>%s</Expiration>
</Credentials>
<Provider>www.amazon.com</Provider>
</AssumeRoleWithWebIdentityResult>
<ResponseMetadata>
<RequestId>request-id</RequestId>
</ResponseMetadata>
</AssumeRoleWithWebIdentityResponse>
`

const getRoleCredentialsResponse = `{
"roleCredentials": {
"accessKeyId": "SSO_AKID",
Expand Down
19 changes: 19 additions & 0 deletions config/testdata/config_source_shared
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,22 @@ sso_start_url = https://THIS_SHOULD_NOT_BE_IN_TESTDATA_CACHE/start
[profile sso_invalid]
sso_account_id = 012345678901
sso_role_name = TestRole

[profile sso_mixed_credproc]
sso_account_id = 012345678901
sso_region = us-west-2
sso_role_name = TestRole
sso_start_url = https://127.0.0.1/start
credential_process = cat ./testdata/test_json.json

[profile sso_mixed_webident]
web_identity_token_file = ./testdata/wit.txt
role_arn = sso_mixed_webident_arn
sso_account_id = 012345678901
sso_region = us-west-2
sso_role_name = TestRole
sso_start_url = https://127.0.0.1/start

[profile webident]
web_identity_token_file = ./testdata/wit.txt
role_arn = webident_arn
19 changes: 19 additions & 0 deletions config/testdata/config_source_shared_for_windows
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,22 @@ credential_process = type .\testdata\test_json.json
[profile chained_cred_proc]
role_arn = assume_role_w_creds_proc_source_prof
source_profile = cred_proc_no_arn_set

[profile sso_mixed_credproc]
sso_account_id = 012345678901
sso_region = us-west-2
sso_role_name = TestRole
sso_start_url = https://127.0.0.1/start
credential_process = type .\testdata\test_json.json

[profile sso_mixed_webident]
web_identity_token_file = .\testdata\wit.txt
role_arn = sso_mixed_webident_arn
sso_account_id = 012345678901
sso_region = us-west-2
sso_role_name = TestRole
sso_start_url = https://127.0.0.1/start

[profile webident]
web_identity_token_file = .\testdata\wit.txt
role_arn = webident_arn
14 changes: 14 additions & 0 deletions config/testdata/gh1204_config
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[default]
region=us-west-2
external_id=FooBar
role_session_name=FooSessionName
role_arn=foofoofoofoofoofoofoofoofoofoofoofoofoofoo
duration_seconds = 43200
source_profile=A

[profile A]
external_id=AExternalID
role_session_name=ASessionName
role_arn=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
duration_seconds = 43210
source_profile=B
9 changes: 9 additions & 0 deletions config/testdata/gh1204_credentials
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[A]
aws_access_key_id=A_PROFILE_ACCESS_KEY_ID
aws_secret_access_key=A_PROFILE_SECRET_ACCESS_KEY
aws_session_token=A_PROFILE_SESSION_TOKEN

[B]
aws_access_key_id=B_PROFILE_ACCESS_KEY_ID
aws_secret_access_key=B_PROFILE_SECRET_ACCESS_KEY
aws_session_token=B_PROFILE_SESSION_TOKEN
7 changes: 7 additions & 0 deletions config/testdata/shared_config
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,10 @@ source_profile = multiple_assume_role_with_credential_source
[profile source_sso_and_assume]
role_arn = source_sso_and_assume_arn
source_profile = sso_and_assume

[profile sso_mixed_credproc]
sso_account_id = 012345678901
sso_region = us-west-2
sso_role_name = TestRole
sso_start_url = https://127.0.0.1/start
credential_process = /path/to/process
1 change: 1 addition & 0 deletions config/testdata/wit.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
YXdzIHNkayBmb3IgZ28gd2ViIGlkZW50aXR5IHRva2Vu
2 changes: 0 additions & 2 deletions internal/ini/visitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ func (v *DefaultVisitor) VisitStatement(stmt AST) error {
name = names[0] + " " + strings.TrimLeft(names[1], " ")
}

// lower casing name to handle duplicates correctly.
name = strings.ToLower(name)
// attach profile name on section
if !v.Sections.HasSection(name) {
v.Sections.container[name] = NewSection(name)
Expand Down
3 changes: 0 additions & 3 deletions internal/ini/walker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ func TestValidDataFiles(t *testing.T) {
}

for profile, tableIface := range e {
// standardize by lower casing
profile = strings.ToLower(profile)

p, ok := v.Sections.GetSection(profile)
if !ok {
t.Fatal("could not find profile " + profile)
Expand Down

0 comments on commit c47c648

Please sign in to comment.