search

BetterCloud / vault-java-driver

Zero-dependency Java client for HashiCorp's Vault
https://bettercloud.github.io/vault-java-driver/
336 stars 223 forks source link

Implement AWS login #90

Closed thinkharderdev closed 6 years ago

thinkharderdev commented 6 years ago

Implemented AWS auth backend (just the login).

To mock dependency on AWS, I added a new MockVault type which validates requests and send a canned response if the request matches one of a set of Predicates.

steve-perkins commented 6 years ago

Thanks so much, @dshva!

In a follow-up commit, I moved your AuthBackendAwsTests class from the /src/test-integration subdirectory to the /src/test subdirectory. The former is meant for true "integration" tests, communication with a real Vault instance running inside a Docker container. Your test suite seems to be a true unit test, communicating with a Jetty instance serving as a Vault mock, like the other test suites in that latter subdirectory.

Also, I've had to apply the @Ignore annotation on both of your test methods, to effectively disable that test suite altogether. It's not clear to me what's happening here... can you please check out the master branch to take a look?

It looks like you're instructing the Jetty mock to simply verify that the incoming request was properly formatted. It LOOKS like the isValidEc2pkcs7Request (on AuthBackendAwsTests.java, line 26) should pass... as the request URI is /v1/auth/aws/login and the request body is {"pkcs7":"pkcs7","role":"role"}. However, the AuthRequestValidatingMockVault.handle() method is still falling through to the block which returns a 400 status code.

thinkharderdev commented 6 years ago

My apologies. Apparently I was just running the unit tests when I was testing before so didn't realize my tests were not being executed. It turns out I made a number of stupid errors.....

Passing a list of predicates was apparently too clever by half. The stream in the request was being consumed by the first predicate and then was empty when the next predicate executed. I was trying to avoid spinning up multiple server instances per test but I don't see any straightforward way around it now, so I did it the easy (and inefficient) way to make things work.

Anyway, I fixed the unit tests. I've attached the patch here patch.diff.tar.gz

Or I can submit another PR with the updated if you world prefer.

Thanks! Dan

steve-perkins commented 6 years ago

Awesome! If it's not too much trouble, could you just send over a PR from that branch so I can do the merge from GitHub? Thanks!