login: Allow non-temporary credentials from the environment

[christophetd]

• f887580 fixed a bug introduced in 67d1aea that causes the AWS console to show "Invalid credentials" when running aws-vault login and only non-temporary credentials are available

• 5798fba allows to use aws-vault login when credentials are in the environment, no matter their type

  • if temporary credentials, the behavior is unchanged
  • otherwise, sts:GetFederationToken is used to retrieve temporary credentials, then generate a sign-in link

This has the nice consequence of being able to run aws-vault login at any time, no matter which type of credentials is available in your current shell

[mtibben]

f887580 fixed a bug #864 (comment) in 67d1aea that causes the AWS console to show "Invalid credentials" when running aws-vault login and only non-temporary credentials are available

This sounds like the correct behaviour to me. aws-vault tries to be predictable. If the credentials in the environment aren't able to be used, then isn't that a user error that should display an error message? "Magic" behaviour I typically try to stay away from

[mtibben]

(but open to being convinced differently)

[christophetd]

I see it as: aws-vault loginbehavior should be the same, no matter where the credentials are coming from. The previous behavior for keychain credentials was:

• If long-lived IAM credentials, call sts:GetFederationToken first and use the subsequent STS creds
• If already STS creds, do nothing
• Then, generate a sign-in link

So I see no reason to change this behavior if credentials are sourced from the environment. We aren't changing the way aws-vault login works, just making sure it's consistent independently of where creds are sourced from

To make it more graphical, this is the current behavior:

flowchart TB
    A[aws-vault login] --> B{Profile name specified?}
    B -->|Yes| C[Retrieve credentials from the keychain]

    C --> D{Temporary credentials?}
    D -->|No|F[Call sts:GetFederationToken to retrieve temporary credentials]

    F --> E
    D -->|Yes|E[Generate magic sign-in link using temporary credentials]

    B -->|No| M[Read credentials from the environment]

    M --> N{Temporary credentials?}
    N -->|No|O[Error]

    N -->|Yes|E

Loading

This PR changes the behavior so it becomes the same no matter whether credentials are sourced from the keychain or from the environment:

flowchart TB
    A[aws-vault login] --> B{Profile name specified?}

    B -->|Yes| C[Retrieve credentials from the keychain]
   

    B -->|No| M[Read credentials from the environment]

    C --> D{Temporary credentials?}
    M --> D
    D -->|No|F[Call sts:GetFederationToken to retrieve temporary credentials]
    F --> E
    D -->|Yes|E[Generate magic sign-in link using temporary credentials]

   

Loading

[christophetd]

@mtibben What do you think of the above? The argument here is to make sure the behavior of aws-vault login is the same when running it against a profile vs. credentials in the environment.

[christophetd]

@mtibben Any thoughts? I heard back from folks that this is the behavior they would expect

[sftim]

If aws-vault login starts generating temporary credentials automatically, how does it decide what parameters (eg session duration) to use?

[christophetd]

Note that aws-login is already generating temporary credentials automatically. The relevant existing parameters are:

$ aws-vault login --help
usage: aws-vault login [<flags>] [<profile>]

Generate a login link for the AWS Console

  -d, --duration=DURATION        Duration of the assume-role or federated session. Defaults to 1h
  -n, --no-session               Skip creating STS session with GetSessionToken

This PR is simply about making sure these are also generated when sourcing credentials from the environment rather than from the keychain.

[sftim]

Fair enough, that's legit then!

[christophetd]

👋🏼 @mtibben Any feedback on the above! Looking forward to get this merged!

[christophetd]

Now rebased on latest master, waiting to be merged.

[christophetd]

Rebased on latest master. @mtibben what's the best way to get this merged? Thanks!

[christophetd]

Rebased on latest master again, eager to get it merged! It makes sure the aws-vault login behavior is consistent. And if I need it on a daily basis, I'm sure other people would benefit from it as well. :-)

@mtibben @lox Let me know if anything is blocking! Thank you

[mtibben]

The logic here is not particularly clear to me.

Previously above we had a very clear section of code

	if input.ProfileName == "" {
		// When no profile is specified, source credentials from the environment
		credsProvider = vault.NewEnvironmentCredentialsProvider()
	} else {
		// Use a profile from the AWS config file
		ckr := &vault.CredentialKeyring{Keyring: keyring}
		if config.HasRole() || config.HasSSOStartURL() {
			// If AssumeRole or sso.GetRoleCredentials isn't used, GetFederationToken has to be used for IAM credentials
			credsProvider, err = vault.NewTempCredentialsProvider(config, ckr)
		} else {
			credsProvider, err = vault.NewFederationTokenCredentialsProvider(input.ProfileName, ckr, config)
		}
		if err != nil {
			return fmt.Errorf("profile %s: %w", input.ProfileName, err)
		}
	}

This retrieveTemporaryCredsFromEnvironment function makes things less understandable by separating out a new codepath and generating federated tokens separately from this. So we end up with functions at different levels of abstraction

Is it possible for the main logic around this to stay in one place?

[christophetd]

Let me see what I can do!

[mtibben]

This PR changes the behavior so it becomes the same no matter whether credentials are sourced from the keychain or from the environment:

The actual code path does not look like the diagram you posted

[christophetd]

If aws-vault login is run without a profile:

• credentials are sourced from the environment (https://github.com/99designs/aws-vault/blob/master/cli/login.go#L98-L99)
• the code path ends up generating a login URL based on these, and fails if no AWS_SESSION_TOKEN is in the environment ("Some of your credentials are missing. Please contact your administrator." displayed on an AWS error page)

If aws-vault login is run with a profile:

• credentials are sourced from the appropriate credentials provider (https://github.com/99designs/aws-vault/blob/master/cli/login.go#L85-L89)
• If credentials retrieved don't correspond to temporary credentials (i.e. SSO or with a role ARN), then GetFederationToken is used (https://github.com/99designs/aws-vault/blob/master/cli/login.go#L107) to generate temporary credentials. This is the step that's missing if you run aws-vault login without a profile and which should be more consistent

So yes, it looks like the code path does have the logic I was pointing out above, unless I missed something?

[mtibben]

Hey @christophetd, does #1150 work for you to resolve this issue?

[christophetd]

@mtibben Left some comments in #1150, thanks!

[christophetd]

closing this PR as it's being addressed by #1150