Provide support for AWS credentials obtained through SSO
mxro opened this issue · 13 comments
While #3 will provide a way to consume credentials from ~/.aws/credentials
this will probably not work for users logging in through SSO:
There is a node library that allows consuming these credentials:
https://github.com/ryansonshine/aws-sso-creds-helper/blob/main/src/sso-creds.ts#L48
However, it seems never versions of the JavaScript SDK also support using these credentials directly. However, I think that would require a major upgrade of the AWS SDK used by Goldstack:
See fromSSO on https://www.npmjs.com/package/@aws-sdk/credential-providers
I hit this issue today while evaluating goldstack (which looks to be great btw)
An alternate approach might be to add support for credential_process
. This is neatly summarised in a comment in the same thread referenced above: aws/aws-cli#4982 (comment)
This would not require an upgrade of the AWS SDK since it exists in JS v2 as AWS.ProcessCredentials. Note that credential_process
should be found in ~/.aws/config
according to most other docs (not in ~/.aws/credentials
)
It seems to be a valid workaround with plenty of CLI tools that fill the gap: aws-sso-creds-helper, aws-sso-util, aws-vault, aws2-wrap
Would this be a good path forward?
Thank you that looks very promising. So with this, configuration would only need to contain the following:
{
"users": [
{
"name": "dev-user",
"type": "credentialProcess",
"config": {
"profile": "default"
}
}
]
}
(will need to implement that of course)
Then for this type of users it would instantiate AWS.ProcessCredentials in infraAws.ts#L282 and that should then provide a good way to get SSO credentials, correct?
And just using the profile
provider would not work, correct? infraAws.ts#L262
Yes. It would probably be worth adding an option for fileName
which "defaults to ~/.aws/credentials
or defined by AWS_SHARED_CREDENTIALS_FILE
process env var" and seems to be wrong in most cases.
My ~/.aws/config
file looks something like this:
[profile goldstackApp.GoldstackAccess]
sso_start_url = https://d-1234567890.awsapps.com/start
sso_region = eu-west-1
sso_account_name = goldstackApp
sso_account_id = 123456789012
sso_role_name = GoldstackAccess
credential_process = aws-sso-util credential-process --profile goldstackApp.GoldstackAccess
So config.json
might need to look something like this (assuming the config gets passed to AWS.ProcessCredentials
unaltered)
{
"users": [
{
"name": "dev-user",
"type": "credentialProcess",
"config": {
"profile": "profile goldstackApp.GoldstackAccess",
"fileName": "~/.aws/config",
}
}
]
}
Then for this type of users it would instantiate AWS.ProcessCredentials in infraAws.ts#L282 and that should then provide a good way to get SSO credentials, correct?
If you're taking an explicit approach, this makes good sense.
And just using the
profile
provider would not work, correct? infraAws.ts#L262
Just using the current implementation of profile
provider doesn't seem to work for me and the error message 'Please perform aws login
for the profile using the AWS CLI.' (infraAws.ts#L275) threw me off a bit.
Reading the docs, AWS.SharedIniFileCredentials expects to find aws_access_key_id
and aws_secret_access_key
in the INI file and therefore would ignore credential_process
If you did want to keep this under the profile
provider, I think something like AWS.CredentialProviderChain
could be used. e.g. https://gist.github.com/pahud/836481ae759147d3f493d3ead1f5406a
NB. I didn't see a contributor guide here and I'm not sure how to work on a submodule in the yarn workspace / pnp setup. If you can help with that, I'd be happy to either contribute a PR and/or test it with my setup here.
Thanks for the thoughts and pointers. I have put together a draft PR: #95
A couple of questions:
- Does the way to override the default path for the credentials file look okay? (To me a bit confusing here is that they talk about the
configuration
path but it would always be the path to thecredentials
file rather than theconfig
file?) - Instead of adding a new user type, I've added an additional proprty to the
profile
user type,"processCredentials": true,
. Does that make sense?
{
"name": "process",
"type": "profile",
"config": {
"profile": "with-process",
"awsDefaultRegion": "us-west-2",
"processCredentials": true,
"awsConfigFileName": "~/.aws/mycredentials"
}
}
- Not sure how well this would work with assume-role? (The way AWS credentials are acquired really seems to be a science all to itself!)
I have also started putting together a Contribute section (#94) - but this is still WIP. Anything that stand out to you that would be worthwhile to add?
Does the way to override the default path for the credentials file look okay? (To me a bit confusing here is that they talk about the configuration path but it would always be the path to the credentials file rather than the config file?)
Yeah, it's definitely confusing. In fact I thought it was the other way around (i.e. AWS.ProcessCredentials talks about the credential path in their docs but it would always be the path to config.).
Every other doc I've seen puts the credential_process
property in ~/.aws/config
.
I ran some tests today and if you set AWS_SDK_LOAD_CONFIG=1
, then it will search for profiles in both config
& credentials
. No need to provide a filename.
Instead of adding a new user type, I've added an additional proprty to the profile user type, "processCredentials": true,. Does that make sense?
Yes, this makes good sense. it's definitely feels like it should sit under profile
instead of being a new user type. I also like that you can specify which provider/behaviour you want to use rather than relying on some opaque precedence rules.
Elsewhere I've noticed credential_source can be used to influence the logic in some places so naming might be better as credentialSource: "process"
. Leaving it blank could perhaps try a few methods?
Not sure how well this would work with assume-role?
Not sure either, I suspect it would be OK. The expected response from credential_process
is very similar to response from sts assume-role
. I wouldn't be surprised if many of those tools mentioned above use sts assume-role
behind the scenes.
The way AWS credentials are acquired really seems to be a science all to itself!
Yes, unfortunately I'm just a lab assistant :). @benkehoe is the chief scientist in my book theres-a-better-way
btw, the SSO credentials expire after 60 minutes and some of the terraform scripts can take a while to run (especially if waiting on DNS propagation). The full batch can easily take over an hour on a fresh install.
getPromise()
will refresh the credentials if they are due to expire (15 second window). Passing these values to a docker container as environment vars effectively removes the ability to refresh / extend the session for the lifetime of the container.
I'll see if I can find a mechanism to allow terraform to refresh credentials from within a docker container.
Thanks for testing things out and for your ideas!
I think "credentialsSource": "process",
should work very nicely and changed the config in the PR to reflect that https://github.com/goldstack/goldstack/pull/95/files#diff-d3e9e88cc2f0ee2aad83c3b15bdd7ddc45341b6c988a2bbbe1b06e63f2947667R85
Somehow setting AWS_SDK_LOAD_CONFIG=1
led to the test to fail (disabled it for now https://github.com/goldstack/goldstack/pull/95/files#diff-e43f64484d679c0de71cca9f787c4dc4d20e104fbd0faada8b38efc275670918R84)
➤ YN0000: ● AWS User config › Should read from AWS config in cli provider
➤ YN0000:
➤ YN0000: ENOENT: no such file or directory, open '/home/runner/.aws/config'
➤ YN0000:
➤ YN0000: 486[17](https://github.com/goldstack/goldstack/runs/5256339838?check_suite_focus=true#step:14:17) |
➤ YN0000: 486[18](https://github.com/goldstack/goldstack/runs/5256339838?check_suite_focus=true#step:14:18) | openSync(p, flags, mode) {
➤ YN0000: > 486[19](https://github.com/goldstack/goldstack/runs/5256339838?check_suite_focus=true#step:14:19) | return this.realFs.openSync(npath.fromPortablePath(p), flags, mode);
➤ YN0000: | ^
➤ YN0000: 486[20](https://github.com/goldstack/goldstack/runs/5256339838?check_suite_focus=true#step:14:20) | }
➤ YN0000: 486[21](https://github.com/goldstack/goldstack/runs/5256339838?check_suite_focus=true#step:14:21) |
➤ YN0000: 486[22](https://github.com/goldstack/goldstack/runs/5256339838?check_suite_focus=true#step:14:22) | async opendirPromise(p, opts) {
➤ YN0000:
➤ YN0000: at NodeFS.openSync (../../../../.pnp.cjs:48619:30)
➤ YN0000: at makeCallSync.subPath.subPath (../../../../.pnp.cjs:51771:34)
➤ YN0000: at ZipOpenFS.makeCallSync (../../../../.pnp.cjs:52776:32)
➤ YN0000: at ZipOpenFS.openSync (../../../../.pnp.cjs:51768:[23](https://github.com/goldstack/goldstack/runs/5256339838?check_suite_focus=true#step:14:23))
➤ YN0000: at VirtualFS.openSync (../../../../.pnp.cjs:49232:[30](https://github.com/goldstack/goldstack/runs/5256339838?check_suite_focus=true#step:14:30))
➤ YN0000: at PosixFS.openSync (../../../../.pnp.cjs:492[32](https://github.com/goldstack/goldstack/runs/5256339838?check_suite_focus=true#step:14:32):30)
➤ YN0000: at URLFS.openSync (../../../../.pnp.cjs:49232:30)
➤ YN0000: at NodeFS.readFileSync (../../../../.pnp.cjs:49119:30)
https://github.com/goldstack/goldstack/runs/5256339838?check_suite_focus=true
Seems like it was looking up the default file location rather than the one provided in the constructor. Will try to investigate further tomorrow. Agreed it seems to make more sense to put the credentials_process
into the config
since it's just config and not a credential!
Somehow setting
AWS_SDK_LOAD_CONFIG=1
led to the test to fail
Seems like it was looking up the default file location rather than the one provided in the constructor. Will try to investigate further tomorrow.
Ah yes, I did read the code path in IniLoader (the underlying file loader) and it seems to ignore the file name property if AWS_SDK_LOAD_CONFIG
is set.
I suspect that if you set AWS_CONFIG_FILE
then it will work.
Chris
Got it working with both using credentials
and config
files, see https://github.com/goldstack/goldstack/pull/95/files#diff-d3e9e88cc2f0ee2aad83c3b15bdd7ddc45341b6c988a2bbbe1b06e63f2947667R110. Had to do some fiddinling around with the environment variables to make it work 😱 https://github.com/goldstack/goldstack/pull/95/files#diff-e43f64484d679c0de71cca9f787c4dc4d20e104fbd0faada8b38efc275670918R83 So happy for any better ideas.
I have merged and released what is there so far, which should now support using process credentials with new templates (or by upgrading a package to the latest version of the template dependency).
Also updated the configuration with a section on Process Credentials.
I raised a seperate issue for tracking the issue you identified regarding long running infra operations: #99
I think this should (if it works!) provide at least some way to use an SSO-based login for local development? Thanks for all your work so far!!!
Thanks for the quick response @mxro, this is working for me now (with a workaround**) 🚀
I found some typos and other minor inconsistencies so I'll put a review on the PR for you.
I still can't make changes locally using yarn link *
or yarn patch
but I made a bit of progress and I'll raise a few issues related to that. Happy to be added as a collaborator if that's a useful path.
** I had to either provide awsConfigFileName
with the default ~/.aws/config
location, OR execute yarn infra
with a prefix (e.g. AWS_SDK_LOAD_CONFIG=1 yarn infra init dev
) to get this working. I'll add this to the PR review also!
That's great to hear. Yeah a lots of bits and pieces of logic esp around the env variables, so definitely need to tweak this a bit to make it work reliabily. Thanks for your help. Started a new PR based on the comments #102 - but still WIP for now.
** I had to either provide awsConfigFileName with the default ~/.aws/config location, OR execute yarn infra with a prefix (e.g. AWS_SDK_LOAD_CONFIG=1 yarn infra init dev) to get this working.
Will want to get this working but not sure if the new PR above would resolve this.
Also added you as a collaborator. This should enable you to create and contribute to branches and the CI should run on them I hope.
Does this work now and can be closed?