FlowCrypt / flowcrypt-browser

FlowCrypt Browser extension for Chrome and Firefox
https://flowcrypt.com
Other
372 stars 48 forks source link

#5666 Add compose button on Thunderbird #5759

Open martgil opened 3 weeks ago

martgil commented 3 weeks ago

This PR adds a functional compose button on top of Thunderbird mail client.

close #5666


Tests (delete all except exactly one):


To be filled by reviewers

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

martgil commented 3 weeks ago

Hi @sosnovsky - I hope you are well.

I would like to hear your suggestion with my progress and approach on how I enable secure compose on Thunderbird mail. What i did is that, I re-use the pop-up we have and injected them on messageDisplay and composeDisplay as a popup, that when clicked, will redirect then to the web extension page where they can directly compose their secure email.

I come up with this idea because, an extension can only inject a single option_ui so by having a way to compose email (encrypted, signed or both) its so flexible to so in MailExtension view. This is pretty much it for this PR.

To test it out:

  1. Install and it up the Thunderbird extension build on a Thunderbird email client.
  2. Open any message and click on "FlowCrypt" injected option ui -> choose secure compose -> choose which inbox you want to use.
  3. Upon clicking the account, you'll be greeter with secure compose - which would let you send secure email (encrypted, signed or combination of both).

I look forward to your feedback.

Thanks!

martgil commented 2 weeks ago

Hi @sosnovsky - Just a quick question for "FlowCrypt button in compose window should open Secure compose". Do you mean by this is that, upon clicking the FlowCrypt button on the compose display, it should directly go to the first account's inbox without the ability to choose for which inbox the user to use? Or would mean only the text.

For example, if the placement of the FlowCrypt button was on the toolbar - it should say "Encrypted inbox", then when accessed from the compose display should say "Secure compose" but without showing the setting button?

sosnovsky commented 2 weeks ago

Hi @sosnovsky - Just a quick question for "FlowCrypt button in compose window should open Secure compose". Do you mean by this is that, upon clicking the FlowCrypt button on the compose display, it should directly go to the first account's inbox without the ability to choose for which inbox the user to use? Or would mean only the text.

Here we can use the same behavior as the current one - show list of accounts and then open encrypted compose for chosen account. As it's probably quite complicated to get current account from Thunderbird UI. Also for simplification we can automatically open encrypted compose if it's only one account is configured.

For example, if the placement of the FlowCrypt button was on the toolbar - it should say "Encrypted inbox", then when accessed from the compose display should say "Secure compose" but without showing the setting button?

I like this idea - it'll make more clear what will be opened after button click.

martgil commented 2 weeks ago

Also for simplification we can automatically open encrypted compose if it's only one account is configured.

I agree on everything else - sounds like a plan. I'm on it now.

martgil commented 2 weeks ago

This PR is almost done.

For example, if the placement of the FlowCrypt button was on the toolbar - it should say "Encrypted inbox", then when accessed from the compose display should say "Secure compose" but without showing the setting button?

I like this idea - it'll make more clear what will be opened after button click.

Just need to show "Secure compose" instead of "Encrypted Inbox" when accessing in an composeAction Button in thunder bird and hide the settings page.

martgil commented 2 weeks ago

Hi @sosnovsky - I believe this one is ready for a review. Thanks!

To test this PR manually, there would be two cases: 1.) using secure compose with single Gmail account 2.) using secure compose with two user accounts from which one account will be selected out of the accounts.

martgil commented 1 week ago

Hi @sosnovsky - I can't seem commit my recent changes after PR for using flat config. All I do is commit my recent changes and push it using VS code's GitHub integration -- the green "Commit" button. After that, I get the following error:

> git -c user.useConfigOnly=true commit --quiet --allow-empty-message --file -
[STARTED] Preparing lint-staged...
[COMPLETED] Preparing lint-staged...
[STARTED] Running tasks for staged files...
[STARTED] package.json — 2 files
[STARTED] *.ts — 2 files
[STARTED] *.{css,htm} — 0 files
[SKIPPED] *.{css,htm} — no files
[STARTED] npm run test_eslint
[COMPLETED] npm run test_eslint
[STARTED] npm run test_patterns
[COMPLETED] npm run test_patterns
[STARTED] prettier --write
[COMPLETED] prettier --write
[COMPLETED] *.ts — 2 files
[COMPLETED] package.json — 2 files
[COMPLETED] Running tasks for staged files...
[STARTED] Applying modifications from tasks...
[FAILED] Prevented an empty git commit!
[STARTED] Reverting to original state because of errors...
[COMPLETED] Reverting to original state because of errors...
[STARTED] Cleaning up temporary files...
[COMPLETED] Cleaning up temporary files...

  ⚠ lint-staged prevented an empty git commit.
  Use the --allow-empty option to continue, or check your task configuration

husky - pre-commit script failed (code 1)
image

Please help - thank you!

EDIT:

I'm not sure why lint-staged didn't recognize that I have commit message - so when I pass a --allow-empty flag, my last commit were pushed with the message.

martgil commented 1 week ago

Hi @sosnovsky I still can't commit and push changes. My changes and commit message are not empty - I'm not so sure why lint-staged sees it as empty.

image
sosnovsky commented 1 week ago

Hi @martgil, probably here is explanation of your situation - https://stackoverflow.com/questions/71420124/how-do-i-solve-this-empty-git-commit-warning/77799678#77799678

Your commit includes only reverted prettier changes, but lint-staged automatically runs prettier and re-applies reverted changes. As a result - your commit becomes empty.

martgil commented 1 week ago

Thanks @sosnovsky for the help. I'm now able to push new changes again :)

martgil commented 1 week ago

Hi @sosnovsky - Its ready for review. Thank you!

At this point, you may check the following:

Quoted email rendering if you wish to reply from a compose display. Eg. You opened an email from Thunderbird -> click reply -> open Secure Compose automatically if single account or Choose account depending on how many user account configured in FlowCrypt -> opens Secure Compose with quoted email rendered.

I've tested out that its not possible to obtain the quoted email if the user came from the messageDisplay as the message object doesn't return an object similar to what composeDisplay returns which includes complete email details.

As part of my tests and checking, I can obtain the messageId from either getComposeDetails() or getDisplayedMessage() but the object with full email details like email body, plain text email body were all available only if interacted from composeDisplay - I think that's were the other JSON object were populated most by purpose so we have that limitation for rendering quoted email

sosnovsky commented 1 week ago

Hi @martgil, thanks for detailed explanation of Thunderbird API! In this case, I think, it'll be more convenient to have ability to open current thread in encrypted inbox, so user will be able to see decrypted version and then reply to it. As currently when user receives encrypted message it's not possible to see decrypted content, the only way is to click flowcrypt button on toolbar, open encrypted inbox and then search for this message there.

I propose to rename Secure compose button to FlowCrypt and clicking on it will open current thread in encrypted inbox (it should be possible by sending threadId parameter). What do you think, should it be possible to implement it this way?

Screenshot 2024-06-21 at 14 53 20
martgil commented 1 week ago

Thank you @sosnovsky for the comments.

I think, it'll be more convenient to have ability to open current thread in encrypted inbox, so user will be able to see decrypted version and then reply to it.

Ah, yes. I see what you what you mean. It will be a convenient option. I'll check, but I'm not 100% sure since Google data such as replyMsgId, threadId are normally not in the raw email/mime so I'm not sure were to pull them since this email data I'm currently pulling are from the API offered by Thunderbird going to our pop-up going to our view (eg. Inbox). This is also the issue I get when trying to pass data to background script to inbox.js for example since the inbox.js are not loaded yet where the background script tries to pass the message. But I will thoroughly check first and let you know.

martgil commented 1 week ago

Hi @sosnovsky - Good news - I have figured out that its possible for us to obtain the threadId by querying an email's Message-Id using Gmail API - I'm currently exploring it out. I'll continue working on this tomorrow.

For my reference: https://stackoverflow.com/questions/33938816/how-to-get-thread-id-from-gmail-for-a-certain-email

martgil commented 5 days ago

Just a little bit more... I've implement almost everything and able to render the message inbox through threadId parameter after parsing the actual threadId from the message-id header. This can be tested out by simply clicking the secure compose on Thunderbird's message Display and should bring you to the thread message.

The things I would still be including is the account cross checking making sure that the account can only interact with their emails and not from other accounts' emails.

sosnovsky commented 5 days ago

@martgil does clicking on secure compose on message display works well for you? For me message decryption stucks in Loading... state:

Screenshot 2024-06-25 at 19 48 53

However messages decrypted correctly when opening directly from encrypted inbox. Seems to be related to https://github.com/FlowCrypt/flowcrypt-browser/issues/5770, as thunderbird and firefox probably use the same web engine.

martgil commented 5 days ago

Hi @sosnovsky,

When opening an encrypted email with Thunderbird's secure compose, I also experience the same issue. One thing I have observed is that there was an error during a process we might have tried to perform in Thunderbird. The specific error says, "Unchecked lastError value: Error: DataCloneError: Promise object could not be cloned." I will check that now and let you know everything I can gather.

EDIT: It seems that the message, which has been successfully decrypted, sometimes causes the system to get stuck. As a result, it is not working 100% of the time, whether accessed from the inbox or secure compose. There might be some interference happening behind the scenes.

Seems to be related to https://github.com/FlowCrypt/flowcrypt-browser/issues/5770, as thunderbird and firefox probably use the same web engine.

Yes, it might be - can confirm issue on firefox too.

martgil commented 5 days ago

Hi @sosnovsky - I have found a minor flaw of how we planned to implement the ability to reply to an email. Assume there was 3 accounts in thunderbird name a, b, c... with two of them have their FlowCrypt setup let say a and b set up FlowCrypt. In this current PR, assuming you are acting as user inbox a and u tried to reply securely with it with user b... user b wont have user b's thread id to that email of user a.

So to cut the story short, I believe we no longer need to list the FlowCrypt account list in secure reply at all since we do not need to put extra efforts for cross account checking which should not exists at all. So I propose, when secure compose is clicked if in 1.) compose Display - will show full screen secure compose with quoted text present and if 2.) in message Display will load up the email thread for that account. Would you agree on that?

sosnovsky commented 4 days ago

So to cut the story short, I believe we no longer need to list the FlowCrypt account list in secure reply at all since we do not need to put extra efforts for cross account checking which should not exists at all. So I propose, when secure compose is clicked if in 1.) compose Display - will show full screen secure compose with quoted text present and if 2.) in message Display will load up the email thread for that account. Would you agree on that?

Sounds good, will be quicker and more convenient than manually choosing account from list 👍

martgil commented 4 days ago

Sounds good, will be quicker and more convenient than manually choosing account from list

Thank you - I have also trim down tremendous unnecessary code. I'll update you really soon.

martgil commented 3 days ago

just need to wait for #5770 fix, as currently it's not possible to test some functionality because of the message stuck

Ah, yes. I'm settling all this requested change while were are looking into #5770 root case. The root cause is not so obvious as sometimes it works well quite randomly when I investigated about the issue.

martgil commented 2 days ago

Needs to wait for https://github.com/FlowCrypt/flowcrypt-browser/issues/5770.

martgil commented 2 days ago

Hi @sosnovsky - I have found a reliable way for you to test message decryption on FlowCrypt inbox without us having to wait for the official fix for #5770.

On thunderbird, open encrypted inbox and wait for at least 10 seconds for the page to load completely. Then after that, click on the encrypted email which would open the email thread.

That method works for me:

image

The same method of waiting for 10s works for me with my Firefox build of the extension. The message stuck on Gmail's extension page dont work with that waiting method.