Générisation des adresses magiques de réponse aux autres instances et…

… environnements (#4807)

* refactor TransferEmailReplyJob to encapsulate perform specs

* Générisation des adresses magiques de réponse aux autres instances et environnements

* rename SENDINBLUE_INBOUND_PASSWORD env var to BREVO

* ajout d’une route /inbound_emails/brevo pour déprécier celle en sendinblue

app/controllers/inbound_emails_controller.rb

@@ -4,19 +4,19 @@
 class InboundEmailsController < ActionController::Base
   skip_before_action :verify_authenticity_token
 
-  before_action :authenticate_sendinblue
+  before_action :authenticate_brevo
 
-  def sendinblue
+  def brevo
     payload = request.params["items"].first
     TransferEmailReplyJob.perform_later(payload)
   end
 
   private
 
-  def authenticate_sendinblue
-    return if ActiveSupport::SecurityUtils.secure_compare(ENV["SENDINBLUE_INBOUND_PASSWORD"], params[:password])
+  def authenticate_brevo
+    return if ActiveSupport::SecurityUtils.secure_compare(ENV["BREVO_INBOUND_PASSWORD"], params[:password])
 
-    Sentry.capture_message("Sendinblue inbound controller was called without valid password", fingerprint: ["sib_inbound_pw_invalid"])
+    Sentry.capture_message("Brevo inbound controller was called without valid password", fingerprint: ["brevo_inbound_pw_invalid"])
     head :unauthorized
   end
 end

app/jobs/transfer_email_reply_job.rb

@@ -7,10 +7,16 @@ class TransferEmailReplyJob < ApplicationJob
   self.log_arguments = false
 
   def self.reply_address_for_rdv(rdv)
-    "rdv+#{rdv.uuid}@reply.rdv-solidarites.fr"
+    return nil if rdv.domain.reply_host_name.nil?
+
+    "rdv+#{rdv.uuid}@#{rdv.domain.reply_host_name}"
   end
 
-  UUID_EXTRACTOR = /rdv\+([a-f0-9\-]*)@reply\.rdv-solidarites\.fr/
+  UUID_EXTRACTOR = /rdv\+([a-f0-9\-]*)@reply\.[a-z\-\.]+$/
+
+  def self.uuid_from_email_address(email_address)
+    email_address.match(UUID_EXTRACTOR)&.captures&.first
+  end
 
   def perform(sendinblue_hash)
     @sendinblue_hash = sendinblue_hash.with_indifferent_access
@@ -50,7 +56,7 @@ def user
   end
 
   def uuid
-    source_mail.to.first.match(UUID_EXTRACTOR)&.captures&.first
+    self.class.uuid_from_email_address(source_mail.to.first)
   end
 
   def extracted_response

app/models/domain.rb

@@ -109,8 +109,8 @@ def host_name
         URI.parse(ENV.fetch("HOST", nil)).host
       elsif ENV["RDV_SOLIDARITES_INSTANCE_NAME"] == "STAGING"
         {
-          RDV_SOLIDARITES => "staging.rdv-solidarites.fr",
-          RDV_AIDE_NUMERIQUE => "staging.rdv-aide-numerique.fr",
+          RDV_SOLIDARITES => "staging.rdv-solidarites.fr", # sous-domaine pas configuré
+          RDV_AIDE_NUMERIQUE => "staging.rdv-aide-numerique.fr", # sous-domaine pas configuré
           RDV_MAIRIE => "staging.rdv-service-public.fr",
         }.fetch(self)
       elsif ENV["RDV_SOLIDARITES_INSTANCE_NAME"] == "DEMO"
@@ -143,6 +143,46 @@ def host_name
     end
   end
 
+  def reply_host_name
+    case Rails.env.to_sym
+    when :production
+      if ENV["IS_REVIEW_APP"] == "true"
+        nil
+      elsif ENV["RDV_SOLIDARITES_INSTANCE_NAME"] == "STAGING"
+        {
+          RDV_MAIRIE => "reply.staging.rdv-service-public.fr",
+          # c’est le seul staging réellement ouvert pour l’instant
+        }.fetch(self, nil)
+      elsif ENV["RDV_SOLIDARITES_INSTANCE_NAME"] == "DEMO"
+        {
+          RDV_SOLIDARITES => "reply.demo.rdv-solidarites.fr",
+          RDV_AIDE_NUMERIQUE => "reply.demo.rdv-aide-numerique.fr",
+          RDV_MAIRIE => "reply.demo.rdv-service-public.fr",
+        }.fetch(self)
+      else
+        {
+          RDV_SOLIDARITES => "reply.rdv-solidarites.fr",
+          RDV_AIDE_NUMERIQUE => "reply.rdv-aide-numerique.fr",
+          RDV_MAIRIE => "reply.rdv-service-public.fr", # TODO: remplacer par anct.gouv.fr une fois les DNS déployés
+        }.fetch(self)
+      end
+    when :development
+      {
+        RDV_SOLIDARITES => "reply.rdv-solidarites.localhost",
+        RDV_AIDE_NUMERIQUE => "reply.rdv-aide-numerique.localhost",
+        RDV_MAIRIE => "reply.rdv-mairie.localhost",
+      }.fetch(self)
+    when :test
+      {
+        RDV_SOLIDARITES => "reply.rdv-solidarites-test.localhost",
+        RDV_AIDE_NUMERIQUE => "reply.rdv-aide-numerique-test.localhost",
+        RDV_MAIRIE => "reply.rdv-mairie-test.localhost",
+      }.fetch(self)
+    else
+      raise "Rails.env not recognized: #{Rails.env.inspect}"
+    end
+  end
+
   def default?
     self == RDV_MAIRIE
   end

config/routes.rb

@@ -341,7 +341,8 @@ def format_redirect_params(params)
   get "/organisations/*rest", to: redirect("admin/organisations/%{rest}")
   # old agenda rule was bookmarked by some agents
   get "admin/organisations/:organisation_id/agents/:agent_id", to: redirect("/admin/organisations/%{organisation_id}/agent_agendas/%{agent_id}")
-  post "/inbound_emails/sendinblue", controller: :inbound_emails, action: :sendinblue
+  post "/inbound_emails/sendinblue", controller: :inbound_emails, action: :brevo # TODO: supprimer après la transition
+  post "/inbound_emails/brevo", controller: :inbound_emails, action: :brevo
 
   # This route redirects invitations to rdv-insertion so that rdv-insertion
   # can use rdvs domain name in their emails

scalingo.json

@@ -77,7 +77,7 @@
       "value": "change_me_if_needed"
     },
 
-    "SENDINBLUE_INBOUND_PASSWORD": {
+    "BREVO_INBOUND_PASSWORD": {
       "description": "Cette valeur est passée en paramètre dans le webhook que nous avons défini chez SendInBlue. C'est un secret, donc non hérité en review app.",
       "value": "change_me_if_needed"
     },

spec/jobs/transfer_email_reply_job_spec.rb

@@ -1,97 +1,150 @@
 RSpec.describe TransferEmailReplyJob do
-  subject(:perform_job) { described_class.perform_now(sendinblue_payload) }
-
-  before do
-    # Set a fixed date so we can assert on dates within email body
-    travel_to(Time.zone.parse("2022-05-17 16:00:00"))
+  describe "#uuid_from_email_address" do
+    %w[
+      [email protected]
+      [email protected]
+      [email protected]
+      [email protected]
+      [email protected]
+      [email protected]
+      [email protected]
+    ].each do |email_address|
+      it "should extract UUID from #{email_address}" do
+        expect(described_class.uuid_from_email_address(email_address)).to eq("b1a2-3c4f")
+      end
+    end
   end
 
-  let!(:user) { create(:user, email: "[email protected]", first_name: "Bénédicte", last_name: "Ficiaire") }
-  let!(:agent) { create(:agent, email: "[email protected]") }
-  let(:rdv_uuid) { "8fae4d5f-4d63-4f60-b343-854d939881a3" }
-  let!(:rdv) { create(:rdv, users: [user], agents: [agent], uuid: rdv_uuid) }
-
-  let(:sendinblue_valid_payload) do
-    # The usual payload has more info, but I removed non-essential fields for readability.
-    # See: https://developers.sendinblue.com/docs/inbound-parsing-api-1#sample-payload
-    {
-      Cc: [],
-      ReplyTo: nil,
-      Subject: "coucou",
-      Attachments: [],
-      Headers: {
-        "Message-ID": "<[email protected]>",
-        Subject: "coucou",
-        From: "Bénédicte Ficiaire <[email protected]>",
-        To: "rdv+8fae4d5f-4d63-4f60-b343-854d939881a3@reply.rdv-solidarites.fr",
-        Date: "Thu, 12 May 2022 12:22:15 +0200",
-      },
-      ExtractedMarkdownMessage: "Je souhaite annuler mon RDV",
-      ExtractedMarkdownSignature: nil,
-      RawHtmlBody: %(<html dir="ltr"><head></head><body style="text-align:left; direction:ltr;"><div>Je souhaite annuler mon RDV</div>\n</body></html>\n),
-      RawTextBody: "Je souhaite annuler mon RDV\n",
-    }
-  end
-  let(:sendinblue_payload) { sendinblue_valid_payload } # use valid payload by default
-
-  context "when all goes well" do
-    it "sends a notification email to the agent, containing the user reply" do
-      expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
-      transferred_email = ActionMailer::Base.deliveries.last
-      expect(transferred_email.to).to eq(["[email protected]"])
-      expect(transferred_email[:from].to_s).to eq(%("RDV Solidarités" <[email protected]>))
-      expect(transferred_email.html_part.body.to_s).to include("Dans le cadre du RDV du 20 mai, l'usager⋅e Bénédicte FICIAIRE a envoyé")
-      expect(transferred_email.html_part.body.to_s).to include("Je souhaite annuler mon RDV") # reply content
-      expect(transferred_email.html_part.body.to_s).to include(%(href="http://www.rdv-solidarites-test.localhost/admin/organisations/#{rdv.organisation_id}/rdvs/#{rdv.id}))
+  describe "#reply_address_for_rdv" do
+    it "uses rdv domain reply_host_name" do
+      rdv = build(:rdv)
+      domain = instance_double(Domain)
+
+      allow(rdv).to receive(:uuid).and_return("aabb-1122")
+      allow(rdv).to receive(:domain).and_return(domain)
+      allow(domain).to receive(:reply_host_name).and_return "reply.rdv-solidarites.fr"
+
+      expect(described_class.reply_address_for_rdv(rdv)).to eq("[email protected]")
     end
-  end
 
-  context "when reply token does not match any in DB" do
-    let(:rdv_uuid) { "6df62597-632e-4be1-a273-708ab58e4765" }
+    it "returns nil for a rdv whose domain reply_host_name is nil" do
+      rdv = build(:rdv)
+      domain = instance_double(Domain)
 
-    it "sends a notification email to the default mailbox, containing the user reply" do
-      expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
-      transferred_email = ActionMailer::Base.deliveries.last
-      expect(transferred_email.to).to eq(["[email protected]"])
-      expect(transferred_email.from).to eq(["[email protected]"])
-      expect(transferred_email.html_part.body.to_s).to include(%(L'usager⋅e "Bénédicte Ficiaire" &lt;[email protected]&gt; a répondu))
-      expect(transferred_email.html_part.body.to_s).to include("Je souhaite annuler mon RDV") # reply content
+      allow(rdv).to receive(:domain).and_return(domain)
+      allow(domain).to receive(:reply_host_name).and_return nil
+
+      expect(described_class.reply_address_for_rdv(rdv)).to be_nil
     end
   end
 
-  context "when an e-mail address does not match our pattern" do
-    let(:sendinblue_payload) do
-      sendinblue_valid_payload.tap { |hash| hash[:Headers][:To] = "[email protected]" }
+  describe "#perform" do
+    subject(:perform_job) { described_class.perform_now(sendinblue_payload) }
+
+    before do
+      # Set a fixed date so we can assert on dates within email body
+      travel_to(Time.zone.parse("2022-05-17 16:00:00"))
+    end
+
+    let!(:user) { create(:user, email: "[email protected]", first_name: "Bénédicte", last_name: "Ficiaire") }
+    let!(:agent) { create(:agent, email: "[email protected]") }
+    let(:rdv_uuid) { "8fae4d5f-4d63-4f60-b343-854d939881a3" }
+    let!(:rdv) { create(:rdv, users: [user], agents: [agent], uuid: rdv_uuid) }
+
+    let(:sendinblue_valid_payload) do
+      # The usual payload has more info, but I removed non-essential fields for readability.
+      # See: https://developers.sendinblue.com/docs/inbound-parsing-api-1#sample-payload
+      {
+        Cc: [],
+        ReplyTo: nil,
+        Subject: "coucou",
+        Attachments: [],
+        Headers: {
+          "Message-ID": "<[email protected]>",
+          Subject: "coucou",
+          From: "Bénédicte Ficiaire <[email protected]>",
+          To: "rdv+8fae4d5f-4d63-4f60-b343-854d939881a3@reply.rdv-solidarites.fr",
+          Date: "Thu, 12 May 2022 12:22:15 +0200",
+        },
+        ExtractedMarkdownMessage: "Je souhaite annuler mon RDV",
+        ExtractedMarkdownSignature: nil,
+        RawHtmlBody: %(<html dir="ltr"><head></head><body style="text-align:left; direction:ltr;"><div>Je souhaite annuler mon RDV</div>\n</body></html>\n),
+        RawTextBody: "Je souhaite annuler mon RDV\n",
+      }
     end
+    let(:sendinblue_payload) { sendinblue_valid_payload } # use valid payload by default
 
-    it "is forwarded to default mailbox" do
-      expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
-      transferred_email = ActionMailer::Base.deliveries.last
-      expect(transferred_email.to).to eq(["[email protected]"])
-      expect(transferred_email.html_part.body.to_s).to include(%(L'usager⋅e "Bénédicte Ficiaire" &lt;[email protected]&gt; a répondu))
+    context "when all goes well" do
+      it "sends a notification email to the agent, containing the user reply" do
+        expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
+        transferred_email = ActionMailer::Base.deliveries.last
+        expect(transferred_email.to).to eq(["[email protected]"])
+        expect(transferred_email[:from].to_s).to eq(%("RDV Solidarités" <[email protected]>))
+        expect(transferred_email.html_part.body.to_s).to include("Dans le cadre du RDV du 20 mai, l'usager⋅e Bénédicte FICIAIRE a envoyé")
+        expect(transferred_email.html_part.body.to_s).to include("Je souhaite annuler mon RDV") # reply content
+        expect(transferred_email.html_part.body.to_s).to include(%(href="http://www.rdv-solidarites-test.localhost/admin/organisations/#{rdv.organisation_id}/rdvs/#{rdv.id}))
+      end
     end
-  end
 
-  context "when several agents are linked to the RDV" do
-    let!(:other_agent) { create(:agent, email: "[email protected]").tap { |a| rdv.agents << a } }
+    context "when reply token does not match any in DB" do
+      let(:rdv_uuid) { "6df62597-632e-4be1-a273-708ab58e4765" }
 
-    it "sends one email with all agents in the TO: field" do
-      perform_job
-      expect(ActionMailer::Base.deliveries.last.to).to contain_exactly("[email protected]", "[email protected]")
+      it "sends a notification email to the default mailbox, containing the user reply" do
+        expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
+        transferred_email = ActionMailer::Base.deliveries.last
+        expect(transferred_email.to).to eq(["[email protected]"])
+        expect(transferred_email.from).to eq(["[email protected]"])
+        expect(transferred_email.html_part.body.to_s).to include(%(L'usager⋅e "Bénédicte Ficiaire" &lt;[email protected]&gt; a répondu))
+        expect(transferred_email.html_part.body.to_s).to include("Je souhaite annuler mon RDV") # reply content
+      end
+    end
+
+    context "when an e-mail address does not match our pattern" do
+      let(:sendinblue_payload) do
+        sendinblue_valid_payload.tap { |hash| hash[:Headers][:To] = "[email protected]" }
+      end
+
+      it "is forwarded to default mailbox" do
+        expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
+        transferred_email = ActionMailer::Base.deliveries.last
+        expect(transferred_email.to).to eq(["[email protected]"])
+        expect(transferred_email.html_part.body.to_s).to include(%(L'usager⋅e "Bénédicte Ficiaire" &lt;[email protected]&gt; a répondu))
+      end
+    end
+
+    context "when an e-mail address matches our pattern for a demo host" do
+      let(:sendinblue_payload) do
+        sendinblue_valid_payload.tap { |hash| hash[:Headers][:To] = "rdv+8fae4d5f-4d63-4f60-b343-854d939881a3@reply.demo.rdv-solidarites.fr" }
+      end
+
+      it "sends a notification email to the agent" do
+        expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
+        transferred_email = ActionMailer::Base.deliveries.last
+        expect(transferred_email.to).to eq(["[email protected]"])
+      end
     end
-  end
 
-  context "when attachments are present" do
-    let(:sendinblue_payload) do
-      sendinblue_valid_payload.tap do |hash|
-        hash[:Attachments] = [{ Name: "mon_scan.pdf", ContentType: "application/pdf" }]
+    context "when several agents are linked to the RDV" do
+      let!(:other_agent) { create(:agent, email: "[email protected]").tap { |a| rdv.agents << a } }
+
+      it "sends one email with all agents in the TO: field" do
+        perform_job
+        expect(ActionMailer::Base.deliveries.last.to).to contain_exactly("[email protected]", "[email protected]")
       end
     end
 
-    it "mentions the attachments in the notification e-mail" do
-      expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
-      transferred_email = ActionMailer::Base.deliveries.last
-      expect(transferred_email.html_part.body.to_s).to include(%(Le mail de l'usager⋅e avait en pièce jointe "mon_scan.pdf".))
+    context "when attachments are present" do
+      let(:sendinblue_payload) do
+        sendinblue_valid_payload.tap do |hash|
+          hash[:Attachments] = [{ Name: "mon_scan.pdf", ContentType: "application/pdf" }]
+        end
+      end
+
+      it "mentions the attachments in the notification e-mail" do
+        expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
+        transferred_email = ActionMailer::Base.deliveries.last
+        expect(transferred_email.html_part.body.to_s).to include(%(Le mail de l'usager⋅e avait en pièce jointe "mon_scan.pdf".))
+      end
     end
   end
 end

spec/mailers/users/rdv_mailer_spec.rb

@@ -8,7 +8,7 @@
     it "renders the headers" do
       expect(mail[:from].to_s).to eq(%("RDV Solidarités" <[email protected]>))
       expect(mail.to).to eq([user.email])
-      expect(mail.reply_to).to eq(["rdv+#{rdv.uuid}@reply.rdv-solidarites.fr"])
+      expect(mail.reply_to).to eq(["rdv+#{rdv.uuid}@reply.rdv-solidarites-test.localhost"])
     end
 
     it "renders the subject" do
@@ -82,7 +82,7 @@
       mail = described_class.with(rdv: rdv, user: user, token: token).rdv_updated(old_starts_at: previous_starting_time, lieu_id: nil)
       expect(mail[:from].to_s).to eq(%("RDV Solidarités" <[email protected]>))
       expect(mail.to).to eq([user.email])
-      expect(mail.reply_to).to eq(["rdv+#{rdv.uuid}@reply.rdv-solidarites.fr"])
+      expect(mail.reply_to).to eq(["rdv+#{rdv.uuid}@reply.rdv-solidarites-test.localhost"])
     end
 
     it "indicates the previous and current values" do
@@ -118,7 +118,7 @@
 
       expect(mail[:from].to_s).to eq(%("RDV Solidarités" <[email protected]>))
       expect(mail.to).to eq([user.email])
-      expect(mail.reply_to).to eq(["rdv+#{rdv.uuid}@reply.rdv-solidarites.fr"])
+      expect(mail.reply_to).to eq(["rdv+#{rdv.uuid}@reply.rdv-solidarites-test.localhost"])
     end
 
     it "subject contains date of cancelled rdv" do
@@ -176,7 +176,7 @@
       mail = described_class.with(rdv: rdv, user: user, token: token).rdv_upcoming_reminder
       expect(mail[:from].to_s).to eq(%("RDV Solidarités" <[email protected]>))
       expect(mail.to).to eq([user.email])
-      expect(mail.reply_to).to eq(["rdv+#{rdv.uuid}@reply.rdv-solidarites.fr"])
+      expect(mail.reply_to).to eq(["rdv+#{rdv.uuid}@reply.rdv-solidarites-test.localhost"])
       expect(mail.html_part.body).to include("Nous vous rappellons que vous avez un RDV prévu")
       expect(mail.html_part.body.raw_source).to include("/users/rdvs/#{rdv.id}?invitation_token=12345")
     end