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

#1233 Decrypt attachments on tap #1323

Merged
merged 9 commits into from
Jan 21, 2022

Conversation

sosnovsky
Copy link
Collaborator

@sosnovsky sosnovsky commented Jan 20, 2022

This PR implements attachments decrypt on tap, so they're not decrypted on message open.

close #1233


Tests (delete all except exactly one):

  • Tests added or updated - updated user is able to view message processing errors and user is able to view encrypted email with attachment tests

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@sosnovsky sosnovsky requested a review from tomholub as a code owner January 20, 2022 15:10
tomholub
tomholub previously approved these changes Jan 20, 2022
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. That made me think: there are three types of attachments:

  • plain / not encrypted
  • encrypted as a separate file
  • encrypted with pgp/mime (a mime message with attachments, encrypted whole)

Could you double check that the third case works after this change? Please try email with subject [enigmail] encrypted+signed+file PGP/MIME on flowcrypt.compatibiltiy account.

I supose the TS core will auto-decrypt it and return back as decrypted (plain), which should work. Worth checking though.

@sosnovsky
Copy link
Collaborator Author

Here is how it works in this branch

Simulator.Screen.Recording.-.iPhone.13.-.2022-01-20.at.22.28.17.mp4

Signature can't be verified, but I get the same error on master branch, is it okay?

@sosnovsky
Copy link
Collaborator Author

The same signature error is in browser extension, so should be good

@tomholub
Copy link
Collaborator

Looks perfect 👍

@tomholub
Copy link
Collaborator

Just the tests to fix else looks great to me.

tomholub
tomholub previously approved these changes Jan 21, 2022
@tomholub tomholub enabled auto-merge (squash) January 21, 2022 09:38
@tomholub tomholub merged commit 0162fa3 into master Jan 21, 2022
@tomholub tomholub deleted the feature/issue-1233-attachments-decrypt branch January 21, 2022 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

don't decrypt attachments before tapping + show modal on error
2 participants