Skip to content

Commit

Permalink
Add token refresh support to SSOCredentialProvider (#1903)
Browse files Browse the repository at this point in the history
This commit adds support for token refresh to the SSOCredentialProvider (by using the SSOTokenProvider) while maintaining support for legacy SSO configurations (i.e. SSO configurations that do not use an sso-session but specify sso fields directly in a profile section of a shared config file)
  • Loading branch information
isaiahvita committed Nov 11, 2022
1 parent 0966539 commit 3d79564
Show file tree
Hide file tree
Showing 11 changed files with 209 additions and 72 deletions.
9 changes: 9 additions & 0 deletions .changelog/2f60f375046b466789bff94b7406ba45.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"id": "2f60f375-046b-4667-89bf-f94b7406ba45",
"type": "feature",
"description": "Adds token refresh support (via SSOTokenProvider) when using the SSOCredentialProvider",
"modules": [
"config",
"credentials"
]
}
9 changes: 9 additions & 0 deletions .changelog/e5c705b5a4f34a99a85f12cee232ae51.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"id": "e5c705b5-a4f3-4a99-a85f-12cee232ae51",
"type": "announcement",
"description": "When using the SSOTokenProvider, a previous implementation incorrectly compensated for invalid SSOTokenProvider configurations in the shared profile. This has been fixed via PR #1903 and tracked in issue #1846",
"modules": [
"config",
"credentials"
]
}
15 changes: 2 additions & 13 deletions config/resolve_bearer_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,9 @@ func resolveBearerAuthTokenProviderChain(ctx context.Context, cfg *aws.Config, c

var provider smithybearer.TokenProvider

if sharedConfig.SSOSession != nil || (sharedConfig.SSORegion != "" && sharedConfig.SSOStartURL != "") {
ssoSession := sharedConfig.SSOSession
if ssoSession == nil {
// Fallback to legacy SSO session config parameters, if the
// sso-session section wasn't used.
ssoSession = &SSOSession{
Name: sharedConfig.SSOStartURL,
SSORegion: sharedConfig.SSORegion,
SSOStartURL: sharedConfig.SSOStartURL,
}
}

if sharedConfig.SSOSession != nil {
provider, err = resolveBearerAuthSSOTokenProvider(
ctx, cfg, ssoSession, configs)
ctx, cfg, sharedConfig.SSOSession, configs)
}

if err == nil && provider != nil {
Expand Down
2 changes: 1 addition & 1 deletion config/resolve_bearer_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestResolveBearerAuthToken(t *testing.T) {
SSOStartURL: "https://example.aws/start",
},
},
expectProvider: true,
expectProvider: false,
expectToken: smithybearer.Token{
Value: "access token",
CanExpire: true,
Expand Down
26 changes: 25 additions & 1 deletion config/resolve_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/aws/aws-sdk-go-v2/credentials/stscreds"
"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
"github.com/aws/aws-sdk-go-v2/service/sso"
"github.com/aws/aws-sdk-go-v2/service/ssooidc"
"github.com/aws/aws-sdk-go-v2/service/sts"
)

Expand Down Expand Up @@ -171,7 +172,30 @@ func resolveSSOCredentials(ctx context.Context, cfg *aws.Config, sharedConfig *S
}

cfgCopy := cfg.Copy()
cfgCopy.Region = sharedConfig.SSORegion

if sharedConfig.SSOSession != nil {
ssoTokenProviderOptionsFn, found, err := getSSOTokenProviderOptions(ctx, configs)
if err != nil {
return fmt.Errorf("failed to get SSOTokenProviderOptions from config sources, %w", err)
}
var optFns []func(*ssocreds.SSOTokenProviderOptions)
if found {
optFns = append(optFns, ssoTokenProviderOptionsFn)
}
cfgCopy.Region = sharedConfig.SSOSession.SSORegion
cachedPath, err := ssocreds.StandardCachedTokenFilepath(sharedConfig.SSOSession.Name)
if err != nil {
return err
}
oidcClient := ssooidc.NewFromConfig(cfgCopy)
tokenProvider := ssocreds.NewSSOTokenProvider(oidcClient, cachedPath, optFns...)
options = append(options, func(o *ssocreds.Options) {
o.SSOTokenProvider = tokenProvider
o.CachedTokenFilepath = cachedPath
})
} else {
cfgCopy.Region = sharedConfig.SSORegion
}

cfg.Credentials = ssocreds.New(sso.NewFromConfig(cfgCopy), sharedConfig.SSOAccountID, sharedConfig.SSORoleName, sharedConfig.SSOStartURL, options...)

Expand Down
8 changes: 8 additions & 0 deletions config/resolve_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,14 @@ func TestSharedConfigCredentialSource(t *testing.T) {
expectedSecretKey: "WEB_IDENTITY_SECRET",
expectedSessionToken: "WEB_IDENTITY_SESSION_TOKEN",
},
"SSO Session missing region": {
envProfile: "sso-session-missing-region",
expectedError: "profile \"sso-session-missing-region\" is configured to use SSO but is missing required configuration: sso_region",
},
"SSO Session mismatched region": {
envProfile: "sso-session-mismatched-region",
expectedError: "sso_region in profile \"sso-session-mismatched-region\" must match sso_region in sso-session",
},
"web identity": {
envProfile: "webident",
expectedAccessKey: "WEB_IDENTITY_AKID",
Expand Down
124 changes: 80 additions & 44 deletions config/shared_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,10 @@ type SSOSession struct {
SSOStartURL string
}

func (s *SSOSession) setFromIniSection(section ini.Section) error {
func (s *SSOSession) setFromIniSection(section ini.Section) {
updateString(&s.Name, section, ssoSessionNameKey)
updateString(&s.SSORegion, section, ssoRegionKey)
updateString(&s.SSOStartURL, section, ssoStartURLKey)

if s.SSORegion == "" || s.SSOStartURL == "" {
return fmt.Errorf(
"%v and %v are required parameters in sso-session section",
ssoRegionKey, ssoStartURLKey,
)
}

return nil
}

// SharedConfig represents the configuration fields of the SDK config files.
Expand Down Expand Up @@ -846,9 +838,8 @@ func (c *SharedConfig) setFromIniSections(profiles map[string]struct{}, profile
// profile only have credential provider options.
c.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 := c.validateCredentialsConfig(profile); err != nil {
return err
}
Expand Down Expand Up @@ -900,31 +891,20 @@ func (c *SharedConfig) setFromIniSections(profiles map[string]struct{}, profile
// 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 c.SSOSessionName != "" {
c.SSOSession, err = getSSOSession(c.SSOSessionName, sections, logger)
if err != nil {
return err
if c.hasSSOTokenProviderConfiguration() {
section, ok := sections.GetSection(ssoSectionPrefix + strings.TrimSpace(c.SSOSessionName))
if !ok {
return fmt.Errorf("failed to find SSO session section, %v", c.SSOSessionName)
}
var ssoSession SSOSession
ssoSession.setFromIniSection(section)
ssoSession.Name = c.SSOSessionName
c.SSOSession = &ssoSession
}

return nil
}

func getSSOSession(name string, sections ini.Sections, logger logging.Logger) (*SSOSession, error) {
section, ok := sections.GetSection(ssoSectionPrefix + strings.TrimSpace(name))
if !ok {
return nil, fmt.Errorf("failed to find SSO session section, %v", name)
}

var ssoSession SSOSession
if err := ssoSession.setFromIniSection(section); err != nil {
return nil, fmt.Errorf("failed to load SSO session %v, %w", name, err)
}
ssoSession.Name = name

return &ssoSession, nil
}

// setFromIniSection loads the configuration from the profile section defined in
// the provided INI file. A SharedConfig pointer type value is used so that
// multiple config file loadings can be chained.
Expand Down Expand Up @@ -1088,17 +1068,66 @@ func (c *SharedConfig) validateCredentialType() error {
len(c.CredentialProcess) != 0,
len(c.WebIdentityTokenFile) != 0,
) {
return fmt.Errorf("only one credential type may be specified per profile: source profile, credential source, credential process, web identity token, or sso")
return fmt.Errorf("only one credential type may be specified per profile: source profile, credential source, credential process, web identity token")
}

return nil
}

func (c *SharedConfig) validateSSOConfiguration() error {
if !c.hasSSOConfiguration() {
if c.hasSSOTokenProviderConfiguration() {
err := c.validateSSOTokenProviderConfiguration()
if err != nil {
return err
}
return nil
}

if c.hasLegacySSOConfiguration() {
err := c.validateLegacySSOConfiguration()
if err != nil {
return err
}
}
return nil
}

func (c *SharedConfig) validateSSOTokenProviderConfiguration() error {
var missing []string

if len(c.SSOSessionName) == 0 {
missing = append(missing, ssoSessionNameKey)
}

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, ssoStartURLKey)
}
}

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", ssoStartURLKey, c.Profile, ssoStartURLKey, ssoSectionPrefix)
}

return nil
}

func (c *SharedConfig) validateLegacySSOConfiguration() error {
var missing []string

if len(c.SSORegion) == 0 {
Expand All @@ -1109,11 +1138,18 @@ func (c *SharedConfig) validateSSOConfiguration() error {
missing = append(missing, ssoStartURLKey)
}

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
}

Expand All @@ -1133,15 +1169,15 @@ func (c *SharedConfig) hasCredentials() bool {
}

func (c *SharedConfig) hasSSOConfiguration() bool {
switch {
case len(c.SSOAccountID) != 0:
case len(c.SSORegion) != 0:
case len(c.SSORoleName) != 0:
case len(c.SSOStartURL) != 0:
default:
return false
}
return true
return c.hasSSOTokenProviderConfiguration() || c.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) clearAssumeRoleOptions() {
Expand Down
16 changes: 16 additions & 0 deletions config/shared_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,22 @@ func TestNewSharedConfig(t *testing.T) {
CredentialProcess: "/path/to/process",
},
},
"SSO Session success": {
ConfigFilenames: []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",
},
},
},
"profile names are case-sensitive (Mixed)": {
ConfigFilenames: []string{testConfigFilename},
CredentialsFilenames: []string{testCredentialsFilename},
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 @@ -76,6 +76,25 @@ sso_region = us-west-2
sso_role_name = TestRole
sso_start_url = https://127.0.0.1/start

[profile sso-session-missing-region]
region = us-east-1
sso_session = sso-session-missing-region-dev
sso_account_id = 123456789012
sso_role_name = testRole

[sso-session sso-session-missing-region-dev]
sso_start_url = https://d-123456789a.awsapps.com/start

[profile sso-session-mismatched-region]
sso_session = sso-session-mismatched-region-dev
sso_region = us-east-1
sso_account_id = 123456789012
sso_role_name = testRole

[sso-session sso-session-mismatched-region-dev]
sso_start_url = https://d-123456789a.awsapps.com/start
sso_region = us-west-2

[profile webident]
web_identity_token_file = ./testdata/wit.txt
role_arn = webident_arn
11 changes: 11 additions & 0 deletions config/testdata/shared_config
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,17 @@ sso_role_name = TestRole
sso_start_url = https://127.0.0.1/start
credential_process = /path/to/process

[profile sso-session-success]
region = us-east-1
sso_session = sso-session-success-dev
sso_account_id = 123456789012
sso_role_name = testRole

[sso-session sso-session-success-dev]
sso_region = us-east-1
sso_start_url = https://d-123456789a.awsapps.com/start
sso_registration_scopes = sso:account:access

[profile DoNotNormalize]
aws_access_key_id = DoNotNormalize_config_akid
aws_secret_access_key = DoNotNormalize_config_secret
Expand Down

0 comments on commit 3d79564

Please sign in to comment.