Commit Graph

160 Commits

Author SHA1 Message Date
Martin Brennan
48d13cb231
UX: Use a dropdown for SSL mode for group SMTP ()
Our old group SMTP SSL option was a checkbox,
but this was not ideal because there are actually
3 different ways SSL can be used when sending
SMTP:

* None
* SSL/TLS
* STARTTLS

We got around this before with specific overrides
for Gmail, but it's not flexible enough and now people
want to use other providers. It's best to be clear,
though it is a technical detail. We provide a way
to test the SMTP settings before saving them so there
should be little chance of messing this up.

This commit also converts GroupEmailSettings to a glimmer
component.
2024-07-18 10:33:14 +10:00
Martin Brennan
0783bfbbfe
FIX: Use login SMTP auth for office365 in group mailer ()
Followup 7b627dc14b

In this other commit, I changed the email settings validator
to always use the `login` authentication method for
office365 and outlook, but I didn't change the actual
group SMTP mailer to do this.

This commit fixes that issue and does some minor refactoring.
2024-07-16 16:21:14 +10:00
Régis Hanol
5f6b6e9818 FIX: correctly compute the window for email summaries
In 958437e7dd we ensured that the email summaries are properly sent based on 'digest_attempted_at' for people who barely/never visit the forum.

This fixed the "frequency" of the email summaries but introduced a bug where the digest would be sent even though there wasn't anything new since for some users.

The logic we use to compute the threshold date for the content to be included in the digest was

```ruby
@since = opts[:since] || user.last_seen_at || user.user_stat&.digest_attempted_at || 1.month.ago
```

It was working as expected for users who haven never been seen but for users who have connected at least once, we would use their "last_seen_at" date as the "threshold date" for the content to be sent in a summary 😬

This fix changes the logic to be the most recent date amongst the `last_seen_at`, `digest_attempted_at` and `1.month.ago` so it's correctly handling cases where

- user has never been seen nor emailed a summary
- user has been seen in a while but has recently been sent a summary
- user has been sent a summary recently but hasn't been seen in a while.
2024-05-27 22:33:51 +02:00
Ted Johansson
f3cad5f3a2
FIX: Correctly re-attach allowed images in activity summary e-mail ()
For e-mails, secure uploads redacts all secure images, and later uses the access control post to re-attached allowed ones. We pass the ID of this post through the X-Discourse-Post-Id header. As the name suggests, this assumes there's only ever one access control post. This is not true for activity summary e-mails, as they summarize across posts.

This adds a new header, X-Discourse-Post-Ids, which is used the same way as the old header, but also works for the case where an e-mail is associated with multiple posts.
2024-04-18 10:27:46 +08:00
Penar Musaraj
8f52fd1051
FIX: Ensure invalid timezone does not block critical emails ()
See https://meta.discourse.org/t/invalid-user-timezone-jams-up-their-mail/303306

This ensures that "account silenced" and "account suspended" emails don't
fail to send if the user has an invalid timezone.
2024-04-12 10:53:44 -04:00
Loïc Guitaut
f7d7092a7a DEV: Update rubocop-discourse to latest version
The lastest version of rubocop-discourse enables rules regarding
plugins.
2024-03-04 15:08:35 +01:00
Ted Johansson
f0a46f8b6f
DEV: Automatically update groups for test users with explicit TL ()
For performance reasons we don't automatically add fabricated users to trust level auto-groups. However, when explicitly passing a trust level to the fabricator, in 99% of cases it means that trust level is relevant for the test, and we need the groups.

This change makes it so that when a trust level is explicitly passed to the fabricator, the auto-groups are refreshed. There's no longer a need to also pass refresh_auto_groups: true, which means clearer tests, fewer mistakes, and less confusion.
2024-01-29 17:52:02 +08:00
Ted Johansson
6ab1a19e93
DEV: Convert min_trust_level_to_allow_invite to groups ()
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_level_to_allow_invite  site setting to invite_allowed_groups.

Nothing much of note. This is used in one place and there's no fallout.
2023-12-18 12:07:36 +08:00
Jarek Radosz
694b5f108b
DEV: Fix various rubocop lints ()
These (21 + 3 from previous PRs) are soon to be enabled in rubocop-discourse:

Capybara/VisibilityMatcher
Lint/DeprecatedOpenSSLConstant
Lint/DisjunctiveAssignmentInConstructor
Lint/EmptyConditionalBody
Lint/EmptyEnsure
Lint/LiteralInInterpolation
Lint/NonLocalExitFromIterator
Lint/ParenthesesAsGroupedExpression
Lint/RedundantCopDisableDirective
Lint/RedundantRequireStatement
Lint/RedundantSafeNavigation
Lint/RedundantStringCoercion
Lint/RedundantWithIndex
Lint/RedundantWithObject
Lint/SafeNavigationChain
Lint/SafeNavigationConsistency
Lint/SelfAssignment
Lint/UnreachableCode
Lint/UselessMethodDefinition
Lint/Void

Previous PRs:
Lint/ShadowedArgument
Lint/DuplicateMethods
Lint/BooleanSymbol
RSpec/SpecFilePathSuffix
2023-12-06 23:25:00 +01:00
Martin Brennan
3e639e4aa7
FIX: Add higher read & open timeouts for group SMTP emails ()
When sending SMTP for group SMTP functionality, we
are running into timeouts for both read and open
when sending mail occassionally, which can cause issues
like the email only being sent to _some_ of the recipients
or to fail altogether.

The defaults of 5s are too low, so bumping them up to
the defaults of the `net-smtp` gem.
2023-11-28 15:32:59 +10:00
Daniel Waterworth
6e161d3e75
DEV: Allow fab! without block ()
The most common thing that we do with fab! is:

    fab!(:thing) { Fabricate(:thing) }

This commit adds a shorthand for this which is just simply:

    fab!(:thing)

i.e. If you omit the block, then, by default, you'll get a `Fabricate`d object using the fabricator of the same name.
2023-11-09 16:47:59 -06:00
Alan Guo Xiang Tan
9c9058d0c3
FIX: Order tags shown in email subject by topics count and name ()
Why this change?

Prior to this change, the ordering of the tags shown in the email subject
was non-deterministic as there was no specific order specified. This
problem was exposed by a flaky test which we had.

What is the fix?

This commit orders the tags used in the email subject first by the
`Tag#public_topic_count` column in descending order and then the `Tag#name`
column in ascending order.
2023-07-13 15:39:58 +08:00
Loïc Guitaut
0f4beab0fb DEV: Update the rubocop-discourse gem
This enables cops related to RSpec `subject`.

See https://github.com/discourse/rubocop-discourse/pull/32
2023-06-26 11:41:52 +02:00
Vinoth Kannan
19133af057
FIX: don't add "Re:" prefix in email subject for first post of group PMs. ()
Previously, it will add a "Re:" prefix to all the emails passing through the group SMTP mailer. It's creating confusion while receiving the mail for the first time on a PM.
2023-06-19 18:52:00 +05:30
Juan David Martínez Cubillos
8b39125985
FEATURE: Implement max_tags_per_email_subject ()
* FEATURE: Implement max_tags_per_email_subject

* made it so only max_tags_per_email_subject is responsible for tags in emails when the feature is enabled

* added locales for implemented siteSettings

* reworded locale for enable_max_tags_per_email_subject

* added min value for max_tags_per_email_subject

* Implemented suggested changes to spec description
2023-06-14 12:22:14 -05:00
Martin Brennan
7a1d60c60e
FIX: Likes received count in digest email ()
This commit fixes an issue where the Likes Received notification
count in the user digest email was not affected by the
since/last_seen date for the user, which meant that no matter
how long it had been since the user visited the count was
always constant.

Now instead for the Likes Received count, we only count the
unread notifications of that type since the user was last
seen.
2023-05-09 19:19:26 +02:00
David Taylor
cb932d6ee1
DEV: Apply syntax_tree formatting to spec/* 2023-01-09 11:49:28 +00:00
Martin Brennan
c4ea158656
FIX: Improve tags in email subjects and add filter headers ()
This commit does a couple of things:

1. Changes the limit of tags to include a subject for a
   notification email to the `max_tags_per_topic` setting
   instead of the arbitrary 3 limit
2. Adds both an X-Discourse-Tags and X-Discourse-Category
   custom header to outbound emails containing the tags
   and category from the subject, so people on mail clients
   that allow advanced filtering (i.e. not Gmail) can filter
   mail by tags and category, which is useful for mailing
   list mode users

c.f. https://meta.discourse.org/t/headers-for-email-notifications-so-that-gmail-users-can-filter-on-tags/249982/17
2023-01-06 10:03:02 +10:00
Alan Guo Xiang Tan
ab3a032b4b
SECURITY: BCC active user emails from group SMTP ()
When sending emails out via group SMTP, if we
are sending them to non-staged users we want
to mask those emails with BCC, just so we don't
expose them to anyone we shouldn't. Staged users
are ones that have likely only interacted with
support via email, and will likely include other
people who were CC'd on the original email to the
group.

Co-authored-by: Martin Brennan <martin@discourse.org>
2023-01-05 06:07:50 +08:00
Daniel Waterworth
84c83e8d4a
SECURITY: Filter tags in user notifications for visibility () 2022-11-29 10:35:41 -06:00
Loïc Guitaut
3eaac56797 DEV: Use proper wording for contexts in specs 2022-08-04 11:05:02 +02:00
Phil Pirozhkov
493d437e79
Add RSpec 4 compatibility ()
* Remove outdated option

04078317ba

* Use the non-globally exposed RSpec syntax

https://github.com/rspec/rspec-core/pull/2803

* Use the non-globally exposed RSpec syntax, cont

https://github.com/rspec/rspec-core/pull/2803

* Comply to strict predicate matchers

See:
 - https://github.com/rspec/rspec-expectations/pull/1195
 - https://github.com/rspec/rspec-expectations/pull/1196
 - https://github.com/rspec/rspec-expectations/pull/1277
2022-07-28 10:27:38 +08:00
Andrei Prigorshnev
25e4095c9c
FIX: respect user timezone in emails about silencing and suspending () 2022-05-27 13:58:54 +04:00
Gerhard Schlager
1a56ce3674 FEATURE: Site setting to cap the recipient list in notification emails
* Adds a hidden site setting: `max_participant_names`
* Replaces duplicate code in `GroupSmtpMailer` and `UserNotifications`
* Groups are sorted by the number of users (decreasing)
* Replaces the query to count users of each group with `Group#user_count`)
* Users are sorted by their last reply in the topic (most recent first)
* Adds lots of tests
2022-04-21 10:43:13 +02:00
Gerhard Schlager
b7230d14a3 REFACTOR: Add full_url to Group
Lets stop writing the same code over and over again.
2022-04-14 11:53:57 +02:00
David Taylor
c9dab6fd08
DEV: Automatically require 'rails_helper' in all specs ()
It's very easy to forget to add `require 'rails_helper'` at the top of every core/plugin spec file, and omissions can cause some very confusing/sporadic errors.

By setting this flag in `.rspec`, we can remove the need for `require 'rails_helper'` entirely.
2022-03-01 17:50:50 +00:00
Jarek Radosz
2fc70c5572
DEV: Correctly tag heredocs ()
This allows text editors to use correct syntax coloring for the heredoc sections.

Heredoc tag names we use:

languages: SQL, JS, RUBY, LUA, HTML, CSS, SCSS, SH, HBS, XML, YAML/YML, MF, ICS
other: MD, TEXT/TXT, RAW, EMAIL
2022-02-28 20:50:55 +01:00
Dan Ungureanu
fa8cd629f1
DEV: Hash tokens stored from email_tokens ()
This commit adds token_hash and scopes columns to email_tokens table.
token_hash is a replacement for the token column to avoid storing email
tokens in plaintext as it can pose a security risk. The new scope column
ensures that email tokens cannot be used to perform a different action
than the one intended.

To sum up, this commit:

* Adds token_hash and scope to email_tokens

* Reuses code that schedules critical_user_email

* Refactors EmailToken.confirm and EmailToken.atomic_confirm methods

* Periodically cleans old, unconfirmed or expired email tokens
2021-11-25 09:34:39 +02:00
Martin Brennan
d3678f6930
FIX: Do not show recipient user in email participants list ()
This commit removes the recipient's username from the
respond to / participants list that is shown at the bottom
of user notification emails. For example if the recipient's
username was jsmith, and there were participants ljones and
bmiller, we currently show this:

> "reply to this email to respond to jsmith, ljones, bmiller"

or

> "Participants: jsmith, ljones, bmiller"

However this is a bit redundant, as you are not replying to
yourself here if you are the recipient user. So we omit the
recipient user's username from this list, which is only used
in the text of the email and not elsewhere.
2021-10-19 15:26:22 +10:00
Arpit Jalan
d5424161f8
FIX: add locales for group mention PM variants () 2021-09-16 23:07:45 +05:30
Martin Brennan
87684f7c5e
FEATURE: Use group SMTP job and mailer instead of UserNotifications change ()
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
Martin Brennan
b463a80cbf
FIX: Do not enqueue :group_smtp_email job if IMAP disabled for the group ()
When a group only has SMTP enabled and not IMAP, we do not
want to enqueue the :group_smtp_email job because using the group's
SMTP credentials for sending user_private_message emails is
handled by the UserNotifications class.

We do not want the :group_smtp_email job to be enqueued because
that uses a reply key instead of the group.email_username
for the reply-to address which is not what we want for SMTP
only, and also creates an IncomingEmail record to prevent IMAP
double syncing which we do not need either.

There is an open question about what happens when IMAP is
enabled after SMTP has been enabled for a while, and also questions
around whether we could do away with :group_smtp_email altogether
and handle everything via EmailLog and UserNotifications, adding
additional columns to the former and modifying the Imap::Sync
class to take this into account...a lot more further testing
for IMAP needs to be done to answer those questions.

For now, this fix should be sufficient to get the correct
reply-to address for user_private_response messages sent in
response to emails sent directly to the group's
email_username SMTP address.

Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
2021-06-07 14:17:35 +10:00
Martin Brennan
eb2c399445
FEATURE: Use group SMTP settings for sending user notification emails (initial) ()
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
Martin Brennan
964da21817
FEATURE: Improve group email settings UI ()
This overhauls the user interface for the group email settings management, aiming to make it a lot easier to test the settings entered and confirm they are correct before proceeding. We do this by forcing the user to test the settings before they can be saved to the database. It also includes some quality of life improvements around setting up IMAP and SMTP for our first supported provider, GMail. This PR does not remove the old group email config, that will come in a subsequent PR. This is related to https://meta.discourse.org/t/imap-support-for-group-inboxes/160588 so read that if you would like more backstory.

### UI

Both site settings of `enable_imap` and `enable_smtp` must be true to test this. You must enable SMTP first to enable IMAP.

You can prefill the SMTP settings with GMail configuration. To proceed with saving these settings you must test them, which is handled by the EmailSettingsValidator.

If there is an issue with the configuration or credentials a meaningful error message should be shown.

IMAP settings must also be validated when IMAP is enabled, before saving.

When saving IMAP, we fetch the mailboxes for that account and populate them. This mailbox must be selected and saved for IMAP to work (the feature acts as though it is disabled until the mailbox is selected and saved):

### Database & Backend

This adds several columns to the Groups table. The purpose of this change is to make it much more explicit that SMTP/IMAP is enabled for a group, rather than relying on settings not being null. Also included is an UPDATE query to backfill these columns. These columns are automatically filled when updating the group.

For GMail, we now filter the mailboxes returned. This is so users cannot use a mailbox like Sent or Trash for syncing, which would generally be disastrous.

There is a new group endpoint for testing email settings. This may be useful in the future for other places in our UI, at which point it can be extracted to a more generic endpoint or module to be included.
2021-05-28 09:28:18 +10:00
Dan Ungureanu
a5273b37f7
FIX: Show inviter name in email's from field ()
'From' field of the email contained the name of the user who posted the
shared post. Instead, it should contain the name of the inviter.
2021-05-26 12:55:07 +10:00
Roman Rizzi
958fbfb719
FEATURE: Send an email notification when a post is approved. ()
We now send an email when a queued post is approved, and we create a notification.
2021-04-12 12:08:23 -03:00
Dan Ungureanu
fb19ee9eee
FIX: Correctly use invite to topic email templates ()
It was used both when inviting from a topic page and when creating
invites with "Send to topic on first login", while it should be used
only in the former case.
2021-03-16 17:08:54 +02:00
Gerhard Schlager
3b2f6e129a
FEATURE: Add English (UK) as locale ()
* "English" gets renamed into "English (US)"
* "English (UK)" replaces "English"

@discourse-translator-bot keep_translations_and_approvals
2021-01-20 21:32:22 +01:00
Martin Brennan
632942e697
FIX: Ensure group SMTP and message builder always uses from address for Reply-To when IMAP is enabled ()
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
Martin Brennan
6e2be3e60b
FIX: When admin changes an email for the user the user must confirm the change ()
See https://meta.discourse.org/t/changing-a-users-email/164512 for additional context.

Previously when an admin user changed a user's email we assumed that they would need a password reset too because they likely did not have access to their account. This proved to be incorrect, as there are other reasons a user needs admin to change their email. This PR:

* Changes the admin change email for user flow so the user is sent an email to confirm the change
* We now record who the email change request was requested by
* If the requested by user is admin and not the user we note this in the email sent to the user
* We also make the confirm change email route open to anonymous users, so it can be clicked by the user even if they do not have access to their account. If there is a logged in user we make sure the confirmation matches the current user.
2020-10-07 13:02:24 +10:00
Martin Brennan
dede942007
FEATURE: Allow email image embed with secure media ()
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
Robin Ward
6da7a97eee FIX: Exclude shared drafts from digests 2020-08-04 13:35:48 -04:00
Jarek Radosz
a96ff82e53
FIX: Set the lang/xml:lang html attrs in emails ()
In some rare cases emails are incorrectly sent out with lang placeholders in body (`xml:lang="%{html_lang}"`)
2020-07-13 16:39:40 +02:00
Dan Ungureanu
c72bc27888
FEATURE: Implement support for IMAP and SMTP email protocols. ()
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
2020-07-10 12:05:55 +03:00
Sam Saffron
38a30a6e96
DEV: correct regression and correct tests
etag change in 31976ecf was incorrect, revert it

Also correct regression in test suite.
2020-07-06 10:56:19 +10:00
Bianca Nenciu
75151f0457
FIX: Use correct URL for unsubscribe () 2020-06-24 09:31:20 +02:00
Jarek Radosz
7ff889574d
DEV: Add rubocop-rspec ()
This adds rubocop-rspec, and enables some cops that were either already passing or are passing now, after fixing them in this commit.

Some new cops are disabled for now, with annotation: "TODO" or "To be decided". Those either need to be discussed first, or require manual changes, or the number of found and fixed offenses is too large to bundle them up in a single PR.

Includes:

* DEV: Update rubocop's `TargetRubyVersion` to 2.6
* DEV: Enable RSpec/VoidExpect
* DEV: Enable RSpec/SharedContext
* DEV: Enable RSpec/EmptyExampleGroup (Removed an obsolete empty spec file)
* DEV: Enable RSpec/ItBehavesLike
* DEV: Remove RSpec/ScatteredLet (It's too strict, as it doesn't recognize fab! as a let-like)
* DEV: Remove RSpec/MultipleExpectations
2020-03-27 17:35:40 +01:00
Kris
e69ffa86b3 Fix test another way 2020-02-11 17:07:18 -05:00
Kris
5e6f0d9f1c Fix test 2020-02-11 16:09:41 -05:00
Joffrey JAFFEUX
0d3d2c43a0
DEV: s/\$redis/Discourse\.redis ()
This commit also adds a rubocop rule to prevent global variables.
2019-12-03 10:05:53 +01:00