Initial commit of assume role cred provider #990
Conversation
|
I would vote for this going into botocore. It's really important functionality that would be useful in a lot of places, boto3 for starters. Even if that doesn't happen right away I really think it is the right place for it to live. Also, is there any way that we could also add support for sessions that use MFA? For example, allowing the MFA serial number to be added to a profile and then, perhaps by prompting, request the MFA token to send with the AssumeRole request? cc @ehammond |
|
I think the MFA part would be really useful, and would work well for the CLI given it's typically a short lived process. However, there are some scenarios, mostly with I believe the two options we MFA is involved are that we can either try to pause the S3 transfer and re-prompt for the new MFA, or we can just error out in that scenario. I'll look into adding this. |
|
+1 for MFA. Defaulting to an error sounds best, with perhaps a future option to prompt for interactive use. Unexpected pauses for prompts can be problematic in automation. |
This builds on the work of several existing pull requests combining what I believe is the combination of all the suggested changes including: * Use role_arn to trigger assume role behavior. Do not require an explicit session to be created and ended (aws#667). * Cache the temporary credentials. If the credentials are not expired then we should reuse them instead of making the assume role call every time (aws#656 and boto/botocore#226). I think a strong case can be made to move this to botocore. However, the file caching makes me somewhat hesitant to move this into botocore (but I think the AssumeRoleProvider without the caching could be added to botocore). I'd like to evaluate this in the future but for now, I don't think it's unreasonable to leave this in the AWS CLI for the time being. At any rate, this is a straightforward and compatible move if we decide to do so. Also, the ability to open a browser with the currently scoped session has not been ported over. I'd like to address that in a separate pull request.
|
LGTM aside from the test failure. |
|
|
||
|
|
||
| def _create_creds_from_response(self, response): | ||
| #return credentials.Credentials( |
There was a problem hiding this comment.
Delete the commented out code.
|
I think that makes sense. So the plan is:
As for botocore, I do want to get something in botocore, the interactive prompting makes me a little more hesitant, but I do think we should eventually move this into botocore. |
|
Yeah looks good to me as well. Test should pass now with the botocore pull request fixing that issue. 🚢 |
jamesls
force-pushed
the
assume-role-provider
branch
from
c62b135 to
df8fcb7
Compare
|
Ok, so update on the plan. The MFA changes are an additive change so I'd like to get this initial version merged and tested to ensure there's no issues with the baseline code. I'll then send another PR for the MFA changes shortly. |
Feedback from aws#990, this adds support for MFA when assuming a role. To enable this, in addition to role_arn and source_profile, you can specify an mfa_serial option in your config file:: [profile foo] role_arn = ... source_profile = development mfa_serial = ..... This is the the mfa arn/device id. If an mfa_serial is provided then a user will be prompted for the token code when the AssumeRole call happens. As mentioned in the original PR, for now when the temporary credentials expire, an exception will be raised if MFA is required. We can look into updating this in the future to support reprompting the user. This only affects the case where the credentials expire within the duration of the AWS CLI process. Aside from some of the ``aws s3 cp/sync`` commands, the AWS CLI is generally a short lived process so this won't affect the common usage scenarios.
Feedback from aws#990, this adds support for MFA when assuming a role. To enable this, in addition to role_arn and source_profile, you can specify an mfa_serial option in your config file:: [profile foo] role_arn = ... source_profile = development mfa_serial = ..... This is the the mfa arn/device id. If an mfa_serial is provided then a user will be prompted for the token code when the AssumeRole call happens. As mentioned in the original PR, for now when the temporary credentials expire, an exception will be raised if MFA is required. We can look into updating this in the future to support reprompting the user. This only affects the case where the credentials expire within the duration of the AWS CLI process. Aside from some of the ``aws s3 cp/sync`` commands, the AWS CLI is generally a short lived process so this won't affect the common usage scenarios.
Feedback from aws#990, this adds support for MFA when assuming a role. To enable this, in addition to role_arn and source_profile, you can specify an mfa_serial option in your config file:: [profile foo] role_arn = ... source_profile = development mfa_serial = ..... This is the the mfa arn/device id. If an mfa_serial is provided then a user will be prompted for the token code when the AssumeRole call happens. As mentioned in the original PR, for now when the temporary credentials expire, an exception will be raised if MFA is required. We can look into updating this in the future to support reprompting the user. This only affects the case where the credentials expire within the duration of the AWS CLI process. Aside from some of the ``aws s3 cp/sync`` commands, the AWS CLI is generally a short lived process so this won't affect the common usage scenarios.
This builds on the work of several existing
pull requests combining what I believe is
the combination of all the suggested changes
including:
Do not require an explicit session to be created
and ended (Add ability to use role ARN as configuration variable #667).
are not expired then we should reuse them instead of
making the assume role call every time
(WIP Commit. #656 and Add temporary credentials boto/botocore#226).
I think a strong case can be made to move this to botocore.
However, the file caching makes me somewhat hesitant to move
this into botocore (but I think the AssumeRoleProvider without
the caching could be added to botocore). I'd like to evaluate this in
the future but for now, I don't think it's unreasonable to leave
this in the AWS CLI for the time being. At any rate, this is
a straightforward and compatible move if we decide to do so.
Also, the ability to open a browser with the currently scoped
session has not been ported over. I'd like to address that
in a separate pull request.
cc @kyleknap @danielgtaylor