From 36a944d67fe445fcd7d4897bc195847f48fcdb99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Villeneuve?= <4990201+Michaelvilleneuve@users.noreply.github.com> Date: Wed, 15 Jan 2025 16:03:35 +0100 Subject: [PATCH 1/5] feat(rgpd): partition motifs by org type (#2540) * participation motifs by org type * fix minor issues * fix validation issue * fix spec * minor refacto * minor refacto * moved validation to service * extract unit * potential fix for missing status * potential fix * fix potential flaky source * potential flaky source * minor fixes * potential fix for flaky --- .../category_configurations_controller.rb | 7 ++- app/models/category_configuration.rb | 4 +- app/models/motif_category.rb | 2 + app/models/organisation.rb | 1 + app/policies/motif_category_policy.rb | 15 +++++ .../category_configurations/create.rb | 12 +++- .../_category_configuration_form.html.erb | 2 +- db/seeds.rb | 2 +- spec/factories/motif_category.rb | 2 +- spec/factories/organisation.rb | 4 +- spec/factories/user.rb | 8 +-- ..._can_create_category_configuration_spec.rb | 63 +++++++++++++++++++ ...nt_can_edit_category_configuration_spec.rb | 1 - ..._user_by_convocation_or_invitation_spec.rb | 7 +++ ...vite_user_from_user_follow_up_page_spec.rb | 2 + .../category_configurations/create_spec.rb | 23 +++++++ spec/services/invitations/send_sms_spec.rb | 5 ++ spec/support/stub_helper.rb | 8 +++ 18 files changed, 155 insertions(+), 13 deletions(-) create mode 100644 app/policies/motif_category_policy.rb create mode 100644 spec/features/agent_can_create_category_configuration_spec.rb diff --git a/app/controllers/category_configurations_controller.rb b/app/controllers/category_configurations_controller.rb index 0653463d8..107395f69 100644 --- a/app/controllers/category_configurations_controller.rb +++ b/app/controllers/category_configurations_controller.rb @@ -14,7 +14,8 @@ class CategoryConfigurationsController < ApplicationController only: [:index, :new, :create, :show, :edit, :update, :destroy] before_action :set_category_configuration, :set_file_configuration, :set_template, only: [:show, :edit, :update, :destroy] - before_action :set_department, :set_file_configurations, only: [:new, :create, :edit, :update] + before_action :set_department, :set_file_configurations, :set_authorized_motif_categories, + only: [:new, :create, :edit, :update] before_action :set_back_to_users_list_url, :set_messages_configuration, :set_category_configurations, only: [:index] def index @@ -107,6 +108,10 @@ def set_organisation @organisation = current_organisation end + def set_authorized_motif_categories + @authorized_motif_categories = MotifCategoryPolicy.authorized_for_organisation(@organisation) + end + def user_count_by_tag_id User.joins(:tags, :organisations) diff --git a/app/models/category_configuration.rb b/app/models/category_configuration.rb index 8ed33a990..c35356c32 100644 --- a/app/models/category_configuration.rb +++ b/app/models/category_configuration.rb @@ -5,7 +5,9 @@ class CategoryConfiguration < ApplicationRecord validates :organisation, uniqueness: { scope: :motif_category, message: "a déjà une category_configuration pour cette catégorie de motif" } - validate :minimum_invitation_duration, :invitation_formats_validity, :periodic_invites_can_be_activated + validate :minimum_invitation_duration, + :invitation_formats_validity, + :periodic_invites_can_be_activated validates :email_to_notify_no_available_slots, :email_to_notify_rdv_changes, format: { diff --git a/app/models/motif_category.rb b/app/models/motif_category.rb index 69205344e..e707ed767 100644 --- a/app/models/motif_category.rb +++ b/app/models/motif_category.rb @@ -3,6 +3,8 @@ class MotifCategory < ApplicationRecord :name, :short_name ].freeze + RSA_RELATED_TYPES = %w[rsa_orientation rsa_accompagnement].freeze + has_many :category_configurations, dependent: :restrict_with_exception has_many :follow_ups, dependent: :restrict_with_exception has_many :motifs, dependent: :restrict_with_exception diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 5214b031f..a7cf17d1b 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -62,6 +62,7 @@ def france_travail? = safir_code? def with_parcours_access? organisation_type.in?(ORGANISATION_TYPES_WITH_PARCOURS_ACCESS) end + alias_method :rsa_related?, :with_parcours_access? def requires_dpa_acceptance? dpa_agreement.nil? diff --git a/app/policies/motif_category_policy.rb b/app/policies/motif_category_policy.rb new file mode 100644 index 000000000..b3f3d7644 --- /dev/null +++ b/app/policies/motif_category_policy.rb @@ -0,0 +1,15 @@ +class MotifCategoryPolicy < ApplicationPolicy + def self.authorized_for_organisation(organisation) + if organisation.rsa_related? + MotifCategory.where(motif_category_type: MotifCategory::RSA_RELATED_TYPES) + else + MotifCategory.where(motif_category_type: organisation.organisation_type) + end + end + + def self.authorized_for_organisation?(motif_category, organisation) + return MotifCategory::RSA_RELATED_TYPES.include?(motif_category.motif_category_type) if organisation.rsa_related? + + organisation.organisation_type == motif_category.motif_category_type + end +end diff --git a/app/services/category_configurations/create.rb b/app/services/category_configurations/create.rb index 3e653fd14..c68aac583 100644 --- a/app/services/category_configurations/create.rb +++ b/app/services/category_configurations/create.rb @@ -6,13 +6,23 @@ def initialize(category_configuration:) def call CategoryConfiguration.transaction do - activate_motif_category_on_rdvs_territory + ensure_motif_category_is_authorized save_record!(@category_configuration) + activate_motif_category_on_rdvs_territory end end private + def ensure_motif_category_is_authorized + return if MotifCategoryPolicy.authorized_for_organisation?( + @category_configuration.motif_category, + @category_configuration.organisation + ) + + fail!("La catégorie de motif n'est pas autorisée pour cette organisation") + end + def activate_motif_category_on_rdvs_territory @activate_motif_category_on_rdvs_territory ||= call_service!( RdvSolidaritesApi::CreateMotifCategoryTerritory, diff --git a/app/views/category_configurations/_category_configuration_form.html.erb b/app/views/category_configurations/_category_configuration_form.html.erb index cefbc9202..890e684b5 100644 --- a/app/views/category_configurations/_category_configuration_form.html.erb +++ b/app/views/category_configurations/_category_configuration_form.html.erb @@ -22,7 +22,7 @@ required="true" > - <% MotifCategory.all.each do |motif_category| %> + <% @authorized_motif_categories.each do |motif_category| %> <% end %> diff --git a/db/seeds.rb b/db/seeds.rb index 1b0229e84..9ec298b67 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -346,7 +346,7 @@ rdv_solidarites_organisation_id: 3, # rdv_solidarites_organisation_id: vérifier l'id de l'organisation correspondante sur RDV-Solidarites department_id: yonne.id, - organisation_type: "siae" + organisation_type: "france_travail" ) CategoryConfiguration.create!( diff --git a/spec/factories/motif_category.rb b/spec/factories/motif_category.rb index f6d004994..481333b54 100644 --- a/spec/factories/motif_category.rb +++ b/spec/factories/motif_category.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :motif_category do template - motif_category_type { "autre" } + motif_category_type { "rsa_orientation" } sequence(:short_name) { |n| "rsa_orientation_#{n}" } name { "RSA orientation" } end diff --git a/spec/factories/organisation.rb b/spec/factories/organisation.rb index ec8e04ca7..93f9d388e 100644 --- a/spec/factories/organisation.rb +++ b/spec/factories/organisation.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :organisation do - sequence(:name) { |n| "Organisation n°#{n}" } - sequence(:email) { |n| "organisation#{n}@rdv-insertion.fr" } + sequence(:name) { |n| "Organisation n°#{n + Process.pid}" } + sequence(:email) { |n| "organisation#{n + Process.pid}@rdv-insertion.fr" } rdv_solidarites_organisation_id { rand(1..10_000_000_000) } department phone_number { "0101010101" } diff --git a/spec/factories/user.rb b/spec/factories/user.rb index 81f1d0a24..13f0f24d4 100644 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -1,13 +1,13 @@ FactoryBot.define do factory :user do - sequence(:uid) { |n| "uid#{n}" } + sequence(:uid) { |n| "uid#{n + Process.pid}" } sequence(:rdv_solidarites_user_id) { |n| n + Process.pid } - sequence(:affiliation_number) { |n| "numero_#{n}" } + sequence(:affiliation_number) { |n| "numero_#{n + Process.pid}" } department_internal_id { rand(4000..5000).to_s } role { "demandeur" } title { "monsieur" } - sequence(:first_name) { |n| "john#{n}" } - sequence(:last_name) { |n| "doe#{n}" } + sequence(:first_name) { |n| "john#{n + Process.pid}" } + sequence(:last_name) { |n| "doe#{n + Process.pid}" } sequence(:email) { |n| "johndoe#{n + Process.pid}@yahoo.fr" } address { "27 avenue de Ségur 75007 Paris" } phone_number { "+33782605941" } diff --git a/spec/features/agent_can_create_category_configuration_spec.rb b/spec/features/agent_can_create_category_configuration_spec.rb new file mode 100644 index 000000000..122b4ba29 --- /dev/null +++ b/spec/features/agent_can_create_category_configuration_spec.rb @@ -0,0 +1,63 @@ +describe "Agents can edit category configuration", :js do + let!(:agent) { create(:agent) } + let!(:organisation) { create(:organisation, organisation_type: "delegataire_rsa") } + let!(:agent_role) { create(:agent_role, organisation: organisation, agent: agent, access_level: "admin") } + let!(:motif_category) do + create( + :motif_category, + name: "RSA Follow up", + short_name: "rsa_follow_up", + motif_category_type: "rsa_accompagnement" + ) + end + + let!(:category_configuration) { create(:category_configuration, organisation:, file_configuration:) } + let(:file_configuration) { create(:file_configuration) } + + before do + stub_rdv_solidarites_activate_motif_category_territories( + organisation.rdv_solidarites_organisation_id, + motif_category.short_name + ) + setup_agent_session(agent) + end + + context "category configuration edit" do + it "allows to create category configuration" do + visit new_organisation_category_configuration_path(organisation) + + fill_in "category_configuration_phone_number", with: "3234" + fill_in "category_configuration_email_to_notify_rdv_changes", with: "test@test.com" + fill_in "category_configuration_email_to_notify_no_available_slots", with: "test@test.com" + + select "RSA Follow up", from: "category_configuration[motif_category_id]" + + find("input[name=\"category_configuration[file_configuration_id]\"]").click + + click_button "Enregistrer" + + expect(page).to have_content("3234") + expect(page).to have_content("test@test.com") + + new_category_configuration = CategoryConfiguration.last + expect(new_category_configuration.reload.phone_number).to eq("3234") + expect(new_category_configuration.email_to_notify_rdv_changes).to eq("test@test.com") + expect(new_category_configuration.email_to_notify_no_available_slots).to eq("test@test.com") + end + + context "motif category selection" do + let!(:motif_category2) do + create(:motif_category, name: "Autre", short_name: "autre", motif_category_type: "autre") + end + + it "allows to select authorized motif categories" do + visit new_organisation_category_configuration_path(organisation) + + expect(page).to have_select( + "category_configuration[motif_category_id]", options: ["-", "RSA Follow up", + category_configuration.motif_category.name] + ) + end + end + end +end diff --git a/spec/features/agent_can_edit_category_configuration_spec.rb b/spec/features/agent_can_edit_category_configuration_spec.rb index bab6ecf74..cc8f0c7b8 100644 --- a/spec/features/agent_can_edit_category_configuration_spec.rb +++ b/spec/features/agent_can_edit_category_configuration_spec.rb @@ -12,7 +12,6 @@ it "allows to edit category configuration" do visit edit_organisation_category_configuration_path(organisation, organisation.category_configurations.first) - fill_in "category_configuration_phone_number", with: "3234" fill_in "category_configuration_phone_number", with: "3234" fill_in "category_configuration_email_to_notify_rdv_changes", with: "test@test.com" fill_in "category_configuration_email_to_notify_no_available_slots", with: "test@test.com" diff --git a/spec/features/agent_can_filter_user_by_convocation_or_invitation_spec.rb b/spec/features/agent_can_filter_user_by_convocation_or_invitation_spec.rb index 853a73b72..7e100aa3c 100644 --- a/spec/features/agent_can_filter_user_by_convocation_or_invitation_spec.rb +++ b/spec/features/agent_can_filter_user_by_convocation_or_invitation_spec.rb @@ -28,6 +28,13 @@ let(:participation2) { create(:participation, user: user1, follow_up: follow_up2, convocable: true) } let!(:notification2) { create(:notification, participation: participation2, created_at: 1.day.ago) } + before do + follow_up.set_status + follow_up.save + follow_up2.set_status + follow_up2.save + end + context "with convocation date before" do before do setup_agent_session(agent) diff --git a/spec/features/agent_can_invite_user_from_user_follow_up_page_spec.rb b/spec/features/agent_can_invite_user_from_user_follow_up_page_spec.rb index e1c5dc124..c7a9b3656 100644 --- a/spec/features/agent_can_invite_user_from_user_follow_up_page_spec.rb +++ b/spec/features/agent_can_invite_user_from_user_follow_up_page_spec.rb @@ -25,6 +25,8 @@ stub_geo_api_request(user.address) stub_brevo stub_creneau_availability(true) + follow_up.set_status + follow_up.save end shared_examples "agent can invite user" do diff --git a/spec/services/category_configurations/create_spec.rb b/spec/services/category_configurations/create_spec.rb index 9fbda3e80..7c6139f07 100644 --- a/spec/services/category_configurations/create_spec.rb +++ b/spec/services/category_configurations/create_spec.rb @@ -65,5 +65,28 @@ expect(category_configuration).not_to be_persisted end end + + describe "motif validatity" do + let(:organisation) { create(:organisation, organisation_type: "delegataire_rsa") } + let(:category_configuration) do + build(:category_configuration, organisation: create(:organisation), motif_category:) + end + + context "when motif is not authorized" do + let(:motif_category) { create(:motif_category, name: "SIAE", short_name: "siae", motif_category_type: "siae") } + + it "stores the error" do + expect(subject.errors[0]).to include("La catégorie de motif n'est pas autorisée") + end + end + + context "when motif is authorized" do + let(:motif_category) { create(:motif_category, motif_category_type: "rsa_orientation") } + + it "is a success" do + is_a_success + end + end + end end end diff --git a/spec/services/invitations/send_sms_spec.rb b/spec/services/invitations/send_sms_spec.rb index 96454b0f6..577ea1f5c 100644 --- a/spec/services/invitations/send_sms_spec.rb +++ b/spec/services/invitations/send_sms_spec.rb @@ -456,6 +456,7 @@ end context "for siae_interview" do + let!(:organisation) { create(:organisation, department: department, organisation_type: "siae") } let!(:follow_up) { build(:follow_up, motif_category: category_siae_interview) } let!(:category_configuration) do create(:category_configuration, organisation: organisation, motif_category: category_siae_interview) @@ -506,6 +507,7 @@ end context "for siae_collective_information" do + let!(:organisation) { create(:organisation, department: department, organisation_type: "siae") } let!(:follow_up) { build(:follow_up, motif_category: category_siae_collective_information) } let!(:category_configuration) do create(:category_configuration, organisation: organisation, @@ -557,6 +559,7 @@ end context "for siae_follow_up" do + let!(:organisation) { create(:organisation, department: department, organisation_type: "siae") } let!(:follow_up) { build(:follow_up, motif_category: category_siae_follow_up) } let!(:category_configuration) do create(:category_configuration, organisation: organisation, motif_category: category_siae_follow_up) @@ -607,6 +610,7 @@ end context "for psychologue" do + let!(:organisation) { create(:organisation, department: department, organisation_type: "autre") } let!(:follow_up) { build(:follow_up, motif_category: category_psychologue) } let!(:category_configuration) do create(:category_configuration, organisation: organisation, motif_category: category_psychologue) @@ -659,6 +663,7 @@ end context "for atelier_enfants_ados" do + let!(:organisation) { create(:organisation, department: department, organisation_type: "autre") } let!(:follow_up) { build(:follow_up, motif_category: category_atelier_enfants_ados) } let!(:category_configuration) do create(:category_configuration, organisation: organisation, motif_category: category_atelier_enfants_ados) diff --git a/spec/support/stub_helper.rb b/spec/support/stub_helper.rb index 8ff21f866..07ded0773 100644 --- a/spec/support/stub_helper.rb +++ b/spec/support/stub_helper.rb @@ -15,6 +15,14 @@ def stub_rdv_solidarites_assign_organisations(rdv_solidarites_user_id) ) end + def stub_rdv_solidarites_activate_motif_category_territories(rdv_solidarites_organisation_id, motif_name) + stub_request( + :post, "http://www.rdv-solidarites-test.localhost/api/rdvinsertion/motif_category_territories" + ).with( + body: "{\"motif_category_short_name\":\"#{motif_name}\",\"organisation_id\":#{rdv_solidarites_organisation_id}}" + ).to_return(status: 200, body: "", headers: {}) + end + def stub_rdv_solidarites_assign_many_referents(rdv_solidarites_user_id) stub_request( :post, "#{ENV['RDV_SOLIDARITES_URL']}/api/rdvinsertion/referent_assignations/create_many" From 0b24fef9ce9c1193969f1313b2f8c0afc162c421 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Villeneuve?= <4990201+Michaelvilleneuve@users.noreply.github.com> Date: Thu, 16 Jan 2025 10:06:04 +0100 Subject: [PATCH 2/5] feat(brevo): make sure webhooks come from brevo (#2563) * make sure webhook come from brevo * minor improvement * refacto brevo tests * fix linter * prevent flaky --- app/controllers/brevo/ip_whitelist_concern.rb | 30 ++++++++++ .../brevo/mail_webhooks_controller.rb | 1 + .../brevo/sms_webhooks_controller.rb | 1 + .../assign_delivery_status_and_date_base.rb | 5 -- .../assign_mail_delivery_status_and_date.rb | 7 --- .../assign_sms_delivery_status_and_date.rb | 7 --- .../brevo/mail_webhooks_controller_spec.rb | 8 +++ .../brevo/sms_webhooks_controller_spec.rb | 8 +++ spec/rails_helper.rb | 1 + spec/requests/brevo_mail_webhooks_spec.rb | 24 +------- spec/requests/brevo_sms_webhooks_spec.rb | 24 +------- ...sign_mail_delivery_status_and_date_spec.rb | 17 ------ ...ssign_sms_delivery_status_and_date_spec.rb | 3 - spec/support/brevo_ip_helper.rb | 58 +++++++++++++++++++ 14 files changed, 111 insertions(+), 83 deletions(-) create mode 100644 app/controllers/brevo/ip_whitelist_concern.rb create mode 100644 spec/support/brevo_ip_helper.rb diff --git a/app/controllers/brevo/ip_whitelist_concern.rb b/app/controllers/brevo/ip_whitelist_concern.rb new file mode 100644 index 000000000..2c062dba0 --- /dev/null +++ b/app/controllers/brevo/ip_whitelist_concern.rb @@ -0,0 +1,30 @@ +require "ipaddr" + +module Brevo::IpWhitelistConcern + extend ActiveSupport::Concern + + # IP list comes from + # https://help.brevo.com/hc/en-us/articles/15127404548498-Brevo-IP-ranges-List-of-publicly-exposed-services#h_01HENC062K8KJKJE7BJNYMPM77 + IP_WHITELIST_RANGE = "1.179.112.0/20".freeze + + included do + before_action :ensure_ip_comes_from_brevo_ips + end + + private + + def ensure_ip_comes_from_brevo_ips + # In case Brevo decides to use some other IP range without notice + # we need a quick way to skip this check + return if ENV["DISABLE_BREVO_IP_WHITELIST"].present? + + return if IPAddr.new(IP_WHITELIST_RANGE).include?(request.remote_ip) + + Sentry.capture_message("Brevo Webhook received with following non whitelisted IP", { + extra: { + ip: request.remote_ip + } + }) + head :forbidden + end +end diff --git a/app/controllers/brevo/mail_webhooks_controller.rb b/app/controllers/brevo/mail_webhooks_controller.rb index 51b82b7cc..2f447dfed 100644 --- a/app/controllers/brevo/mail_webhooks_controller.rb +++ b/app/controllers/brevo/mail_webhooks_controller.rb @@ -1,5 +1,6 @@ module Brevo class MailWebhooksController < ApplicationController + include Brevo::IpWhitelistConcern skip_before_action :authenticate_agent!, :verify_authenticity_token PERMITTED_PARAMS = %i[ diff --git a/app/controllers/brevo/sms_webhooks_controller.rb b/app/controllers/brevo/sms_webhooks_controller.rb index f40c4894c..927c7ca2b 100644 --- a/app/controllers/brevo/sms_webhooks_controller.rb +++ b/app/controllers/brevo/sms_webhooks_controller.rb @@ -1,5 +1,6 @@ module Brevo class SmsWebhooksController < ApplicationController + include Brevo::IpWhitelistConcern skip_before_action :authenticate_agent!, :verify_authenticity_token PERMITTED_PARAMS = %i[ diff --git a/app/services/inbound_webhooks/brevo/assign_delivery_status_and_date_base.rb b/app/services/inbound_webhooks/brevo/assign_delivery_status_and_date_base.rb index 368c27033..8ce9eb3a3 100644 --- a/app/services/inbound_webhooks/brevo/assign_delivery_status_and_date_base.rb +++ b/app/services/inbound_webhooks/brevo/assign_delivery_status_and_date_base.rb @@ -10,7 +10,6 @@ def call return if old_update? return if @record.delivered? - alert_sentry_if_webhook_mismatch set_last_brevo_webhook_received_at return unless delivery_status.in?(record_class.delivery_statuses.keys) @@ -29,10 +28,6 @@ def record_class @record_class ||= @record.class end - def alert_sentry_if_webhook_mismatch - raise NoMethodError - end - def delivery_status raise NoMethodError end diff --git a/app/services/inbound_webhooks/brevo/assign_mail_delivery_status_and_date.rb b/app/services/inbound_webhooks/brevo/assign_mail_delivery_status_and_date.rb index 48cd20c2e..d2b294657 100644 --- a/app/services/inbound_webhooks/brevo/assign_mail_delivery_status_and_date.rb +++ b/app/services/inbound_webhooks/brevo/assign_mail_delivery_status_and_date.rb @@ -6,13 +6,6 @@ class AssignMailDeliveryStatusAndDate < AssignDeliveryStatusAndDateBase def delivery_status @delivery_status ||= @webhook_params[:event] end - - def alert_sentry_if_webhook_mismatch - return if @record.email.casecmp?(@webhook_params[:email]) - - Sentry.capture_message("#{record_class} email and webhook email does not match", - extra: { record: @record, webhook: @webhook_params }) - end end end end diff --git a/app/services/inbound_webhooks/brevo/assign_sms_delivery_status_and_date.rb b/app/services/inbound_webhooks/brevo/assign_sms_delivery_status_and_date.rb index e6cec3e16..8a1f082b5 100644 --- a/app/services/inbound_webhooks/brevo/assign_sms_delivery_status_and_date.rb +++ b/app/services/inbound_webhooks/brevo/assign_sms_delivery_status_and_date.rb @@ -6,13 +6,6 @@ class AssignSmsDeliveryStatusAndDate < AssignDeliveryStatusAndDateBase def delivery_status @delivery_status ||= @webhook_params[:msg_status] end - - def alert_sentry_if_webhook_mismatch - return if @record.user.phone_number == PhoneNumberHelper.format_phone_number(@webhook_params[:to]) - - Sentry.capture_message("#{record_class} mobile phone and webhook mobile phone does not match", - extra: { record: @record, webhook: @webhook_params }) - end end end end diff --git a/spec/controllers/brevo/mail_webhooks_controller_spec.rb b/spec/controllers/brevo/mail_webhooks_controller_spec.rb index 58d96d35c..055840fbb 100644 --- a/spec/controllers/brevo/mail_webhooks_controller_spec.rb +++ b/spec/controllers/brevo/mail_webhooks_controller_spec.rb @@ -1,4 +1,6 @@ describe Brevo::MailWebhooksController do + include_context "with ip whitelist" + describe "#create" do before do allow(InboundWebhooks::Brevo::ProcessMailDeliveryStatusJob).to receive(:perform_later) @@ -13,6 +15,12 @@ } end + context "when called with non-matching IP" do + let(:params) { valid_mail_params } + + include_examples "returns 403 for non-whitelisted IP", "18.12.12.12" + end + context "when X-Mailin-custom header is present and environment matches" do it "enqueues the job for processing mail delivery status" do expect(InboundWebhooks::Brevo::ProcessMailDeliveryStatusJob).to receive(:perform_later) diff --git a/spec/controllers/brevo/sms_webhooks_controller_spec.rb b/spec/controllers/brevo/sms_webhooks_controller_spec.rb index d506654e5..350918481 100644 --- a/spec/controllers/brevo/sms_webhooks_controller_spec.rb +++ b/spec/controllers/brevo/sms_webhooks_controller_spec.rb @@ -1,4 +1,6 @@ describe Brevo::SmsWebhooksController do + include_context "with ip whitelist" + describe "#create" do before do allow(InboundWebhooks::Brevo::ProcessSmsDeliveryStatusJob).to receive(:perform_later) @@ -8,6 +10,12 @@ { to: "1234567890", msg_status: "delivered", date: "2023-06-07T12:34:56Z", record_identifier: "invitation_1" } end + context "when called with non-matching IP" do + let(:params) { valid_sms_params } + + include_examples "returns 403 for non-whitelisted IP", "18.12.12.12" + end + it "enqueues the job for processing SMS delivery status" do expect(InboundWebhooks::Brevo::ProcessSmsDeliveryStatusJob).to receive(:perform_later) .with({ to: "1234567890", msg_status: "delivered", date: "2023-06-07T12:34:56Z" }, "invitation_1") diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 4907be76d..d5379bf72 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -45,6 +45,7 @@ config.include MotifCategoriesHelper config.include NirHelper config.include SortingHelper + config.include BrevoIpHelper config.include ConfirmModalHelper config.include StubHelper config.include UploadHelper diff --git a/spec/requests/brevo_mail_webhooks_spec.rb b/spec/requests/brevo_mail_webhooks_spec.rb index cbfbba297..3c7f9aaee 100644 --- a/spec/requests/brevo_mail_webhooks_spec.rb +++ b/spec/requests/brevo_mail_webhooks_spec.rb @@ -1,4 +1,6 @@ RSpec.describe "BrevoMailWebhooks" do + include_context "with ip whitelist" + let(:webhook_params) do { email: "test@example.com", @@ -29,28 +31,6 @@ expect(invitation.last_brevo_webhook_received_at).to eq(Time.zone.parse("2023-06-07T12:34:56Z")) end - context "when emails doesnt match" do - let(:mismatched_webhook_params) do - { - email: "email_changed@example.com", - event: "delivered", - date: "2023-06-07T12:34:56Z", - :"X-Mailin-custom" => "{\"record_identifier\": \"#{invitation.record_identifier}\"}" - } - end - - it "update the invitation but captures an error" do - post "/brevo/mail_webhooks", params: mismatched_webhook_params, as: :json - expect(response).to be_successful - - invitation.reload - expect(invitation.delivery_status).to eq("delivered") - expect(invitation.last_brevo_webhook_received_at).to eq(Time.zone.parse("2023-06-07T12:34:56Z")) - expect(Sentry).to have_received(:capture_message).with("Invitation email and webhook email does not match", - any_args) - end - end - context "with an event not in enum" do let(:unprocessed_event_webhook_params) do { diff --git a/spec/requests/brevo_sms_webhooks_spec.rb b/spec/requests/brevo_sms_webhooks_spec.rb index 20ccc32ef..9844083cc 100644 --- a/spec/requests/brevo_sms_webhooks_spec.rb +++ b/spec/requests/brevo_sms_webhooks_spec.rb @@ -1,4 +1,6 @@ RSpec.describe "BrevoSmsWebhooks" do + include_context "with ip whitelist" + let(:webhook_params) do { to: "0601010101", @@ -27,28 +29,6 @@ expect(invitation.last_brevo_webhook_received_at).to eq(Time.zone.parse("2023-06-07T12:34:56Z")) end - context "when phones doesnt match" do - let(:mismatched_webhook_params) do - { - to: "0987654321", - msg_status: "delivered", - date: "2023-06-07T12:34:56Z" - } - end - - it "update the invitation but captures an error" do - post "/brevo/sms_webhooks/#{invitation.record_identifier}", params: mismatched_webhook_params, as: :json - expect(response).to be_successful - - invitation.reload - expect(invitation.delivery_status).to eq("delivered") - expect(invitation.last_brevo_webhook_received_at).to eq(Time.zone.parse("2023-06-07T12:34:56Z")) - expect(Sentry).to have_received(:capture_message).with( - "Invitation mobile phone and webhook mobile phone does not match", any_args - ) - end - end - context "when event is not in enum" do let(:unprocessed_event_webhook_params) do { diff --git a/spec/services/inbound_webhooks/brevo/assign_mail_delivery_status_and_date_spec.rb b/spec/services/inbound_webhooks/brevo/assign_mail_delivery_status_and_date_spec.rb index c4054c143..4e7f303ab 100644 --- a/spec/services/inbound_webhooks/brevo/assign_mail_delivery_status_and_date_spec.rb +++ b/spec/services/inbound_webhooks/brevo/assign_mail_delivery_status_and_date_spec.rb @@ -22,25 +22,8 @@ end end - context "when emails does not match" do - let(:webhook_params) do - { email: "email_changed@example.com", event: "delivered", date: "2023-06-07T12:34:56Z" } - end - - it "update the #{record_type} but capture an error" do - expect(Sentry).to receive(:capture_message).with( - "#{record_type.capitalize} email and webhook email does not match", - any_args - ) - subject - expect(record.delivery_status).to eq("delivered") - expect(record.last_brevo_webhook_received_at).to eq(Time.zone.parse("2023-06-07T12:34:56Z")) - end - end - it "updates the #{record_type} with the correct delivery status and date" do subject - expect(Sentry).not_to receive(:capture_message).with(any_args) record.reload expect(record.delivery_status).to eq("delivered") expect(record.last_brevo_webhook_received_at).to eq(Time.zone.parse("2023-06-07T12:34:56Z")) diff --git a/spec/services/inbound_webhooks/brevo/assign_sms_delivery_status_and_date_spec.rb b/spec/services/inbound_webhooks/brevo/assign_sms_delivery_status_and_date_spec.rb index 4c458bb5b..1414a7873 100644 --- a/spec/services/inbound_webhooks/brevo/assign_sms_delivery_status_and_date_spec.rb +++ b/spec/services/inbound_webhooks/brevo/assign_sms_delivery_status_and_date_spec.rb @@ -26,9 +26,6 @@ let(:webhook_params) { { to: "0987654321", msg_status: "delivered", date: "2023-06-07T12:34:56Z" } } it "update the #{record_type} but capture an error" do - expect(Sentry).to receive(:capture_message).with( - "#{record_type.capitalize} mobile phone and webhook mobile phone does not match", any_args - ) subject expect(record.delivery_status).to eq("delivered") expect(record.last_brevo_webhook_received_at).to eq(Time.zone.parse("2023-06-07T12:34:56Z")) diff --git a/spec/support/brevo_ip_helper.rb b/spec/support/brevo_ip_helper.rb new file mode 100644 index 000000000..c6a75cd20 --- /dev/null +++ b/spec/support/brevo_ip_helper.rb @@ -0,0 +1,58 @@ +module BrevoIpHelper + def stub_brevo_webhook_ip(remote_ip = "1.179.112.1") + [ + Brevo::MailWebhooksController, + Brevo::SmsWebhooksController + ].each do |controller| + allow_any_instance_of(controller).to receive(:request).and_wrap_original do |original_request| + original_request.call.tap do |request| + allow(request).to receive(:remote_ip).and_return(remote_ip) + end + end + end + end +end + +RSpec.shared_context "with ip whitelist", shared_context: :metadata do |ip| + let(:remote_ip) { ip || "1.179.112.1" } + + before do + stub_brevo_webhook_ip(remote_ip) + end +end + +RSpec.shared_examples "returns 403 for non-whitelisted IP" do |ip| + include_context "with ip whitelist", ip + + context "when called with non-matching IP" do + it "returns 403" do + expect(Sentry).to( + receive(:capture_message).with("Brevo Webhook received with following non whitelisted IP", { + extra: { ip: ip } + }) + ) + post :create, params:, as: :json + expect(response).to have_http_status(:forbidden) + end + end + + context "when whitelisting is disabled" do + before do + ENV["DISABLE_BREVO_IP_WHITELIST"] = "true" + end + + after do + ENV["DISABLE_BREVO_IP_WHITELIST"] = nil + end + + it "skips whitelisting" do + expect(Sentry).not_to( + receive(:capture_message).with("Brevo Webhook received with following non whitelisted IP", { + extra: { ip: ip } + }) + ) + post :create, params:, as: :json + expect(response).not_to have_http_status(:forbidden) + end + end +end From 3b65d335fe3d94c5fe8958c49da293ba65232875 Mon Sep 17 00:00:00 2001 From: Amine Dhobb Date: Mon, 20 Jan 2025 14:07:54 +0100 Subject: [PATCH 3/5] fix(migration): Remove null constraint on agent_id for parcours_documents (#2570) --- db/schema.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index de31218af..ed361432c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_12_16_135605) do +ActiveRecord::Schema[7.1].define(version: 2025_01_20_125249) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -363,7 +363,7 @@ create_table "parcours_documents", force: :cascade do |t| t.bigint "department_id", null: false t.bigint "user_id", null: false - t.bigint "agent_id", null: false + t.bigint "agent_id" t.string "type" t.datetime "created_at", null: false t.datetime "updated_at", null: false From e475fb3b844bd80af8d65e7de0dd3c2a688fc4bc Mon Sep 17 00:00:00 2001 From: Amine Dhobb Date: Mon, 20 Jan 2025 14:37:13 +0100 Subject: [PATCH 4/5] fix: Add migration file to remove agent_id null constraint (#2571) --- ..._change_agent_id_null_constraint_on_parcours_documents.rb | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 db/migrate/20250120125249_change_agent_id_null_constraint_on_parcours_documents.rb diff --git a/db/migrate/20250120125249_change_agent_id_null_constraint_on_parcours_documents.rb b/db/migrate/20250120125249_change_agent_id_null_constraint_on_parcours_documents.rb new file mode 100644 index 000000000..f98d9ef26 --- /dev/null +++ b/db/migrate/20250120125249_change_agent_id_null_constraint_on_parcours_documents.rb @@ -0,0 +1,5 @@ +class ChangeAgentIdNullConstraintOnParcoursDocuments < ActiveRecord::Migration[7.1] + def change + change_column_null :parcours_documents, :agent_id, true + end +end From a8f3277a34918a4daeb37d7724ee476df41a0b4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Villeneuve?= <4990201+Michaelvilleneuve@users.noreply.github.com> Date: Wed, 22 Jan 2025 16:53:52 +0100 Subject: [PATCH 5/5] fix(teleprocedure): change wording (#2582) --- app/views/website/teleprocedure_landings/_13.html.erb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/views/website/teleprocedure_landings/_13.html.erb b/app/views/website/teleprocedure_landings/_13.html.erb index febd66f91..a8c9b36e4 100644 --- a/app/views/website/teleprocedure_landings/_13.html.erb +++ b/app/views/website/teleprocedure_landings/_13.html.erb @@ -1,6 +1,5 @@ -

Si vous résidez dans l’une des 29 communes listées ci-dessous, vous êtes concerné par une expérimentation du Conseil départemental des Bouches-du-Rhône visant à faciliter votre prise de rendez-vous RSA.

-
À la suite de votre demande de RSA, un email et/ou un SMS va vous être transmis par le Conseil départemental pour fixer un premier rendez-vous d’orientation. Veuillez cliquer sur le lien contenu dans ce mail/SMS pour choisir un créneau à votre convenance.
-
-

Communes concernées : Marseille - 2e arrondissement, Marseille - 3e arrondissement, Arles, Aureille, Barbentane, Baux-de-Provence, Boulbon, Cabannes, Chateaurenard, Eygalières, Eyragues, Fontvieille, Graveson, Maillane, Mas-Blanc-des-Alpilles, Maussane-les-Alpilles, Saint-Pierre-de-Mézoargues, Molleges, Mouries, Noves, Orgon, Paradou, Plan-d'Orgon, Rognonas, Saint-Andiol, Saint-Etienne-du-Gres, Saintes-Maries-de-la-Mer, Saint-Martin-de-Crau, Saint-Rémy-de-Provence, Tarascon, Verquieres.

-
-

Si vous ne résidez pas dans l’une des 29 communes listées ci-dessus, vous n’êtes pas concerné par ces modalités de prise de rendez-vous et serez convié à votre rendez-vous d’orientation par courrier postal avec accusé de réception.

+

Vous pouvez désormais choisir le jour et l’heure, les plus appropriés à votre emploi du temps, pour rencontrer le conseiller d’orientation qui définira avec vous les démarches et actions à mettre en œuvre pour faciliter votre insertion.

+ +

À la suite de votre demande de RSA, un email et/ou un SMS vous sera transmis par le Conseil départemental pour fixer votre premier rendez-vous d’orientation. Cliquez sur le lien contenu dans ce mail/SMS pour choisir un créneau à votre convenance.

+ +

Simple, rapide, facile le service « RENDEZ-VOUS INSERTION » est disponible sur tous les Pôles d’insertion des Bouches-du- Rhône est accessible en un clic.