Add gopass support to qute-pass via a CLI switch #5143

bascht · 2019-12-10T13:22:48Z

The new CLI-switch --pass-executable makes qute-pass compatible with the gopass password store (https://github.com/gopasspw/gopass). While gopass itself is mostly compatible with pass, it offers the possibility to mount multiple shared password stores. qute-pass way of just traversing PASSWORD_STORE_DIR won't help in that case.

--

I've tried to keep backwards-compatibility with the old behaviour as much as possible. The method find_pass_candidates will now just branch out into two different modes depending on which password executable is used. In the pass way, the code stayed the same.

Closes #5142

The-Compiler · 2019-12-10T13:28:33Z

@cryzed @wildente @Holzhaus @jtyers @brianclemens @lufte Can you help with reviewing/testing this?

(Also, if you'd rather not be pinged for future qute-pass changes, let me know! I figured I'd ping some people who contributed to qute-pass in the past, as I don't use it myself)

lufte

It works correctly, and the code looks good.

Here's how I tested it:
1- Moved my current store somewhere else so I don't break anything.
2- Installed gopass and created a new store.
3- Added a password for somesite.
4- Mounted a new store with gopass mounts add store1.
5- Added another password for somesite under store1 with gopass generate store1/somesite/user.
6- Used the new userscript from somesite and it correctly showed me the two options.

Regarding the documentation of the new parameter (which says Executable to start (pass or gopass)) I wonder if we should limit the options of allowed arguments (with choices) or simply remove the (pass or gopass) part, since the code is not really limiting to those two options.

Besides that, looks great.

bascht · 2019-12-11T12:27:06Z

@lufte Wow, thanks for the extensive test and feedback! Re the new start parameter. I wonder whether it could be something like --gopass-compatibility-mode (or rather something more catchy).

I recon some people could have gopass aliased to pass, since it behaves almost in the same way.

Holzhaus · 2019-12-13T02:40:16Z

How about --mode gopass? If a third pass implementation pops up, we can just add a third choice for that argument instead of messing around with multiple mutually exclusive flags.

The-Compiler · 2019-12-30T21:35:22Z

The current argument/documentation indeed feels a bit confusing to me - it reads like I could do --pass-executable /some/path/to/gopass but that won't work.

I can see two ways forward:

Keep --pass-executable, but check if the os.path.basename() of it is gopass, and use the given path to execute the binary.
Use --mode gopass / --mode pass like proposed by @Holzhaus (and then use either gopass or pass in $PATH).

I don't really mind either way, so I'll leave it up to you (plural) which one you prefer.

Holzhaus · 2019-12-31T01:13:01Z

I'd prefer the latter. We could also have --mode auto (selected by default) which guesses the mode from the executable name.

bascht · 2020-01-02T07:52:37Z

Yip, I think the --mode suggestion is the most sensible, I'm gonna rework the PR and go with the --mode auto as a default, like @Holzhaus suggested.

The new CLI-switch --mode makes qute-pass compatible with the gopass password store (https://github.com/gopasspw/gopass). While gopass itself is mostly compatible with pass, it offers the possibility to mount multiple shared password stores. qute-pass way of just traversing PASSWORD_STORE_DIR won't help in that case. Closes qutebrowser#5142

bascht · 2020-01-06T21:50:44Z

Soooo – once I had everything typed down I noticed that --mode auto might be more confusing than helpful, since

pass is in my opinion a sensible default and

FileNotFoundError … No such file or directory: 'pass': 'pass'

is a helpful error message for users that only have gopass.
pass and gopass might both be in $PATH but the user wants to use pass
pass and gopass are both used for different password stores (actual use-case from a co-worker :D)

So, 5bf7c8e adds a simple --mode switch with the default set to pass so it doesn't break backwards compatibility.

@Holzhaus @lufte wdyt?

Holzhaus · 2020-01-07T17:55:50Z

@bascht That is okay for me. However, you could also rename auto to something more descriptive like prefer-gopass or prefer-pass (which falls back to the other tool if it's not in $PATH) and make that the default, but that's just an idea. I'm just using plain pass anyway ;-)

lufte · 2020-01-09T00:27:41Z

Ah, a fellow force-pusher like myself. I had to dig a little bit to be able to compare exactly what you changed since the last commit, instead of reviewing the whole diff again 😅

I think this is the best option and also we shouldn't keep bikeshedding this issue.

bascht · 2020-01-09T09:39:23Z

Oh yeah, sorry 'bout the unannounced force push – I was in my regular work projects workflow where I constantly rebase to keep the history tight. 😬

So, if everyone is fine with the current solution:
Thank you for your help and feedback!

The-Compiler · 2020-01-09T09:45:20Z

Awesome, thanks everyone! I'll get back to this ASAP, currently working on some remaining Qt 5.14 changes.

@bascht @Holzhaus @lufte Is it okay if I ping you when there are future qute-pass changes to be reviewed? If so, please give this comment a 👍.

lufte · 2020-01-09T13:34:06Z

🙈 you're right @mschilli87

Holzhaus · 2020-01-09T14:08:48Z

misc/userscripts/qute-pass

 argument_parser.add_argument('url', nargs='?', default=os.getenv('QUTE_URL'))
 argument_parser.add_argument('--password-store', '-p',
                             default=os.getenv('PASSWORD_STORE_DIR', default=os.path.expanduser('~/.password-store')),
                             help='Path to your pass password-store')


Can you add (only used in pass-mode) to the help text?

Good catch, added. 👍

misc/userscripts/qute-pass

Co-Authored-By: Jan Holthuis <holthuis.jan@googlemail.com>

bascht · 2020-01-14T10:58:01Z

Build failed but, but this looks unrelated 👼 – should I rebase on master?

The-Compiler · 2020-01-14T11:07:37Z

Meh, flaky tests - feel free to just ignore them.

The-Compiler · 2020-01-21T09:06:01Z

Thanks @bascht for the change and @lufte @Holzhaus for the reviews! 👍

There was a small conflict with #5199, but I think I fixed things up correctly.

bascht · 2020-01-22T10:13:27Z

Thank you!

lufte approved these changes Dec 11, 2019

View reviewed changes

The-Compiler added the status: work in progress label Dec 30, 2019

The-Compiler mentioned this pull request Dec 31, 2019

add basic gopass support #5170

Closed

bascht force-pushed the add-gopass-support-to-qute-pass branch from 3b8c5b9 to 5bf7c8e Compare January 6, 2020 21:41

The-Compiler removed the status: work in progress label Jan 9, 2020

This comment has been minimized.

Sign in to view

Holzhaus suggested changes Jan 9, 2020

View reviewed changes

bascht and others added 2 commits January 13, 2020 21:45

Reword help text with a more clearer description

dc4ce53

Co-Authored-By: Jan Holthuis <holthuis.jan@googlemail.com>

Add note that --password-store is only available in pass-mode

481261a

The-Compiler merged commit 481261a into qutebrowser:master Jan 21, 2020

bascht deleted the add-gopass-support-to-qute-pass branch January 22, 2020 10:13

Uh oh!

Add gopass support to qute-pass via a CLI switch #5143

Add gopass support to qute-pass via a CLI switch #5143

Uh oh!

Conversation

bascht commented Dec 10, 2019

Uh oh!

The-Compiler commented Dec 10, 2019

Uh oh!

lufte left a comment

Choose a reason for hiding this comment

Uh oh!

bascht commented Dec 11, 2019

Uh oh!

Holzhaus commented Dec 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

The-Compiler commented Dec 30, 2019

Uh oh!

Holzhaus commented Dec 31, 2019

Uh oh!

bascht commented Jan 2, 2020

Uh oh!

bascht commented Jan 6, 2020

Uh oh!

Holzhaus commented Jan 7, 2020

Uh oh!

lufte commented Jan 9, 2020

Uh oh!

bascht commented Jan 9, 2020

Uh oh!

The-Compiler commented Jan 9, 2020

Uh oh!

This comment has been minimized.

lufte commented Jan 9, 2020

Uh oh!

Holzhaus Jan 9, 2020

Choose a reason for hiding this comment

Uh oh!

bascht Jan 13, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bascht commented Jan 14, 2020

Uh oh!

The-Compiler commented Jan 14, 2020

Uh oh!

The-Compiler commented Jan 21, 2020

Uh oh!

bascht commented Jan 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Holzhaus commented Dec 13, 2019 •

edited

Loading