abedra / libvault

A C++ library for Hashicorp Vault
MIT License
34 stars 26 forks source link

Is Token Auth method supported? #77

Closed zspasztori closed 3 years ago

zspasztori commented 3 years ago

Hi,

The main readme says that libvault has Partial implementation for tokens auth. https://www.vaultproject.io/api-docs/auth/token

I am wondering if the methods such as create token, renew token, revoke token etc. are implemented somewhere.

abedra commented 3 years ago

That's correct, the only thing implemented currently is the authentication strategy

https://github.com/abedra/libvault/blob/master/include/VaultClient.h#L265-L275

It should be fairly trivial to implement if you would like to send a pull request. I'll happily support quickly if this is for another open source project.

zspasztori commented 3 years ago

I will send a pull request. I just have some questions.

Is there a reason the library separates AuthenticationMethods from AuthenticationStrategy ? All authentication methods (except token) has a login method. Wouldn't it make more sense for all auth methods to inherit from the same interface, and the vault client could just call login inside. Currently I do not even see how you can use an auth method without a strategy, since AuthMethods needs explicitly a client for creation.

Unfortunately, according to vault documentation token has no login method. Looking at your implementation if someone uses TokenStrategy for authentication there is no way to tell if the client is actually authenticated/live. This is a bit of an inconsistency compared to other strategies.

abedra commented 3 years ago

Ultimately, tokens are unique in how they work within Vault. To have a token is to be authenticated. There's nothing else to do. The library aims to enforce the consistency, which it does in the interface provided. Authentication methods are separated to highlight the management of the authentication vs the authentication itself. See AppRole for an example https://github.com/abedra/libvault/blob/master/include/VaultClient.h#L418-L445.

In terms of inheritance and consistency, authentication strategies to all implement the same interface https://github.com/abedra/libvault/blob/master/include/VaultClient.h#L212-L216. If you feel there is a different way to model things that provides more clarity, please feel free to submit a pull request. While I do feel that having foundational interfaces helps provide clarity, I don't believe that layers of inheritance do. I feel that building layers with the context required provides a more testable foundation, but that's just my opinion.

zspasztori commented 3 years ago

in Vault::Params is it possible to set different types for the values?

for example create token (https://www.vaultproject.io/api-docs/auth/token#create-token) command has num_uses field which expects an integer type, or the policies field which expects an array of strings.

I am wondering that in this case should I use instead a native nlohmann::json, or do you have any suggestion?

abedra commented 3 years ago

Not the way Vault::Parameters is currently defined. It's an alias for std::map<std::string, std::string> as written. There are several ways to consider this. A template parameter can be added to achieve the desired outcome. That would result in quite a bit of churn, but a good editor can do most of the lifting for you. I've been considering going down the path to define pods for the various inputs to provide a richer overall experience, so if you wanted to start that trend it would be reasonable.

If you don't want to chip away at the larger stuff above, have you tried using a string for the parameter? I have found vault to be fairly tolerant of this in the past and have seen it convert properly in every situation where I have used strings for number based parameters. I would agree that it's less than ideal, but it may be worth a shot if you aren't looking to do the larger lift.

zspasztori commented 3 years ago

did you manage to look into the pull request? I have updated it a bit.