diff --git a/app/controllers/legacy_api/send_controller.rb b/app/controllers/legacy_api/send_controller.rb index 48adedf09..05fa489f2 100644 --- a/app/controllers/legacy_api/send_controller.rb +++ b/app/controllers/legacy_api/send_controller.rb @@ -12,7 +12,8 @@ class SendController < BaseController "FromAddressMissing" => "The From address is missing and is required", "UnauthenticatedFromAddress" => "The From address is not authorised to send mail from this server", "AttachmentMissingName" => "An attachment is missing a name", - "AttachmentMissingData" => "An attachment is missing data" + "AttachmentMissingData" => "An attachment is missing data", + "InvalidMessageID" => "Message-ID must be in RFC 5322 format: local-part@domain" }.freeze # Send a message with the given options @@ -32,6 +33,8 @@ class SendController < BaseController # custom_headers => A hash of custom headers # attachments => An array of attachments # (name, content_type and data (base64)) + # message_ids => A hash of recipient emails to Message-IDs + # (enables idempotency) # # Response: A array of hashes containing message information # OR an error if there is an issue sending the message @@ -50,6 +53,7 @@ def message attributes[:bounce] = api_params["bounce"] ? true : false attributes[:tag] = api_params["tag"] attributes[:custom_headers] = api_params["headers"] if api_params["headers"] + attributes[:message_ids] = api_params["message_ids"] if api_params["message_ids"].is_a?(Hash) attributes[:attachments] = [] (api_params["attachments"] || []).each do |attachment| @@ -112,8 +116,23 @@ def raw # Store the result ready to return result = { message_id: nil, messages: {} } + + # Extract Message-ID from raw message for duplicate detection + mail_for_message_id = Mail.new(raw_message) + extracted_message_id = mail_for_message_id.message_id&.gsub(/^<|>$/, '') + if api_params["rcpt_to"].is_a?(Array) api_params["rcpt_to"].uniq.each do |rcpt_to| + # Check for duplicate if Message-ID is present + if extracted_message_id.present? + existing = @current_credential.server.message_db.select(:messages, fields: [:id, :token, :message_id], where: { message_id: extracted_message_id, rcpt_to: rcpt_to }).first + if existing + result[:message_id] = existing["message_id"] if result[:message_id].nil? + result[:messages][rcpt_to] = { id: existing["id"], token: existing["token"], message_id: existing["message_id"], existing: true } + next + end + end + message = @current_credential.server.message_db.new_message message.rcpt_to = rcpt_to message.mail_from = api_params["mail_from"] @@ -125,11 +144,11 @@ def raw message.bounce = api_params["bounce"] ? true : false message.save result[:message_id] = message.message_id if result[:message_id].nil? - result[:messages][rcpt_to] = { id: message.id, token: message.token } + result[:messages][rcpt_to] = { id: message.id, token: message.token, message_id: message.message_id } end end render_success result end end -end +end \ No newline at end of file diff --git a/app/models/outgoing_message_prototype.rb b/app/models/outgoing_message_prototype.rb index 8584b72c5..27ea2b7e4 100644 --- a/app/models/outgoing_message_prototype.rb +++ b/app/models/outgoing_message_prototype.rb @@ -18,6 +18,7 @@ class OutgoingMessagePrototype attr_accessor :tag attr_accessor :credential attr_accessor :bounce + attr_accessor :message_ids def initialize(server, ip, source_type, attributes) @server = server @@ -76,7 +77,19 @@ def create_messages if valid? all_addresses.each_with_object({}) do |address, hash| if address = Postal::Helpers.strip_name_from_address(address) - hash[address] = create_message(address) + # Check for existing message if message_ids provided + message_id = @message_ids.is_a?(Hash) ? @message_ids[address] : nil + if message_id + # Strip angle brackets if present + message_id = message_id.gsub(/^<|>$/, '') + # Check for duplicate + existing = @server.message_db.select(:messages, fields: [:id, :token, :message_id], where: { message_id: message_id }).first + if existing + hash[address] = { id: existing["id"], token: existing["token"], message_id: existing["message_id"], existing: true } + next + end + end + hash[address] = create_message(address, message_id) end end else @@ -105,6 +118,14 @@ def attachments end # rubocop:enable Lint/DuplicateMethods + def valid_message_id_format?(message_id) + return false if message_id.blank? + # Strip angle brackets if present for validation + id = message_id.gsub(/^<|>$/, '') + # RFC 5322: local-part@domain (must have @ and both parts non-empty, no spaces) + id.match?(/\A[^@\s]+@[^@\s]+\z/) + end + def validate @errors = [] @@ -145,57 +166,76 @@ def validate end end end + + if @message_ids.is_a?(Hash) + @message_ids.each do |recipient, message_id| + unless valid_message_id_format?(message_id) + @errors << "InvalidMessageID" unless @errors.include?("InvalidMessageID") + break + end + end + end @errors end def raw_message - @raw_message ||= begin - mail = Mail.new - if @custom_headers.is_a?(Hash) - @custom_headers.each { |key, value| mail[key.to_s] = value.to_s } - end - mail.to = to_addresses.join(", ") if to_addresses.present? - mail.cc = cc_addresses.join(", ") if cc_addresses.present? - mail.from = @from - mail.sender = @sender - mail.subject = @subject - mail.reply_to = @reply_to - mail.part content_type: "multipart/alternative" do |p| - if @plain_body.present? - p.text_part = Mail::Part.new - p.text_part.body = @plain_body - end - if @html_body.present? - p.html_part = Mail::Part.new - p.html_part.content_type = "text/html; charset=UTF-8" - p.html_part.body = @html_body - end + @raw_message ||= raw_message_for_recipient(nil, @message_id) + end + + def raw_message_for_recipient(address, message_id) + mail = Mail.new + if @custom_headers.is_a?(Hash) + @custom_headers.each { |key, value| mail[key.to_s] = value.to_s } + end + mail.to = to_addresses.join(", ") if to_addresses.present? + mail.cc = cc_addresses.join(", ") if cc_addresses.present? + mail.from = @from + mail.sender = @sender + mail.subject = @subject + mail.reply_to = @reply_to + mail.part content_type: "multipart/alternative" do |p| + if @plain_body.present? + p.text_part = Mail::Part.new + p.text_part.body = @plain_body end - attachments.each do |attachment| - mail.attachments[attachment[:name]] = { - mime_type: attachment[:content_type], - content: attachment[:data] - } + if @html_body.present? + p.html_part = Mail::Part.new + p.html_part.content_type = "text/html; charset=UTF-8" + p.html_part.body = @html_body end - mail.header["Received"] = ReceivedHeader.generate(@server, @source_type, @ip, :http) - mail.message_id = "<#{@message_id}>" - mail.to_s end + attachments.each do |attachment| + mail.attachments[attachment[:name]] = { + mime_type: attachment[:content_type], + content: attachment[:data] + } + end + mail.header["Received"] = ReceivedHeader.generate(@server, @source_type, @ip, :http) + mail.message_id = "<#{message_id}>" + mail.to_s end - def create_message(address) + def create_message(address, message_id = nil) + # Use provided message_id or generate one + msg_id = message_id || "#{SecureRandom.uuid}@#{Postal::Config.dns.return_path_domain}" + message = @server.message_db.new_message message.scope = "outgoing" message.rcpt_to = address message.mail_from = from_address message.domain_id = domain.id - message.raw_message = raw_message + message.raw_message = raw_message_for_recipient(address, msg_id) message.tag = tag message.credential_id = credential&.id message.received_with_ssl = true message.bounce = @bounce message.save - { id: message.id, token: message.token } + # Include message_id in response only if message_ids parameter was provided + if @message_ids.is_a?(Hash) + { id: message.id, token: message.token, message_id: message.message_id } + else + { id: message.id, token: message.token } + end end end diff --git a/spec/apis/legacy_api/send/idempotency_spec.rb b/spec/apis/legacy_api/send/idempotency_spec.rb new file mode 100644 index 000000000..54f320715 --- /dev/null +++ b/spec/apis/legacy_api/send/idempotency_spec.rb @@ -0,0 +1,400 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe "Legacy Send API - Idempotency", type: :request do + let(:server) { create(:server) } + let(:credential) { create(:credential, server: server) } + let(:domain) { create(:domain, owner: server) } + + describe "/api/v1/send/message with message_ids" do + let(:default_params) do + { + to: ["test1@example.com", "test2@example.com"], + from: "test@#{domain.name}", + subject: "Test", + plain_body: "Test body" + } + end + + context "when message_ids are provided" do + let(:message_id_1) { "unique-id-1@example.com" } + let(:message_id_2) { "unique-id-2@example.com" } + let(:params) do + default_params.merge( + message_ids: { + "test1@example.com" => message_id_1, + "test2@example.com" => message_id_2 + } + ) + end + + it "creates messages with the provided Message-IDs" do + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params.to_json + + parsed_body = JSON.parse(response.body) + expect(parsed_body["status"]).to eq "success" + + message1 = server.message(parsed_body["data"]["messages"]["test1@example.com"]["id"]) + message2 = server.message(parsed_body["data"]["messages"]["test2@example.com"]["id"]) + + expect(message1.message_id).to eq message_id_1 + expect(message2.message_id).to eq message_id_2 + end + + it "includes message_id in response" do + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params.to_json + + parsed_body = JSON.parse(response.body) + expect(parsed_body["data"]["messages"]["test1@example.com"]["message_id"]).to eq message_id_1 + expect(parsed_body["data"]["messages"]["test2@example.com"]["message_id"]).to eq message_id_2 + end + + context "when Message-IDs have angle brackets" do + let(:params) do + default_params.merge( + message_ids: { + "test1@example.com" => "<#{message_id_1}>", + "test2@example.com" => "<#{message_id_2}>" + } + ) + end + + it "strips angle brackets and stores without them" do + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params.to_json + + parsed_body = JSON.parse(response.body) + message1 = server.message(parsed_body["data"]["messages"]["test1@example.com"]["id"]) + message2 = server.message(parsed_body["data"]["messages"]["test2@example.com"]["id"]) + + expect(message1.message_id).to eq message_id_1 + expect(message2.message_id).to eq message_id_2 + end + end + + context "when a duplicate Message-ID is sent" do + before do + # First request + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params.to_json + end + + it "returns the existing message" do + # Second request with same Message-IDs + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params.to_json + + parsed_body = JSON.parse(response.body) + expect(parsed_body["status"]).to eq "success" + expect(parsed_body["data"]["messages"]["test1@example.com"]["existing"]).to eq true + expect(parsed_body["data"]["messages"]["test2@example.com"]["existing"]).to eq true + end + + it "does not create a new message" do + initial_count = server.message_db.messages.count + + # Second request + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params.to_json + + expect(server.message_db.messages.count).to eq initial_count + end + + it "returns the same message ID and token" do + first_response = JSON.parse(response.body) + first_id_1 = first_response["data"]["messages"]["test1@example.com"]["id"] + first_token_1 = first_response["data"]["messages"]["test1@example.com"]["token"] + + # Second request + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params.to_json + + second_response = JSON.parse(response.body) + expect(second_response["data"]["messages"]["test1@example.com"]["id"]).to eq first_id_1 + expect(second_response["data"]["messages"]["test1@example.com"]["token"]).to eq first_token_1 + end + end + + context "when partial duplicate (one recipient duplicate, one new)" do + let(:params) do + default_params.merge( + message_ids: { + "test1@example.com" => message_id_1, + "test2@example.com" => message_id_2 + } + ) + end + + before do + # First request with only test1 + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: default_params.merge( + to: ["test1@example.com"], + message_ids: { "test1@example.com" => message_id_1 } + ).to_json + end + + it "returns existing for test1 and creates new for test2" do + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params.to_json + + parsed_body = JSON.parse(response.body) + expect(parsed_body["data"]["messages"]["test1@example.com"]["existing"]).to eq true + expect(parsed_body["data"]["messages"]["test2@example.com"]["existing"]).to be_nil + end + end + end + + context "when message_ids are not provided" do + it "generates unique Message-IDs for each recipient" do + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: default_params.to_json + + parsed_body = JSON.parse(response.body) + message1 = server.message(parsed_body["data"]["messages"]["test1@example.com"]["id"]) + message2 = server.message(parsed_body["data"]["messages"]["test2@example.com"]["id"]) + + expect(message1.message_id).to be_a String + expect(message2.message_id).to be_a String + expect(message1.message_id).not_to eq message2.message_id + end + end + + context "when message_ids is provided but not a hash" do + let(:params) { default_params.merge(message_ids: "not-a-hash") } + + it "ignores the parameter and generates Message-IDs" do + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params.to_json + + parsed_body = JSON.parse(response.body) + expect(parsed_body["status"]).to eq "success" + message1 = server.message(parsed_body["data"]["messages"]["test1@example.com"]["id"]) + expect(message1.message_id).to match(/\A[a-f0-9-]+@/) + end + end + + context "when message_ids have invalid format" do + let(:params) do + default_params.merge( + message_ids: { + "test1@example.com" => "invalid-no-domain", + "test2@example.com" => "valid-id@example.com" + } + ) + end + + it "returns an error" do + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params.to_json + + parsed_body = JSON.parse(response.body) + expect(parsed_body["status"]).to eq "error" + expect(parsed_body["data"]["code"]).to eq "InvalidMessageID" + end + end + + context "when message_ids have missing local part" do + let(:params) do + default_params.merge( + message_ids: { + "test1@example.com" => "@example.com" + } + ) + end + + it "returns an error" do + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params.to_json + + parsed_body = JSON.parse(response.body) + expect(parsed_body["status"]).to eq "error" + expect(parsed_body["data"]["code"]).to eq "InvalidMessageID" + end + end + + context "when message_ids have spaces" do + let(:params) do + default_params.merge( + message_ids: { + "test1@example.com" => "id with spaces@example.com" + } + ) + end + + it "returns an error" do + post "/api/v1/send/message", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params.to_json + + parsed_body = JSON.parse(response.body) + expect(parsed_body["status"]).to eq "error" + expect(parsed_body["data"]["code"]).to eq "InvalidMessageID" + end + end + end + + describe "/api/v1/send/raw with Message-ID in headers" do + let(:raw_email) do + "Message-ID: \r\n" \ + "From: test@#{domain.name}\r\n" \ + "To: recipient@example.com\r\n" \ + "Subject: Test\r\n" \ + "\r\n" \ + "Test body" + end + + let(:default_params) do + { + rcpt_to: ["test1@example.com", "test2@example.com"], + mail_from: "test@#{domain.name}", + data: Base64.encode64(raw_email) + } + end + + context "when Message-ID is present in raw email" do + it "extracts and uses the Message-ID" do + post "/api/v1/send/raw", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: default_params.to_json + + parsed_body = JSON.parse(response.body) + expect(parsed_body["status"]).to eq "success" + + message1 = server.message(parsed_body["data"]["messages"]["test1@example.com"]["id"]) + message2 = server.message(parsed_body["data"]["messages"]["test2@example.com"]["id"]) + + expect(message1.message_id).to eq "raw-test-id@example.com" + expect(message2.message_id).to eq "raw-test-id@example.com" + end + + it "includes message_id in response" do + post "/api/v1/send/raw", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: default_params.to_json + + parsed_body = JSON.parse(response.body) + expect(parsed_body["data"]["messages"]["test1@example.com"]["message_id"]).to eq "raw-test-id@example.com" + expect(parsed_body["data"]["messages"]["test2@example.com"]["message_id"]).to eq "raw-test-id@example.com" + end + + context "when duplicate Message-ID is sent" do + before do + # First request + post "/api/v1/send/raw", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: default_params.to_json + end + + it "returns existing messages for all recipients" do + # Second request with same Message-ID + post "/api/v1/send/raw", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: default_params.to_json + + parsed_body = JSON.parse(response.body) + expect(parsed_body["status"]).to eq "success" + expect(parsed_body["data"]["messages"]["test1@example.com"]["existing"]).to eq true + expect(parsed_body["data"]["messages"]["test2@example.com"]["existing"]).to eq true + end + + it "does not create new messages" do + initial_count = server.message_db.messages.count + + # Second request + post "/api/v1/send/raw", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: default_params.to_json + + expect(server.message_db.messages.count).to eq initial_count + end + + it "returns the same message IDs and tokens" do + first_response = JSON.parse(response.body) + first_id_1 = first_response["data"]["messages"]["test1@example.com"]["id"] + first_token_1 = first_response["data"]["messages"]["test1@example.com"]["token"] + + # Second request + post "/api/v1/send/raw", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: default_params.to_json + + second_response = JSON.parse(response.body) + expect(second_response["data"]["messages"]["test1@example.com"]["id"]).to eq first_id_1 + expect(second_response["data"]["messages"]["test1@example.com"]["token"]).to eq first_token_1 + end + end + + context "when partial duplicate (one recipient already exists)" do + before do + # First request with only test1 + post "/api/v1/send/raw", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: default_params.merge(rcpt_to: ["test1@example.com"]).to_json + end + + it "returns existing for test1 and creates new for test2" do + post "/api/v1/send/raw", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: default_params.to_json + + parsed_body = JSON.parse(response.body) + expect(parsed_body["data"]["messages"]["test1@example.com"]["existing"]).to eq true + expect(parsed_body["data"]["messages"]["test2@example.com"]["existing"]).to be_nil + end + + it "only creates one new message" do + initial_count = server.message_db.messages.count + + post "/api/v1/send/raw", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: default_params.to_json + + expect(server.message_db.messages.count).to eq initial_count + 1 + end + end + + context "when Message-ID has angle brackets" do + let(:raw_email_with_brackets) do + "Message-ID: <>\r\n" \ + "From: test@#{domain.name}\r\n" \ + "To: recipient@example.com\r\n" \ + "Subject: Test\r\n" \ + "\r\n" \ + "Test body" + end + + let(:params_with_brackets) do + default_params.merge(data: Base64.encode64(raw_email_with_brackets)) + end + + it "strips angle brackets before storage" do + post "/api/v1/send/raw", + headers: { "x-server-api-key" => credential.key, "content-type" => "application/json" }, + params: params_with_brackets.to_json + + parsed_body = JSON.parse(response.body) + message = server.message(parsed_body["data"]["messages"]["test1@example.com"]["id"]) + + expect(message.message_id).to eq "raw-bracketed-id@example.com" + end + end + end + end +end diff --git a/spec/apis/legacy_api/send/raw_spec.rb b/spec/apis/legacy_api/send/raw_spec.rb index e63370f0f..8633ab978 100644 --- a/spec/apis/legacy_api/send/raw_spec.rb +++ b/spec/apis/legacy_api/send/raw_spec.rb @@ -117,8 +117,8 @@ expect(parsed_body["data"]["message_id"]).to be_a String expect(parsed_body["data"]["messages"]).to be_a Hash expect(parsed_body["data"]["messages"]).to match({ - "test1@example.com" => { "id" => kind_of(Integer), "token" => /\A[a-zA-Z0-9]{16}\z/ }, - "test2@example.com" => { "id" => kind_of(Integer), "token" => /\A[a-zA-Z0-9]{16}\z/ } + "test1@example.com" => { "id" => kind_of(Integer), "token" => /\A[a-zA-Z0-9]{16}\z/, "message_id" => kind_of(String) }, + "test2@example.com" => { "id" => kind_of(Integer), "token" => /\A[a-zA-Z0-9]{16}\z/, "message_id" => kind_of(String) } }) end @@ -157,8 +157,8 @@ expect(parsed_body["data"]["message_id"]).to be_a String expect(parsed_body["data"]["messages"]).to be_a Hash expect(parsed_body["data"]["messages"]).to match({ - "test1@example.com" => { "id" => kind_of(Integer), "token" => /\A[a-zA-Z0-9]{16}\z/ }, - "test2@example.com" => { "id" => kind_of(Integer), "token" => /\A[a-zA-Z0-9]{16}\z/ } + "test1@example.com" => { "id" => kind_of(Integer), "token" => /\A[a-zA-Z0-9]{16}\z/, "message_id" => kind_of(String) }, + "test2@example.com" => { "id" => kind_of(Integer), "token" => /\A[a-zA-Z0-9]{16}\z/, "message_id" => kind_of(String) } }) end end