1226026 - Reports of passwords data loss on connected devices

Status: RESOLVED INVALID

(Firefox for iOS :: Sync, defect)

Description

[Richard Newman [:rnewman]]

https://input.mozilla.org/en-US/dashboard/response/5695015

---
Got very excited about iOS version of Firefox - mostly about sync. So I happily put in my details and yes, my Desktop bookmarks were now available on my mobile. Bliss.

And then HORROR!!!

All my saved passwords are now lost!
They are getting lost from any Firefox I dare to open and go on-line, because Sync server is loosing them all!!!
I do have backups, but dear Gods (all of them) - this is BAD!
---

https://support.mozilla.org/en-US/questions/1093850#answer-806124

---
After installation of Firefox for iOS I synced it with my default account [e-mail] and the correct password. After the sync the bookmarks are synced correctly over all devices and all my stored credentials are gone. I need my credentials restored otherwise I will never be able to access a lot of web pages. 
…
My credentials are gone on all my systems (iOS, Windows, Linux) after setting up Firefox Sync on my iOS. On my desktops my firefoxes were updated when then lastest update came out. There is such a file but it has the present time stamp.
---


This is very reminiscent of Bug 1024100 and Bug 1072707. I expect we could find more if we looked.

At least one of these users denies using Clear Private Data, which rules out Bug 1202810.


I have very thoroughly read our current code, and the code we shipped in 1.1:

https://github.com/mozilla/firefox-ios/tree/builds/AuroraV1113


There are two ways a user might see no passwords:

* They're deleted.
* They're all invalid/malformed/empty.


The only way I see this happening is if the user runs Clear Private Data (which copies every login to loginsL and flips is_deleted to 1) or if somehow every incoming login is reconciled to the same local deleted login.

I don't think we support deleting saved logins at all, so there *should be no deleted logins at all* in the local database.

(Incoming synced deletions are immediately deleted with SQL DELETE from both loginsL and loginsM.)


The only way invalid records would be uploaded is if:

 * An incoming record reconciles against a local record.
 * The delta resolution process produces a mess.

We have to have an existing local record for this to happen -- otherwise we don't reconcile at all. That seems unlikely.

My best guess, then, is that this is a desktop Sync bug

Comment 1

[Richard Newman [:rnewman]]

Brian, Steph: do we support deleting logins on the iOS device?

Nick: could you spend half an hour reading Logins/LoginsSynchronizer/SQLiteLogins to see if you see any problems I miss?

Comment 2

[Richard Newman [:rnewman]]

> …and yes, my Desktop bookmarks were now available
> on my mobile.

… which implies that the initial sync completed syncing logins.


> They are getting lost from any Firefox I dare to open and go on-line,

… which implies that there's either a wipe command or real records (deleted or malformed) on the server.

Comment 3

[Stephan Leroux [:sleroux]]

Looks like we have PasswordsClearable which calls into SQLiteLogins:removeAll(). PasswordClearable is called when clearing private data from the Settings. From SQLiteLogins:removeAll(),

1. Clear all newly created local logins
"DELETE FROM \(TableLoginsLocal) WHERE sync_status = \(SyncStatus.New.rawValue)"

2. Mark older local logins for deletetion
"UPDATE \(TableLoginsLocal) SET local_modified = \(nowMillis), sync_status = \(SyncStatus.Changed.rawValue), is_deleted = 1, password = '', hostname = '', username = '' WHERE is_deleted = 0"

3. Copy mirrored bookmarks into our local table and mark them as deleted
"INSERT OR IGNORE INTO \(TableLoginsLocal) (guid, local_modified, is_deleted, sync_status, hostname, timeCreated, timePasswordChanged, password, username) " +
"SELECT guid, \(nowMillis), 1, \(SyncStatus.Changed.rawValue), '', timeCreated, \(nowMillis)000, '', '' FROM \(TableLoginsMirror)"

4. Update entries in the mirror table to is_overridden = 1
"UPDATE \(TableLoginsMirror) SET is_overridden = 1"

Only things I can see that might be problematic is step 3 and 4.

Also, for some reason we don't actually use removeLoginByGUID in the app anywhere - just in tests...

Comment 4

[Richard Newman [:rnewman]]

(In reply to Stephan Leroux [:sleroux] from comment #3)

> Also, for some reason we don't actually use removeLoginByGUID in the app
> anywhere - just in tests...

Yup, that's what I thought -- nothing but CPD. So a user who's never cleared will never have deleted records in the DB.

Comment 5

[[:philipp]]

another user with the same symptoms confirmed that he cleared private data: http://www.camp-firefox.de/forum/viewtopic.php?f=1&t=115558 (in german). 
he makes a point there that maybe it would be better if "saved logins" weren't selected by default while clearing data...

Comment 6

[Nick Alexander :nalexander [he/him]]

(In reply to Richard Newman [:rnewman] from comment #1)
> Brian, Steph: do we support deleting logins on the iOS device?
> 
> Nick: could you spend half an hour reading
> Logins/LoginsSynchronizer/SQLiteLogins to see if you see any problems I miss?

I never got to this, but I'm not likely to get to it any time soon :(

Comment 7

[Lougenia Bailey]

Closing this as the Firefox iOS sync code referenced here in 2015 was replaced by the application services logins rust component in 2019.