opening openpgp key manager with no email account: exception
Categories
(MailNews Core :: Security: OpenPGP, defect, P1)
Tracking
(thunderbird_esr78 fixed, thunderbird80 fixed)
People
(Reporter: KaiE, Assigned: aleca)
Details
Attachments
(1 file, 1 obsolete file)
8.02 KB,
patch
|
KaiE
:
review+
wsmwk
:
approval-comm-beta+
wsmwk
:
approval-comm-esr78+
|
Details | Diff | Splinter Review |
Create a fresh profile.
Enable OpenPGP pref.
Cancel the request to configure an email account.
Use menu tools/openpgp key manager.
Result:
Uncaught TypeError: identity is undefined
openKeyManager chrome://messenger/content/accountUtils.js:579
oncommand chrome://messenger/content/messenger.xhtml:1
accountUtils.js:579:5
openKeyManager chrome://messenger/content/accountUtils.js:579
oncommand chrome://messenger/content/messenger.xhtml:1
Assignee | ||
Comment 1•4 years ago
|
||
I thought I covered this scenario in bug 1656391 with the null condition.
Should we prevent the usage of the Key Manager entirely if not even 1 email account have been configured?
Reporter | ||
Comment 2•4 years ago
|
||
It's certainly an edge case, but I use it frequently for testing, after creating a fresh profile.
I don't see a technical justification to prevent it.
in
mailnews/base/prefs/content/accountUtils.js
in
openKeyManager()
you have:
let args = {
identity: identity[0],
cancelCallback: null,
okCallback: null,
};
This is the place where the error happens, and where the fix would have to be done. If there's no entry in the identity array, then identity[0] is an exception.
This could probably be fixed with:
identity: identity.length ? identity[0] : null
Reporter | ||
Comment 3•4 years ago
|
||
Better:
identity: (identity && identity.length) ? identity[0] : null
Reporter | ||
Comment 4•4 years ago
|
||
I'll attach the patch, so you can review it.
Assignee | ||
Comment 5•4 years ago
|
||
Perfect
Reporter | ||
Comment 6•4 years ago
|
||
Assignee | ||
Comment 7•4 years ago
|
||
I think we should also disable the Generate new Key
button in the manager menu if the identity is null, otherwise we will have the same exception in the key wizard.
Reporter | ||
Comment 8•4 years ago
|
||
(In reply to Alessandro Castellani (:aleca) from comment #7)
I think we should also disable the
Generate new Key
button in the manager menu if the identity is null, otherwise we will have the same exception in the key wizard.
Agreed. Patch updated, with lint fixes.
Assignee | ||
Updated•4 years ago
|
Reporter | ||
Comment 9•4 years ago
|
||
Based on the comments in phab, these changes weren't ideal (from bug 1652537):
https://hg.mozilla.org/comm-central/diff/5fffded97b81e12ede2b165d10599dfa03c1a8d8/mailnews/base/prefs/content/accountUtils.js
Alessandro, it seems this needs to be improved, based on guidance from Magnus - or the code should be removed (don't stick in context when opening key manager).
I'm fine with both. Can you and Magnus please resolve this?
As this needs a revision to Alessandro's original, I'd like to assign this bug to you.
For me, I primarily want the exception gone, one way or the other.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 11•4 years ago
|
||
Let's simplify things where we can.
If the KeyWizard is opened directly from the Account Settings, we get the current identity in order to allow users to set up a new key with the correct identity without the necessity of changing it.
If the KeyWizard is opened from the Key Manager, we don't pass any identity and we fallback to the default one.
If no default identity is available, we allow opening the Key Manager but we disable the Key Wizard menuitem.
Reporter | ||
Comment 12•4 years ago
|
||
Comment on attachment 9168947 [details] [diff] [review]
1657497-openpgp-manager.diff
Thank you!
This removes the code that was previously added, which was added without a review from Magnus, and which Magnus didn't like - I think we don't need his review for that removal.
Comment 13•4 years ago
|
||
Pushed by kaie@kuix.de:
https://hg.mozilla.org/comm-central/rev/34c08922aa2e
Allow opening the OpenPGP Key Manager without a default identity. r=mkmelin DONTBUILD
Reporter | ||
Comment 14•4 years ago
|
||
Comment on attachment 9168947 [details] [diff] [review]
1657497-openpgp-manager.diff
OpenPGP UI correctness and simplification fix, no risk.
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Comment 15•4 years ago
|
||
Apologies, I landed the commit with incorrect user attribution and incorrect reviewer :(
Comment 16•4 years ago
|
||
Comment on attachment 9168947 [details] [diff] [review]
1657497-openpgp-manager.diff
[Triage Comment]
Approved for beta
Approved for esr78
Comment 17•4 years ago
|
||
bugherder uplift |
Thunderbird 80.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/88e243a20848
Reporter | ||
Comment 18•4 years ago
|
||
Description
•