Remove special case handling of an FxA saved password by Sync

RESOLVED FIXED in Firefox 47

Status

()

Toolkit
Password Manager
P2
normal
RESOLVED FIXED
2 years ago
10 months ago

People

(Reporter: rfeeley, Assigned: markh)

Tracking

unspecified
mozilla47
Points:
---
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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.
(Reporter)

Comment 1

2 years ago
I should also note that whatever Sync credentials are saved appear to be deleted when the browser disconnects from Sync.
(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.
(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?
Flags: needinfo?(markh)
(Reporter)

Comment 4

2 years ago
(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.
(Assignee)

Comment 5

2 years ago
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)
Flags: needinfo?(markh)
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.
(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.
(Reporter)

Comment 8

2 years ago
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
> 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.
Flags: firefox-backlog+
Priority: -- → P2
Summary: Discuss to save or not to save the Firefox Accounts password → Remove special case handling of an FxA saved password by Sync
(Assignee)

Comment 10

2 years ago
Created attachment 8721829 [details] [diff] [review]
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)
Assignee: nobody → markh
Status: NEW → ASSIGNED
Attachment #8721829 - Flags: review?(rnewman)
Attachment #8721829 - Flags: review?(rnewman) → review+

Comment 11

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

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e0453757fb6c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
(Reporter)

Updated

a year ago
See Also: → bug 1296584
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.
See Also: → bug 1325271
(Assignee)

Comment 14

10 months ago
(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?
(Assignee)

Comment 15

10 months ago
Bug 1162778 is a version of that bug for iOS.
Discussion is in Bug 1325271.
(Reporter)

Comment 17

10 months ago
I don't see disconnecting Sync as related to erasing Firefox data like passwords, bookmarks, history.
(Assignee)

Updated

10 months ago
See Also: → bug 1343269
You need to log in before you can comment on or make changes to this bug.