Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support MFA device for cross account role switching #100

Merged
merged 5 commits into from
Dec 16, 2015
Merged

Support MFA device for cross account role switching #100

merged 5 commits into from
Dec 16, 2015

Conversation

pdecat
Copy link

@pdecat pdecat commented Nov 30, 2015

Support MFA device for cross account role switching, reusing STS credentials between API calls as MFA Token cannot be used more than once.

This introduces two new optional command line arguments:

  -M MFA_SERIAL_NUMBER, --mfa-serial-number MFA_SERIAL_NUMBER
                    MFA Serial Number to use when assuming a role via STS
  -T MFA_TOKEN, --mfa-token MFA_TOKEN
                    MFA Token to use when assuming a role via STS

@pdecat
Support MFA device for cross account role switching, reusing STS cred…
2409f6e 
…entials between API calls as MFA Token cannot be used more than once.
@codecov-io
Copy link

Current coverage is 99.85%

Merging #100 into develop will decrease coverage by -0.15% as of 47c50f4

@@            develop    #100   diff @@
=======================================
  Files            17      17       
  Stmts          1403    1414    +11
  Branches        222     223     +1
  Methods           0       0       
=======================================
+ Hit            1403    1412     +9
- Partial           0       1     +1
- Missed            0       1     +1

Review entire Coverage Diff as of 47c50f4

Powered by Codecov. Updated on successful CI builds.

@jantman
Copy link
Owner

jantman commented Dec 16, 2015

@pdecat Please accept my most sincere apologies; it seems like I never got the notification for this.

The only Travis failure was a minor formatting issue in one of the doc strings, and coverage is complete. I'll look this over tonight or tomorrow and get it merged. I'm hoping to have a patch-level release out within the next few days.

jantman
jantman reviewed Dec 16, 2015
View reviewed changes
awslimitchecker/checker.py
@@ -83,6 +83,12 @@ def __init__(self, warning_threshold=80, critical_threshold=99,
com/IAM/latest/UserGuide/id_roles_create_for-user_externalid.html>`_
string to use when assuming a role via STS.
:type external_id: str
:param mfa_serial_number: (optional) the `MFA Serial Number` string to_
use when assuming a role via STS.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The continuation line here and on line 89 needs to be indented two spaces. That's why the Travis doc test is failing.

@jantman
Copy link
Owner

jantman commented Dec 16, 2015

It looks like there are a few more places that have indentation issues causing the docs build to fail. Also, there should probably be something added to the documentation about this - I'd really like to see it in the "Credentials" section of the "Getting Started" page.

Since these are such trivial changes, I'll leave it up to you whether you want to fix them, or just want me to pull this in to my own branch and clean it up a bit.

I've never used MFA with STS before, so I'm going to need to just take your word that this works...

@jantman
Copy link
Owner

jantman commented Dec 16, 2015

Ok, thanks! I'll merge this and have a release out tonight or early tomorrow morning (GMT-5)

@pdecat
Copy link
Author

pdecat commented Dec 16, 2015

You're welcome.

I'll try to find some time to add this to the Credentials section.

@jantman
Copy link
Owner

jantman commented Dec 16, 2015

@pdecat don't worry about it, I'll add the docs after merging.

Thanks so much for contributing this!

@jantman jantman added this to the 0.2.3 milestone Dec 16, 2015
jantman added a commit that referenced this pull request Dec 16, 2015
@jantman
Merge pull request #100 from pdecat/develop
8d4c225 
Support MFA device for cross account role switching
@jantman jantman merged commit 8d4c225 into jantman:develop Dec 16, 2015
jantman added a commit that referenced this pull request Dec 16, 2015
@jantman
PR #100 - add documentation on MFA to Getting Started
4f63e02
jantman added a commit that referenced this pull request Dec 16, 2015
@jantman
PR #100 - regenerate documentation
4164054
jantman added a commit that referenced this pull request Dec 16, 2015
@jantman
PR #100 - add changelog
cff9bbf
@jantman
Copy link
Owner

jantman commented Dec 17, 2015

@pdecat this has been released as 0.2.3 and is now live on pypi. Thanks again so much for the contribution!

jantman added a commit that referenced this pull request Dec 17, 2015
@jantman
PR #100 - add coverage for additional code path in Connectable
88cb837
jantman added a commit that referenced this pull request Dec 17, 2015
@jantman
changelog for previous two commits - #112 and PR #100
c1bc86d
@pdecat pdecat deleted the develop branch December 30, 2015 06:31
@pdecat pdecat restored the develop branch December 30, 2015 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.2.3
Development

Successfully merging this pull request may close these issues.
3 participants
@pdecat @codecov-io @jantman