Add credential caching for AssumeRole

[mtibben]

This implements caching for AssumeRole calls.

This has been a long time coming, and fixes #552, #532 and probably other issues too.

Approach

The approach to this was to

1. Create a wrapper around keyring, (using an almost identical interface to Keyring and CredentialKeyring)
2. Generalise the session caching provider (now CachedSessionProvider) so that it could cater for any type of sts.Credentials.

As anticipated, I hit a snag with the new SSO provider, and needed to simplify it to move this forward. The SSO provider was using 2 layers of caching - one for the OIDC token and one for the SSO credentials. The SSO credentials was straightforward to use with the generalised CachedSessionProvider, but the OIDC was not, so the OIDC caching was removed. Apologies @rickardl @itsdalmo, I'd like to follow up with another PR to resolve this. Additionally the tests were a little brittle as it seemed to test the codepath rather than the behaviour. Perhaps with this simplification the tests are not necessary. Again lets follow up in another issue to address this @rickardl @itsdalmo

How it works

The role caching kicks in when an MFA device is used. This is the same behaviour as GetSessionToken, the rationale being that we only really want to store credentials when it's "useful" to do so. When there's no MFA device to worry about, the credential can be generated on demand without any friction.

Additionally, the new SessionKeyring makes more metadata available, so the list command can show the session expiry

$ aws-vault list
Profile                  Credentials              Sessions
=======                  ===========              ========
default                  -                        -
tibbo                    tibbo                    sts.GetSessionToken:7h59m55s
myrole                   -                        sts.AssumeRole:59m19s

You can test this out with v5.5.0-beta2

[mtibben]

@rickardl @itsdalmo

1. Are you able to test this to make sure it hasn't broken SSO in any way?
2. What was the reasoning for the 2 levels of caching? Is it because the start_url could be used on multiple profiles?
3. Any thoughts on the tests?

[itsdalmo]

Hi @mtibben!

1. Are you able to test this to make sure it hasn't broken SSO in any way?
2. What was the reasoning for the 2 levels of caching? Is it because the start_url could be used on multiple profiles?

The two levels of caching was there because the start_url is the same for multiple profiles as you say. What was being cached was a device ID and an access token that could be used to exchange for role credentials, and by removing the caching users will need to go through login-flow (opening the browser and clicking sign-in etc) every time the role credentials expire (typically 1h) instead of just when the access token expires (e.g. every 8h). The device ID and access tokens are cached in the v2 AWS CLI as well, so I think AWS expects these values to be "long lived".

3. Any thoughts on the tests?

Not 100% sure why you removed the existing tests? With reference to the above; the tests were deliberately written to break if a code-change also broke caching, because the caching was a key aspect of the desired behaviour for the SSO Provider. Not sure what you would want the tests to cover if not the behaviour of the code? 🤷‍♂️

[mtibben]

We should be able to separate those concerns now @itsdalmo - caching is orthogonal to the credential creation code and tested separately

[mtibben]

Not sure what you would want the tests to cover if not the behaviour of the code?

I'm more interested in the behaviour of the interface, testing the internal code behaviour leads to brittle tests and makes refactoring difficult

[gws]

@mtibben This (possibly) breaks 5.5.0-beta2 for me with the pass backend on Linux. Every operation (this one was just list, but it happens on exec and rm too that I tested) results in this panic, but only after starting a session:

$ aws-vault --debug list
2020/04/30 12:02:56 aws-vault v5.5.0-beta2
2020/04/30 12:02:56 [keyring] Considering backends: [pass]
2020/04/30 12:02:56 Loading config file /home/me/.aws/config
2020/04/30 12:02:56 Parsing config file /home/me/.aws/config
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x9a8d1f]

goroutine 1 [running]:
github.com/99designs/aws-vault/v5/vault.(*SessionKeyring).GetMetadata(0xc000161b80, 0xc0001386fd, 0x13, 0xc00002373e, 0x2, 0xc000026c60, 0x24, 0x0, 0x0, 0x0, ...)
        github.com/99designs/aws-vault/v5/vault/sessionkeyring.go:172 +0x1af
github.com/99designs/aws-vault/v5/vault.(*SessionKeyring).GetAllMetadata(0xc000161b80, 0xc0000712c0, 0x4, 0x4, 0x0, 0x0)
        github.com/99designs/aws-vault/v5/vault/sessionkeyring.go:190 +0x143
github.com/99designs/aws-vault/v5/cli.LsCommand(0xc000000000, 0xc00000dca0, 0xd57da0, 0xc0001fb9b0, 0x0, 0x0)
        github.com/99designs/aws-vault/v5/cli/ls.go:66 +0xdfe
github.com/99designs/aws-vault/v5/cli.ConfigureListCommand.func1(0xc0001f19e0, 0x40a5b3, 0xacec20)
        github.com/99designs/aws-vault/v5/cli/ls.go:45 +0xd0
gopkg.in/alecthomas/kingpin%2ev2.(*actionMixin).applyActions(0xc00021a0d8, 0xc0001f19e0, 0x0, 0x0)
        gopkg.in/alecthomas/kingpin.v2@v2.2.6/actions.go:28 +0x6d
gopkg.in/alecthomas/kingpin%2ev2.(*Application).applyActions(0xc000218690, 0xc0001f19e0, 0x0, 0x0)
        gopkg.in/alecthomas/kingpin.v2@v2.2.6/app.go:557 +0xdc
gopkg.in/alecthomas/kingpin%2ev2.(*Application).execute(0xc000218690, 0xc0001f19e0, 0xc000055a40, 0x1, 0x1, 0x0, 0x0, 0x0, 0xae11a0)
        gopkg.in/alecthomas/kingpin.v2@v2.2.6/app.go:390 +0x8f
gopkg.in/alecthomas/kingpin%2ev2.(*Application).Parse(0xc000218690, 0xc00001e0a0, 0x2, 0x2, 0xc000218690, 0x405aff, 0xc00002a118, 0x0)
        gopkg.in/alecthomas/kingpin.v2@v2.2.6/app.go:222 +0x1fe
main.main()
        github.com/99designs/aws-vault/v5/main.go:26 +0x184

I was using 5.5.0-beta1 successfully in order to test out the YubiKey support which worked well. I can work around the panic by removing the session file from the backend manually and at least start up a session that way, but it's painful.

I'm using MFA via YubiKey which is what led me to believe this change is the culprit.

[mtibben]

OK thanks @gws I created #573 to track that issue