Commit Graph

576 Commits

Author SHA1 Message Date
Martin Brennan 7b392cee50
FIX: Correct the forwarded by user small post for group inbox (#14252)
When 2ac9fd9dff was done, this
affected the small post that is created when forwarding an email
into the group inbox. Instead of using the name and the email of
the user who forwarded the email, it used the original from email
and name to create the small post. So instead of something like
"Discourse Team forwarded the above email" we ended up with
"John Smith forwarded the above email" which is incorrect.

This fixes the issue by creating a staged user for the forwarding
email address (if such a user does not yet exist) and uses that
for the "forwarded" small post instead.
2021-09-06 15:02:13 +10:00
Bianca Nenciu f0005025c4
FIX: Correctly display GitHub code oneboxes (#14149)
Code fragments without the code lines were not displayed correctly.

Follow-up-to 721a946d77.
2021-08-26 12:55:46 +03:00
Bianca Nenciu 721a946d77
FIX: Remove spacing from GitHub oneboxes (#14147)
Each code line was surrounded by a lot of white space. The new styles
are similar to what the web interface is using.
2021-08-25 16:34:01 +03:00
Martin Brennan 2ac9fd9dff
FIX: Use original from address when forwarding to group inbox (#14114)
When emails were forwarded to a group inbox by the email address
of the group, for example when an email ends up in spam and must
be manually forwarded to the group+site@discoursemail.com address,
the OP of the topic ended up being the group's email address instead
of the sender who originally sent the email to the group inbox.

This commit detects that an email has been forwarded using existing
tools, and if the from address matches one of the group incoming
email addresses, then we look at the forwarded email's from address
and use that instead for the incoming email from address as well as
the staged/regular user used for the Topic.user.

This will make it much cleaner to forward emails into a group inbox,
and will prevent issues with PostAlerter where the OP is double-notified
for these emails.
2021-08-24 08:57:28 +10:00
Bianca Nenciu 5c9abab0cd
FIX: Remove markers from all translated languages (#14081)
Emails can include the marker in a different language, depending on
site and user settings. The email receiver always looked for the marker
in default language.
2021-08-18 19:42:04 +03:00
Joffrey JAFFEUX 644441852e
FIX: prevents exception on malformatted messages (#13997)
The following example message would generate an exception:

```
Return-Path: <discourse@bar.com>
From: Foo Bar <discourse@bar.com>
To: reply+4f97315cc828096c9cb34c6f1a0d6fe8@bar.com
Date: Fri, 15 Jan 2016 00:12:43 +0100
Message-ID: <21@foo.bar.mail>
Mime-Version: 1.0
Content-Type: text/html; charset=UTF-8

</div>

```

Exception:

```
NoMethodError:
       undefined method `split' for nil:NilClass
```
2021-08-10 15:49:32 +02:00
Bianca Nenciu 54e2b95539
FIX: Remove 'reply above line' marker (#13927)
This marker is automatically added by Discourse's group SMTP mailer.
When user responded to emails sent by it, Discourse did not trim this
marker.
2021-08-03 20:08:19 +03:00
Bianca Nenciu 52520638ca
FIX: Inline secure images with duplicated names (#13926)
Inlining secure images with the same name was not possible because they
were indexed by filename. If an email contained two files with the same
name, only the first image was used for both of them. The other file
was still attached to the email.
2021-08-03 18:58:34 +03:00
Martin Brennan b88d8c8894
FIX: Use reply-to address for incoming emails if present (#13896)
When the Reply-To header is present for incoming emails we
want to use it instead of the from address. This is usually the
case when forwarding an email via a mailing list into Discourse.

For now we are only using the Reply-To header if the email has
been forwarded via Google Groups, which is why we are checking the
X-Original-From header too. In future we may want to use the Reply-To
header in more cases.
2021-08-03 08:01:17 +10:00
Martin Brennan d3e27cabf6
FIX: Improve participant display in group SMTP emails (#13539)
This PR makes several changes to the group SMTP email contents to make it look more like a support inbox message.

* Remove the context posts, they only add clutter to the email and replies
* Display email addresses of staged users instead of odd generated usernames
* Add a "please reply above this line" message to sent emails
2021-06-28 10:42:06 +10:00
Martin Brennan 87684f7c5e
FEATURE: Use group SMTP job and mailer instead of UserNotifications change (#13489)
This PR backtracks a fair bit on this one https://github.com/discourse/discourse/pull/13220/files.

Instead of sending the group SMTP email for each user via `UserNotifications`, we are changing to send only one email with the existing `Jobs::GroupSmtpEmail` job and `GroupSmtpMailer`. We are changing this job and mailer along with `PostAlerter` to make the first topic allowed user the `to_address` for the email and any other `topic_allowed_users` to be the CC address on the email. This is to cut down on emails sent via SMTP, which is subject to daily limits from providers such as Gmail. We log these details in the `EmailLog` table now.

In addition to this, we have changed `PostAlerter` to no longer rely on incoming email email addresses for sending the `GroupSmtpEmail` job. This was unreliable as a user's email could have changed in the meantime. Also it was a little overcomplicated to use the incoming email records -- it is far simpler to reason about to just use topic allowed users.

This also adds a fix to include cc_addresses in the EmailLog.addressed_to_user scope.
2021-06-28 08:55:13 +10:00
Jarek Radosz 203d56719d
UX: Improve blockquote styling in emails (#13527) 2021-06-25 20:13:46 +02:00
Jarek Radosz 61472d6aaa
DEV: Rename `hilight` to `highlight` (#13526) 2021-06-25 18:05:50 +02:00
Martin Brennan 5222247746
FEATURE: Add more columns to outbound EmailLog (#13449)
This adds the following columns to EmailLog:

* cc_addresses
* cc_user_ids
* topic_id
* raw

This is to bring the EmailLog table closer in parity to
IncomingEmail so it can be better utilized for Group SMTP
and IMAP mailing.

The raw column contains the full content of the outbound email,
but _only_ if the new hidden site setting
enable_raw_outbound_email_logging is enabled. Most sites do not
need it, and it's mostly required for IMAP and SMTP sending.

In the next pull request, there will be a migration to backfill
topic_id on the EmailLog table, at which point we can remove the
topic fallback method on EmailLog.
2021-06-22 08:32:01 +10:00
Martin Brennan 22b96c9ce1
FIX: Prevent resurrecting old topics via email reply for group inboxes with SMTP enabled (#13382)
We already reject email replies to public topics via `SiteSetting.disallow_reply_by_email_after_days` and raising the `OldDestinationError`. This PR introduces similar behaviour for group inboxes, but without the rejection, and **only when SMTP is enabled for the group**.

If a reply is sent via email and the post is older than `SiteSetting.disallow_reply_by_email_after_days` days ago, then we create a new topic instead of making a reply in the old one and link back to the original topic. This is done to prevent long running group inbox discussions.
2021-06-21 11:45:00 +10:00
Martin Brennan ff6114d83f
FIX: Do not add mailing list headers to group SMTP emails (#13431)
When we are emailing people from a group inbox, we are having
a PM conversation with them, as a support account would. In this
case mailing list headers do not make sense. It is not like a forum
topic where you may have tens or hundreds of participants -- it is a
conversation between the group and a small handful of people
directly contacting the group, often just one person.

The only header left in tact was List-Unsubsribe which is important
for letting people opt out to notifications.
2021-06-18 14:36:17 +10:00
Martin Brennan 7fca7fb7ff
DEV: Add SMTP group ID to EmailLog (#13381)
Adds a new `smtp_group_id` column to `EmailLog` which is filled in if the mail `from_address` matches a group's `email_username`. This is for easier debugging, so we know which emails have been sent via group SMTP.
2021-06-15 11:29:46 +10:00
Martin Brennan e9dc88a7b6
FIX: Link up reply to post correctly when emailing group (#13339)
When replying to a user_private_message email originating from
a group PM that does _not_ have a reply key (e.g. when replying
directly to the group's SMTP address), we were mistakenly linking
the new post created from the reply to the OP and the user who
created the topic, based on the first IncomingEmail message ID in
the topic, rather than using the correct reply to user and post number
that the user actually replied to.

We now use the In-Reply-To header to look up the corresponding EmailLog
record when the user who replied was sent a user_private_message email,
and use the post from that as the reply_to_user/post.

This also removes superfluous filtering of incoming_email records. After
already filtering by message_id and then addressed_to_user (which only
returns incoming emails where the to, from, or cc address includes any
of the user's emails), we were filtering again but in the ruby code for
the exact same conditions. After removing this all existing tests still
pass.
2021-06-10 15:28:50 +10:00
Martin Brennan eb2c399445
FEATURE: Use group SMTP settings for sending user notification emails (initial) (#13220)
This PR changes the `UserNotification` class to send outbound `user_private_message` using the group's SMTP settings, but only if:

* The first allowed_group on the topic has SMTP configured and enabled
* SiteSetting.enable_smtp is true
* The group does not have IMAP enabled, if this is enabled the `GroupSMTPMailer` handles things

The email is sent using the group's `email_username` as both the `from` and `reply-to` address, so when the user replies from their email it will go through the group's SMTP inbox, which needs to have email forwarding set up to send the message on to a location (such as a hosted site email address like meta@discoursemail.com) where it can be POSTed into discourse's handle_mail route.

Also includes a fix to `EmailReceiver#group_incoming_emails_regex` to include the `group.email_username` so the group does not get a staged user created and invited to the topic (which was a problem for IMAP), as well as updating `Group.find_by_email` to find using the `email_username` as well for inbound emails with that as the TO address.

#### Note

This is safe to merge without impacting anyone seriously. If people had SMTP enabled for a group they would have IMAP enabled too currently, and that is a very small amount of users because IMAP is an alpha product, and also because the UserNotification change has a guard to make sure it is not used if IMAP is enabled for the group. The existing IMAP tests work, and I tested this functionality by manually POSTing replies to the SMTP address into my local discourse.

There will probably be more work needed on this, but it needs to be tested further in a real hosted environment to continue.
2021-06-03 14:47:32 +10:00
Dan Ungureanu ccbe3bea79
UX: Improve style of GitHub PR body in emails (#13198)
On the web, we display only an excerpt in a monospace font and the rest
of the body is hidden under ellipsis. The email displayed both of them
and it did not use the same style. This commit leaves only the excerpt
in emails and makes it use a monospace font to display it.
2021-05-31 14:03:19 +03:00
Bianca Nenciu e6329d3007
FIX: Gracefully handle inline images in emails (#12855) 2021-04-29 10:17:33 +03:00
Régis Hanol cd93d1b5f7
FEATURE: new 'trim_incoming_emails' site setting (#12874)
This setting allows admin to de/activate automatic trimming of incoming email.
There are instances where it does wonders in trimming all the garbage content and other
instances where it's so bad that it trims the most important part of the email.

FIX: don't remove hidden content using the style attribute when converting HTML to Markdown.
The regexp used was doing more harm than good. It was way too broad.

FIX: properly elide signatures from emails sent with Front App.
This is fairly safe as Front App nicely identifies signatures in the HTML part.
2021-04-28 17:08:48 +02:00
Bianca Nenciu 129aeb8c2f
FIX: Improvements to email styles (#12451) 2021-03-22 14:09:38 +02:00
Bianca Nenciu 3246c3cc92
DEV: Update mail and use fork (#10639)
Version 2.8 brings some changes to how address fields are handled and
this commits updates that and should also include a fix which handles
encoded attachment filenames.

The fork contains a bugfix to correctly decode mail attachments.
2021-02-18 20:15:02 +02:00
Kris ded0f5b4e5
UX: Remove Helvetica from our font stack (#11876)
Arial is more consistently available across operating systems and aligns better.
2021-02-05 17:01:21 -05:00
Martin Brennan 4af4d36175
FIX: IMAP allow unknown senders to reply to group topics via email (#11877)
Adds a new column/setting to groups, allow_unknown_sender_topic_replies, which is default false. When enabled, this scenario is allowed via IMAP:

* OP sends an email to the support email address which is synced to a group inbox via IMAP, creating a group topic
* Group user replies to the group topic
* An email notification is sent to the OP of the topic via GroupSMTPMailer
* The OP has several email accounts and the reply is sent to all of them, or they forward their reply to another email account
* The OP replies from a different email address than the OP (gloria@gmail.com instead of gloria@hey.com for example)
* The a new staged user is created, the new reply is accepted and added to the topic, and the staged user is added to the topic allowed users

Without allow_unknown_sender_topic_replies enabled the new reply creates an entirely new topic (because the email address it is sent from is not previously part of the topic email chain).
2021-01-29 09:59:10 +10:00
Martin Brennan 9ee8a01c3a
FIX: Change default for IncomingEmail#created_via to 0 (unknown) and make NOT NULL (#11782)
Follow up to https://review.discourse.org/t/dev-add-created-via-column-to-incomingemail-pr-11751/18366/6
2021-01-21 12:59:50 +10:00
Martin Brennan fb184fed06
DEV: Add created_via column to IncomingEmail (#11751)
This should make it easier to track down how the incoming email was created, which is one of four locations:

The POP3 poller (which picks up reply via email replies)
The admin email controller #handle_mail (which is where hosted mail is sent)
The IMAP sync tool
The group SMTP mailer, which sends emails when replying to IMAP topics, pre-emptively creating IncomingEmail records to avoid double syncing
2021-01-20 13:22:41 +10:00
Martin Brennan bd25627198
FIX: IMAP post alerter race condition and code improvements (#11711)
This PR fixes a race condition with the IMAP notification code. In the `Email::Receiver` we call the `NewPostManager` to create the post and enqueue jobs and sends alerts via `PostAlerter`. However, if the post alerter reaches the `notify_pm_users` and the `group_notifying_via_smtp` method _before_ the incoming email is updated with the post and topic, we unnecessarily send a notification to the person who just posted. The result of this is that the IMAP syncer re-imports the email sent to the user about their own post, which looks like this in the group inbox:

To fix this, we skip the jobs enqueued by `NewPostManager` and only enqueue them with `PostJobsEnqueuer` manually _after_ the incoming email record has been updated with the post and topic.

Other improvements:

* Moved code to calculate email addresses from `IncomingEmail` records into the topic, with a group passed in, for easier testing and debugging. It is not the responsibility of the post alerter to figure this stuff out.
* Add shortcut methods on `IncomingEmail` to split or provide an empty array for to and cc addresses to avoid repetition.
2021-01-15 10:54:46 +10:00
Martin Brennan 64ba5b1d21
FIX: Group SMTP email improvements (#11633)
Fixes a rare race condition causing the `Imap::Sync` class to create an incoming email and associated post/topic, which then kicks off the PostAlerter to notify others in the PM about a reply in the topic, but for the OP which is not necessary (because the person emailing the IMAP inbox already knows about the OP). Basically, we should never be sending the group SMTP email for the first post in a topic.

Also in this PR:

* Custom attribute accessors for the to/from/cc addresses on `IncomingEmail`, to parse them from an array to a joined string so the logic for this is only in one place.
* Store extra detail against the `IncomingEmail` created in `GroupSmtpMailer`
* regex test Mail header Reply-To as string instead of Field, which fixes `warning: deprecated Object#=~ is called on Mail::Field; it always returns nil`
* Add DEBUG_IMAP to log all IMAP logs as warnings for easier debugging
* Changed the Rails logging to `ImapSyncLog` in the `GroupSmtpMailer`
2021-01-05 15:32:04 +10:00
Martin Brennan 28db835c4c
FIX: Calculate email attachment size limit correctly (#11321)
When calculating whether the attached uploads went over the SiteSetting.email_total_attachment_size_limit_kb.kilobytes limit, we were using the original_upload for the calculation instead of the actually attached_upload, which will be smaller in most cases because it can be an optimized image.
2020-11-23 11:16:08 +10:00
Martin Brennan 879e4a9e29
FIX: Inline avatar style for onebox when embedding secure images (#11229)
When embedding secure images that are inline-avatars for oneboxes we weren't applying the correct sizing/style.
2020-11-16 09:58:40 +10:00
Martin Brennan 2bb7676ba4
FIX: Stop bypassing email checks for invite emails (#11113)
See #10794 for original context.

I did not mean to add invite to the BYPASS_TYPES for Email::Sender, it was supposed to be invite_password_instructions.
2020-11-04 09:18:22 +10:00
Rafael dos Santos Silva b70b3e867c
FIX: Safely skip secure_media steps when it's not enabled (#11110)
* FIX: Safely skip secure_media steps when it's not enabled

* DEV: Secure media tests should enable secure media
2020-11-03 13:53:15 -03:00
Kane York 789e3775df
FIX: Make all email subject vars available in notification subjects (#11064)
A site owner attempting to use both the email_subject site setting and translation overrides for normal post notification
email subjects would find themselves frusturated at the lack of template argument parity.
Make all the variables available for translation overrides by adding the subject variables to the custom interpolation keys list and applying them.

Reported at https://meta.discourse.org/t/customize-subject-format-for-standard-emails/20801/47?u=riking
2020-11-02 20:00:11 -08:00
Martin Brennan 3655062c60
FIX: Ensure oneboxed secure images which are optimized and also lightboxed optimized images are embedded in email (#11061)
We had an issue where onebox thumbnail was too large and thus was optimized, and we are using the image URLs in post to redact and re-embed, based on the sha1 in the URL. Optimized image URLs have extra stuff on the end like _99x99 so we were not parsing out the sha1 correctly. Another issue I found was for posts that have giant images, the original was being used to embed in the email and thus would basically never get included because it is huge.

For example the URL 787b17ea61_2_690x335.jpeg was not parsed correctly; we would end up with 787b17ea6140f4f022eb7f1509a692f2873cfe35_2_690x335.jpeg as the sha1 which would not find the image to re-embed that was already attached to the email.

This fix will use the first optimized image of the detected upload when we are redacting and then re-embedding to make sure we are not sending giant things in email. Also, I detect if it is a onebox thumbnail or the site icon and force appropriate sizes and styles.
2020-11-02 09:52:21 +10:00
Martin Brennan 632942e697
FIX: Ensure group SMTP and message builder always uses from address for Reply-To when IMAP is enabled (#11037)
There is a site setting reply_by_email_enabled which when combined with reply_by_email_address creates a Reply-To header in emails in the format "test+%{reply_key}@test.com" along with a PostReplyKey record, so when replying Discourse knows where to route the reply.

However this conflicts with the IMAP implementation. Since we are sending the email for a group via SMTP and from their actual email account, we want all replys to go to that email account as well so the IMAP sync job can pick them up and put them in the correct place. So if the group has IMAP enabled and configured, then the reply-to header will be correct.

This PR also makes a further fix to 64b0b50 by using the correct recipient user for the PostReplyKey record. If the post user is used we encounter this error:

if destination.user_id != user.id && !forwarded_reply_key?(destination, user)
  raise ReplyUserNotMatchingError, "post_reply_key.user_id => #{destination.user_id.inspect}, user.id => #{user.id.inspect}"
end
This is because the user above is found from the from_address, but the destination which is the PostReplyKey is made by the post.user, which will be different people.
2020-10-28 07:01:58 +10:00
Krzysztof Kotlarek b2481adb40
FIX: persist secure image width and height if is given (#10994)
`max-width: 50%; max-height: 400px;` is a good fallback, however, if width and height are given and are smaller than fallback -  we should persist that smaller size.
2020-10-22 13:25:09 +11:00
Martin Brennan 64b0b50ac0
FIX: Pass user to Email::Sender to avoid broken reply key for group_smtp email (#10978)
Our Email::Sender class accepts an optional user argument, which is used to create a PostReplyKey record when present. This record is used to sub out the %{reply_key} placeholder in the Reply-To mail header, so if we do not pass in the user we get a broken Reply-To header.

This is especially problematic in the IMAP group SMTP situation, because these emails go to customers that we are replying to, and when they reply to us the email bounces! This fixes the issue by passing user to the Email::Sender when sending a group_smtp email but there is still more to do in another PR.

This Email::Sender optional user is a bit of a footgun IMO, especially because most of the time we use it there is a user we can source. I would like to do another PR for this after this one to make the parameter not optional, so we don't end up with these reply issues down the line again.
2020-10-22 10:49:08 +10:00
Daniel Waterworth 48c3172913
DEV: Remove preserve_email_structure_when_styling setting (#10956)
This was made adjustable to allow rolling back quickly if problems came
up. The new behaviour was made default in 93137066 and no problems with
this have been reported.
2020-10-19 12:19:30 +01:00
Martin Brennan f63da1c146
FIX: Confirm new email not sent for staff if email disabled with "non-staff" option (#10794)
See https://meta.discourse.org/t/email-address-change-confirmation-email-not-sent-but-every-other-notification-emails-are/165358

In short: with disable emails set to non-staff, email address change confirmation emails (those sent to the new address) are not sent for staff or admin members.

This was happening because we were looking up the staff user with the to_address of the email, but the to address was the new email address because we are sending a confirm email change email, and thus the user could not be found. We didn't need to do this anyway because we are passing the user into the Email::Sender class anyway.
2020-10-08 13:52:17 +10:00
Gerhard Schlager 99181bb3b8
FIX: Stripping lines from incoming email shouldn't fail for blank body (#10800) 2020-10-02 15:44:35 +02:00
Martin Brennan a8ed0b4612
FIX: Correct corrupt encoding in emails containing attachments 2020-09-29 14:10:57 +10:00
Arpit Jalan 376255ed84 UX: add width and height to GitHub onebox inline avatar in emails. 2020-09-21 11:47:09 +05:30
Martin Brennan dede942007
FEATURE: Allow email image embed with secure media (#10563)
This PR introduces a few important changes to secure media redaction in emails. First of all, two new site settings have been introduced:

* `secure_media_allow_embed_images_in_emails`: If enabled we will embed secure images in emails instead of redacting them.
* `secure_media_max_email_embed_image_size_kb`: The cap to the size of the secure image we will embed, defaulting to 1mb, so the email does not become too big. Max is 10mb. Works in tandem with `email_total_attachment_size_limit_kb`.

`Email::Sender` will now attach images to the email based on these settings. The sender will also call `inline_secure_images` in `Email::Styles` after secure media is redacted and attachments are added to replace redaction messages with attached images. I went with attachment and `cid` URLs because base64 image support is _still_ flaky in email clients.

All redaction of secure media is now handled in `Email::Styles` and calls out to `PrettyText.strip_secure_media` to do the actual stripping and replacing with placeholders. `app/mailers/group_smtp_mailer.rb` and `app/mailers/user_notifications.rb` no longer do any stripping because they are earlier in the pipeline than `Email::Styles`.

Finally the redaction notice has been restyled and includes a link to the media that the user can click, which will show it to them if they have the necessary permissions.

![image](https://user-images.githubusercontent.com/920448/92341012-b9a2c380-f0ff-11ea-860e-b376b4528357.png)
2020-09-10 09:50:16 +10:00
Martin Brennan 95b71b35d6
FEATURE: IMAP delete email sync for group inboxes (#10392)
Adds functionality to reflect topic delete in Discourse to IMAP inbox (Gmail only for now) and reflecting Gmail deletes in Discourse.

Adding lots of tests, various refactors and code improvements.

When Discourse topic is destroyed in PostDestroyer mark the topic incoming email as imap_sync: true, and do the opposite when post is recovered.
2020-08-12 10:16:26 +10:00
Martin Brennan ed68ac054d
DEV: Cover other case for generated message ID in Email::Receiver 2020-08-03 13:37:06 +10:00
Martin Brennan 7f55ed1a4a
DEV: Clarify Email::Receiver existing incoming email behaviour 2020-08-03 13:31:34 +10:00
Martin Brennan 2920988b3a
FIX: IMAP sync email update uniqueness across groups and minor improvements (#10332)
Adds a imap_group_id column to IncomingEmail to deal with an issue where we were trying to update emails in the mailbox, calling IncomingEmail.where(imap_sync: true). However UID and UIDVALIDITY could be the same across accounts. So if group A used IMAP details for Gmail account A, and group B used IMAP details for Gmail account B, and both tried to sync changes to an email with UID of 3 (e.g. changing Labels), one account could affect the other. This even applied to Archiving!

Also in this PR:

* Fix error occurring if we do a uid_fetch and no emails are returned
* Allow for creating labels within the target mailbox (previously we would not do this, only use existing labels)
* Improve consistency for log messages
* Add specs for generic IMAP provider (Gmail specs still to come)
* Add custom archiving support for Gmail
* Only use Message-ID for uniqueness of IncomingEmail if it was generated by us
* Various refactors and improvements
2020-08-03 13:10:17 +10:00
Krzysztof Kotlarek e0d9232259
FIX: use allowlist and blocklist terminology (#10209)
This is a PR of the renaming whitelist to allowlist and blacklist to the blocklist.
2020-07-27 10:23:54 +10:00