1653300 - OpenPGP e2ee pref UI should show if an external key was configured

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, enhancement, P1)

Description

[Kai Engert (:KaiE:)]

OpenPGP e2ee pref UI should show if an external key was configured

More details to follow

Comment 1

[Kai Engert (:KaiE:)]

Bug 1603782 adds a new preference, that distinguishes if the configured OpenPGP is an external key managed by GnuPG.

It can be queried using identity.getBoolAttribute("is_gnupg_key_id");
It would be good to indicate to display that this configuration is used.
However, I suggest that initially we use a simplify as much as possible.

A minimal way could be:

• require that the user manually edits the prefs.js file to set this pref
• if the pref is set, don't show the list of keys. Don't offer the button "add key".
• instead, show a text that says "Manual configuration active. Using external GnuPG key with ID (value from openpgp_key_id)"

I'm guessing that this is very simple to do, and it would be sufficient to avoid that the UI is used to disturb the manual configuration.

Alternatively, if you think it can be done quickly, prior to the string freeze, you could an additional selection choice, similar to "None". It could use label "GnuPG Key (key ID)".
Have an edit box inside that selection, without any field validation.
Don't offer any details, don't offer any properties, don't offer any actions.

If this special entry is selected, set the identitys pref to true. Set the value of openpgp_key_id to the string entered in the input box. No validation.

There will be documentation for this mode. Several additional (manual) steps will be required to ensure this external configuration works. Therefore I think it isn't worth it rushing a more pretty UI.

Comment 2

[Kai Engert (:KaiE:)]

And as previously mentioned, if global pref "mail.openpgp.allow_external_gnupg" is false, hide the GnuPG offer.

If mail.openpgp.allow_external_gnupg it false, ignore the account's value of is_gnupg_key_id

Comment 3

[Kai Engert (:KaiE:)]

Alessandro, do you think this could be done?

I'm happy with the minimal implementation (hide key selection and button, don't offer input, just offer that external gnupg is used, require the user to manually edit prefs.js).

Comment 4

[Alessandro Castellani [:aleca]]

Attachment: 1653300-key-external.diff

Comment 5

[Kai Engert (:KaiE:)]

"External GnuPG Key saved successfully!"

Could confuse the user, because we're not saving a key. Also, we have not idea if the configuration was successful. I suggest to remove the optimism and say:

"External Key configuration saved."

Comment 6

[Alessandro Castellani [:aleca]]

Attachment: 1653300-key-external.diff

Strings udpated

Comment 7

[Kai Engert (:KaiE:)]

Thanks a lot for the patch. I've reviewed with Alessandro interactively in chat. We agreed on two string corrections, and he gave me another JS snippet to fix saved prefs.

I'll upload the updated patch and mark as r=kaie

Comment 8

[Kai Engert (:KaiE:)]

Attachment: 1653300-v3.patch

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/16c5f3137158
Implement the UI to handle OpenPGP External keys. r=KaiE DONTBUILD

Comment 10

[Kai Engert (:KaiE:)]

Comment on attachment 9166030 [details] [diff] [review]
1653300-v3.patch

Only if the external-gnupg pref is enabled, this will allow users to conveniently configure their external key ID that they want to use. Avoids that users have to fiddle with account IDs in prefs.
Important for smartcard and Qubes OS users.

Comment 11

[Kai Engert (:KaiE:)]

received a=wsmwk for both comm-beta and comm-esr78 on Matrix

Comment 12

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/releases/comm-beta/rev/5b3c2123f52edfa375b07212486be221365d1bed
https://hg.mozilla.org/releases/comm-esr78/rev/0164fcd413f59a68782f07c742fd2d9263d7d15a

Comment 13

[Alessandro Castellani [:aleca]]

I found a small issue here:

// Always update the GnuPG boolean pref to be sure the currently used key is
  // internal or external.
  gIdentity.setBoolAttribute(
    "is_gnupg_key_id",
    newKey ==
      gIdentity.getUnicharAttribute("last_entered_external_gnupg_key_id")
  );

With this condition, is_gnupg_key_id will be set to TRUE if the user selects NONE and no previous external key was saved in the last_entered_external_gnupg_key_id.
Quick fix coming up.

Comment 14

[Alessandro Castellani [:aleca]]

Attachment: 1653300-key-external-PART2.diff

This should fix it.

Comment 15

[Kai Engert (:KaiE:)]

Comment on attachment 9166033 [details] [diff] [review]
1653300-key-external-PART2.diff

will uplift this obvious correctness fix, and reuse the existing a=

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/16c3f6fc057e
Fix the toggle of the external GnuPG preference. r=KaiE DONTBUILD

Comment 17

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/releases/comm-esr78/rev/e0421c7babd858cbb289e65079dd438d1272e3dc
https://hg.mozilla.org/releases/comm-beta/rev/e1710043ac0de0ad6c986ab904aae70cb3a034f2

Comment 18

[Wayne Mery (:wsmwk)]

Comment on attachment 9166033 [details] [diff] [review]
1653300-key-external-PART2.diff

Per previous comment Approved for esr78, and beta 79.0b3

Comment 19

[Wayne Mery (:wsmwk)]

Comment on attachment 9166030 [details] [diff] [review]
1653300-v3.patch

Per previous comment Approved for esr78, and beta 79.0b3

However, Kai, you missed this patch in the uplifts to beta and 78.

For beta, I suggest we skip the uplift since it will be in 80 by default.  Unless having attachment 9166033 [details] [diff] [review] but not having this patch will badly break 79.0b3 (which will exist for several days before 80 comes out)

Comment 20

[Kai Engert (:KaiE:)]

(In reply to Wayne Mery (:wsmwk) from comment #19)

However, Kai, you missed this patch in the uplifts to beta and 78.

I don't think I missed them.

See above, there were two uplifts.
Comment 12 and comment 17.

Comment 21

[Wayne Mery (:wsmwk)]

super - I was looking for comments closer together. We're golden