Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Local storege fails if account contains non-decryptable email. #129

Closed
nloomans opened this issue Apr 10, 2019 · 6 comments
Closed

Local storege fails if account contains non-decryptable email. #129

nloomans opened this issue Apr 10, 2019 · 6 comments

Comments

@nloomans
Copy link

Steps to reproduce

  1. Send an email to your self using a PGP Key your ProtonMail has not been setup to decrypt.
  2. Enable local storage
  3. The following error will now popup: Error decrypting message: Session key decryption failed

This is how such an email is shown when viewing it within the web client:

2019-04-10-212837_1920x1080_scrot

Expected behavior

Instead of failing to enable local storage entirely, it would be nice if ElectronMail just ignored such emails.

Workaround

  1. Disable local store
  2. Delete the email (not just trash it!) which contains un-decryptable content.
  3. Enable local storage

(The enabling and disable part might not be necessary)

@vladimiry
Copy link
Owner

Thanks for the thoughtful report. There will be a need to unambiguously distinguish the decryption error due to the absence of PGP key from other types of errors so the app skips only one specific error. So there is a need to, first of all, debugging the case in order to determine if it's feasible to catch the exact error since the same error type might potentially be thrown by protonmail/pgp for different cases.

@vladimiry
Copy link
Owner

I was thinking about how to solve the body decryption issues once and forever and not hurting the UX in along with that and I think I came up with a unified solution that will even enhance the UX:

  • The app won't stop the emails downloading process regardless of the body decrypting result.
  • If any kind of error occurred during the body decryption process the app will set the body to empty but will also add to the email the bodyDecryptiongError property with at least the following properties: error message, error stack trace, date, app version. This is a better idea than skipping/ignoring the problematic email.
  • If a user opens the email with filled bodyDecryptiongError property in the database view mode the app will show the re-download button instead of the body content. The app will also display near to the button the original error happened during decryption. So there will be a way to individually for email re-download the body content without publishing a new app release in the case of for example upstream issue like this one got fixed.
  • Emails with filled bodyDecryptiongError property will get the respective indication in the list view of the database view mode. It also won't hurt to add the respective filter to the full-text search criteria list.
  • Exported to EML files emails with filled bodyDecryptiongError property will get the original decryption error message instead of the body content.

So having the described idea implemented the app won't face a blockers happening due to the decryption problems during the emails bootstrap fetch process.

This is going to be a unified solution for the problem so there will be no need to unambiguously distinguish the decryption error type as I wrote in the previous message.

@nloomans of course this will solve the original issue.

@ihubgit this is going to solve the issue raised by you here without a need to wait for the upstream issue resolving. So you will be able to safely enable the local database option.

PS all of that is protonmail related only.

@vladimiry
Copy link
Owner

@nloomans can you try pre 3.2.0 build?

It works as described in the previous message:
body-decryption-failed

@nloomans
Copy link
Author

@vladimiry I have tested the build and it works perfectly! Both with an partial update and after removing database.bin

Thanks!

@vladimiry
Copy link
Owner

Thanks for testing. A new release is going to be published soon.

@vladimiry
Copy link
Owner

vladimiry commented Apr 14, 2019

The described above improvement has been released with 3.2.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants