Provide a recovery solution for AVG users who had `logins.json` access from Firefox prevented
Categories
(Toolkit :: Password Manager, task, P1)
Tracking
()
People
(Reporter: MattN, Assigned: MattN)
References
()
Details
User Story
Affected users: * First release Firefox version with new signing certificate was 67.0.2 * Windows only * AVG users at the time 67.0.2 was first started (may have uninstalled AVG in the meantime) * Should have a `logins.json.corrupt*` files modified (which timestamp actually changes in this case) since 67.0.2 release (with some buffer for clock skew) Recovery: * Close Login Manager access to logins.json * Move any existing `logins.json` aside. If that fails then maybe the user doesn't have the AVG fix yet and it's preventing us from accessing it? * Let the user choose the file to restore if they have more than one otherwise just give a confirmation dialog. * Restart the browser to re-initialize the password manager * Translations: https://pontoon.mozilla.org/projects/restore-logins-extension/
Attachments
(2 files, 10 obsolete files)
In case we need it I will starting writing code that could be run on user's machines to recover their logins from the logins.json.corrupt*
files caused by AVG interfering with Firefox's access to logins.json
.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Comment 1•5 years ago
|
||
Copy for this bug currently being worked out here: https://docs.google.com/document/d/1wSI9ee5o6WVwH6iU67J6eFA5ix9xDB8-M0Z9CKQlVY8/edit#
Assignee | ||
Comment 2•5 years ago
|
||
Work in progress for initial feedback
Assignee | ||
Updated•5 years ago
|
Comment 3•5 years ago
|
||
Comment on attachment 9072306 [details] [diff] [review] WIP 1 for feedback Review of attachment 9072306 [details] [diff] [review]: ----------------------------------------------------------------- ::: experiments/skeleton/api.js @@ +2,5 @@ > +/* global ExtensionAPI */ > +ChromeUtils.defineModuleGetter(this, "OS", "resource://gre/modules/osfile.jsm"); > +ChromeUtils.defineModuleGetter(this, "AddonManager", "resource://gre/modules/AddonManager.jsm"); > + > +ChromeUtils.import("resource://gre/modules/Console.jsm"); This is a no-op. Please remove. @@ +7,5 @@ > +const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm"); > + > + > +const env = Cc["@mozilla.org/process/environment;1"].getService(Ci.nsIEnvironment); > +const ADDON_ID = "hotfix-avg-logins-bug-1559458@mozilla.com"; There's no reason to hardcode this. You can just get `extension.id` from your API instance. @@ +12,5 @@ > + > +function fix(desiredFile) { > + let loginsJSONPath = OS.Path.join(OS.Constants.Path.profileDir, "logins.json"); > + let recoveryBackupPath = OS.Path.join(OS.Constants.Path.profileDir, "logins.json.before-recovery"); > + if (OS.File.exists(loginsJSONPath)) { This needs an `await`. `OS.File.exists` returns a promise, which will always evaluate to true. @@ +24,5 @@ > +async function getCorruptFiles() { > + let iterator = new OS.File.DirectoryIterator(OS.Constants.Path.profileDir); > + let files = []; > + try { > + await iterator.forEach(async function onEntry(entry) { You probably want `for await (let entry of iterator) ...` @@ +33,5 @@ > + let file = { > + entry, > + stats, > + }; > + if ("winLastWriteDate" in entry) { Should this not be "winCreationDate"? @@ +49,5 @@ > + }); > + } finally { > + iterator.close(); > + } > + console.log("e", files); This looks wrong. @@ +56,5 @@ > + > +function stageFix(path) { > + console.log("stage fix ", path); > + // TODO: test directory names with spaces and non-ascii > + env.set("MOZ_RESTORE_LOGINS", path); I'm not entirely clear on the purpose of requiring a restart for this... @@ +62,5 @@ > +} > + > +function uninstall() { > + console.log("uninstalling"); > + AddonManager.getAddonByID(ADDON_ID, function(addon) { These APIs don't accept callbacks anymore. They return promises.
Assignee | ||
Comment 4•5 years ago
|
||
Address feedback
Assignee | ||
Comment 5•5 years ago
|
||
Comment 6•5 years ago
|
||
Comment on attachment 9072315 [details] [diff] [review] v.1 Extension Review of attachment 9072315 [details] [diff] [review]: ----------------------------------------------------------------- ::: api.js @@ +7,5 @@ > + > +async function fix(desiredFile) { > + Services.logins.wrappedJSObject._storage.wrappedJSObject.terminate(); > + try { > + await Services.logins.wrappedJSObject._storage.wrappedJSObject._store.finalize(); Maybe just store the value of `Services.logins.wrappedJSObject._storage.wrappedJSObject` and reuse it? @@ +21,5 @@ > + } > + > + console.log("restoring", desiredFile); > + await OS.File.copy(desiredFile, loginsJSONPath); > + await uninstall(); This needs to pass the extension ID... @@ +31,5 @@ > + let files = []; > + try { > + for await (let entry of iterator) { > + if (!entry.name.startsWith("logins.json") || !entry.name.includes(".corrupt")) { > + return null; `continue` @@ +49,5 @@ > + } finally { > + iterator.close(); > + } > + > + return files; It seems strange that we sort the files if there's no exception but return them unsorted if their is an exception... @@ +92,5 @@ > + let labels = corrupt.map(file => { > + console.log(file.creationDate); > + let msDiff = now - file.creationDate; > + let days = Math.floor(msDiff / MS_IN_DAY); > + console.log(days); These two console logs seem unnecessary. At the very least they need some context...
Comment 7•5 years ago
|
||
Comment on attachment 9072316 [details] [diff] [review] v1.1 Extension Review of attachment 9072316 [details] [diff] [review]: ----------------------------------------------------------------- ::: api.js @@ +38,5 @@ > + let file = { > + entry, > + stats, > + }; > + file.creationDate = entry.winCreationDate || 0; // TODO This should probably just be `entry.winCreationDate || entry.lastModificationDate` @@ +42,5 @@ > + file.creationDate = entry.winCreationDate || 0; // TODO > + files.push(file); > + } > + > + return files.sort(function compare(a, b) { I think just move this to after the finally clause.
Assignee | ||
Comment 8•5 years ago
|
||
Updated•5 years ago
|
Assignee | ||
Comment 9•5 years ago
|
||
Can you sign this so QA can start testing?
Comment 10•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 11•5 years ago
|
||
QA's testing on Windows 10 was good so I sent an email to AMO admins to have the signed en-US attachment 9072334 [details] uploaded there. Localization has started at https://pontoon.mozilla.org/projects/restore-logins-extension/ and we will upload a new version around Tuesday with localizations for top locales if everything goes well.
Assignee | ||
Comment 12•5 years ago
|
||
AMO rejects strict_min_version
s which aren't on the list at https://addons.mozilla.org/en-US/firefox/pages/appversions/ so unless TheOne is able to override that for us we will have to loosen it. I don't think it's a big deal since it means we will also help users who downgraded Firefox but still don't know how to fix their logins.json
in their profile folder.
Changes since the last signed version (for en-US): https://github.com/mozilla/restore-logins-extension/compare/v1.0.0-beta.1...v1.2.x This also includes some minor copy updates from PMM.
Apologies for flagging so many people… I'm not sure who is checking bugmail this weekend. Could someone please sign this with the Mozilla signing cert.?
Thank you very much
Comment 13•5 years ago
|
||
The add-on code looks ok from an AMO perspective, just needs a signature now.
Comment 14•5 years ago
|
||
Wei or Adrian, could you please sign the file in comment 12 with the internal certificate?
Comment 15•5 years ago
|
||
SIgned file attached. Please test it.
Thanks.
Updated•5 years ago
|
Comment 16•5 years ago
•
|
||
Since v1.2.0 would need QA again (due to changed minVersion), we built another version that includes localization while we are working on getting QA folks
tl;dr: This unsigned version (1.3) introduces localization.
Comment 17•5 years ago
|
||
This is the signed version of v1.3, ready for QA.
Assignee | ||
Comment 18•5 years ago
|
||
Version 1.3 is published on AMO: https://addons.mozilla.org/firefox/addon/restore-logins/
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 19•5 years ago
|
||
Added pt-BR, fr, & es-ES translations. I believe we aren't planning on adding more languages after this.
Please sign this extension and wait for approval in the Slack channel before publishing on AMO.
Anyone else CC'd who can sign, feel free to do so.
Comment 20•5 years ago
|
||
Unfortunately, I don't have the key to sign.
Maybe Wei or Adrian are around?
Comment 21•5 years ago
|
||
Assignee | ||
Comment 22•5 years ago
|
||
There were no objections on Slack so please make 1.3.1 the latest version on AMO.
Thank you
Assignee | ||
Updated•5 years ago
|
Comment 23•5 years ago
|
||
Version 1.3.1 is posted and approved. There were some slight issues during upload, let me know if this caused any trouble.
Description
•