Several Mailvelope-related fixes and features #7348
Conversation
gurnec
force-pushed
the
mailvelope-fixes
branch
from
e6a785f
to
332f068
Compare
gurnec
force-pushed
the
mailvelope-fixes
branch
from
332f068
to
49476bd
Compare
|
Maybe we should make |
|
Commit 2367aa7 is also working for Firefox 75, just verified it. |
|
I cherry-picked fixes from here and backported to 1.4. The rest is a feature request. Please, rebase |
|
@alecpl Thanks! Rebased, and I'll update the initial comment. |
|
@alecpl One other quick note: I believe this one also qualifies as only-a-bugfix: |
@alecpl, @gurnec I agree this might be worth a user preference and not a fixed setting for all. |
|
Question is just to which preference section that option could be added to... |
|
I suggest to merge this into master and then find the right place to add that user preference. The other changes are very valuable and should go into 1.5-beta |
|
Am I right in believing the |
|
Yes, Mailvelope is a browser plugin that does what the Enigma plugin does but client side (PGP Encryption and Decryption). It has an API that Roundcube integrates. Usage is only activated if the Mailvelope plugin is installed and your Roundcube domain has the API option turned on. |
| @@ -69,6 +69,11 @@ RELEASE 1.4.4 | |||
| - Security: Fix remote code execution via crafted 'im_convert_path' or 'im_identify_path' settings [CVE-2020-12641] | |||
| - Security: Fix local file inclusion (and code execution) via crafted 'plugins' option [CVE-2020-12640] | |||
| - Security: Fix CSRF bypass that could be used to log out an authenticated user [CVE-2020-12626] (#7302) | |||
| - Mailvelope: Fix size of iframe for PGP-inlined mail (#7348) | |||
There was a problem hiding this comment.
Please, rebase and put these on top of the changelog.
| @@ -86,3 +86,7 @@ $config['plugins'] = array( | |||
|
|
|||
| // skin name: folder from skins/ | |||
| $config['skin'] = 'elastic'; | |||
|
|
|||
| // Use the Main Keyring in Mailvelope? If not, use (creating if required) | |||
There was a problem hiding this comment.
Remove this. We want to have the sample config short.
| @@ -4021,7 +4031,8 @@ function rcube_webmail() | |||
| } | |||
|
|
|||
| ref.hide_message(msgid); | |||
| $(selector).addClass('mailvelope').children().not('iframe').hide(); | |||
| $(selector).children().not('iframe').hide(); | |||
| $('#messagebody').addClass('mailvelope'); | |||
There was a problem hiding this comment.
You should not use skin-specific selectors. If needed add messagebody property to env via steps/mail/show.inc::rcmail_message_body().
| // check for mailvelope API | ||
| this.pgpmime_support_check = function(action) | ||
| { | ||
| if (typeof window.mailvelope !== 'undefined') |
There was a problem hiding this comment.
wouldn't be better to just do if (window.mailvelope) ?
|
@gurnec Have you seen my last comments, will you be able to work on these? |
Each commit is listed below. I'm open to any disagreements/critiques, so please fire away!
24d0cf5Merged. Let Mailvelope use sender's address to find pubkeys to check signaturesAlso remove the showExternalContent option which was removed from the Mailvelope API January 2019.
cec240d Fix size of Mailvelope iframe for PGP-inlined mail
Emails with PGP attachments appear correctly like this, but emails with the PGP in the message body before this commit looked squished like this. Now they both appear correctly. (Only tested in Chromium and Firefox 75, sorry.)
c3d4598Merged. Add missing\s to regexes inrcube_check_email()An unrelated minor bug fix, I can remove it if you'd prefer.
7f64384 Avoid initializing Mailvelope if we don't need to
Small efficiency fix, but it also makes the next two commits a bit smaller in scope.
cc3779f Add config option
mailvelope_main_keyringto use Mailvelope Main KeyringDefaults to false for backwards compatibility. It's probably only useful for new installs. Related: Use preferred/main keyring from Mailvelope #7157
2f71928 Add config option
mailvelope_keysizefor size of new Mailvelope keysDefaults to 4096 for backwards compatibility.
a121960 Always ask before discarding composed Mailvelopes
The unpatched code appears to be intentionally not asking before discarding emails currently being composed. This does the exact opposite, which can be minorly annoying if one saves and then leaves while composing, but is much safer in other cases IMO. If others disagree, perhaps this could be a config option instead? Related: Unsaved changes warning not used in Mailvelope mode #7017
9d39c0a Don't warn to re-add attachments when restoring a Mailvelope draft
Currently, if you save a Mailvelope email draft, and then select to edit it from your Drafts folder, the "Already uploaded attachments cannot be encrypted"... warning message is unnecessarily displayed.
edit: I just (Apr 29) amended this last commit to remove the use of
Object.values()which isn't supported by IE.Object.keys()apparently is supported.40ecd47Merged. Show Encrypt button w/Mailvelope, even if disabled (added Apr 30)