browserpass / browserpass-legacy

Legacy Browserpass repo, development is now happening at:
https://github.com/browserpass/browserpass-extension
MIT License
1k stars 80 forks source link

Add an option to autofill the login form after URL is opened via browserpass #249

Closed jcromero closed 5 years ago

jcromero commented 6 years ago

Should it be possible that g shortcut opens login page in the current tab, aufofills login fields and autosubmits login form? That way it can be avoided to have duplicated tabs in the browser for the same domain.

maximbaz commented 6 years ago

It was not designed for this, but it is an interesting idea.

For current tab vs new tab, I propose this approach:

I know it's a breaking change, but I saw this behavior in vimium and surfingkeys extensions, and I find it very intuitive.

When the tab is loaded, initiating the "autofill + submit" should not be a big problem.

PR is welcome!

jcromero commented 6 years ago

Could you please explain then what g shortcut was designed for?

erayd commented 6 years ago

@jcromero As currently implemented, it opens the target website in a new tab, and handles HTTP basic authentication if required.

@maximbaz Rather than break the current behaviour, can we designate a different hotkey for what @jcromero wants to do? I prefer the current behaviour as-is; I generally want the site in a new tab, and unless it's basic auth, I do not want it attempting to autofill logon forms on the newly opened page. No objection to somebody adding that behaviour, but I'd prefer that it be additional, rather than instead of how things work now.

jcromero commented 6 years ago

@erayd Why do you want the site duplicated in a new tab? I find myself always switching and closing tabs to avoid those duplications.

maximbaz commented 6 years ago

@jcromero: as mentioned already, one reason why the button was needed is to support basic auth (it's when browser stops loading page and asks for credentials, often seen on home router pages). An extension can only submit credentials to such websites if and only if this URL was opened by this extension.

Second reason is that some people want to configure custom URLs in their password files and use browserpass to open the URLs for them. For example, one could have a file called github.com.gpg, have url: https://github.com/login, on a new tab the user would fire up browserpass, type "git", press g on the github.com entry, and the login page will be opened in a new tab.

And your proposal fits nicely in the second scenario, it was not implemented yet because we didn't think about it before and nobody needed this, but I like the idea 🙂

@erayd: there's a thing of having too many options 😃 What do you think about pressing Shift+G or holding Shift when clicking on a button to open it in a new tab? When you click a link on a browser, it opens in the current tab by default, but when you hold a modifier (Ctrl) it opens it in a new tab.

As for auto-filling, it will happen only if the login form is present, and nothing will be auto-filled if there is no login form. Is that okay for you?

erayd commented 6 years ago

@jcromero

Why do you want the site duplicated in a new tab?

I don't. I open some new sites via the extension, I do not launch the site from the current tab a second time. As per @maximbaz's second scenario above, I open URLs via the extension in a new tab after searching.

@maximbaz I'm OK with shift+g to launch in a new tab (and have g change to opening in the current tab). I'd prefer that they be the other way around, but I don't feel strongly enough about that to push hard on it. However, I feel very strongly that autofill should not be the default behavior - it's a security risk. Could we maybe have autofill-on-launch be another option checkbox, that defaults to off?

maximbaz commented 6 years ago

Could you explain a case when this would a security risk? Just want to have an example in mind. Suppose you open github.com via browserpass, you see a login form and not only you don't want to fill out the form, you don't even want to login? Why would you open this website from the browserpass in the first place then?

Of course we would add a domain name check, in case there was a redirect.

erayd commented 6 years ago

@maximbaz The primary risk is where the page opened is not the intended login page. For example, browserpass might launch work.com based on the file path (e.g logins/work.com/payroll), but what the user actually wants to do is log in to a different application on that domain, or to another location altogether that happens to be linked to from work.com (e.g.a staff portal link to the company payroll app).

Autofill on launch can result in login credentials being leaked to a page that isn't associated with those credentials, which the user did not intend to log in to.

maximbaz commented 6 years ago

Good examples, thanks. We should also keep in mind that we guess domain based on the file path, but our guess is not guaranteed to be always correct. Since there is a chance to leak credentials to a wrong party, I agree that we should not have this new behavior enabled by default.

maximbaz commented 6 years ago

I made g shortcut use the current tab and added Shift+g that opens a new tab, because it's two against one 😛. But jokes aside, I honestly find this behavior more intuitive, and it matches better the experience I see in the browser and in other extensions.

This ticket remains open for the functionality to auto-inject the form fields and auto-submit the form. Auto-submit is already disabled by default, auto-inject needs a new option that will also be disabled by default, as agreed above.

jcromero commented 6 years ago

@erayd I think, then, that I'm missing anything, because when I press Shift+L and introduce a domain already stored in my password manager, I get nothing. I only get domains matching the one in the browser's URL bar. I see that you are using the extension like a kind of bookmarks manager (for domains with accounts stored in your password manager). I can not do that :/

I'm using chromium (debian stable) and the last version of the extension (just updated)

@maximbaz Thanks for the partial implementation of the feature.

jcromero commented 6 years ago

Ups!! I have to press Enter to get the results of the search!! I thought they were showed automatically when pressing keys in the search box. It would be cool to have search as-you-type :)

Sorry for the noise.

erayd commented 6 years ago

@jcromero Yup - the as-you-type stuff filters existing search results. You can press backspace to clear the existing search, or type something that isn't in the list, then press enter to run a new search :-).

If you want to search everything in your password store interactively, just search for something like / or . - that should return most or all of the items in your password store, and then you can just use the type-to-filter behavior as you would for any other search result.

jcromero commented 6 years ago

@erayd Thanks for the tip!!

maximbaz commented 6 years ago

@erayd thanks for explaining (and implementing 😉), since it's not the first time such question arises, I added this info to the README so that next time we can refer to the docs 😉

b192134 and 5557ab6

jcromero commented 6 years ago

@maximbaz Thanks for completing the documentation. I have a final request with respect to that documentation. Could you please explain how can I get the green tags showed in the first gif of README.md, when searching for domains in the search box ("personal" and "work" tags)? I'm not able to do it.

maximbaz commented 6 years ago

@jcromero sure, done: https://github.com/dannyvankooten/browserpass/commit/9233a667623acf981d92c2e8dd7ad0fa1fa3ae9c

maximbaz commented 6 years ago

When this is implemented, in addition to introducing the setting in extension options, I'd like to also be able to override the configuration on a per-site basis. The implementation will be very similar to https://github.com/dannyvankooten/browserpass/pull/252.

That way, I can keep "auto-fill after open" disabled by default, but enable on a few sites that I trust and open often.

maximbaz commented 5 years ago

After a thorough consideration, I decided to close this request as out of scope, sorry.

Browserpass is not intended as bookmark manager, I wish we didn't have "launch URL" functionality altogether, but we need to have it in order to support Modal HTTP authentication. I don't want to have any functionality that encourages using Browserpass as a bookmark manager.

In fact in v3 I'm removing the "Open URL" button from popup altogether, so we only have keyboard shortcuts to launch URLs.