Closed
Bug 1388238
Opened 6 years ago
Closed 6 years ago
Create MasterPassword.jsm to provide async wrappers around crypto-SDR and handle existing dialogs
Categories
(Toolkit :: Form Manager, enhancement, P1)
Toolkit
Form Manager
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: MattN, Assigned: steveck)
References
(Blocks 1 open bug)
Details
(Whiteboard: [form autofill:M4])
Attachments
(2 files, 2 obsolete files)
As a starting point for bug 1271851 and to use for Form Autofill, we can start providing cleaner APIs for a master password with helpers to avoid existing issues like triggering a new MP dialog while one is already open. For now we'll implement this in the formautofill system extension so we can iterate quickly but eventually this module can move to toolkit and be used by password manager and other consumers like sync.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Reporter | ||
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8894734 [details] Bug 1388238 - Implement waitForMasterPasswordDialog helper which handles open dialogs. https://reviewboard.mozilla.org/r/165908/#review171004
Attachment #8894734 -
Flags: review?(MattN+bmo) → review+
Reporter | ||
Comment 4•6 years ago
|
||
mozreview-review |
Comment on attachment 8894735 [details] Bug 1388238 - Add encrypt/decrypt methods to MasterPassword.jsm. https://reviewboard.mozilla.org/r/165910/#review171006
Attachment #8894735 -
Flags: review?(MattN+bmo) → review+
Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/autoland/rev/7b7aa4e9ae5a Implement waitForMasterPasswordDialog helper which handles open dialogs. r=MattN https://hg.mozilla.org/integration/autoland/rev/55f44886a839 Add encrypt/decrypt methods to MasterPassword.jsm. r=MattN
Comment 6•6 years ago
|
||
Backed out in https://hg.mozilla.org/integration/autoland/rev/b676d5b34892 for eslint failures, https://treeherder.mozilla.org/logviewer.html#?job_id=121866954&repo=autoland
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/autoland/rev/8e2f4f651250 Implement waitForMasterPasswordDialog helper which handles open dialogs. r=MattN https://hg.mozilla.org/integration/autoland/rev/99a525a70041 Add encrypt/decrypt methods to MasterPassword.jsm. r=MattN
Comment 10•6 years ago
|
||
Backed out in https://hg.mozilla.org/integration/autoland/rev/c93fa2271ee7 for xpcshell crashes, https://treeherder.mozilla.org/logviewer.html#?job_id=121879918&repo=autoland
Comment 11•6 years ago
|
||
Also, and unreferenced file error, https://treeherder.mozilla.org/logviewer.html#?job_id=121889675&repo=autoland
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8896181 -
Flags: review?(MattN+bmo) → review?(lchang)
Attachment #8896182 -
Flags: review?(MattN+bmo) → review?(lchang)
Assignee | ||
Comment 14•6 years ago
|
||
Comment on attachment 8896182 [details] Bug 1388238 - Add encrypt/decrypt methods to MasterPassword.jsm. I tried to remove the token reset and needInit checking in the new patch, but it's still failed in the end... It'll return different error code(-11) with message [unknown top frame]. I still have no idea why it's always busted in the debug build only...
Flags: needinfo?(MattN+bmo)
Attachment #8896182 -
Flags: review?(lchang)
Assignee | ||
Updated•6 years ago
|
Attachment #8896181 -
Flags: review?(lchang)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8896181 -
Flags: review?(MattN+bmo)
Attachment #8896182 -
Flags: review?(MattN+bmo)
Assignee | ||
Comment 17•6 years ago
|
||
Comment on attachment 8896182 [details] Bug 1388238 - Add encrypt/decrypt methods to MasterPassword.jsm. I found that assertion for the prompt case would be crashed only in the debug build, so the only way to verify the prompt safely is using the mock prompt in the xpcshell test.
Reporter | ||
Comment 18•6 years ago
|
||
mozreview-review |
Comment on attachment 8896182 [details] Bug 1388238 - Add encrypt/decrypt methods to MasterPassword.jsm. https://reviewboard.mozilla.org/r/167458/#review174198
Attachment #8896182 -
Flags: review?(MattN+bmo) → review+
Reporter | ||
Comment 19•6 years ago
|
||
mozreview-review |
Comment on attachment 8896181 [details] Bug 1388238 - Implement waitForMasterPasswordDialog helper which handles open dialogs. https://reviewboard.mozilla.org/r/167456/#review174202
Attachment #8896181 -
Flags: review?(MattN+bmo) → review+
Comment 20•6 years ago
|
||
Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/autoland/rev/5b4b8c71b9fb Implement waitForMasterPasswordDialog helper which handles open dialogs. r=MattN https://hg.mozilla.org/integration/autoland/rev/02f07f0cb11d Add encrypt/decrypt methods to MasterPassword.jsm. r=MattN
Reporter | ||
Updated•6 years ago
|
Attachment #8894734 -
Attachment is obsolete: true
Reporter | ||
Updated•6 years ago
|
Attachment #8894735 -
Attachment is obsolete: true
Reporter | ||
Updated•6 years ago
|
Flags: needinfo?(MattN+bmo)
Reporter | ||
Comment 21•6 years ago
|
||
This is getting backed out since comment 11 wasn't addressed.
Comment 22•6 years ago
|
||
Backout by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/0149c32ca8b7 Backed out changeset 02f07f0cb11d because comment 11 wasn't addressed before landing.
Backed out part 1 as well: https://hg.mozilla.org/integration/autoland/rev/08b6dcd7d8f776c09390497b648a66fd15767aef
Flags: needinfo?(schung)
Assignee | ||
Comment 24•6 years ago
|
||
We'll push patch in Bug 1337314 together with this one to make sure there's no unreferenced file error(because Bug 1337314 will need API in this patch).
Flags: needinfo?(schung)
Comment 25•6 years ago
|
||
Pushed by lchang@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9f15f3ee6cc1 Implement waitForMasterPasswordDialog helper which handles open dialogs. r=MattN https://hg.mozilla.org/integration/mozilla-inbound/rev/8c1ef3d6ed6d Add encrypt/decrypt methods to MasterPassword.jsm. r=MattN
Comment 26•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9f15f3ee6cc1 https://hg.mozilla.org/mozilla-central/rev/8c1ef3d6ed6d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 27•6 years ago
|
||
Steve and Matt, sadly this hunk from your patches + if (xulDialog) { + xulDialog.setAttribute("windowtype", "prompt:" + this.args.promptType); + } + breaks Thunderbird's entire Mozmill test suite. Is it really valid to change attribution on the XUL dialogue? Looks like you want to detect the password window let promptWin = Services.wm.getMostRecentWindow("prompt:promptPassword"); so why not just prepend the "prompt:" when the rest is already "promptPassword"?
Flags: needinfo?(schung)
Flags: needinfo?(MattN+bmo)
![]() |
||
Comment 28•6 years ago
|
||
It seems more like they needed a way to find the password dialog via windowtype. Normally dialogs have no windowtype, but have an id. So the dialog code generates a windowtype unconditionally. This produces prompt:promptPassword for the password dialog, but e.g. prompt:alert (and prompt:confirmEx) for normal dialogs that only had id=commonDialog.
Reporter | ||
Comment 29•6 years ago
|
||
See bug 1391182 comment 13. (In reply to Jorg K (GMT+2) from comment #27) > Looks like you want to detect the password window > let promptWin = Services.wm.getMostRecentWindow("prompt:promptPassword"); Correct. > so why not just prepend the "prompt:" when the rest is already > "promptPassword"? Because it will be useful for others in the future too.
Flags: needinfo?(schung)
Flags: needinfo?(MattN+bmo)
Reporter | ||
Comment 30•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/966d4487c72e https://hg.mozilla.org/releases/mozilla-beta/rev/d0ea76640bfd
status-firefox56:
--- → fixed
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•