Adding support for a custom RoleSessionName when assuming a role

[pgcp]

These changes allow a user to specify the "role_session_name" parameter in a role-based profile. The value of role_session_name is used as the RoleSessionName parameter in the STS AssumeRole call.

[pgcp]

This pull request was targeted for issue 1333. This being my first pull request on Github, I'm not aware of how to link this pull request to that issue.

[jamesls]

Thanks for the pull request, sorry for the delay. I think this looks great (thanks for adding tests), and would be a great feature addition to the CLI.

I'm looking at this now.

[quiver]

I found one problem with the current implementation.

STS token is stored in cache directory using (account_id, role) as the key and AWS-CLI reuse its tokens as long as (account_id, role) pair is same util it expires, and STS assumed-role principal ARN is created based on RoleSessionName parameter.

So if the cachedRoleSessionName and subsequent requests' RoleSessionName differ, restricting access using STS ARN does not always work. This is @pgcp 's original use-case.

The purpose of RoleSessionName parameter is to uniquely identity a session when the same role is assumed by different principals or for different reasons, so separating cache files based on (account_id, role, role_session_name) seems reasonable to me.(Of course, if role_session_name parameter is not specified(=default), share the cache file and this is the current behavior.)

[pgcp]

@quiver, thanks for finding this. I agree that this is a problem, although a fairly minor one. Minor or not, of course it should be fixed.

To be clear, the effect of this problem is that cached credentials will be used when they shouldn't be if the role_session_name is modified for a profile. The no-longer-matching cached credentials will be used from the time that the role_session_name is modified to the expiration time of the cached credentials. That is at most 1 hour.

I agree that separating cache files based on (account_id, role, role_session_name) seems to be the fix. I can update the cache key to include the role_session_name, if present. Then this problem should be resolved.

[jamesls]

The proposed changes sound good to me. Thanks for catching @quiver .

[pgcp]

I've reverted a change on my branch that didn't have anything to do with this change. I was unaware that pull requests follow branches. I thought it just followed an exact commit.

I've added a commit that adds the role_session_name into the cache key if it is present. None of the characters allowed in a role_session_name ([\w+=,.@-]*) seems like they should be a problem if they're in a file name.

One problem is that the role_session_name is case sensitive so the cache file may present a problem on a case-insensitive file system. This will only be a problem if 1) a user has the cache file on a case-insensitive file system, and 2) they're changing the role_session_name such that only the case is different from the previous value, and 3) it has been less than an hour since the file was cached (because the expiration time for credentials is one hour). This is such a narrow case that I didn't think it required the effort to fix.

[quiver]

Updated PR looks good to me. 👍

@pgcp

This is such a narrow case that I didn't think it required the effort to fix.

Agreed.

Sorry for nit-picking.

[jamesls]

Rebased against develop and merged. Thanks again for the pull request!