[Differential] [Updated] D3140: Add formatters for application/pgp-keys and application/vnd.gnupg.wks body parts

[aheinecke (Andre Heinecke)]

aheinecke added a subscriber: emanuel.
aheinecke added a comment.


  Great, I think it's nice to finally have an application/pgp-keys handler. But as written in the inline comments I think that if we have a handler we need to offer the import action.
  
  Regarding the WKS Handler I think the "This key is already published" message / check if the key is already in the WKD is probably wrong, there might be use cases (e.g. after changing the expiry date of a key or adding / removing subkeys) where a user wants to update the published key even if a key with the same fingerprint was already published.
  I would have thought we could handle this like invitations. So that after accepting an invitation the actual invitation mail is deleted. (Here after replying to a confirmation request we would delete the request). This would remove the need here problem to check if the key is already published.
  
  Regarding sending, you use SMTPJob. I am concerned that this might be a bit too low level.  It looks to me for example that if you are offline when trying to reply the SMTPJob would just error out where I expected that the response stays in the outbox and is sent the next time you are online. Again invitations could be the example how to handle this.

INLINE COMMENTS

> dvratil wrote in gnupgwksmemento.cpp:47
> > Perhaps it will good to detect if we have gpg2 in system no ?
> 
> It's a bit more difficult than that. Some distros have GnuPG2 binaries called "gpg", while some others have GnuPG2 binaries called "gpg2" (and GnuPG1 binaries called "gpg", possibly installed at the same time). We need to use GnuPG2 for this, so we try "gpg2" first, and if we get `QProcess::FailedToStart` error we try "gpg". If we would get error with "gpg", it means that either there's no "gpg" binary, or it's GnuPG1 binary (which does not understand the arguments).
> 
> I think it's easier and simpler to just try, rather than locating the binaries and parsing their "--version" output.

Ultimately I think we need API in gpgme for this. But in the meantime its better to use GpgME engine info:

GpgME::engineInfo(GpgME::GpgEngine).fileName();

There is also GpgME::engineInfo(GpgME::GpgEngine).engineVersion() that you could use for version checks like:

if (!(GpgME::engineInfo(GpgME::GpgEngine).engineVersion() < "2.1.13"))
...

(2.1.13 is the first version that fully supports wkd lookup)

But as written in the general comment I don't think we should do the check here but rather delete the confirmation request mail once we have sent a response.

> pgpkeyformatter.cpp:93
> +    const auto key = mp.key();
> +    if (key.isNull()) {
> +        block.setProperty("uid", mp.userID());

In this case we should offer to import the key. So that we handle the case where someone attaches their PGP key to a message and the recipient imports it. QGpgME::ImportJob

This would also mean to store the raw data in the PgpKeyMessagePart. I think without this we might have a regression because application/pgp-keys was previously associated with kleopatra so you could import an attached public key by "Opening the attachment in Kleopatra".

REPOSITORY
  rKDEPIMADDONS KDE PIM Addons

REVISION DETAIL
  https://phabricator.kde.org/D3140

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dvratil, bjoernbalazs, mlaurent, aheinecke
Cc: emanuel, mlaurent, kde-pim, #kde_pim, spencerb, dvasin, winterz, vkrause, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20161024/3b3cba40/attachment.html>