-
Notifications
You must be signed in to change notification settings - Fork 110
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
Add support for refreshable AWS SSO tokens #616
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrnorm
commented
Mar 1, 2024
go.mod
Outdated
@@ -18,9 +18,11 @@ require ( | |||
) | |||
|
|||
require ( | |||
github.com/alessio/shellescape v1.4.2 |
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.
I ran go mod tidy
and cleaned up go.mod
and go.sum
chrnorm
commented
Mar 1, 2024
} | ||
|
||
// Attempts to store the token, any errors will be logged to debug logging | ||
func (s *SSOTokensSecureStorage) StoreSSOToken(profileKey string, ssoTokenValue SSOToken) { | ||
err := s.SecureStorage.Store(profileKey, ssoTokenValue) | ||
if err != nil { | ||
clio.Debugf("%s\n", errors.Wrap(err, "writing sso token to credentials cache").Error()) | ||
clio.Debugf("writing sso token to credentials cache: %s", err.Error()) |
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.
I've fixed the syntax up here - the previous syntax was difficult to read
shwethaumashanker
approved these changes
Mar 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changed?
This PR adds opt-in support for the new IAM Identity Center refreshable token login flow. You can opt-in to using refreshable tokens by configuring your AWS profiles to include
granted_sso_registration_scopes
as follows:[profile example] granted_sso_account_id = 123456789012 granted_sso_role_name = AWSAdministratorAccess + granted_sso_registration_scopes = sso:account:access granted_sso_start_url = https://commonfate.awsapps.com/start granted_sso_region = ap-southeast-2 credential_process = granted credential-process --profile example
At the moment, support for SSO registration scopes has been implemented only for profiles using the Granted Credential Process configuration.
Some background here; the native AWS CLI way of configuring this is by using the
sso-session
entry as follows:I would like Granted to support this configuration too, however the AWS v2 Go SDK does not yet support reading
sso_registration_scopes
. So the initial implementation here just usesgranted_sso_registration_scopes
.In future it's likely we'll make the refreshable token flow the default, but for now this behaviour is opt-in.
Additionally, this PR tidies up some of our AWS SSO login flow code. In the Granted codebase the
cfaws
package has grown to be quite large. I've moved the SSO login flow code out of this package into a newidclogin
package.Why?
Fixes #615.
How did you test it?
Tested and verified on MacOS by manually logging in a few times to verify the functionality was correct. I tested the refresh flow by manipulating the
Expiry
value of the token inside my keychain.I also tested the case where the refresh token is invalid, by overriding the
refreshToken
value inside my keychain and overriding theExpiry
to force a refresh. The behaviour here is that an error is logged to stderr and the user is prompted to reauthenticate, which is what we want.Given that no platform-specific keychain functionality has been modified I expect this to work across other OSes too.
Potential risks