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.
This commit is contained in:
Martin Brennan 2021-05-06 12:59:52 +10:00 committed by GitHub
parent c697efc186
commit 5f7bef6d20
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 46 additions and 8 deletions

View File

@ -143,14 +143,24 @@ class Admin::EmailController < Admin::AdminController
end end
def handle_mail 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 retry_count = 0
begin 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 rescue JSON::GeneratorError => e
if retry_count == 0 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_count += 1
retry retry
else else
@ -158,7 +168,13 @@ class Admin::EmailController < Admin::AdminController
end end
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 end
def raw_email def raw_email

View File

@ -49,7 +49,7 @@ class Auth::DefaultCurrentUserProvider
methods: :post, methods: :post,
actions: "admin/email#handle_mail", actions: "admin/email#handle_mail",
formats: nil formats: nil
) ),
] ]
# do all current user initialization here # do all current user initialization here

View File

@ -202,10 +202,32 @@ describe Admin::EmailController do
end end
describe '#handle_mail' do describe '#handle_mail' do
it 'should enqueue the right job' do it "returns a bad request if neither email parameter is present" do
expect { post "/admin/email/handle_mail.json", params: { email: email('cc') } } post "/admin/email/handle_mail.json"
.to change { Jobs::ProcessEmail.jobs.count }.by(1) 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.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
end end