From 1cde276656a9ed72c1dd6488b86a60d25da260ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 7 Dec 2015 17:01:08 +0100 Subject: [PATCH] FEATURE: ability to send emails to a group --- app/models/group.rb | 4 + app/models/user.rb | 2 +- lib/email/receiver.rb | 105 +++++++++++++++---------- lib/topic_creator.rb | 2 +- spec/components/email/receiver_spec.rb | 52 +++++++++--- 5 files changed, 111 insertions(+), 54 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index 7684e4f9e..76ae67ded 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -317,6 +317,10 @@ class Group < ActiveRecord::Base self.group_users.create(user_id: user.id, owner: true) end + def self.find_by_email(email) + self.find_by(incoming_email: Email.downcase(email)) + end + protected def name_format_validator diff --git a/app/models/user.rb b/app/models/user.rb index e0e7299dd..1a3dbd47e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -560,7 +560,7 @@ class User < ActiveRecord::Base # Takes into account admin, etc. def has_trust_level?(level) raise "Invalid trust level #{level}" unless TrustLevel.valid?(level) - admin? || moderator? || TrustLevel.compare(trust_level, level) + admin? || moderator? || staged? || TrustLevel.compare(trust_level, level) end # a touch faster than automatic diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index f149bb919..07f89304e 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -1,8 +1,5 @@ require_dependency 'new_post_manager' -require 'email/html_cleaner' -# -# Handles an incoming message -# +require_dependency 'email/html_cleaner' module Email @@ -34,37 +31,32 @@ module Email message = Mail.new(@raw) - body = parse_body message + body = parse_body(message) - dest_info = {type: :invalid, obj: nil} + dest_info = { type: :invalid, obj: nil } message.to.each do |to_address| - if dest_info[:type] == :invalid - dest_info = check_address to_address - end + dest_info = check_address(to_address) + break if dest_info[:type] != :invalid end - raise BadDestinationAddress if dest_info[:type] == :invalid + raise BadDestinationAddress if dest_info[:type] == :invalid raise AutoGeneratedEmailError if message.header.to_s =~ /auto-generated/ || message.header.to_s =~ /auto-replied/ # TODO get to a state where we can remove this @message = message @body = body - if dest_info[:type] == :category + user_email = @message.from.first + @user = User.find_by_email(user_email) + + case dest_info[:type] + when :group raise BadDestinationAddress unless SiteSetting.email_in - category = dest_info[:obj] - @category_id = category.id - @allow_strangers = category.email_in_allow_strangers + group = dest_info[:obj] - user_email = @message.from.first - @user = User.find_by_email(user_email) - - # create staged account when user doesn't exist - if @user.blank? && @allow_strangers + if @user.blank? if SiteSetting.allow_staged_accounts - username = UserNameSuggester.suggest(user_email) - name = User.suggest_name(user_email) - @user = User.create(email: user_email, username: username, name: name, staged: true) + @user = create_staged_account(user_email) else wrap_body_in_quote(user_email) @user = Discourse.system_user @@ -72,38 +64,67 @@ module Email end raise UserNotFoundError if @user.blank? - raise UserNotSufficientTrustLevelError.new @user unless @allow_strangers || @user.has_trust_level?(TrustLevel[SiteSetting.email_in_min_trust.to_i]) + raise UserNotSufficientTrustLevelError.new(@user) unless @user.has_trust_level?(TrustLevel[SiteSetting.email_in_min_trust.to_i]) - create_new_topic - else + create_new_topic(archetype: Archetype.private_message, target_group_names: [group.name]) + when :category + raise BadDestinationAddress unless SiteSetting.email_in + category = dest_info[:obj] + + if @user.blank? && category.email_in_allow_strangers + if SiteSetting.allow_staged_accounts + @user = create_staged_account(user_email) + else + wrap_body_in_quote(user_email) + @user = Discourse.system_user + end + end + + raise UserNotFoundError if @user.blank? + raise UserNotSufficientTrustLevelError.new(@user) unless category.email_in_allow_strangers || @user.has_trust_level?(TrustLevel[SiteSetting.email_in_min_trust.to_i]) + + create_new_topic(category: category.id) + when :reply @email_log = dest_info[:obj] - raise EmailLogNotFound if @email_log.blank? + raise EmailLogNotFound if @email_log.blank? raise TopicNotFoundError if Topic.find_by_id(@email_log.topic_id).nil? - raise TopicClosedError if Topic.find_by_id(@email_log.topic_id).closed? + raise TopicClosedError if Topic.find_by_id(@email_log.topic_id).closed? create_reply end + rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError => e raise EmailUnparsableError.new(e) end - def check_address(address) - category = Category.find_by_email(address) - return {type: :category, obj: category} if category + def create_staged_account(email) + User.create( + email: email, + username: UserNameSuggester.suggest(email), + name: User.suggest_name(email), + staged: true + ) + end - regex = Regexp.escape SiteSetting.reply_by_email_address + def check_address(address) + group = Group.find_by_email(address) + return { type: :group, obj: group } if group + + category = Category.find_by_email(address) + return { type: :category, obj: category } if category + + regex = Regexp.escape(SiteSetting.reply_by_email_address) regex = regex.gsub(Regexp.escape('%{reply_key}'), "(.*)") - regex = Regexp.new regex + regex = Regexp.new(regex) match = regex.match address if match && match[1].present? reply_key = match[1] email_log = EmailLog.for(reply_key) - - return {type: :reply, obj: email_log} + return { type: :reply, obj: email_log } end - {type: :invalid, obj: nil} + { type: :invalid, obj: nil } end def parse_body(message) @@ -211,13 +232,13 @@ module Email reply_to_post_number: @email_log.post.post_number) end - def create_new_topic - result = create_post_with_attachments(@user, - raw: @body, - title: @message.subject, - category: @category_id) + def create_new_topic(topic_options={}) + topic_options[:raw] = @body + topic_options[:title] = @message.subject + result = create_post_with_attachments(@user, topic_options) topic_id = result.post.present? ? result.post.topic_id : nil + EmailLog.create( email_type: "topic_via_incoming_email", to_address: @message.from.first, # pick from address because we want the user's email @@ -228,10 +249,10 @@ module Email result end - def create_post_with_attachments(user, post_opts={}) + def create_post_with_attachments(user, post_options={}) options = { cooking_options: { traditional_markdown_linebreaks: true }, - }.merge(post_opts) + }.merge(post_options) raw = options[:raw] diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb index 7d6691e99..a69f17938 100644 --- a/lib/topic_creator.rb +++ b/lib/topic_creator.rb @@ -166,7 +166,7 @@ class TopicCreator def add_groups(topic, groups) return unless groups - names = groups.split(',') + names = groups.split(',').flatten len = 0 Group.where(name: names).each do |group| diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index a7d7d26aa..b4af327b6 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -437,7 +437,7 @@ This is a link http://example.com" end end - describe "posting a new topic" do + describe "posting a new topic in a category" do let(:category_destination) { raise "Override this in a lower describe block" } let(:email_raw) { raise "Override this in a lower describe block" } let(:allow_strangers) { false } @@ -536,7 +536,6 @@ greatest show ever created. Everyone should watch it. SiteSetting.email_in = true end - it "correctly can target categories" do to = "some@email.com" @@ -595,27 +594,60 @@ greatest show ever created. Everyone should watch it. }.to raise_error(Discourse::InvalidAccess) end - end - describe "processes an unknown email sender to category" do + let(:email_in) { "bob@bob.com" } + let(:user_email) { "#{SecureRandom.hex(32)}@foobar.com" } + let(:body) { "This is a new topic created\n\ninside a category ! :)" } + before do SiteSetting.email_in = true + SiteSetting.allow_staged_accounts = true end it "rejects anon email" do - Fabricate(:category, email_in_allow_strangers: false, email_in: "bob@bob.com") - expect { process_email(from: "test@test.com", to: "bob@bob.com") }.to raise_error(Email::Receiver::UserNotFoundError) + Fabricate(:category, email_in_allow_strangers: false, email_in: email_in) + + expect { + process_email(from: user_email, to: email_in, body: body) + }.to raise_error(Email::Receiver::UserNotFoundError) end it "creates a topic for allowed category" do - Fabricate(:category, email_in_allow_strangers: true, email_in: "bob@bob.com") - process_email(from: "test@test.com", to: "bob@bob.com") + Fabricate(:category, email_in_allow_strangers: true, email_in: email_in) + process_email(from: user_email, to: email_in, body: body) - # This is the current implementation but it is wrong, it should register an account - expect(Discourse.system_user.posts.order("id desc").limit(1).pluck(:raw).first).to include("Hey folks") + staged_account = User.find_by_email(user_email) + expect(staged_account).to be + expect(staged_account.staged).to be(true) + expect(staged_account.posts.order(id: :desc).limit(1).pluck(:raw).first).to eq(body) + end + end + + describe "processes an unknown email sender to group" do + let(:incoming_email) { "foo@bar.com" } + let(:user_email) { "#{SecureRandom.hex(32)}@foobar.com" } + let(:body) { "This is a message to\n\na group ;)" } + + before do + SiteSetting.email_in = true + SiteSetting.allow_staged_accounts = true + end + + it "creates a message for allowed group" do + Fabricate(:group, incoming_email: incoming_email) + process_email(from: user_email, to: incoming_email, body: body) + + staged_account = User.find_by_email(user_email) + expect(staged_account).to be + expect(staged_account.staged).to be(true) + + post = staged_account.posts.order(id: :desc).first + expect(post).to be + expect(post.raw).to eq(body) + expect(post.topic.private_message?).to eq(true) end end