From 3d79564f49870ccdce722ed337245265b237872b Mon Sep 17 00:00:00 2001 From: Isaiah Vita <82135527+isaiahvita@users.noreply.github.com> Date: Thu, 10 Nov 2022 17:22:07 -0800 Subject: [PATCH] Add token refresh support to SSOCredentialProvider (#1903) 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) --- .../2f60f375046b466789bff94b7406ba45.json | 9 ++ .../e5c705b5a4f34a99a85f12cee232ae51.json | 9 ++ config/resolve_bearer_token.go | 15 +-- config/resolve_bearer_token_test.go | 2 +- config/resolve_credentials.go | 26 +++- config/resolve_credentials_test.go | 8 ++ config/shared_config.go | 124 +++++++++++------- config/shared_config_test.go | 16 +++ config/testdata/config_source_shared | 19 +++ config/testdata/shared_config | 11 ++ .../ssocreds/sso_credentials_provider.go | 42 ++++-- 11 files changed, 209 insertions(+), 72 deletions(-) create mode 100644 .changelog/2f60f375046b466789bff94b7406ba45.json create mode 100644 .changelog/e5c705b5a4f34a99a85f12cee232ae51.json diff --git a/.changelog/2f60f375046b466789bff94b7406ba45.json b/.changelog/2f60f375046b466789bff94b7406ba45.json new file mode 100644 index 00000000000..37f99e8c794 --- /dev/null +++ b/.changelog/2f60f375046b466789bff94b7406ba45.json @@ -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" + ] +} \ No newline at end of file diff --git a/.changelog/e5c705b5a4f34a99a85f12cee232ae51.json b/.changelog/e5c705b5a4f34a99a85f12cee232ae51.json new file mode 100644 index 00000000000..471c7be585a --- /dev/null +++ b/.changelog/e5c705b5a4f34a99a85f12cee232ae51.json @@ -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" + ] +} \ No newline at end of file diff --git a/config/resolve_bearer_token.go b/config/resolve_bearer_token.go index ae5fb27bd7f..a8ebb3c0a39 100644 --- a/config/resolve_bearer_token.go +++ b/config/resolve_bearer_token.go @@ -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 { diff --git a/config/resolve_bearer_token_test.go b/config/resolve_bearer_token_test.go index ee3567ca69a..0d2d0a7fb45 100644 --- a/config/resolve_bearer_token_test.go +++ b/config/resolve_bearer_token_test.go @@ -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, diff --git a/config/resolve_credentials.go b/config/resolve_credentials.go index 28705f47fb0..1bb6addf3a1 100644 --- a/config/resolve_credentials.go +++ b/config/resolve_credentials.go @@ -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" ) @@ -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...) diff --git a/config/resolve_credentials_test.go b/config/resolve_credentials_test.go index 7111251aa27..f099594c77d 100644 --- a/config/resolve_credentials_test.go +++ b/config/resolve_credentials_test.go @@ -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", diff --git a/config/shared_config.go b/config/shared_config.go index 48aa7a8cf0b..c23ca9a2697 100644 --- a/config/shared_config.go +++ b/config/shared_config.go @@ -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. @@ -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 } @@ -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. @@ -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 { @@ -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 } @@ -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() { diff --git a/config/shared_config_test.go b/config/shared_config_test.go index b0c628bee34..8b35b23f405 100644 --- a/config/shared_config_test.go +++ b/config/shared_config_test.go @@ -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}, diff --git a/config/testdata/config_source_shared b/config/testdata/config_source_shared index 0de4dc6dee5..10bc5c95a0d 100644 --- a/config/testdata/config_source_shared +++ b/config/testdata/config_source_shared @@ -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 diff --git a/config/testdata/shared_config b/config/testdata/shared_config index 0680a759dab..27284b49573 100644 --- a/config/testdata/shared_config +++ b/config/testdata/shared_config @@ -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 diff --git a/credentials/ssocreds/sso_credentials_provider.go b/credentials/ssocreds/sso_credentials_provider.go index bd7603bbc4c..b3cf7853e76 100644 --- a/credentials/ssocreds/sso_credentials_provider.go +++ b/credentials/ssocreds/sso_credentials_provider.go @@ -45,6 +45,10 @@ type Options struct { // If custom cached token filepath is used, the Provider's startUrl // parameter will be ignored. CachedTokenFilepath string + + // Used by the SSOCredentialProvider if a token configuration + // profile is used in the shared config + SSOTokenProvider *SSOTokenProvider } // Provider is an AWS credential provider that retrieves temporary AWS @@ -78,27 +82,39 @@ func New(client GetRoleCredentialsAPIClient, accountID, roleName, startURL strin // Retrieve retrieves temporary AWS credentials from the configured Amazon // Single Sign-On (AWS SSO) user portal by exchanging the accessToken present -// in ~/.aws/sso/cache. +// in ~/.aws/sso/cache. However, if a token provider configuration exists +// in the shared config, then we ought to use the token provider rather then +// direct access on the cached token. func (p *Provider) Retrieve(ctx context.Context) (aws.Credentials, error) { - if p.cachedTokenFilepath == "" { - cachedTokenFilepath, err := StandardCachedTokenFilepath(p.options.StartURL) + var accessToken *string + if p.options.SSOTokenProvider != nil { + token, err := p.options.SSOTokenProvider.RetrieveBearerToken(ctx) if err != nil { - return aws.Credentials{}, &InvalidTokenError{Err: err} + return aws.Credentials{}, err + } + accessToken = &token.Value + } else { + if p.cachedTokenFilepath == "" { + cachedTokenFilepath, err := StandardCachedTokenFilepath(p.options.StartURL) + if err != nil { + return aws.Credentials{}, &InvalidTokenError{Err: err} + } + p.cachedTokenFilepath = cachedTokenFilepath } - p.cachedTokenFilepath = cachedTokenFilepath - } - tokenFile, err := loadCachedToken(p.cachedTokenFilepath) - if err != nil { - return aws.Credentials{}, &InvalidTokenError{Err: err} - } + tokenFile, err := loadCachedToken(p.cachedTokenFilepath) + if err != nil { + return aws.Credentials{}, &InvalidTokenError{Err: err} + } - if tokenFile.ExpiresAt == nil || sdk.NowTime().After(time.Time(*tokenFile.ExpiresAt)) { - return aws.Credentials{}, &InvalidTokenError{} + if tokenFile.ExpiresAt == nil || sdk.NowTime().After(time.Time(*tokenFile.ExpiresAt)) { + return aws.Credentials{}, &InvalidTokenError{} + } + accessToken = &tokenFile.AccessToken } output, err := p.options.Client.GetRoleCredentials(ctx, &sso.GetRoleCredentialsInput{ - AccessToken: &tokenFile.AccessToken, + AccessToken: accessToken, AccountId: &p.options.AccountID, RoleName: &p.options.RoleName, })