-
Notifications
You must be signed in to change notification settings - Fork 825
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add support for AWS SSO profiles #549
Conversation
Co-authored-by: Rickard Löfström <rickard.lofstrom@gmail.com>
vault/ssorolecredentialsprovider.go
Outdated
|
||
for { | ||
// Sleep to allow the user to complete the login flow | ||
time.Sleep(3 * time.Second) |
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 wonder about this delay, should it be configurable? I am not very fast on the keyboard, it could definitely take me longer than 3 seconds to enter my credentials in AWS SSO via my 3rd party IdP, including finding my MFA device and completing the authentication. Or is it that the user should already be logged into the browser ahead of time and this time is for clicking a single approve button?
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.
Just to clarify, retries every 3 seconds. If you have a SSO session against your IDP, it might not require you to re-authenticate with the MFA, so waiting longer might be annoying as it’s almost instant after the first initial authentication.
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 did some digging around to see how the AWS CLI does this, and ended up here: https://github.com/boto/botocore/blob/v2/botocore/utils.py#L1663
And from their comments around the default retry interval and slow down delay:
https://tools.ietf.org/html/draft-ietf-oauth-device-flow-15#section-3.5
I.e., we (like the AWS CLI) should just be using the interval that is specified in the response from the API when calling StartDeviceAuthorization
, and fall back to the 5s defaults defined by the RFC. d7ce500 implements this, and also adds handling for the SlowDownException
error code.
From my testing, this has effectively reduced the retry interval to 1s, which makes AWS Vault more snappy to respond after I had completed the browser login.
I've tested this branch with my AWS SSO configured to use a 3rd Party IdP (Google Apps SAML), and it works well so far 😍 |
Hi @mtibben! Sorry to ping you like this; I was wondering when you might have time to look at this PR (since you seem to be the one maintaining |
vault/ssorolecredentialsprovider.go
Outdated
return nil, err | ||
} | ||
|
||
fmt.Printf(authorizationTemplate, aws.StringValue(auth.VerificationUriComplete)) |
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.
If we dump this to stdout with fmt.Printf
it may interfere with the output of whatever command you're executing... can we get this output to stderr, preferably through kingpin somehow? Alternatively should we be using the prompt
driver to achieve this?
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.
Great catch! Since prompt
is only used for requesting user input (which we are not doing here), and kingpin
is not currently used anywhere outside the cli/
page right now, I think it makes most sense to use Fprintf(os.Stderr, ...)
here.
|
||
expiration := aws.MillisecondsTimeValue(resp.RoleCredentials.Expiration) | ||
|
||
// This is needed because sessions.Store expects a sts.Credentials object. |
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.
yes I'm hoping to refactor sessions.Store completely and turn it into an ordinary Provider
} | ||
return &SSOAccessToken{ | ||
Token: aws.StringValue(t.AccessToken), | ||
Expiration: time.Now().Add(time.Duration(aws.Int64Value(t.ExpiresIn)) * time.Second), |
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.
Can we break this up to make it more readable?
@@ -222,6 +222,25 @@ role_arn = arn:aws:iam::123456789012:role/target | |||
|
|||
You can also set the `mfa_serial` with the environment variable `AWS_MFA_SERIAL`. | |||
|
|||
## AWS Single Sign-On (AWS SSO) |
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.
Add this to the TOC at the top
) | ||
|
||
// CachedSSORoleCredentialsProvider uses the keyring to cache SSO Role sessions. | ||
type CachedSSORoleCredentialsProvider struct { |
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.
Hmm I was planning a sessions refactor, but it may need to encompass this. Mind if I ping you if I get to this?
retryInterval = time.Duration(i) * time.Second | ||
} | ||
|
||
for { |
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.
Would it be possible to capture the retry logic in its own separate function? There's an example of this in
Line 149 in 877981b
func retry(maxTime time.Duration, sleep time.Duration, f func() error) (err error) { |
This is fantastic, thank you @itsdalmo! I know from experience that testing this stuff is really really hard as you basically have to mock out everything. But if there are any simple tests you can write to prevent me breaking this going forward that would be ideal |
Co-authored-by: Kristian Dalmo Olsen <kristian@doingit.no>
Does anyone have this working with edit: I've been able to get this to work with credentials process and terraform by having two profiles: [profile xyz]
region=us-east-1
credential_process=aws-vault exec --json xyz-sso
[profile xyz-sso]
sso_start_url = https://someone.awsapps.com/start
sso_region = us-east-1
sso_account_id = 045464924556
sso_role_name = TheRole
region = us-east-1 |
This PR hopefully closes #449 by adding support for AWS SSO.
Given a AWS SSO Profile configuration in
~/.aws/confg
:This PR allows
aws-vault
to obtain short lived credentials for an SSO profile, which can be used by e.g.exec
(andlogin
), and produces output as seen below:In the above example,
aws-vault
would take care of:sso_start_url
in your config).exec
command.The client credentials (ID and secret), access token and session credentials will all be cached in the Keyring, and automatically refreshed (by repeating the relevant steps above) when they expire. The AWS SSO Client is valid for 3 months by default, while the access token and session expiration depends on the settings for your AWS SSO Organization.
Please take a look and let us know what you think 😅