diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index 7faf2cfb9..6226461f1 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -8,14 +8,14 @@ require_dependency 'email/message_builder' module Jobs class PollMailbox < Jobs::Scheduled - every SiteSetting.pop3s_polling_period_mins.minutes + every SiteSetting.pop3_polling_period_mins.minutes sidekiq_options retry: false include Email::BuildEmailHelper def execute(args) @args = args - if SiteSetting.pop3s_polling_enabled? - poll_pop3s + if SiteSetting.pop3_polling_enabled? + poll_pop3 end end @@ -72,14 +72,11 @@ module Jobs end end - def poll_pop3s - if !SiteSetting.pop3s_polling_insecure - Net::POP3.enable_ssl(OpenSSL::SSL::VERIFY_NONE) - end - Net::POP3.start(SiteSetting.pop3s_polling_host, - SiteSetting.pop3s_polling_port, - SiteSetting.pop3s_polling_username, - SiteSetting.pop3s_polling_password) do |pop| + def poll_pop3 + connection = Net::POP3.new(SiteSetting.pop3_polling_host, SiteSetting.pop3_polling_port) + connection.enable_ssl if SiteSetting.pop3_polling_ssl + + connection.start(SiteSetting.pop3_polling_username, SiteSetting.pop3_polling_password) do |pop| unless pop.mails.empty? pop.each do |mail| handle_mail(mail) diff --git a/config/application.rb b/config/application.rb index 357621b23..6ec3c0bc5 100644 --- a/config/application.rb +++ b/config/application.rb @@ -91,7 +91,7 @@ module Discourse # Configure sensitive parameters which will be filtered from the log file. config.filter_parameters += [ :password, - :pop3s_polling_password, + :pop3_polling_password, :s3_secret_access_key, :twitter_consumer_secret, :facebook_app_secret, diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index d2d9d40fc..34b5019e9 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -926,13 +926,13 @@ en: disable_emails: "Prevent Discourse from sending any kind of emails" - pop3s_polling_enabled: "Poll via POP3S for email replies." - pop3s_polling_insecure: "Poll using plain text POP3 without SSL." - pop3s_polling_period_mins: "The period in minutes between checking the POP3S account for email. NOTE: requires restart." - pop3s_polling_port: "The port to poll a POP3S account on." - 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." + pop3_polling_enabled: "Poll via POP3 for email replies." + pop3_polling_ssl: "Use SSL while connecting to the POP3 server. (Recommended)" + pop3_polling_period_mins: "The period in minutes between checking the POP3 account for email. NOTE: requires restart." + pop3_polling_port: "The port to poll a POP3 account on." + pop3_polling_host: "The host to poll for email via POP3." + pop3_polling_username: "The username for the POP3 account to poll for email." + pop3_polling_password: "The password for the POP3 account to poll for email." 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_prefix: "The [label] used in the subject of emails. It will default to 'title' if not set." diff --git a/config/site_settings.yml b/config/site_settings.yml index 2d7546429..a3e3c02b0 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -385,13 +385,13 @@ email: email_custom_headers: 'Auto-Submitted: auto-generated' reply_by_email_enabled: false reply_by_email_address: '' - pop3s_polling_enabled: false - pop3s_polling_insecure: false - pop3s_polling_period_mins: 5 - pop3s_polling_host: '' - pop3s_polling_port: 995 - pop3s_polling_username: '' - pop3s_polling_password: '' + pop3_polling_enabled: false + pop3_polling_ssl: true + pop3_polling_period_mins: 5 + pop3_polling_host: '' + pop3_polling_port: 995 + pop3_polling_username: '' + pop3_polling_password: '' email_in: default: false client: true diff --git a/db/migrate/20140826234625_rename_settings_pop3s_to_pop3.rb b/db/migrate/20140826234625_rename_settings_pop3s_to_pop3.rb new file mode 100644 index 000000000..0672e5168 --- /dev/null +++ b/db/migrate/20140826234625_rename_settings_pop3s_to_pop3.rb @@ -0,0 +1,9 @@ +class RenameSettingsPop3sToPop3 < ActiveRecord::Migration + def up + execute "UPDATE site_settings SET name = replace(name, 'pop3s', 'pop3') WHERE name ILIKE 'pop3%'" + end + + def down + execute "UPDATE site_settings SET name = replace(name, 'pop3', 'pop3s') WHERE name ILIKE 'pop3%'" + end +end diff --git a/spec/jobs/poll_mailbox_spec.rb b/spec/jobs/poll_mailbox_spec.rb index 3084b2b2f..4af0c8d75 100644 --- a/spec/jobs/poll_mailbox_spec.rb +++ b/spec/jobs/poll_mailbox_spec.rb @@ -7,18 +7,18 @@ describe Jobs::PollMailbox do describe ".execute" do - it "does no polling if pop3s_polling_enabled is false" do - SiteSetting.expects(:pop3s_polling_enabled?).returns(false) - poller.expects(:poll_pop3s).never + it "does no polling if pop3_polling_enabled is false" do + SiteSetting.expects(:pop3_polling_enabled?).returns(false) + poller.expects(:poll_pop3).never poller.execute({}) end - describe "with pop3s_polling_enabled" do + describe "with pop3_polling_enabled" do - it "calls poll_pop3s" do - SiteSetting.expects(:pop3s_polling_enabled?).returns(true) - poller.expects(:poll_pop3s).once + it "calls poll_pop3" do + SiteSetting.expects(:pop3_polling_enabled?).returns(true) + poller.expects(:poll_pop3).once poller.execute({}) end @@ -26,19 +26,34 @@ describe Jobs::PollMailbox do end - describe ".poll_pop3s" do + describe ".poll_pop3" do it "logs an error on pop authentication error" do error = Net::POPAuthenticationError.new data = { limit_once_per: 1.hour, message_params: { error: error }} - Net::POP3.expects(:start).raises(error) + Net::POP3.any_instance.expects(:start).raises(error) Discourse.expects(:handle_exception) - poller.poll_pop3s + poller.poll_pop3 end + it "calls enable_ssl when the setting is enabled" do + SiteSetting.pop3_polling_ssl = true + Net::POP3.any_instance.stubs(:start) + Net::POP3.any_instance.expects(:enable_ssl) + + poller.poll_pop3 + end + + it "does not call enable_ssl when the setting is off" do + SiteSetting.pop3_polling_ssl = false + Net::POP3.any_instance.stubs(:start) + Net::POP3.any_instance.expects(:enable_ssl).never + + poller.poll_pop3 + end end # Testing mock for the email objects that you get