From 5f7bef6d20bff8623a76cdba2eb7e4d4f9d8f9dc Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 6 May 2021 12:59:52 +1000 Subject: [PATCH] FEATURE: Add email_encoded parameter to accept inbound base64 encoded emails (#12947) We have found when receiving and posting inbound emails to the handle_mail route, it is better to POST the payload as a base64 encoded string to avoid strange encoding issues. This introduces a new param of `email_encoded` and maintains the legacy param of email, showing a deprecation warning. Eventually the old param of `email` will be dropped and the new one `email_encoded` will be the only way to handle_mail. --- app/controllers/admin/email_controller.rb | 24 ++++++++++++++--- lib/auth/default_current_user_provider.rb | 2 +- spec/requests/admin/email_controller_spec.rb | 28 +++++++++++++++++--- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/app/controllers/admin/email_controller.rb b/app/controllers/admin/email_controller.rb index f4662fa724e..6edc51070bd 100644 --- a/app/controllers/admin/email_controller.rb +++ b/app/controllers/admin/email_controller.rb @@ -143,14 +143,24 @@ class Admin::EmailController < Admin::AdminController end def handle_mail - params.require(:email) + deprecated_email_param_used = false + + if params[:email_encoded].present? + email_raw = Base64.strict_decode64(params[:email_encoded]) + elsif params[:email].present? + deprecated_email_param_used = true + email_raw = params[:email] + else + raise ActionController::ParameterMissing.new("email_encoded or email") + end + retry_count = 0 begin - Jobs.enqueue(:process_email, mail: params[:email], retry_on_rate_limit: true, source: :handle_mail) + Jobs.enqueue(:process_email, mail: email_raw, retry_on_rate_limit: true, source: :handle_mail) rescue JSON::GeneratorError => e if retry_count == 0 - params[:email] = params[:email].force_encoding('iso-8859-1').encode("UTF-8") + email_raw = email_raw.force_encoding('iso-8859-1').encode("UTF-8") retry_count += 1 retry else @@ -158,7 +168,13 @@ class Admin::EmailController < Admin::AdminController end end - render plain: "email has been received and is queued for processing" + # TODO: 2022-05-01 Remove this route once all sites have migrated over + # to using the new email_encoded param. + if deprecated_email_param_used + render plain: "warning: the email parameter is deprecated. all POST requests to this route should be sent with a base64 strict encoded encoded_email parameter instead. email has been received and is queued for processing" + else + render plain: "email has been received and is queued for processing" + end end def raw_email diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index 884b000d4c8..2fe829c6cdf 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -49,7 +49,7 @@ class Auth::DefaultCurrentUserProvider methods: :post, actions: "admin/email#handle_mail", formats: nil - ) + ), ] # do all current user initialization here diff --git a/spec/requests/admin/email_controller_spec.rb b/spec/requests/admin/email_controller_spec.rb index eb6d23fd067..817c5c5f289 100644 --- a/spec/requests/admin/email_controller_spec.rb +++ b/spec/requests/admin/email_controller_spec.rb @@ -202,10 +202,32 @@ describe Admin::EmailController do end describe '#handle_mail' do - it 'should enqueue the right job' do - expect { post "/admin/email/handle_mail.json", params: { email: email('cc') } } - .to change { Jobs::ProcessEmail.jobs.count }.by(1) + it "returns a bad request if neither email parameter is present" do + post "/admin/email/handle_mail.json" + expect(response.status).to eq(400) + expect(response.body).to include("param is missing") + end + + it 'should enqueue the right job, and show a deprecation warning (email_encoded param should be used)' do + expect_enqueued_with( + job: :process_email, + args: { mail: email('cc'), retry_on_rate_limit: true, source: :handle_mail } + ) do + post "/admin/email/handle_mail.json", params: { email: email('cc') } + end expect(response.status).to eq(200) + expect(response.body).to eq("warning: the email parameter is deprecated. all POST requests to this route should be sent with a base64 strict encoded encoded_email parameter instead. email has been received and is queued for processing") + end + + it 'should enqueue the right job, decoding the raw email param' do + expect_enqueued_with( + job: :process_email, + args: { mail: email('cc'), retry_on_rate_limit: true, source: :handle_mail } + ) do + post "/admin/email/handle_mail.json", params: { email_encoded: Base64.strict_encode64(email('cc')) } + end + expect(response.status).to eq(200) + expect(response.body).to eq("email has been received and is queued for processing") end end