search

jmgilman / vaultrs

An asynchronous Rust client library for the Hashicorp Vault API
https://docs.rs/vaultrs
MIT License
97 stars 60 forks source link

Add support for login via certificate #76

Closed avandecreme closed 7 months ago

avandecreme commented 7 months ago

The first commit is cherry picked from #62

The second one is specific to this PR and allow one to use certificate to login to vault.

Haennetz commented 7 months ago

Hey thanks for your pull request. It would be nice if you can add a test for the certificate login. I merged #62 can you rebase your branch then the checks should be able to pass.

gitguardian[bot] commented 7 months ago

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them. Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately. Find here more information about risks.

🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

avandecreme commented 7 months ago

I added two commits. The first one is implementing the tests. The second one is just to show how having https://github.com/jmgilman/dockertest-server/pull/15 merged could clean up this PR.

I committed the keys and certificates as well as the method to generate them. I see that gitguardian is not happy about it even though those are just test certificates.

I guess we could generate new certificates on every test run, provided we can run openssl in the runner. What do you think?

Haennetz commented 7 months ago

Yes I think generating a new certificate each time would be the best solution for that. We can use rcgen as a dev dependency and create the certificate with it.

avandecreme commented 7 months ago

Yes I think generating a new certificate each time would be the best solution for that. We can use rcgen as a dev dependency and create the certificate with it.

Done

Haennetz commented 7 months ago

Hey tanks for implementing this. The test is failing at the moment because the nativ-tls feature didn't support pam only. It requires pkcs8 or pkcs12.

avandecreme commented 7 months ago

I added a commit to disable the test with native-tls since vaultrs does not support client certificates with it anyway. See https://github.com/jmgilman/vaultrs/blob/34e3874ee406723381ba99db9e5c77b64967294d/src/client.rs#L298

avandecreme commented 7 months ago

Suggestions applied

Haennetz commented 7 months ago

Thanks :+1: