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.

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

Expanded Support for AWS Auth Method #482

Merged
merged 1 commit into from Jul 7, 2019

Conversation

Projects
None yet
4 participants
@stevefranks
Copy link
Contributor

commented Jun 28, 2019

[Proposal]
This PR adds support for authentication via the AWS auth backend. I have tested this code against my local vault instance. I'll add more documentation once we are in agreement on the design.

References: #479

cc @jeffwecan

@stevefranks stevefranks force-pushed the stevefranks:develop branch 3 times, most recently from 51ddfd2 to f55e5cf Jun 28, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Jun 28, 2019

Codecov Report

Merging #482 into develop will decrease coverage by 4.77%.
The diff coverage is 26.87%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #482      +/-   ##
===========================================
- Coverage    88.05%   83.28%   -4.78%     
===========================================
  Files           51       52       +1     
  Lines         2680     2907     +227     
===========================================
+ Hits          2360     2421      +61     
- Misses         320      486     +166
Impacted Files Coverage Δ
hvac/constants/aws.py 100% <100%> (ø) ⬆️
hvac/v1/__init__.py 86.5% <100%> (+0.25%) ⬆️
hvac/api/auth_methods/__init__.py 77.27% <100%> (+1.08%) ⬆️
hvac/api/auth_methods/aws.py 21.32% <21.32%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c013371...5c566e5. Read the comment docs.

@stevefranks stevefranks force-pushed the stevefranks:develop branch from f55e5cf to 3f51c3f Jun 28, 2019

@stevefranks stevefranks force-pushed the stevefranks:develop branch from 3f51c3f to 5c566e5 Jun 28, 2019

@stevefranks stevefranks changed the title add support for aws auth method [WIP] add support for aws auth method Jun 28, 2019

@jeffwecan

This comment has been minimized.

Copy link
Collaborator

commented Jul 2, 2019

Just as a heads-up, I personally haven't had time to look over this PR but intend to do so as soon as I am able.

response = self._adapter.get(
url=api_path,
)
return response.json().get('data')

This comment has been minimized.

Copy link
@drewmullen

drewmullen Jul 3, 2019

Contributor

@jeffwecan we should consider standardizing json returns. personally i like this where the data key is returned directly, however, we have some that doesnt extract it. not sure its worth going back and changing since that would have dowstream effects but, we could consider it

example:

return response.json()

@@ -10,6 +10,7 @@
from hvac.api.auth_methods.mfa import Mfa
from hvac.api.auth_methods.okta import Okta
from hvac.api.auth_methods.radius import Radius
from hvac.api.auth_methods.aws import Aws

This comment has been minimized.

Copy link
@drewmullen

drewmullen Jul 3, 2019

Contributor

nit picky - the other imports are alphabetical

@drewmullen drewmullen requested a review from jeffwecan Jul 3, 2019

@drewmullen

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

LGTM! thanks for the PR! i dont have a strong exp with aws auth so ill let @jeffwecan comment b4 approving

@jeffwecan jeffwecan changed the title add support for aws auth method Expanded Support for AWS Auth Method Jul 7, 2019

@jeffwecan
Copy link
Collaborator

left a comment

Checks out to me, thanks so much again @stevefranks!

@jeffwecan jeffwecan merged commit 0943292 into hvac:develop Jul 7, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jeffwecan

This comment has been minimized.

Copy link
Collaborator

commented Jul 8, 2019

Also I realize now that I skipped past the "I'll add more documentation once we are in agreement on the design" point you mentioned when opening the PR. However you're still welcome to fill in those gaps at any point in the future (and it would be much appreciated in fact!). The docstrings will at least cover some things since they end up in "autodoc" output at https://hvac.readthedocs.io/en/stable/source/hvac_api_auth_methods.html.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.