Category not accepting "anonymous email" from known users

Reproduce:

  1. Have a “private” category that only has create/reply/see access for members of a specific group. No other permissions.
  2. Enable “anonymous email in” for that specific via a unique working alias to the incoming POP account.
  3. Have a non-staff user that is not a member of the group send an email report in to the category email address from the email address associated with their account.
  4. Send in an email from an account not associated with any known Discourse account.

Expected Behavior:

  • Both #3 and #4 result in new topics in the private category, and the group can begin discussion.

Actual Behavior:

  • The email in #4 works but email in #3 is rejected with the can_create? failed error.
8 Likes

@zogstrip can you review this?

Is this still a problem?

I’m no longer running a site with incoming email enabled so unfortunately can’t reproduce to say either way.

2 Likes

Pretty sure #3 still isn’t working.

When we receive an email, we first try to associate it to a user, and then we check for permissions. Since the user isn’t part of the group, they can’t post.

The email_in_allow_strangers field on the category only works for staged users.

2 Likes

Can confirm #3 still not working. Is the best workaround right now to have emails directed at a group rather than a category? Not my preferred arrangement but I can switch until a patch comes out.

1 Like

I don’t think we can ever support #3 unless we add another setting that opens up any incoming emails?

This scenario seems to be replaced these days with the group functionality as a type of workaround, although I could see some use cases that it might still be just as valuable for categories/topics/discussions.

Sounds plausible. Is this a big thing to do?

Like you say, this is just a (bad) workaround. IMO this should be controllable on a per-category level. Clearly a bug that this works only for anonymous but not for known users.

1 Like

I just ran into this issue while trying to set up a category for our organizations info list. This category only allows a certain group to access it, but we selected the option to allow emails from anonymous users. Email addresses that are not associated with any of our discourse users can send a message that will show up in the category, but registered users that are not in the group get rejected due to “Insufficient Trust Level”. I think I understand the technical reason why it works this way (only works for staged users) but is there a reason why this would be the desired or expected behavior? It seems to me if we are choosing to allow anonymous users we probably want to allow all registered users as well.

1 Like

We ran into this issue with our site this past week as well. For us, the situation is as follows:

While most of our categories are intended to be for registered users only, we wanted to have a way for a non-member of the community to make contact with the core developers by email to ask a quick question, raise a concern, etc.

We did this by setting up a category to accept emails from anonymous users using a custom address (e.g., ourproject+contact@discoursemail.com). We made the category’s permissions readable only by core project members to avoid generating too much mail or noise for the community at large (which meant making topics create-able and reply-able by core members as well since there’s no way to restrict read access without also limiting create/reply access).

However, we then learned that if the anonymous user went on to create an official account, they could no longer send mails to the contact email address. This seems counter-intuitive in that they are now only more official and registered than they were before, but can do strictly less than they could before.

This caused me a great degree of confusion before I came across this topic, and still seems like an unfortunate situation.

-Brad

2 Likes

This problem still exists 2.7.8. It is confusing that a mail sent to the email associated with a category is accepted when the sender has no registered account in the forum and is rejected if it is.

3 Likes

I believe the following patch (against the current main branch) could solve this issue by skipping validations when email_in_allow_strangers is set for the category, in the same way it is skipped for staged users. Does that sound sensible?

diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb
index 7c76c44d61..dd3bc3cfb0 100644
--- a/lib/email/receiver.rb
+++ b/lib/email/receiver.rb
@@ -762,7 +762,7 @@ module Email
                      elided: elided,
                      title: subject,
                      category: destination.id,
-                     skip_validations: user.staged?)
+                     skip_validations: (user.staged? || destination.email_in_allow_strangers))
 
       elsif destination.is_a?(PostReplyKey)
         # We don't stage new users for emails to reply addresses, exit if user is nil

It does not work because validations are about the content of the post, not about permissions to create a post or not…

I tested the patch below to work. It is a hack do not try this at home :scream: Its merit is to roughly identify where changes should happen. The challenge is to figure out how to do that in a proper way. Any advice would be most welcome :pray:

diff --git a/app/jobs/regular/notify_mailing_list_subscribers.rb b/app/jobs/regular/notify_mailing_list_subscribers.rb
index c535296105..1d3bf79637 100644
--- a/app/jobs/regular/notify_mailing_list_subscribers.rb
+++ b/app/jobs/regular/notify_mailing_list_subscribers.rb
@@ -74,7 +74,7 @@ module Jobs
 
       DiscourseEvent.trigger(:notify_mailing_list_subscribers, users, post)
       users.find_each do |user|
-        if Guardian.new(user).can_see?(post)
+        if Guardian.new(user).can_see?(post) && Guardian.new(user).can_see_category_staged?(post.topic.category)
           if EmailLog.reached_max_emails?(user)
             skip(user.email, user.id, post.id,
               SkippedEmailLog.reason_types[:exceeded_emails_limit]
diff --git a/app/models/category.rb b/app/models/category.rb
index 630a74c425..6c253650c6 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -201,7 +201,7 @@ class Category < ActiveRecord::Base
       end
     else
       permissions = permission_types.map { |p| CategoryGroup.permission_types[p] }
-      where("(:staged AND LENGTH(COALESCE(email_in, '')) > 0 AND email_in_allow_strangers)
+      where("(LENGTH(COALESCE(email_in, '')) > 0 AND email_in_allow_strangers)
           OR categories.id NOT IN (SELECT category_id FROM category_groups)
           OR categories.id IN (
                 SELECT category_id
@@ -209,7 +209,6 @@ class Category < ActiveRecord::Base
                  WHERE permission_type IN (:permissions)
                    AND (group_id = :everyone OR group_id IN (SELECT group_id FROM group_users WHERE user_id = :user_id))
              )",
-        staged: guardian.is_staged?,
         permissions: permissions,
         user_id: guardian.user.id,
         everyone: Group[:everyone].id)
diff --git a/lib/guardian/category_guardian.rb b/lib/guardian/category_guardian.rb
index 94a48466d6..2a4ba8015c 100644
--- a/lib/guardian/category_guardian.rb
+++ b/lib/guardian/category_guardian.rb
@@ -64,6 +64,14 @@ module CategoryGuardian
   end
 
   def can_see_category?(category)
+    return false unless category
+    return true if is_admin?
+    return true if !category.read_restricted
+    return true if category.email_in.present? && category.email_in_allow_strangers
+    secure_category_ids.include?(category.id)
+  end
+
+  def can_see_category_staged?(category)
     return false unless category
     return true if is_admin?
     return true if !category.read_restricted
2 Likes

What do you think of this suggestion @zogstrip?

1 Like

I think the behavior here is deliberate a fix here is going to have to be pretty comprehensive.

The trouble with allowing people to associate a topic with an existing account is that anyone can spoof anyone’s email.

It think the way of resolving this long term is:

  1. sam@somewhere.com has an account on Discourse.
  2. sam@somewhere.com emails a category with anonymous email in
  3. Discourse sends an email to sam@somewhere.com: "looks like you posted: CONTENTS OF POST, to URL of Discourse. Was it really you?
  4. Click yes
  5. Topic is created

Without this protection this feature would be wide open to impersonation which is very risky.

5 Likes

Isn’t this issue already there, regardless the Accept emails from anonymous users with no accounts parameter?

I share that point of view: when an email is associated to a category and the box Accept emails from anonymous users with no accounts is checked, an incoming email

  1. associated to a Discourse account
  2. which is not allowed to read the messages according to the Security parameters of the category

should be accepted.

1 Like

Does the use of SPF/DKIM/DMARC, which is much more prolific now than it was in 2016, not safeguard against this to a good enough degree?

Any good email provider (for the user) will not allow other users to send from addresses that do not belong to them and any good email provider or receiving service (for the Discourse instance) should reject emails that fail origin validation.

Providers will exist that don’t validate the from address and/or don’t set up SPF/etc. records but if a user chooses to use an email provider who does nothing to protect against impersonation, it seems reasonable that they can be impersonated.

I haven’t delved particularly deeply into it but it looks like Email::AuthenticationResults already does some origin validation. Perhaps in the short(er) term, the verdict could be made available (if it isn’t already) such that these emails can be accepted with a pass verdict.

In your long-term solution, the “was it really you?” verification could also be skipped in the case of a pass verdict.