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).
This commit is contained in:
Martin Brennan 2021-01-29 09:59:10 +10:00 committed by GitHub
parent 5114a6421b
commit 4af4d36175
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 78 additions and 14 deletions

View File

@ -234,6 +234,8 @@ const Group = RestModel.extend({
default_notification_level: this.default_notification_level,
membership_request_template: this.membership_request_template,
publish_read_state: this.publish_read_state,
allow_unknown_sender_topic_replies: this
.allow_unknown_sender_topic_replies,
};
["muted", "regular", "watching", "tracking", "watching_first_post"].forEach(

View File

@ -68,4 +68,15 @@
{{i18n "groups.manage.email.mailboxes.none_found"}}
{{/if}}
</div>
<div class="control-group">
<label class="control-label">{{i18n "groups.manage.email.settings.title"}}</label>
</div>
<div class="control-group">
<label class="control-group-inline" for="allow_unknown_sender_topic_replies">
{{input type="checkbox" name="allow_unknown_sender_topic_replies" id="allow_unknown_sender_topic_replies" checked=model.allow_unknown_sender_topic_replies}}
<span>{{i18n "groups.manage.email.settings.allow_unknown_sender_topic_replies"}}</span>
</label>
<p>{{i18n "groups.manage.email.settings.allow_unknown_sender_topic_replies_hint"}}</p>
</div>
{{/if}}

View File

@ -614,7 +614,8 @@ class GroupsController < ApplicationController
:name,
:grant_trust_level,
:automatic_membership_email_domains,
:publish_read_state
:publish_read_state,
:allow_unknown_sender_topic_replies
])
custom_fields = DiscoursePluginRegistry.editable_group_custom_fields

View File

@ -1026,6 +1026,10 @@ end
# membership_request_template :text
# messageable_level :integer default(0)
# mentionable_level :integer default(0)
# publish_read_state :boolean default(FALSE), not null
# members_visibility_level :integer default(0), not null
# flair_icon :string
# flair_upload_id :integer
# smtp_server :string
# smtp_port :integer
# smtp_ssl :boolean
@ -1037,13 +1041,10 @@ end
# imap_last_uid :integer default(0), not null
# email_username :string
# email_password :string
# publish_read_state :boolean default(FALSE), not null
# members_visibility_level :integer default(0), not null
# flair_icon :string
# flair_upload_id :integer
# imap_last_error :text
# imap_old_emails :integer
# imap_new_emails :integer
# allow_unknown_sender_topic_replies :boolean default(FALSE)
#
# Indexes
#

View File

@ -26,7 +26,8 @@ class GroupShowSerializer < BasicGroupSerializer
:imap_last_error,
:imap_old_emails,
:imap_new_emails,
:message_count
:message_count,
:allow_unknown_sender_topic_replies
def self.admin_or_owner_attributes(*attrs)
attributes(*attrs)

View File

@ -663,6 +663,10 @@ en:
imap_ssl: "Use SSL for IMAP"
username: "Username"
password: "Password"
settings:
title: "Settings"
allow_unknown_sender_topic_replies: "Allow unknown sender topic replies."
allow_unknown_sender_topic_replies_hint: "Allows unknown senders to reply to group topics. If this is not enabled, replies from email addresses not already included on the IMAP email thread or invited to the topic will create a new topic."
mailboxes:
synchronized: "Synchronized Mailbox"
none_found: "No mailboxes were found in this email account."

View File

@ -0,0 +1,11 @@
# frozen_string_literal: true
class AddAllowUnknownSenderTopicRepliesToGroup < ActiveRecord::Migration[6.0]
def up
add_column :groups, :allow_unknown_sender_topic_replies, :boolean, default: false, null: false
end
def down
remove_column :groups, :allow_unknown_sender_topic_replies
end
end

View File

@ -741,18 +741,31 @@ module Email
message_ids = Email::Receiver.extract_reply_message_ids(@mail, max_message_id_count: 5)
post_ids = []
incoming_emails = IncomingEmail
.where(message_id: message_ids)
.addressed_to_user(user)
.pluck(:post_id, :from_address, :to_addresses, :cc_addresses)
incoming_emails = IncomingEmail.where(message_id: message_ids)
if !group.allow_unknown_sender_topic_replies
incoming_emails = incoming_emails.addressed_to_user(user)
end
incoming_emails = incoming_emails.pluck(:post_id, :from_address, :to_addresses, :cc_addresses)
incoming_emails.each do |post_id, from_address, to_addresses, cc_addresses|
post_ids << post_id if contains_email_address_of_user?(from_address, user) ||
contains_email_address_of_user?(to_addresses, user) ||
contains_email_address_of_user?(cc_addresses, user)
if group.allow_unknown_sender_topic_replies
post_ids << post_id
else
post_ids << post_id if contains_email_address_of_user?(from_address, user) ||
contains_email_address_of_user?(to_addresses, user) ||
contains_email_address_of_user?(cc_addresses, user)
end
end
if post_ids.any? && post = Post.where(id: post_ids).order(:created_at).last
# this must be done for the unknown user (who is staged) to
# be allowed to post a reply in the topic
if group.allow_unknown_sender_topic_replies
post.topic.topic_allowed_users.find_or_create_by!(user_id: user.id)
end
create_reply(user: user,
raw: body,
elided: elided,

View File

@ -951,8 +951,9 @@ describe Email::Receiver do
expect { process(:email_reply_2) }.to change { topic.posts.count }.by(1).and change { Topic.count }.by(0)
end
it "creates a new topic when the sender is not known" do
it "creates a new topic when the sender is not known and the group does not allow unknown senders to reply to topics" do
IncomingEmail.where(message_id: '34@foo.bar.mail').update(cc_addresses: 'three@foo.com')
group.update(allow_unknown_sender_topic_replies: false)
expect { process(:email_reply_2) }.to change { topic.posts.count }.by(0).and change { Topic.count }.by(1)
end
@ -960,6 +961,25 @@ describe Email::Receiver do
IncomingEmail.where(message_id: '34@foo.bar.mail').update(message_id: '99@foo.bar.mail')
expect { process(:email_reply_2) }.to change { topic.posts.count }.by(0).and change { Topic.count }.by(1)
end
it "includes the sender on the topic when the message id is known, the sender is not known, and the group allows unknown senders to reply to topics" do
IncomingEmail.where(message_id: '34@foo.bar.mail').update(cc_addresses: 'three@foo.com')
group.update(allow_unknown_sender_topic_replies: true)
expect { process(:email_reply_2) }.to change { topic.posts.count }.by(1).and change { Topic.count }.by(0)
end
context "when the sender is not in the topic allowed users" do
before do
user = User.find_by_email("two@foo.com")
topic.topic_allowed_users.find_by(user: user).destroy
end
it "adds them to the topic at the same time" do
IncomingEmail.where(message_id: '34@foo.bar.mail').update(cc_addresses: 'three@foo.com')
group.update(allow_unknown_sender_topic_replies: true)
expect { process(:email_reply_2) }.to change { topic.posts.count }.by(1).and change { Topic.count }.by(0)
end
end
end
end