Skip to content

Provide support for AWS credentials obtained through SSO #17

Closed

Description

@mxro

While #3 will provide a way to consume credentials from ~/.aws/credentials this will probably not work for users logging in through SSO:

aws/aws-cli#4982

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

Activity

  1. chrismilleruk commented on Feb 16, 2022

    @chrismilleruk
    Contributor

    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?

  2. mxro commented on Feb 16, 2022

    @mxro
    CollaboratorAuthor

    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

  3. chrismilleruk commented on Feb 17, 2022

    @chrismilleruk
    Contributor

    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.

  4. mxro commented on Feb 17, 2022

    @mxro
    CollaboratorAuthor

    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 the credentials file rather than the config 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?

  5. chrismilleruk commented on Feb 18, 2022

    @chrismilleruk
    Contributor

    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

  6. chrismilleruk commented on Feb 18, 2022

    @chrismilleruk
    Contributor

    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.

    Screenshot 2022-02-18 at 16 30 38

    Screenshot 2022-02-18 at 16 31 24

  7. chrismilleruk commented on Feb 18, 2022

    @chrismilleruk
    Contributor

    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.

  8. mxro commented on Feb 19, 2022

    @mxro
    CollaboratorAuthor

    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!

  9. chrismilleruk commented on Feb 19, 2022

    @chrismilleruk
    Contributor

    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

  10. mxro commented on Feb 20, 2022

    @mxro
    CollaboratorAuthor

    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!!!

  11. chrismilleruk commented on Feb 21, 2022

    @chrismilleruk
    Contributor

    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!

  12. mxro commented on Feb 21, 2022

    @mxro
    CollaboratorAuthor

    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.

  13. mxro commented on Mar 2, 2022

    @mxro
    CollaboratorAuthor

    Does this work now and can be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions