Closed Bug 1647934 Opened 8 months ago Closed 5 months ago

Clean up a user's login backup when it may no longer be useful

Categories

(Toolkit :: Password Manager, enhancement, P2)

Desktop
All
enhancement

Tracking

()

VERIFIED FIXED
83 Branch
Tracking Status
firefox83 --- verified

People

(Reporter: MattN, Assigned: prathiksha)

References

Details

(Keywords: perf-alert)

Attachments

(2 files)

Bug 1597358 is automatically creating login backups (behind a pref not yet on-by-default) but since this is automatic we should allow the file to be deleted when it's no longer relevant.

Some ideas

  1. Also remove the backup when the last user-visible login is removed by user interaction (e.g. Remove All, multi-select removal, or individual removal). KatieC agrees
  • We don't want to do this if we just have 0 logins because the file was corrupt, this has to be from user action.
  1. Automatically delete the backup after N days if the user has 0 (user-visible) saved logins at that time. This may be a bit tricky and doesn't help users who uninstall Firefox or don't run it again for a long time.
  2. Add an option to delete the backup when the last login is individually deleted e.g. a button on the "no logins view".

Katie, what do you think?

Flags: needinfo?(kcaldwell)

Katie and I discussed this and decided on option 1 (modified to cover all user methods of deletion)

Flags: needinfo?(kcaldwell)
Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:prathiksha, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(prathikshaprasadsuman)
Pushed by prathikshaprasadsuman@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/047eec5c0b1c
Clean up a user's logins backup when it may no longer be useful. r=sfoster
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch

== Change summary for alert #27046 (as of Thu, 24 Sep 2020 15:42:16 GMT) ==

Regressions:

402% decision gecko-decision opt 120.03 -> 602.58

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=27046

Flags: qe-verify+
Flags: needinfo?(prathikshaprasadsuman)

Hey Prathiksha,

When I delete the last remaining login from about:login the "logins-backup.json" file automatically deletes itself and appears again right after that containing the previously deleted login. Meanwhile the logins.json file is without any saved credentials, as it should be: {"nextId":6,"logins":[],"potentiallyVulnerablePasswords":[],"dismissedBreachAlertsByLoginGUID":{},"version":3}

If I choose to save again new credentials, the "logins.json" file will update itself correctly with the new credentials while the backup file will have: {"nextId":6,"logins":[],"potentiallyVulnerablePasswords":[],"dismissedBreachAlertsByLoginGUID":{},"version":3}

Please take a look at this when you have the time, seems like the backup file is one step behind the logins.json (expected as per https://bugzilla.mozilla.org/show_bug.cgi?id=1597358#c37) but it will also restore itself if it was deleted automatically due to removing all saved logins.
Attached a recording from 83.0a1 (2020-09-27) (64-bit).

Flags: needinfo?(prathikshaprasadsuman)

(In reply to Timea Cernea [:tbabos] from comment #7)

Hey Prathiksha,

When I delete the last remaining login from about:login the "logins-backup.json" file automatically deletes itself and appears again right after that containing the previously deleted login. Meanwhile the logins.json file is without any saved credentials, as it should be: {"nextId":6,"logins":[],"potentiallyVulnerablePasswords":[],"dismissedBreachAlertsByLoginGUID":{},"version":3}

If I choose to save again new credentials, the "logins.json" file will update itself correctly with the new credentials while the backup file will have: {"nextId":6,"logins":[],"potentiallyVulnerablePasswords":[],"dismissedBreachAlertsByLoginGUID":{},"version":3}

Please take a look at this when you have the time, seems like the backup file is one step behind the logins.json (expected as per https://bugzilla.mozilla.org/show_bug.cgi?id=1597358#c37) but it will also restore itself if it was deleted automatically due to removing all saved logins.
Attached a recording from 83.0a1 (2020-09-27) (64-bit).

Timea, this seems like a bug. There seems to be a race between code that's deleting the backup and code that's backing up before changing logins.json. I'll file a bug and get to fixing it. Thanks for catching this!

Flags: needinfo?(prathikshaprasadsuman)
Depends on: 1667762

Thanks for looking into it! Will verify this again after Bug 1667762 is fixed.

Verified-fixed on latest Nightly 83.0a1 (2020-10-06) (64-bit) on Windows 7/10, MacOS 10.15, Ubuntu 16.04.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.