Niklas Femerstrand wrote:
Whazzup,
I just released the beta of my plugin that implements OpenPGP into Roundcube through JavaScript by using the OpenPGP.js library. Github repo available here https://github.com/qnrq/rc_openpgpjs and announcement here http://qnrq.se/rc_openpgpjs_ending_seven_years_of_roundcube_insecurity/.
Expect a stable release as soon as I have regulated my dopamine levels; or rather, as soon as I have fixed these https://github.com/qnrq/rc_openpgpjs/issues?milestone=1&page=1&state=open minor issues. I'm estimating it'll take /at most/ one or two weeks to finalize the stable version. In the meantime we can handle the political discussions that this may stir up.
Hi Niklas
Thanks for the hard work you put into this! It's good to have somebody finally taking care about the whole PGP topic as we're too busy with other stuff.
After a first sight at your work, I'd like to drop some comments about what I discovered. From both, a developer's as well as a user's point of view, there are some issues which should be addressed:
Development
- You bind events to UI buttons by using their object IDs. This doesn't
work reliably because these are internal IDs and they can vary with different skins. With the new Larry skin, the plugin doesn't work because #rcmbtn114 isn't the send button (but the CC+ button of the address book widget). Solution: there are generic events triggered by the Roundcube core script, which you can bind on [1]. In your case, binding to the 'send' command would look like this:
rcmail.addEventListener('beforesend', function(e){ /* do your magic */ });
- Pubkey selection: $("#_to").val().split(",") doesn't always work. There
might be recipient addresses like this: "Brüderli, Thomas" thomas@roundcube.net. Solution: grep for email address patterns in the recipients list instead. BTW: there are also CC and BCC fields to consider when encrypting.
- Storing the passphrase as cookie: even if the cookie has a short
expiration period, please be aware that cookies might be sent to the server on every HTTP request. I'd suggest to use local storage functions to store the passphrase.
- There's no encryption for attachments. I know, this is hard or almost
impossible to implement on the client but it's a serious lack of feature. We might consider to implement hooks that'll let you read attachment contents back to the client in order to encrypt them.
- Decryption of encrypted messages is only possible on messages sent with
text/plain but not with multipart/encrypted. This is mainly because Roundcube doesn't even send the encrypted body parts to the client. Solution: there are plugin hooks in PHP available [2] to grab encrypted parts from the message structure and make sure they're sent to the client. Attachment decryption is also consider here.
- No decryption in message preview! One has to open the message in full
screen to have the openpgpjs plugin kick in. Just activate the plugin on 'preview' action as well.
- UI elements such as dialogs and forms are fully created in javascript. In
order to also make them customizable using skins, you should use skins and templates and add them to the html page in PHP.
- Texts are not localized but hard-coded in javascript. The Roundcube
plugin system provides easy ways to maintain localized texts. Those strings can also be made available in javascript. See other plugins how this is done.
- Make key-server to lookup keys configurable. See other plugins how plugin
config works.
Usability
- In the message view screen, a button is added to the top tasks bar.
Besides that it doesn't fit in the Larry skin, this isn't the right place to put that button. In the compose screen, it is placed to the toolbar which is much better. However, I'd consider to find the key management in the Settings section.
- There's no progress/activity display while encrypting/decrypting action
is going on. I rekon this sometimes takes a few seconds and as a user, I don't know what's happening.
- The key management dialogs don't have a proper layout. At least in Larry
skin, the import box is aligned right of the table, forcing the table contents to break. The inputs for importing keys could even be hidden in an additional dialog or just placed below the table.
- The order of the dialog tabs (Generate keys, Private keys, Public keys)
should be reversed. Importing public keys is IMO the most common task while generating keys is rarely used.
- Create individual CSS files for different skins in order to better
integrate with the Roundcube UI.
Please don't take the above points as a rant but as a friendly and constructive feedback. We're happy to see progress on this and we're willing to help you making things nice and shiny.
Best regards, Thomas
[1] http://trac.roundcube.net/wiki/Plugin_Events#beforeandafter [2] http://trac.roundcube.net/wiki/Plugin_Hooks#message_part_structure