Closed BlueDrink9 closed 5 years ago
Note: The failing shellchecks are mostly because of the use of separate files for the configs
Also consider renaming the repo to reflect its generic status
Thanks for the huge effort @BlueDrink9! Sorry for the late response, busy week at work...
Unfortunately, this PR is a bit different from my vision for this plugin. As I understood, This PR allows the users to create their own "adapters" (I'm not settled on this word yet, but this basically means the different implementation for password managers), which makes this plugin to be a "framework", which I don't want. Maintaining such framework means that:
We could break users very easily, as the implementation of the adapters is on their side. This could be solved by making the users lock the plugin to a specific version (using a git tag or a commit hash), but I'm not sure if TPM (which is what I'm using) supports it, and this method also has it's own issues.
We would either need to take care of a lot of cases, or be as generic as possible. Each password manager CLI could have a different way of returning the data (for example, 1pass
only returns name and password - no UUID).
There are more issues with this PR, as I see it:
Very coupled with jq
. Adapters should have their own implementation. Maybe the password manager's CLI does not return a JSON (like 1pass
).
The API seems... unnecessary. As I see it,adapters should expose the same API, so for example each adapter would expose a get_items
function (with that exact name), not variables referring to it. The adapter would then simply be sourced and the plugin will assume that all the function has been declared. This should also make shellcheck
happier.
There is also the $FILTER_URL
thing. I used the sudolikeaboss://local
URL to filter items in order for this plugin to be compatible with the sudolikeaboss project (which I was using at the time), and in order to avoid bloating the entire list with irrelevant items. This should only apply for the 1Password adapter. In addition, some CLIs may not provide you with a URL to filter (such as 1pass
).
This PR adds a ton of option to the main file. Options that are specific for a password manager should be declared in the adapter's file.
There are a lot of Vim's modelines here. As much as I love Vim, these shouldn't be committed.
I noticed the "Theoretically working but totally untested 1pass version" commit. If this was not tested, this shouldn't be in this PR.
As the changes are pretty big, I'll close this PR for now. I'm planning to make a simpler "generalization" for this project in the near future.
Fine by me. I've fixed a few of those issues in my fork to make it more generic anyway, and I'll keep using my fork.
lpass
and on
allow json output (optional, in the lpass
case), the rest can provide a custom filter.on
passwords need that url and lpass and everything else doesn't," so I made it generic. I think 1pass
is an exception, as a wrapper, because most password managers have associated URLs. It was a variable just to make it easier to change for debug reasons.In the meanwhile, my fork works fine with lastpass, so would you be willing to put a link to mine for lastpass users only?
Apologies, but making this generic required a large re-write so it's not a small PR.
This takes the password manager specific commands and replaces them with a generic version, that calls the specific version based on variables. These variables are set in separate files, kinda like plugins.
This PR in particular has only been tested to work with lastpass, not 1password, so needs serious testing. I don't think I've changed any of the specifics though, just moved them, and the same generic code works for the lastpass specifics, so I'm hopeful it still works.
I've updated the readme and added configuration documentation.
There's also the shell of a 1pass version for #6, but again, it's entirely untested and required some extra new custom code.