brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.67k stars 2.31k forks source link

Auto-fill saves personal data on disk without the user's knowledge #31960

Open fmarier opened 1 year ago

fmarier commented 1 year ago

By default, the toggle in brave://settings/addresses is enabled: Screenshot from 2023-07-28 12-48-16

https://fmarier.org/test/autofill.html shows two different ways that this feature works:

  1. Incomplete address form: not all of the fields are present and so the individual components will be saved in the browser (hidden, can't see or delete them individually) without a prompt.
  2. Complete address form: all of the fields are present and a prompt will be shown before the complete address is saved and visible in brave://settings/addresses: Screenshot from 2023-08-02 20-06-47 If denied, the full address is not saved, but the individual components are still saved just as in the previous case (invisibly and without the user's knowledge).

The only indication that portions of an address are saved is in brave://settings/clearBrowserData (see Auto-fill form data) where all of the data for all sites can be deleted at once: clear-site-data

It's also possible to mouse over an element and then hit Ctrl+shift+Del (Shift+Fn+Del on Mac) to delete it from the local auto-fill database.

Note that nothing new is recorded in private windows, but the existing saved auto-fill info is available to be auto-filled in private browsing windows too (brave/brave-browser#9795).

The behavior is the same on desktop and Android, as well as in Chrome and Firefox.

fmarier commented 1 year ago

Saving complete addresses by default sounds good to me since there's a prompt each time (just like before passwords are saved), but the automatic and mostly-invisible saving of incomplete addresses by default seems problematic for a privacy browser.

fmarier commented 1 year ago

According to https://chromium.googlesource.com/chromium/src/+/main/components/autofill/README.md#what_s-the-difference-between-autofill-and-autocomplete, it looks like there is already a split in the code between autofill (what I call complete addresses above) and autocomplete (what I refer to as incomplete addresses).

fmarier commented 1 year ago

Here's a concrete suggestion:

  1. Add a new (default OFF) setting in brave://settings/addresses and alter the description of the existing setting: Screenshot from 2023-08-18 15-24-34

  2. Add a new doorhanger prompt (like the "Save address?" one pictured above) when we're about to save an autocomplete field:

Would you like Brave to save this kind of information and fill it in for you later? person@example.com

[ Yes ] [ No ]

With the following behavior:

szilardszaloki commented 1 year ago

Hey @fmarier, Not sure how wild of a thought it is, but don't we want to disable autocomplete entirely on the autofill path? IIUC, the user is prompted each time Chromium is about to save any autofill/typed information (addresses, credit/debit cards, etc.). If the user hits No, thanks, it doesn't seem to make a whole lot of sense to store the info as autocomplete data (not even if the user agreed to do so via the other prompt we're just about to introduce). If the user says they don't want the data to be saved, they most probably mean "not in any way", rather than "not as autofill data, but autocomplete is okay". WDYT?

fmarier commented 1 year ago

IIUC, the user is prompted each time Chromium is about to save any autofill/typed information (addresses, credit/debit cards, etc.).

I was actually thinking (though I didn't make that clear, thanks for pointing it out) that it would be a one-time prompt. You have to go into the settings to enable/disable later.

So amending my earlier suggestion slightly for clarity:

Would you like Brave to save this kind of information and fill it in for you later? person@example.com

[ Yes, always ] [ No, never ]

That said, I'm open to other people's opinions. I was mainly trying to put something concrete out there to move this forward and encourage comments. I'm not sure I have the best solution.

If the user hits No, thanks, it doesn't seem to make a whole lot of sense to store the info as autocomplete data (not even if the user agreed to do so via the other prompt we're just about to introduce). If the user says they don't want the data to be saved, they most probably mean "not in any way", rather than "not as autofill data, but autocomplete is okay". WDYT?

I think you make a good point. If you say no to saving parts of an address, then you most likely don't want to save that complete address either.

If we make our prompt one-time only, then perhaps the double-prompt is not a big deal. On the other hand, one could say that if we make our prompt one-time only, then it's also not a big deal if you can't save the very first complete address (but you get prompted to save any future ones).

Maybe @aguscruiz has some thoughts here.

szilardszaloki commented 1 year ago

@fmarier I know you were thinking that ours would be a one-time prompt. 🙂 I should have worded better. So what I meant is that if the user says "no" (via Chromium's built-in autofill prompt) to saving the complete address (as autofill data), then they most likely don't want to save parts of the address (as autocomplete data) - that's what I meant by "disabling autocomplete entirely on the autofill path". And therefore it wouldn't really matter what the user's choice was on our prompt (if they've seen our prompt at all), as we would get rid of the autocomplete functionality on the autofill path (that is, when saving typed data, e.g. addresses, credit/debit cards, IBAN addresses, etc.). This would also mean that the double-prompt could never occur.

fmarier commented 1 year ago

So if I understand your suggestion correctly, this is what the two cases would look like (assuming user says yes to prompts):

So you're right that it would eliminate the possibility of a double prompt. However, it would also mean that users who said yes to the autocomplete prompt would also miss out on some data that would otherwise be remembered when filling out full address forms.

szilardszaloki commented 1 year ago

Yeah, and that's why I was wondering if this would be problematic. It does seem to be redundant, as the data is already saved as autofill data.

szilardszaloki commented 1 year ago

But anyway, it would probably be best to not change this behavior.

szilardszaloki commented 1 year ago

I did check what the double-prompt would look like by the way, and it's not the end of the world.

fmarier commented 11 months ago

@szilardszaloki @aguscruiz and myself just had a chat and decided that the following approach would make the most sense:

will-gant commented 6 months ago

Would love to see some changes here. I think my use case is pretty common, but it's not currently well catered to:

  1. I use an external password manager that has a browser extension, and want this (and only this) to autofill usernames and passwords
  2. I want my address autofilled by the browser (my password manager isn't very good at this part).

At the moment it appears the only way to achieve this is by toggling 'Save and fill addresses', but the consequence of that is that whenver I focus on any login prompt I get: a) A suggested login from my password manager (always the thing what I want) b) A separate autofill suggestion from Brave (which, rather irritatingly, tends to float above the password manager's suggestion), proposing three or four usernames - many of which aren't usernames at all.

The only alternative to the above seems to be to disable 'Save and fill addresses', which means I lose the ability to have Brave fill my home address in forms.