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 sso auto run #847
Add sso auto run #847
Conversation
Merge from Parent
… to determine if new creds are needed and automatically open the browser page
Re adding you for review as I corrupt my previous PR when fixing my email in commits |
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.
Hey @dbmurphy , thanks for re-opening a PR after the history got a bit funky!
I agree with the idea of making AWS SSO easier to use, especially when external authentication is required and timeouts require re-authentication outside of Steampipe.
I was concerned at first about requiring the use of the AWS CLI for AWS SSO directly in the plugin, but this seems reasonable based on the fact that a user needs to run aws sso login
to use SSO anyway.
However, I am more worried about if/how we can detect if the user intends to use SSO correctly, as we don't want to try to run AWS CLI SSO commands if a user is attempting to authenticate through a profile not using SSO, an assume role session, or any other supported form.
Do you have any results from trying these changes with the other authentication methods supported in this plugin? How does the awsConfig
data come through in those cases?
Another idea would be to introduce a capability to Steampipe that would allow pre- or post-hooks (similar to npm hooks) to be called before/after creating connections. Then in a pre-hook, you could call a script that runs the necessary AWS SSO login commands before attempting to create the AWS connection.
Would something like this capability be useful to you in your workflow?
@@ -1737,6 +1739,10 @@ func getSessionWithMaxRetries(ctx context.Context, d *plugin.QueryData, region s | |||
|
|||
if awsConfig.Profile != nil { | |||
sessionOptions.Profile = *awsConfig.Profile | |||
// When profile is set but not access or secret key, logically this means it will be SSO login |
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.
@dbmurphy Is this always true? If I have a connection using my default
profile (or any other named profile):
connection "aws" {
plugin = "aws"
regions = ["us-east-1"]
}
Then have my ~/.aws/credentials
file setup so my default
profile uses an AWS access key pair:
[default]
aws_access_key_id = AKIA...
aws_secret_access_key = cD+J...
The awsConfig.AccessKey
and awsConfig.SecretKey
values will still be nil
, as they're not explicitly defined in the config file. Similarly, there are a few other ways you can authenticate with the AWS plugin that do not require an access or secret key explicitly set in the Steampipe AWS config file, like using AssumeRole creds, AWS Vault creds, and EC2 instance profile creds. There are a few examples of each sample connection configuration from https://hub.steampipe.io/plugins/turbot/aws.
Have you tried this code while using any of the above authentication methods?
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.
@dbmurphy Is this always true? If I have a connection using my
default
profile (or any other named profile):connection "aws" { plugin = "aws" regions = ["us-east-1"] }
Then have my
~/.aws/credentials
file setup so mydefault
profile uses an AWS access key pair:[default] aws_access_key_id = AKIA... aws_secret_access_key = cD+J...
The
awsConfig.AccessKey
andawsConfig.SecretKey
values will still benil
, as they're not explicitly defined in the config file. Similarly, there are a few other ways you can authenticate with the AWS plugin that do not require an access or secret key explicitly set in the Steampipe AWS config file, like using AssumeRole creds, AWS Vault creds, and EC2 instance profile creds. There are a few examples of each sample connection configuration from https://hub.steampipe.io/plugins/turbot/aws.Have you tried this code while using any of the above authentication methods?
I have tried some but not all, this is rather complicated, and why I made mention that the auth system really needs some TLC, so we can do things like
if isSSO
if isKey
...
However this means you would need to refactor the Env, SPC, and .aws layer to get a materialized view of what the SDK itself would see. In the current form of this code, it's possible if you had a key type auth and it tried this is would call sts and sso butt hey would fail as no SSO URL is defined in the .aws/config. In this case, it would simply continue and the SDK would still use the key/secret you have provided.
As such, I feel this is a solution that works but could be made cleaner, but such clean is a major refactor of the auth code. Similar to how it should only call SSO login/sts checks once but as steampipe does not have a hierarchical order and all copies of the AWS plugin as started at the same time it calls N open browser or log prints for authorizing in your SSO system.
// checkAWSCallerIdent returns boolean if currently logged in to AWS CLI | ||
func checkAWSCallerIdent(ctx context.Context, profile string) bool { | ||
plugin.Logger(ctx).Trace("getSessionWithMaxRetries", "checkAWSCallerIdent", "Starting for "+profile) | ||
commandInput := strings.Fields("aws sts get-caller-identity --profile " + profile) |
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 the user doesn't have the proper STS permissions to run this particular command, do you know if aws sso login
would still work as expected? I'm concerned that if a user has set themselves up to use aws sso login
but not any STS commands, this command would fail.
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 you don't have St's permission the command will false return false and the code would assume you logger out as it can't verify it. Given STS is the AWS best practice way to check if your logged in. I think you trying to code for a bad practice that is going against aws standards when you ask if STS permissions are missing.
I've got this branch to successfully use But this was after running
On WSL2, this hack (aws/aws-cli#5301) enables the same protocol.
I thought maybe the old Lynx char-mode browser could work, but evidently not. So I'm wondering if, in this situation, we'd still need to tell the Steampipe user to do |
This one confuses me, when the STS failed it should have called the SSO login for you, which would have printed the URL you needed to open a browser with into the log. Where you could grab it and open a browser to authorize the API request via SSO. Offhand I am not sure the best way to detect if browser redirection would work or not, as this is controlled by awscli itself not this plugin. I don't think we would want to print the AWScli message in call cases just when an open browser isn't supported on a system. @judell Is there a programmatic way to predict with your ENV settings that this would not open a browser so we print the info to console vs just the log? I don't think the user would need to call aws sso login themselves but they would need to use the URL that it output in another system which does have a browser they can put the awscli provided link into and click authorize. |
The plot thickens. Yesterday I was still dealing with confusion about using vs not using explicit credentials in Here let's just focus on what the Having done that manually yesterday, I had to wait 12 hours for the creds to expire in order to do a fresh test. (I lack permissions to revoke them.) Today, on an EC2 Linux instance, here is the situation. ~/.aws/config
$ aws sts get-caller-identity --profile SSO-ReadOnly-605...981 The SSO session associated with this profile has expired or is otherwise invalid. To refresh this SSO session run aws sso login with the corresponding profile. $ steampipe query Welcome to Steampipe v0.11.0 Plugin log: Empty Database log: 2022-01-07T17:56:50.392Z [WARN] hub: stream receive error rpc error: code = Unknown desc = SSOProviderInvalidToken: the SSO session has expired or is invalid (0xc000394630) That's true for the stock client and the This suggests to me that the message might instead be: SSO session has expired or is invalid. Please run But I think that should be the responsibility of the plugin, not the plugin SDK? Anyway, before speculating further, it would be good to know if others can replicate this scenario. It's awkward for me to test because I have to wait for creds to expire after every login, but presumably others who can use https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_use_revoke-sessions.html can do so more easily. |
So the real issue is at the aws SDK layer as the stored cred prevent aws SSO login from being useful. However until a materialised view of the aws SDK (not steampipe aws itself) parsing of all config input would make it hard to predict this case from the SPC config alone. For example this same error would me we should call aws SSO login to generate new creds if they were not hard coded. I would question how much golang aws SDK logic would need to be recreated in the aws steampipe plugin to detect all these edge cases. Is that really someone that can be kept up with? Maybe we should expose a allowSSOAutoLogin as a bool in the SPC? |
Let's recap. The title of the original issue was: "app appears to hang when credentials have expired" For me, now that I've removed explicit credentials from my Others who reported the same long delay are @psolomon-opploans and (I think?) @dbmurphy. Do we all agree that Steampipe now promptly reports the above error? If yes, then we can talk about whether/how to streamline the required use of |
My issue is a bit different I have 15 accounts spread over 3 aggregators. I wanted a way to trigger the aws SSO automatically. Which this plugin improvement does. The re Auth solution was along for the ride. |
✔️ |
The AWS SDK leaves ALOT to be desired when it comes to auth detection types, or exposed the parsed .aws/config options however there is progress...
I am still looking into how I would manually trigger a SSO login rather than calling runAWSCLISSOLogin, so that I could better detect if the SSOIDC is failing to redirect to the browser. This is a rather complicated bit of code, and I wonder if it's not worth the error as your key was in the cred file and would have skipped the STS check anyhow. The code logic path I am going down currently is
My argument here is the last item is an edge of an edge of an edge, is it reasonable to chase this when there was never a candidate for "auto sso" anyhow? @cbruno10 @rajlearner17 @judell , What are your thoughts on this? |
@dbmurphy I apologize for the lack of communication on this issue - it seems to have slipped through the cracks. Fundamentally, we would prefer not to have plugins launching browsers for user input; Plugins are essentially server elements that run on the server side. They may or may not be running on the same machine as the steampipe client, and when running as a service may or may not be in your user session. I did some testing of the PR code, and found some other challenges we would need to iron out as well:
I believe that to really provide good SSO support (as well as MFA support and other related authentication items), we would need to rethink and significantly refactor the auth code in the aws plugin and how plugins interact with steampipe for user input, and I don't think that is currently a priority for us. Our current thinking is adding 'hooks' to let users to call their own scripts / executables when certain lifecycle events occur - when steampipe starts, when a connection is created, etc. This would allow you to call your own script when steampipe loads a connection. This is much simpler on our end, but also more flexible. What are your thoughts on such an approach? Would it be sufficient for your needs? |
This makes your product unusable for a general user to not hand off the web browser call to aws cli which doesn't this for you.
I mentioned this before, this is because steampipe calls all 10 plugin copies in parrellel at the same time. The only way to fix this would be to have plugin factory pre executor that would be called before any of the plugin inits occur so you could run the SSO setup.
I do not get this behavior so maybe it's a aws cli setting rather than a plugin on?
I think this would need as mentioned earlier for me to be able to register that the aws plugin depends on my aws_sso_setup plugin tondo this. I don't think such a facility exists today for chaining plugin startuos does it? |
Consider an AWS connection configured (roughly) like this:
The idea being that you could call any script you like on plugin startup ... including to login via SSO. That allows you to manage the process in a flexible way, but avoids dependencies within steampipe itself. The other There are challenges here for use with mullti-account aggregators as well - we want to ensure the SSO setup is only called once. Perhaps we should also support hooks at the Steampipe application level? |
Could we do this hook on an aggregator so it runs before the non aggregator plugins do? If so then the normal plugins could have the hook and the aggregator also has it. The function uses sts checks to see if it needs a logon, which it wouldn't after the aggregator one had setup the connection. Also the plugin level hook would let any use the aggregator or plugin retrigger loggin if a invalid sts was detected. |
Hey @dbmurphy , I'll be closing PR as we'll be looking to offer the functionality discussed in this PR through hooks, which currently have an open feature request in turbot/steampipe#1426. This PR will still be available for reference, but for any future discussion on hooks, including use cases for certain plugins (like this one), feel free to post in the aforementioned Steampipe issue. |
replace #839 which needed to be closed as changing commit email caused history issues
Integration test logs
Logs
Not sure how you want to show this one, as it's very behavioral with SSO but here is the trace from the pluginUpdated the above code to better protect against a nil awsConfig.Profile entry in some cases.
Additionally, I added a bool var we may want to expose in the awsConfig struct which is "isSSO".
I did not include it, but it could make things easier code wise if we had something like: