From 3df3a908ac8ba987e9f0a110101674c6ad21bf5f Mon Sep 17 00:00:00 2001 From: riking Date: Fri, 11 Jul 2014 13:01:01 -0700 Subject: [PATCH 1/5] Remove email_in_address and email_in_category site settings The functionality is entirely covered by per-category email in addresses, and the category being a number was confusing people. --- config/site_settings.yml | 4 ---- ...711193923_remove_email_in_address_setting.rb | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20140711193923_remove_email_in_address_setting.rb diff --git a/config/site_settings.yml b/config/site_settings.yml index fe7b5a0987d..a68035def3a 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -384,13 +384,9 @@ email: email_in: default: false client: true - email_in_address: - default: '' - type: email email_in_min_trust: default: 2 enum: 'TrustLevelSetting' - email_in_category: -1 email_prefix: '' email_site_title: '' diff --git a/db/migrate/20140711193923_remove_email_in_address_setting.rb b/db/migrate/20140711193923_remove_email_in_address_setting.rb new file mode 100644 index 00000000000..cc2f6b63119 --- /dev/null +++ b/db/migrate/20140711193923_remove_email_in_address_setting.rb @@ -0,0 +1,17 @@ +class RemoveEmailInAddressSetting < ActiveRecord::Migration + def up + cat_id_r = ActiveRecord::Base.exec_sql("SELECT value FROM site_settings WHERE name = 'email_in_category'").first + email_r = ActiveRecord::Base.exec_sql("SELECT value FROM site_settings WHERE name = 'email_in_address'").first + if cat_id_r && email_r + category_id = cat_id_r["value"].to_i + email = email_r["value"] + ActiveRecord::Base.exec_sql("UPDATE categories SET email_in = ? WHERE id = ?", email, category_id) + end + + ActiveRecord::Base.exec_sql("DELETE FROM site_settings WHERE name = 'email_in_category' OR name = 'email_in_address'") + end + + def down + # this change is backwards-compatible + end +end From 17db265b414cd69b2ff2db17723a382f3a2147bd Mon Sep 17 00:00:00 2001 From: riking Date: Fri, 11 Jul 2014 13:30:08 -0700 Subject: [PATCH 2/5] Remove use of email_in_address and email_in_category --- config/locales/server.en.yml | 4 +- docs/MAILING-LIST-SETUP.md | 6 +- lib/email/receiver.rb | 8 +-- spec/components/email/receiver_spec.rb | 93 -------------------------- 4 files changed, 6 insertions(+), 105 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index f1d6c5a4748..9aa130752c7 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -912,10 +912,8 @@ en: pop3s_polling_host: "The host to poll for email via POP3S." pop3s_polling_username: "The username for the POP3S account to poll for email." pop3s_polling_password: "The password for the POP3S account to poll for email." - email_in: "Allow users to post new topics via email." - email_in_address: "The email address the users can post new topics to. None means users can't post globally." + email_in: "Allow users to post new topics via email (requires pop3 polling). Configure the addresses in the \"Settings\" tab of each category." email_in_min_trust: "The minimum trust level a user needs to have to be allowed to post new topics via email." - email_in_category: "The category new emails are posted into." email_prefix: "The [label] used in the subject of emails. It will default to 'title' if not set." email_site_title: "The title of the site used as the sender of emails from the site. Default to 'title' if not set. If your 'title' contains characters that are not allowed in email sender strings, use this setting." diff --git a/docs/MAILING-LIST-SETUP.md b/docs/MAILING-LIST-SETUP.md index 25dd3394c6d..594abb0acf8 100644 --- a/docs/MAILING-LIST-SETUP.md +++ b/docs/MAILING-LIST-SETUP.md @@ -7,13 +7,13 @@ Acting like a Mailing list is disabled per default in Discourse. This guide show First of, you need a POP3s enabled server receiving your email. Then make sure to enable "reply_by_email_enabled" and configured the server appropriately in your Admin-Settings under "Email": ![enable-reply-by-email](https://f.cloud.github.com/assets/2879972/2242953/97d5dd52-9d17-11e3-915e-037758cc68a7.png) -Once that is in place, you can enable the "email_in"-feature globally in the same email-section. If you provide another "email_in_address" all emails arriving in the inbox to that address will be handeled and posted to the "email_in_category" (defaults to "uncategorised"). For spam protection only users of a high trust level can post via email per default. You can change this via the "email_in_min_trust" setting. +Once that is in place, you can enable the "email_in"-feature globally in the same email-section. ### Per category email address -Once "email_in" is enabled globally a new configuration option appears in your category settings dialog allowing you to specify an email-address for that category. Emails going to the previously configured inbox to that email-address will be posted in this category instead of the default configuration. **Attention** User-Permissions and the minimum trust levels still apply. +Once "email_in" is enabled globally a new configuration option appears in your category settings dialog allowing you to specify an email-address for that category. Emails going to the previously configured inbox to that email-address will be posted in this category. -Additionally, by checking the "accept non-user emails"-checkbox in the category settings, emails to the given email but from unknown email-addresses will be posted in the category by the System-User in a quoted fashion, showing the original email-address and content in the quotes. +For spam protection only users of a high trust level can post via email per default. You can change this via the "email_in_min_trust" setting. Additionally, by checking the "accept non-user emails"-checkbox in the category settings, emails to the given email but from unknown email-addresses will be posted in the category by the System-User in a quoted fashion, showing the original email-address and content in the quotes. ### Troubleshooting diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 9fc25af104a..b452214f737 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -44,7 +44,7 @@ module Email end raise UserNotFoundError if @user.blank? - raise UserNotSufficientTrustLevelError.new @user unless @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i]) + raise UserNotSufficientTrustLevelError.new @user unless @allow_strangers || @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i]) create_new_topic else @@ -134,11 +134,7 @@ module Email def is_in_email? @allow_strangers = false - - if SiteSetting.email_in && SiteSetting.email_in_address == @message.to.first - @category_id = SiteSetting.email_in_category.to_i - return true - end + return false unless SiteSetting.email_in category = Category.find_by_email(@message.to.first) return false unless category diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 9e0b6c29ce6..b1162a09b35 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -198,99 +198,8 @@ greatest show ever created. Everyone should watch it. end - describe "processes a valid incoming email" do - before do - SiteSetting.stubs(:email_in_address).returns("discourse-in@appmail.adventuretime.ooo") - SiteSetting.stubs(:email_in_category).returns("42") - SiteSetting.stubs(:email_in).returns(true) - end - - let(:incoming_email) { fixture_file("emails/valid_incoming.eml") } - let(:receiver) { Email::Receiver.new(incoming_email) } - let(:user) { Fabricate.build(:user, id: 3456) } - let(:subject) { "We should have a post-by-email-feature." } - let(:email_body) { -"Hey folks, - -I was thinking. Wouldn't it be great if we could post topics via email? Yes it would! - -Jakie" } - - describe "email from non user" do - - before do - User.expects(:find_by_email).returns(nil) - end - - it "raises user not found error" do - expect { receiver.process }.to raise_error(Email::Receiver::UserNotFoundError) - end - - end - - describe "email from untrusted user" do - before do - User.expects(:find_by_email).with( - "jake@adventuretime.ooo").returns(user) - SiteSetting.stubs(:email_in_min_trust).returns(TrustLevel.levels[:elder].to_s) - end - - it "raises untrusted user error" do - expect { receiver.process }.to raise_error(Email::Receiver::UserNotSufficientTrustLevelError) - end - - end - - describe "with proper user" do - - before do - SiteSetting.stubs(:email_in_min_trust).returns(TrustLevel.levels[:newuser].to_s) - User.expects(:find_by_email).with( - "jake@adventuretime.ooo").returns(user) - - topic_creator = mock() - TopicCreator.expects(:new).with(instance_of(User), - instance_of(Guardian), - has_entries(title: subject, - category: 42)) - .returns(topic_creator) - - topic_creator.expects(:create).returns(topic_creator) - topic_creator.expects(:id).twice.returns(12345) - - - post_creator = mock - PostCreator.expects(:new).with(instance_of(User), - has_entries(raw: email_body, - topic_id: 12345, - cooking_options: {traditional_markdown_linebreaks: true})) - .returns(post_creator) - - post_creator.expects(:create) - - EmailLog.expects(:create).with(has_entries( - email_type: 'topic_via_incoming_email', - to_address: "discourse-in@appmail.adventuretime.ooo", - user_id: 3456, - topic_id: 12345 - )) - end - - let!(:result) { receiver.process } - - it "extracts the body" do - expect(receiver.body).to eq(email_body) - end - - end - - end - - describe "processes an email to a category" do before do - SiteSetting.stubs(:email_in_address).returns("") - SiteSetting.stubs(:email_in_category).returns("42") SiteSetting.stubs(:email_in).returns(true) end @@ -398,8 +307,6 @@ Jakie" } describe "processes an unknown email sender to category" do before do - SiteSetting.stubs(:email_in_address).returns("") - SiteSetting.stubs(:email_in_category).returns("42") SiteSetting.stubs(:email_in).returns(true) end From 73ceeda1f9a4f46a45af1bed76b83f39ac5261f4 Mon Sep 17 00:00:00 2001 From: riking Date: Fri, 11 Jul 2014 15:12:15 -0700 Subject: [PATCH 3/5] Stop unnecessarily suppressing settings for uncategorized Fix migration for emailing into uncategorized --- .../discourse/templates/modal/edit-category.js.handlebars | 4 ++-- .../20140711193923_remove_email_in_address_setting.rb | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/discourse/templates/modal/edit-category.js.handlebars b/app/assets/javascripts/discourse/templates/modal/edit-category.js.handlebars index 6f1b67241b5..786de606f49 100644 --- a/app/assets/javascripts/discourse/templates/modal/edit-category.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/edit-category.js.handlebars @@ -3,9 +3,9 @@ {{edit-category-tab selectedTab=selectedTab tab="general"}} {{#unless isUncategorizedCategory}} {{edit-category-tab selectedTab=selectedTab tab="security"}} - {{edit-category-tab selectedTab=selectedTab tab="settings"}} - {{edit-category-tab selectedTab=selectedTab tab="images"}} {{/unless}} + {{edit-category-tab selectedTab=selectedTab tab="settings"}} + {{edit-category-tab selectedTab=selectedTab tab="images"}}
diff --git a/db/migrate/20140711193923_remove_email_in_address_setting.rb b/db/migrate/20140711193923_remove_email_in_address_setting.rb index cc2f6b63119..f81efdb7a60 100644 --- a/db/migrate/20140711193923_remove_email_in_address_setting.rb +++ b/db/migrate/20140711193923_remove_email_in_address_setting.rb @@ -1,9 +1,11 @@ class RemoveEmailInAddressSetting < ActiveRecord::Migration def up + uncat_id = ActiveRecord::Base.exec_sql("SELECT value FROM site_settings WHERE name = 'uncategorized_category_id'").first cat_id_r = ActiveRecord::Base.exec_sql("SELECT value FROM site_settings WHERE name = 'email_in_category'").first email_r = ActiveRecord::Base.exec_sql("SELECT value FROM site_settings WHERE name = 'email_in_address'").first - if cat_id_r && email_r - category_id = cat_id_r["value"].to_i + if email_r + category_id = uncat_id["value"].to_i + category_id = cat_id_r["value"].to_i if cat_id_r email = email_r["value"] ActiveRecord::Base.exec_sql("UPDATE categories SET email_in = ? WHERE id = ?", email, category_id) end From 1682f5d584e9f46b85ca52978dd47afcfce67315 Mon Sep 17 00:00:00 2001 From: riking Date: Thu, 17 Jul 2014 09:43:13 -0700 Subject: [PATCH 4/5] Fix message, and add test, for EmailUnparseableError --- config/locales/server.en.yml | 5 ++-- spec/components/email/receiver_spec.rb | 4 +-- spec/fixtures/emails/no_content_reply.eml | 34 +++++++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 spec/fixtures/emails/no_content_reply.eml diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 9aa130752c7..ef99e1b4619 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1372,14 +1372,15 @@ en: text_body_template: | We're sorry, but your email message to %{destination} didn't work. - We couldn't find any content in the email. + We couldn't find any content in the email. Make sure that you wrote something at the top of the email - we can't parse inline replies. + If you're getting this and you did include content, try again in UTF-8 plain text (no HTML). email_reject_parsing: subject_template: "Email issue -- Content unrecognized" text_body_template: | We're sorry, but your email message to %{destination} didn't work. - The email encoding was unknown or not supported. Try again with UTF-8 plain text. + We couldn't find where your reply was in the email. **Make sure to write your entire reply at the top of the email** -- everything below the start of the replied-to message is discarded. email_reject_post_error: subject_template: "Email issue -- Posting error" diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index b1162a09b35..95d757dcbf1 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -19,8 +19,8 @@ describe Email::Receiver do expect { Email::Receiver.new("asdf" * 30).process}.to raise_error(Email::Receiver::EmptyEmailError) end - pending "raises EmailUnparsableError in some situation" do - expect { Email::Receiver.new("something").process}.to raise_error(Email::Receiver::EmailUnparsableError) + it "raises EmailUnparsableError if there is no reply content" do + expect { Email::Receiver.new(fixture_file("emails/no_content_reply.eml")).process}.to raise_error(Email::Receiver::EmailUnparsableError) end end diff --git a/spec/fixtures/emails/no_content_reply.eml b/spec/fixtures/emails/no_content_reply.eml new file mode 100644 index 00000000000..95eb2055ce6 --- /dev/null +++ b/spec/fixtures/emails/no_content_reply.eml @@ -0,0 +1,34 @@ +Return-Path: +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog +To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo +Message-ID: +Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 + +On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta + wrote: +> +> +> +> eviltrout posted in 'Adventure Time Sux' on Discourse Meta: +> +> --- +> hey guys everyone knows adventure time sucks! +> +> --- +> Please visit this link to respond: http://localhost:3000/t/adventure-time-sux/1234/3 +> +> To unsubscribe from these emails, visit your [user preferences](http://localhost:3000/user_preferences). +> \ No newline at end of file From c8d322d1be6a1cfe50274bd652e82f0c5ab99d2d Mon Sep 17 00:00:00 2001 From: riking Date: Thu, 17 Jul 2014 10:04:02 -0700 Subject: [PATCH 5/5] Include the email subject line in rejection messages This change has a tradeoff. It increases our backscatter vulnerability - the subject could have spammy content - but it's extremely valuable to the user to know exactly which message was rejected. If you sent two at the same time, and only one was rejected, you would have no way of knowing which worked and which to resend without going to the website (which is what email-in is trying to avoid, kinda). --- app/jobs/scheduled/poll_mailbox.rb | 2 +- app/mailers/rejection_mailer.rb | 11 ++++++----- config/locales/server.en.yml | 12 ++++++------ spec/jobs/poll_mailbox_spec.rb | 11 ++--------- 4 files changed, 15 insertions(+), 21 deletions(-) diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index 564d20c6c58..45182ddfceb 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -44,7 +44,7 @@ module Jobs if message_template # inform the user about the rejection message = Mail::Message.new(mail_string) - client_message = RejectionMailer.send_rejection(message.from, message.body, message.to, message_template) + client_message = RejectionMailer.send_rejection(message.from, message.body, message.subject, message.to, message_template) Email::Sender.new(client_message, message_template).send else Discourse.handle_exception(e, { code: "unknown error for incoming email", mail: mail_string} ) diff --git a/app/mailers/rejection_mailer.rb b/app/mailers/rejection_mailer.rb index 03beb06c4a6..20727b459de 100644 --- a/app/mailers/rejection_mailer.rb +++ b/app/mailers/rejection_mailer.rb @@ -3,11 +3,12 @@ require_dependency 'email/message_builder' class RejectionMailer < ActionMailer::Base include Email::BuildEmailHelper - def send_rejection(from, body, to_address, template) - build_email(from, template: "system_messages.#{template}", source: body, destination: to_address) + def send_rejection(message_from, message_body, message_subject, forum_address, template) + build_email(message_from, + template: "system_messages.#{template}", + source: message_body, + former_title: message_subject, + destination: forum_address) end - def send_trust_level(from, template) - build_email(from, template: 'email_reject_trust_level') - end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ef99e1b4619..af6cf412b2f 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1356,21 +1356,21 @@ en: email_reject_trust_level: subject_template: "Email issue -- Insufficient Trust Level" text_body_template: | - We're sorry, but your email message to %{destination} didn't work. + We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work. Your account does not have the required trust level to post new topics to this email address. If you believe this is in error, contact a staff member. email_reject_no_account: subject_template: "Email issue -- No Account" text_body_template: | - We're sorry, but your email message to %{destination} didn't work. + We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work. There is no known account with this email address. Try sending from a different email address, or contact a staff member. email_reject_empty: subject_template: "Email issue -- No Content" text_body_template: | - We're sorry, but your email message to %{destination} didn't work. + We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work. We couldn't find any content in the email. Make sure that you wrote something at the top of the email - we can't parse inline replies. If you're getting this and you did include content, try again in UTF-8 plain text (no HTML). @@ -1378,21 +1378,21 @@ en: email_reject_parsing: subject_template: "Email issue -- Content unrecognized" text_body_template: | - We're sorry, but your email message to %{destination} didn't work. + We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work. We couldn't find where your reply was in the email. **Make sure to write your entire reply at the top of the email** -- everything below the start of the replied-to message is discarded. email_reject_post_error: subject_template: "Email issue -- Posting error" text_body_template: | - We're sorry, but your email message to %{destination} didn't work. + We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work. Some possible causes are: complex formatting, message too large, message too small. Please try again. email_reject_reply_key: subject_template: "Email issue -- Bad Reply Key" text_body_template: | - We're sorry, but your email message to %{destination} didn't work. + We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work. The provided reply key is invalid or unknown, so we don't know what this email is in reply to. Contact a staff member. diff --git a/spec/jobs/poll_mailbox_spec.rb b/spec/jobs/poll_mailbox_spec.rb index f83b25404af..3adcb78c735 100644 --- a/spec/jobs/poll_mailbox_spec.rb +++ b/spec/jobs/poll_mailbox_spec.rb @@ -44,14 +44,7 @@ describe Jobs::PollMailbox do describe "processing email" do let!(:receiver) { mock } - let!(:email_string) { <