node-vault icon indicating copy to clipboard operation
node-vault copied to clipboard

Allow a useHomeToken & tokenFile parameter options

Open RyanCopley opened this issue 7 years ago • 2 comments

Our usage of the node-vault library typically involves a thin wrapper to get the authentication token from various locations. Partly for the development experience, but absolutely as a deployment strategy. I am proposing two new configuration parameters useHomeToken and tokenFile.

useHomeToken is useful because $ vault login places a copy of the token in ~/.vault-token (https://www.vaultproject.io/docs/commands/token-helper.html). This would allow local development to automatically negotiate with the user that is already logged into the system.

tokenFile is useful because of our deployment strategy. We have a docker / kubernetes environment for hosting our applications, and part of the orchestration is a docker container that does the vault authentication for us and places a token on the disk. There does not seem to be a native way to use a token file, so this adds it in.

If this approach is appreciable, I can update the tests 👍

RyanCopley avatar Feb 27 '18 22:02 RyanCopley

Codecov Report

Merging #81 into master will decrease coverage by 2.52%. The diff coverage is 62.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #81      +/-   ##
==========================================
- Coverage     100%   97.47%   -2.53%     
==========================================
  Files           2        2              
  Lines         111      119       +8     
  Branches       21       23       +2     
==========================================
+ Hits          111      116       +5     
- Misses          0        3       +3
Impacted Files Coverage Δ
src/index.js 97.36% <62.5%> (-2.64%) :arrow_down:

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 3fd7a99...ab8f958. Read the comment docs.

codecov[bot] avatar Feb 27 '18 23:02 codecov[bot]

Hi Ryan, thanks for your PR!

We are currently working on 1.0.0 in the #78 PR, I think it's best if we can consider this functionality for that version, based on the refactored codebase. We could add it as another auth, just like the current aws auth that I have on a separate branch in my fork repo (you can check it, along with the latest comments of the PR I mentioned before).

I would need to think a bit more about how this fits with the current proposal and discuss with the other people involved, but this is some quick personal feedback that comes to mind:

  • I think this is gonna be a very common use case, but it needs to be well designed in two important ways:
  1. Needs to be flexible enough to accommodate most valid ways to implement this file-based auth system.
  2. Needs to have a very good default so that no parameters are needed in the maximum of cases possible.

This last point is because to pass parameters you would need to write something like this:

const nodeVault = require('node-vault/auth/file')('filepath')

Or potentially something like:

const nodeVault = require('node-vault/auth/file')({
  filepath: <filepath>,
  regex: <regex expression>,
  transform: <transform function>
})

But if we define a good default then we can do something a bit more compact.

Iguess a decent one would be a simple file in the home of the user that runs Node named .vault-token. Like this:

const nodeVault = require('node-vault/auth/file')({
  filepath: untildify('~/.vault-token')
})

But by using the default it gets way shorter and straightforward:

const nodeVault = require('node-vault/file') // maybe 'fs' instead?

Of course we could also make it possible to use a shorthand for defining just a filepath, because in most cases you will only have a single file where the content is just the token, with no regex or transformations needed. For example:

const nodeVault = require('node-vault/auth/file')('~/.secrets/vault-token') // with untildify support

Let me know your thoughts.

I guess the next step to move this proposal forward is to move it to the right branch, which has not been created yet in this repo because the merge request is directly from my fork branch to this repo's master. I'll get it done sometime soon. Meanwhile you can checkout the branch fork my fork in GitHub and fork it into another branch, then code some stuff that we can later iterate on. I can help with the code also, but it depends on how much free time I have in the next months.

I'll write an update when the 1.0 branch is ready in this repo so you can change the PR destination to it.

If this gets done in time, it can be shipped with 1.0.

DaniGuardiola avatar Feb 27 '18 23:02 DaniGuardiola

closing due to staleness; please reopen if desired

aviadhahami avatar Nov 10 '22 17:11 aviadhahami