1248765 - Remove special case handling of an FxA saved password by Sync

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect, P2)

Description

[Ryan Feeley [:rfeeley]]

Firefox currently offers to save a user’s Firefox Accounts password for Sync, whether or not it’s correct. Should Firefox be saving this password at all? Let’s discuss.

Comment 1

[Ryan Feeley [:rfeeley]]

I should also note that whatever Sync credentials are saved appear to be deleted when the browser disconnects from Sync.

Comment 2

[Chris Karlof [:ckarlof]]

(In reply to Ryan Feeley [:rfeeley] from comment #0)
> Firefox currently offers to save a user’s Firefox Accounts password for
> Sync, whether or not it’s correct. Should Firefox be saving this password at
> all? Let’s discuss.

Why not? It's the user's choice if she wants her FxA password saved. I recall one user sharing a valid workflow: he had a hard-to-remember FxA password that he wanted to sync to his mobile phone to make it easily to recall when logging in to other Desktop devices. Makes sense. Not offering to save it for him makes this harder (or impossible) for him.

Comment 3

[Chris Karlof [:ckarlof]]

(In reply to Ryan Feeley [:rfeeley] from comment #1)
> I should also note that whatever Sync credentials are saved appear to be
> deleted when the browser disconnects from Sync.

Sync uses the password manager to store FxA credentials (in JSON format), but if I ask to save my FxA password during the Sync sign in flow, it does it indeed store a normal looking email and password as well.

Sync *should* delete the FxA credentials stored by Sync when you disconnect, but it shouldn't probably delete the FxA email and password you asked it to save. It’s strange and I can’t find what would be causing this.

Mark, any idea?

Comment 4

[Ryan Feeley [:rfeeley]]

(In reply to Chris Karlof [:ckarlof] from comment #2)
> (In reply to Ryan Feeley [:rfeeley] from comment #0)
> Why not? It's the user's choice if she wants her FxA password saved.

Just spit-balling here, but if we were to leverage the FxA password as a master password (the way Chrome does with the Google account password) isn't it generally a bad idea to store said master password in the list of passwords? I'm only speaking from experience with other password managers, and am personally foggy on why one would want to avoid doing this.

Comment 5

[Mark Hammond [:markh] [:mhammond]]

The code to remove (and not sync) these passwords landed in bug 967047 - but I can't see the rationale for including the FxA hosts, but I think it was "we don't want to sync it" and the fact it is deleted on unlink is just a side-effect of that.

If we wanted to avoid syncing it but also not delete it on unlink we could refactor a few things to make that possible. If we wanted to allow it to sync and not delete it on unlink the change would be trivial (we'd just remove the block at https://dxr.mozilla.org/mozilla-central/rev/0629918a09ae87808efdda432d7852371ba37db6/services/sync/modules/util.js#625)

Comment 6

[Tony Mechelynck [:tonymec]]

Firefox Accounts is now used for more than just Sync: AMO now uses it too.

IMHO the Firefox Accounts login ought to be treated exactly the same as any other login, including AMO logins up to a few days ago.

Comment 7

[Chris Karlof [:ckarlof]]

(In reply to Ryan Feeley [:rfeeley] from comment #4)
> (In reply to Chris Karlof [:ckarlof] from comment #2)
> > (In reply to Ryan Feeley [:rfeeley] from comment #0)
> > Why not? It's the user's choice if she wants her FxA password saved.
> 
> Just spit-balling here, but if we were to leverage the FxA password as a
> master password (the way Chrome does with the Google account password)

As a point of reference, Chrome doesn't prevent you from saving your Google password in Chrome.

> isn't it generally a bad idea to store said master password in the list of
> passwords? 

What's the bad outcome you're worried about? Someone that gets a hold of all your passwords also gets your master password? Sounds like the horse is already out of the barn.

Comment 8

[Ryan Feeley [:rfeeley]]

Some proposed principals:
1. Anything viewable in the list of saved logins should be sync-able
2. Anything viewable in the list of saved logins should be viewable and editable
3. Nothing in saved logins should disappear without explicit user action
4. Anything that the user has chosen to save should appear in the list of saved logins.
5. We don't ask the user to save anything that is now viewable and editable in saved logins

Comment 9

[Chris Karlof [:ckarlof]]

> If we wanted to allow it to sync and not delete it on unlink the change would be trivial (we'd just remove the block at https://dxr.mozilla.org/mozilla-central/rev/0629918a09ae87808efdda432d7852371ba37db6/services/sync/modules/util.js#625)

In support of :tonymec, I feel we should sync and not delete saved passwords for accounts.firefox.com, i.e., treat it like any other saved password.

Comment 10

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0001-Bug-1248765-allow-syncing-of-FxA-password-and-don-t-.patch

This patch allows us to sync the FxA password and doesn't delete it when unlinking the device from Sync. The credentials blob itself is still treated as it always was (ie, is not synced and is removed on unlink)

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/e0453757fb6c

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e0453757fb6c

Comment 13

[Alex Davis [:adavis]]

Just adding a note here. This requires a review in the near future.

This saved password behavior has been brought to my attention twice this week because users think it's a bug and security issue.

The primary user story is:
As a Firefox user, when I sign out of my Firefox account in Firefox, I expect my passwords to be forgotten by the browser.

This story is likely very relevant on a shared/public computer. (e.g. coffee shop, school, shared home computer or tablet)

You can imagine that when users think this is the expected behavior, it's even more shocking to know that the browser can remember your login credentials for your Firefox Account. A malicious person on the same device could log back in to FxA and if your email login information was also saved in the browser, they can even confirm the login via the email sent out.

The solution might not be to delete the information automatically. But rather, it could be to provide users with instructions on how to delete their passwords themselves if they would like to.

Comment 14

[Mark Hammond [:markh] [:mhammond]]

(In reply to Alex Davis [:adavis] [PM FxA+Sync] from comment #13)
> The primary user story is:
> As a Firefox user, when I sign out of my Firefox account in Firefox, I
> expect my passwords to be forgotten by the browser.

While passwords offer the highest risk, I guess it really goes for *all* data - eg, a user expecting passwords to be removed probably also expects history and bookmarks to be removed.

We could add special UI around the disconnect button and then wipe pretty-much everything on the client, but I suspect getting the UX around this will be somewhat tricky - I can imagine users who mis-read or mix-click complaining about data-loss (although I guess that is largely mitigated by the ability to reconnect the account and thus re-grab most of that data).

Regardless, this would require a new bug - Alex, can you please open one and ask rfeeley for UX advice?

Comment 15

[Mark Hammond [:markh] [:mhammond]]

Bug 1162778 is a version of that bug for iOS.

Comment 16

[Richard Newman [:rnewman]]

Discussion is in Bug 1325271.

Comment 17

[Ryan Feeley [:rfeeley]]

I don't see disconnecting Sync as related to erasing Firefox data like passwords, bookmarks, history.