From 2032eb814459d1460c569de84968d567b1daafa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Vantomme?= Date: Mon, 8 Mar 2021 10:24:53 +0100 Subject: [PATCH 01/17] Test (Capybara): add no-sandbox option to headless chrome driver Activating this option fix the 'DevToolsActivePort file doesn't exist' error. https://stackoverflow.com/questions/50642308/webdriverexception-unknown-error-devtoolsactiveport-file-doesnt-exist-while-t --- spec/support/capybara.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index dba0047bc..0275b0f0d 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -9,6 +9,7 @@ end Capybara.register_driver :headless_chrome do |app| options = Selenium::WebDriver::Chrome::Options.new + options.add_argument('--no-sandbox') unless ENV['SANDBOX'] options.add_argument('--headless') unless ENV['NO_HEADLESS'] options.add_argument('--window-size=1440,900') From 62562ff7a593c72f751d3ee3b6a833d8bed265bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Vantomme?= Date: Wed, 26 Jan 2022 16:56:03 +0100 Subject: [PATCH 02/17] feat(landing): improve CTA rendering --- app/assets/stylesheets/landing.scss | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/landing.scss b/app/assets/stylesheets/landing.scss index 589cdbb01..124e9ee2a 100644 --- a/app/assets/stylesheets/landing.scss +++ b/app/assets/stylesheets/landing.scss @@ -295,7 +295,6 @@ $users-breakpoint: 950px; font-size: 24px; font-weight: bold; margin-top: 13px; - color: #FFFFFF; &.grey { color: $g700; @@ -303,15 +302,19 @@ $users-breakpoint: 950px; } .cta-panel-explanation { - font-size: 24px; + font-size: 22px; margin-bottom: 10px; - color: #FFFFFF; &.grey { color: $g700; } } +.half .cta-panel-title, +.half .cta-panel-explanation { + text-align: center; +} + .role-panel-title { font-size: 30px; font-weight: bold; @@ -357,7 +360,7 @@ $cta-panel-button-border-size: 2px; @include vertical-padding(15px); display: block; border-radius: 100px; - font-size: 24px; + font-size: 22px; text-align: center; cursor: pointer; margin-top: 20px; From ab06ec2887f6857de4cdd50dc82580001c89faa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Vantomme?= Date: Fri, 28 Jan 2022 13:01:01 +0100 Subject: [PATCH 03/17] fix(task): deal nicely with dropped tmp_expert_migrated column on avis table --- ...9_backfill_claimant_id_for_experts_on_avis_table.rake | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/tasks/deployment/20210311085419_backfill_claimant_id_for_experts_on_avis_table.rake b/lib/tasks/deployment/20210311085419_backfill_claimant_id_for_experts_on_avis_table.rake index bc11c3a3e..f7b4fe17b 100644 --- a/lib/tasks/deployment/20210311085419_backfill_claimant_id_for_experts_on_avis_table.rake +++ b/lib/tasks/deployment/20210311085419_backfill_claimant_id_for_experts_on_avis_table.rake @@ -3,7 +3,8 @@ namespace :after_party do task backfill_claimant_id_for_experts_on_avis_table: :environment do puts "Running deploy task 'backfill_claimant_id_for_experts_on_avis_table'" - avis_experts_claimant = Avis.where(claimant_type: 'Expert', tmp_expert_migrated: false) + avis_experts_claimant = Avis.where(claimant_type: 'Expert') + avis_experts_claimant = avis_experts_claimant.where(tmp_expert_migrated: false) if Avis.column_names.include?("tmp_expert_migrated") progress = ProgressReport.new(avis_experts_claimant.count) avis_experts_claimant.find_each do |avis| @@ -15,7 +16,10 @@ namespace :after_party do claimant_expert = claimant_instructeur.reload.user.expert ExpertsProcedure.find_or_create_by(procedure: avis.procedure, expert: claimant_expert) end - avis.update_columns(claimant_id: claimant_expert.id, tmp_expert_migrated: true) + + if Avis.column_names.include?("tmp_expert_migrated") + avis.update_columns(claimant_id: claimant_expert.id, tmp_expert_migrated: true) + end else # Avis associated to an Instructeur with no user are bad data: delete it avis.destroy! @@ -23,6 +27,7 @@ namespace :after_party do progress.inc end progress.finish + # Update task as completed. If you remove the line below, the task will # run with every deploy (or every time you call after_party:run). AfterParty::TaskRecord From eea3087ef0abf5724c4c0da834de6af9a76e44fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Vantomme?= Date: Fri, 28 Jan 2022 13:03:04 +0100 Subject: [PATCH 04/17] fix(task): deal nicely with dropped instructeur_id column on avis table --- ...ill_experts_procedure_id_on_avis_table.rake | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/tasks/deployment/20210324081552_backfill_experts_procedure_id_on_avis_table.rake b/lib/tasks/deployment/20210324081552_backfill_experts_procedure_id_on_avis_table.rake index 90a1ffa3f..54bf1f32d 100644 --- a/lib/tasks/deployment/20210324081552_backfill_experts_procedure_id_on_avis_table.rake +++ b/lib/tasks/deployment/20210324081552_backfill_experts_procedure_id_on_avis_table.rake @@ -3,8 +3,19 @@ namespace :after_party do task backfill_experts_procedure_id_on_avis_table: :environment do puts "Running deploy task 'backfill_experts_procedure_id_on_avis_table'" - without_instructeur = Avis.where(experts_procedure_id: nil, instructeur_id: nil).where.not(email: nil) - with_instructeur = Avis.where(experts_procedure_id: nil, email: nil).where.not(instructeur_id: nil) + if Avis.column_names.include?("instructeur_id") + without_instructeur = Avis.where(experts_procedure_id: nil, instructeur_id: nil).where.not(email: nil) + with_instructeur = Avis.where(experts_procedure_id: nil, email: nil).where.not(instructeur_id: nil) + else + without_instructeur = Avis + .where(experts_procedure_id: nil, claimant_type: [nil, "Instructeur"]) + .where.not(email: nil) + + with_instructeur = Avis + .where(experts_procedure_id: nil, email: nil, claimant_type: [nil, "Instructeur"]) + .where.not(claimant_id: nil) + end + progress = ProgressReport.new(without_instructeur.count) progress2 = ProgressReport.new(with_instructeur.count) @@ -22,7 +33,8 @@ namespace :after_party do progress.finish with_instructeur.find_each do |avis| - instructeur = avis.instructeur + instructeur = avis.respond_to?(:instructeur) ? avis.instructeur : avis.claimant + if instructeur && instructeur.user user = User.create_or_promote_to_expert(instructeur.user.email, SecureRandom.hex) user.reload From f3bf0499b605f9234159918c18c8500d53b77303 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Vantomme?= Date: Wed, 5 Jan 2022 15:39:23 +0100 Subject: [PATCH 05/17] chore(rubocop): cleanup dependencies Using rubocop-rails_config is a nonsense because: - it relies on rubocop-minitest which is useless here (we use RSpec) - it relies on rubocop-packaging but disables all its cops - it targets ruby version 2.7, but we use 3.0 --- .rubocop.yml | 14 ++++++++------ Gemfile | 3 ++- Gemfile.lock | 12 ++---------- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index e5eac1a9b..ea5ad74b8 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,15 +1,15 @@ require: + - rubocop-performance + - rubocop-rails - rubocop/rspec/focused - ./lib/cops/add_concurrent_index.rb - ./lib/cops/application_name.rb - ./lib/cops/unscoped.rb -inherit_gem: - rubocop-rails_config: - - config/rails.yml - AllCops: - TargetRubyVersion: 2.7 + TargetRubyVersion: 3.0 + DisabledByDefault: true + SuggestExtensions: false Exclude: - "db/schema.rb" - "db/migrate/20190730153555_recreate_structure.rb" @@ -644,6 +644,9 @@ Performance/RedundantMatch: Performance/RedundantMerge: Enabled: true +Style/HashTransformValues: + Enabled: true + Style/RedundantSortBy: Enabled: true @@ -1373,4 +1376,3 @@ Style/YodaCondition: Style/ZeroLengthPredicate: Enabled: true - diff --git a/Gemfile b/Gemfile index 3377bc729..e6ebfaf0a 100644 --- a/Gemfile +++ b/Gemfile @@ -111,7 +111,8 @@ group :development do gem 'rack-mini-profiler' gem 'rails-erd', require: false # generates `doc/database_models.pdf` gem 'rubocop', require: false - gem 'rubocop-rails_config' + gem 'rubocop-performance', require: false + gem 'rubocop-rails', require: false gem 'rubocop-rspec-focused', require: false gem 'scss_lint', require: false gem 'web-console' diff --git a/Gemfile.lock b/Gemfile.lock index 2ea2134bc..66ef40011 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -620,8 +620,6 @@ GEM unicode-display_width (>= 1.4.0, < 3.0) rubocop-ast (1.4.1) parser (>= 2.7.1.5) - rubocop-packaging (0.5.1) - rubocop (>= 0.89, < 2.0) rubocop-performance (1.9.2) rubocop (>= 0.90.0, < 2.0) rubocop-ast (>= 0.4.0) @@ -629,13 +627,6 @@ GEM activesupport (>= 4.2.0) rack (>= 1.1) rubocop (>= 0.90.0, < 2.0) - rubocop-rails_config (1.3.1) - railties (>= 5.0) - rubocop (>= 1.8) - rubocop-ast (>= 1.0.1) - rubocop-packaging (~> 0.5) - rubocop-performance (~> 1.3) - rubocop-rails (~> 2.0) rubocop-rspec-focused (1.0.0) rubocop (>= 0.51) ruby-graphviz (1.2.5) @@ -874,7 +865,8 @@ DEPENDENCIES rspec-rails rspec_junit_formatter rubocop - rubocop-rails_config + rubocop-performance + rubocop-rails rubocop-rspec-focused ruby-saml-idp sanitize-url From 2bdf58cb793b1ee9c98fc14231c2a680b3bc4ea7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Vantomme?= Date: Wed, 5 Jan 2022 15:48:42 +0100 Subject: [PATCH 06/17] style(rubocop): fix Layout/SpaceInsideBlockBraces offense --- spec/factories/procedure.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb index 75919801c..ec7a25b67 100644 --- a/spec/factories/procedure.rb +++ b/spec/factories/procedure.rb @@ -18,7 +18,7 @@ FactoryBot.define do administrateurs { administrateur.present? ? [administrateur] : [association(:administrateur)] } transient do - administrateur { } + administrateur {} instructeurs { [] } types_de_champ { [] } types_de_champ_private { [] } From 04cfc8ed9d89c041aa457c3f5f2227ff32787671 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 18 Jan 2022 16:56:30 +0100 Subject: [PATCH 07/17] config: use alternate delivery methods to configure ActionMailer Previously `SENDINBLUE_BALANCING` was used only when `SENDINBLUE_ENABLED` was *disabled* (otherwise only SendInBlue was ever used). This commit: - Ensure that `SENDINBLUE_BALANCING` is used only when SendInBlue is *enabled* (which is more intuitive). - Make it easier to add other delivery methods. --- app/lib/mailtrap/smtp.rb | 4 +++ app/lib/sendinblue/smtp.rb | 4 +++ config/environments/production.rb | 33 +++++++++++-------- .../dynamic_smtp_settings_interceptor.rb | 26 ++++++++------- 4 files changed, 43 insertions(+), 24 deletions(-) create mode 100644 app/lib/mailtrap/smtp.rb create mode 100644 app/lib/sendinblue/smtp.rb diff --git a/app/lib/mailtrap/smtp.rb b/app/lib/mailtrap/smtp.rb new file mode 100644 index 000000000..93b57f412 --- /dev/null +++ b/app/lib/mailtrap/smtp.rb @@ -0,0 +1,4 @@ +module Mailtrap + class Smtp < ::Mail::SMTP + end +end diff --git a/app/lib/sendinblue/smtp.rb b/app/lib/sendinblue/smtp.rb new file mode 100644 index 000000000..cccb4872e --- /dev/null +++ b/app/lib/sendinblue/smtp.rb @@ -0,0 +1,4 @@ +module Sendinblue + class Smtp < ::Mail::SMTP + end +end diff --git a/config/environments/production.rb b/config/environments/production.rb index cdfeaed9b..b006c9ec3 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -1,4 +1,6 @@ require "active_support/core_ext/integer/time" +require Rails.root.join("app/lib/mailtrap/smtp") +require Rails.root.join("app/lib/sendinblue/smtp") Rails.application.configure do # Settings specified here will take precedence over those in config/application.rb. @@ -76,8 +78,8 @@ Rails.application.configure do # config.action_mailer.raise_delivery_errors = false if ENV['MAILTRAP_ENABLED'] == 'enabled' - config.action_mailer.delivery_method = :smtp - config.action_mailer.smtp_settings = { + ActionMailer::Base.add_delivery_method :mailtrap, Mailtrap::Smtp + config.action_mailer.mailtrap_settings = { user_name: Rails.application.secrets.mailtrap[:username], password: Rails.application.secrets.mailtrap[:password], address: 'smtp.mailtrap.io', @@ -85,17 +87,22 @@ Rails.application.configure do port: '2525', authentication: :cram_md5 } - elsif ENV['SENDINBLUE_ENABLED'] == 'enabled' - config.action_mailer.delivery_method = :smtp - config.action_mailer.smtp_settings = { - user_name: Rails.application.secrets.sendinblue[:username], - password: Rails.application.secrets.sendinblue[:smtp_key], - address: 'smtp-relay.sendinblue.com', - domain: 'smtp-relay.sendinblue.com', - port: '587', - authentication: :cram_md5 - } - else + config.action_mailer.delivery_method = :mailtrap + elsif + if ENV['SENDINBLUE_ENABLED'] == 'enabled' + ActionMailer::Base.add_delivery_method :sendinblue, Sendinblue::Smtp + config.action_mailer.sendinblue_settings = { + user_name: Rails.application.secrets.sendinblue[:username], + password: Rails.application.secrets.sendinblue[:smtp_key], + address: 'smtp-relay.sendinblue.com', + domain: 'smtp-relay.sendinblue.com', + port: '587', + authentication: :cram_md5 + } + end + + # Default delivery method + # (Actual delivery method will be selected at runtime by DynamicSmtpSettingsInterceptor) config.action_mailer.delivery_method = :mailjet end diff --git a/lib/action_mailer/dynamic_smtp_settings_interceptor.rb b/lib/action_mailer/dynamic_smtp_settings_interceptor.rb index 91d6398f8..2aeef5c5e 100644 --- a/lib/action_mailer/dynamic_smtp_settings_interceptor.rb +++ b/lib/action_mailer/dynamic_smtp_settings_interceptor.rb @@ -6,17 +6,21 @@ class DynamicSmtpSettingsInterceptor def self.delivering_email(message) - if ENV['SENDINBLUE_BALANCING'] == 'enabled' - if rand(0..99) < ENV['SENDINBLUE_BALANCING_VALUE'].to_i - message.delivery_method.settings = { - user_name: ENV['SENDINBLUE_USER_NAME'], - password: ENV['SENDINBLUE_SMTP_KEY'], - address: 'smtp-relay.sendinblue.com', - domain: 'smtp-relay.sendinblue.com', - port: '587', - authentication: :cram_md5 - } - end + if balance_to_sendinblue? + ApplicationMailer.wrap_delivery_behavior(message, :sendinblue) + end + # fallback to the default delivery method + end + + private + + def self.balance_to_sendinblue? + if ENV.fetch('SENDINBLUE_ENABLED') != 'enabled' + false + elsif ENV.fetch('SENDINBLUE_BALANCING') == 'enabled' + rand(0..99) < ENV.fetch('SENDINBLUE_BALANCING_VALUE').to_i + else + true end end end From 27b42fe8ae9424f1b1f567737960b284698efaf0 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 19 Jan 2022 20:30:01 +0000 Subject: [PATCH 08/17] config: better document `SENDINBLUE_BALANCING` --- config/env.example | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/config/env.example b/config/env.example index d2f9de017..8f3e332f8 100644 --- a/config/env.example +++ b/config/env.example @@ -70,28 +70,33 @@ MATOMO_ENABLED="disabled" MATOMO_ID="" MATOMO_HOST="matomo.organisme.fr" -# SMTP Provider: Send In Blue +# Default SMTP Provider: Mailjet +MAILJET_API_KEY="" +MAILJET_SECRET_KEY="" + +# Alternate SMTP Provider: SendInBlue SENDINBLUE_ENABLED="disabled" -SENDINBLUE_BALANCING="" -SENDINBLUE_BALANCING_VALUE="" SENDINBLUE_CLIENT_KEY="" SENDINBLUE_SMTP_KEY="" SENDINBLUE_USER_NAME="" # SENDINBLUE_LOGIN_URL="https://app.sendinblue.com/account/saml/login/truc" -# SMTP Provider: Mailjet -MAILJET_API_KEY="" -MAILJET_SECRET_KEY="" +# Ratio of emails sent using SendInBlue +# When enabled, N % of emails will be sent using SendInBlue +# (and the others using the default SMTP provider) +SENDINBLUE_BALANCING="disabled" +SENDINBLUE_BALANCING_VALUE="50" + +# Alternate SMTP Provider: Mailtrap (mail catcher for staging environments) +# When enabled, all emails will be sent using this provided +MAILTRAP_ENABLED="disabled" +MAILTRAP_USERNAME="" +MAILTRAP_PASSWORD="" # External service: live chat for admins (specific to démarches-simplifiées.fr) CRISP_ENABLED="disabled" CRISP_CLIENT_KEY="" -# External service: mail catcher for staging environments (specific to démarches-simplifiées.fr) -MAILTRAP_ENABLED="disabled" -MAILTRAP_USERNAME="" -MAILTRAP_PASSWORD="" - # API Entreprise credentials # https://api.gouv.fr/api/api-entreprise.html API_ENTREPRISE_KEY="" From 847abca1226466aabc31eaf795ed92a61d3c7fae Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 26 Jan 2022 10:33:12 +0000 Subject: [PATCH 09/17] config: simplify mailer configuration again Move everything to initializers, and replace the email settings interceptor by a BalancerDeliveryMethod. It has the advantage that it can be configured entirely from the `config/environment.rb` file, without an extra file to look at. --- app/lib/balancer_delivery_method.rb | 38 ++++++++ app/lib/mailtrap/smtp.rb | 4 - app/lib/sendinblue/smtp.rb | 4 - config/environments/development.rb | 39 ++------ config/environments/production.rb | 38 +++----- .../dynamic_smtp_settings_interceptor.rb | 14 --- config/initializers/helo.rb | 17 ++++ config/initializers/mailtrap.rb | 17 ++++ config/initializers/sendinblue.rb | 24 ++++- .../dynamic_smtp_settings_interceptor.rb | 26 ------ spec/lib/balancer_delivery_method_spec.rb | 93 +++++++++++++++++++ 11 files changed, 205 insertions(+), 109 deletions(-) create mode 100644 app/lib/balancer_delivery_method.rb delete mode 100644 app/lib/mailtrap/smtp.rb delete mode 100644 app/lib/sendinblue/smtp.rb delete mode 100644 config/initializers/dynamic_smtp_settings_interceptor.rb create mode 100644 config/initializers/helo.rb create mode 100644 config/initializers/mailtrap.rb delete mode 100644 lib/action_mailer/dynamic_smtp_settings_interceptor.rb create mode 100644 spec/lib/balancer_delivery_method_spec.rb diff --git a/app/lib/balancer_delivery_method.rb b/app/lib/balancer_delivery_method.rb new file mode 100644 index 000000000..85f468ae3 --- /dev/null +++ b/app/lib/balancer_delivery_method.rb @@ -0,0 +1,38 @@ +# A Mail delivery method that randomly balances the actual delivery between different +# methods. +# +# Usage: +# +# ```ruby +# ActionMailer::Base.add_delivery_method :balancer, BalancerDeliveryMethod +# config.action_mailer.balancer_settings = { +# smtp: 25, +# sendmail: 75 +# } +# config.action_mailer.delivery_method = :balancer +# ``` +# +# Be sure to restart your server when you modify this file. +class BalancerDeliveryMethod + # Allows configuring the random number generator used for selecting a delivery method, + # mostly for testing purposes. + mattr_accessor :random, default: Random.new + + def initialize(settings) + @delivery_methods = settings + end + + def deliver!(mail) + balanced_delivery_method = delivery_method(mail) + ApplicationMailer.wrap_delivery_behavior(mail, balanced_delivery_method) + mail.deliver + end + + private + + def delivery_method(mail) + @delivery_methods + .flat_map { |delivery_method, weight| [delivery_method] * weight } + .sample(random: self.class.random) + end +end diff --git a/app/lib/mailtrap/smtp.rb b/app/lib/mailtrap/smtp.rb deleted file mode 100644 index 93b57f412..000000000 --- a/app/lib/mailtrap/smtp.rb +++ /dev/null @@ -1,4 +0,0 @@ -module Mailtrap - class Smtp < ::Mail::SMTP - end -end diff --git a/app/lib/sendinblue/smtp.rb b/app/lib/sendinblue/smtp.rb deleted file mode 100644 index cccb4872e..000000000 --- a/app/lib/sendinblue/smtp.rb +++ /dev/null @@ -1,4 +0,0 @@ -module Sendinblue - class Smtp < ::Mail::SMTP - end -end diff --git a/config/environments/development.rb b/config/environments/development.rb index 2b0f92cbc..e0be95cc4 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -75,40 +75,13 @@ Rails.application.configure do config.assets.raise_runtime_errors = true # Action Mailer settings + config.action_mailer.delivery_method = :letter_opener - if ENV['SENDINBLUE_ENABLED'] == 'enabled' - config.action_mailer.delivery_method = :smtp - config.action_mailer.smtp_settings = { - user_name: Rails.application.secrets.sendinblue[:username], - password: Rails.application.secrets.sendinblue[:smtp_key], - address: 'smtp-relay.sendinblue.com', - domain: 'smtp-relay.sendinblue.com', - port: '587', - authentication: :cram_md5 - } - else - # https://usehelo.com - if ENV['HELO_ENABLED'] == 'enabled' - config.action_mailer.delivery_method = :smtp - config.action_mailer.smtp_settings = { - user_name: 'demarches-simplifiees', - password: '', - address: '127.0.0.1', - domain: '127.0.0.1', - port: ENV.fetch('HELO_PORT', '2525'), - authentication: :plain - } - else - config.action_mailer.delivery_method = :letter_opener_web - end - - config.action_mailer.default_url_options = { - host: 'localhost', - port: 3000 - } - - config.action_mailer.asset_host = "http://" + ENV['APP_HOST'] - end + config.action_mailer.default_url_options = { + host: 'localhost', + port: 3000 + } + config.action_mailer.asset_host = "http://" + ENV['APP_HOST'] Rails.application.routes.default_url_options = { host: 'localhost', diff --git a/config/environments/production.rb b/config/environments/production.rb index b006c9ec3..912426727 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -1,6 +1,5 @@ require "active_support/core_ext/integer/time" -require Rails.root.join("app/lib/mailtrap/smtp") -require Rails.root.join("app/lib/sendinblue/smtp") +require Rails.root.join("app/lib/balancer_delivery_method") Rails.application.configure do # Settings specified here will take precedence over those in config/application.rb. @@ -78,31 +77,20 @@ Rails.application.configure do # config.action_mailer.raise_delivery_errors = false if ENV['MAILTRAP_ENABLED'] == 'enabled' - ActionMailer::Base.add_delivery_method :mailtrap, Mailtrap::Smtp - config.action_mailer.mailtrap_settings = { - user_name: Rails.application.secrets.mailtrap[:username], - password: Rails.application.secrets.mailtrap[:password], - address: 'smtp.mailtrap.io', - domain: 'smtp.mailtrap.io', - port: '2525', - authentication: :cram_md5 - } config.action_mailer.delivery_method = :mailtrap - elsif - if ENV['SENDINBLUE_ENABLED'] == 'enabled' - ActionMailer::Base.add_delivery_method :sendinblue, Sendinblue::Smtp - config.action_mailer.sendinblue_settings = { - user_name: Rails.application.secrets.sendinblue[:username], - password: Rails.application.secrets.sendinblue[:smtp_key], - address: 'smtp-relay.sendinblue.com', - domain: 'smtp-relay.sendinblue.com', - port: '587', - authentication: :cram_md5 - } - end - # Default delivery method - # (Actual delivery method will be selected at runtime by DynamicSmtpSettingsInterceptor) + elsif ENV['SENDINBLUE_ENABLED'] == 'enabled' && ENV['SENDINBLUE_BALANCING'] == 'enabled' + ActionMailer::Base.add_delivery_method :balancer, BalancerDeliveryMethod + config.action_mailer.balancer_settings = { + sendinblue: ENV.fetch('SENDINBLUE_BALANCING_VALUE').to_i, + mailjet: 100 - ENV.fetch('SENDINBLUE_BALANCING_VALUE').to_i + } + config.action_mailer.delivery_method = :balancer + + elsif ENV['SENDINBLUE_ENABLED'] == 'enabled' + config.action_mailer.delivery_method = :sendinblue + + else config.action_mailer.delivery_method = :mailjet end diff --git a/config/initializers/dynamic_smtp_settings_interceptor.rb b/config/initializers/dynamic_smtp_settings_interceptor.rb deleted file mode 100644 index b91ee7e19..000000000 --- a/config/initializers/dynamic_smtp_settings_interceptor.rb +++ /dev/null @@ -1,14 +0,0 @@ -# We want to register an interceptor, but we can't make the action idempotent -# (because there's no way to peek at the currently registered interceptors). -# -# To make zeitwerk happy, instead signal that we don't want the -# DynamicSmtpSettingsInterceptor constant to be auto-loaded, by: -# - adding it to a non-autoloaded-path (/lib), -# - requiring it explicitely. -# -# See https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#autoloading-when-the-application-boots -require 'action_mailer/dynamic_smtp_settings_interceptor' - -ActiveSupport.on_load(:action_mailer) do - ActionMailer::Base.register_interceptor DynamicSmtpSettingsInterceptor -end diff --git a/config/initializers/helo.rb b/config/initializers/helo.rb new file mode 100644 index 000000000..ed119ba81 --- /dev/null +++ b/config/initializers/helo.rb @@ -0,0 +1,17 @@ +if ENV['HELO_ENABLED'] == 'enabled' + ActiveSupport.on_load(:action_mailer) do + module Helo + class SMTP < ::Mail::SMTP; end + end + + ActionMailer::Base.add_delivery_method :helo, Helo::SMTP + ActionMailer::Base.helo_settings = { + user_name: 'demarches-simplifiees', + password: '', + address: '127.0.0.1', + domain: '127.0.0.1', + port: ENV.fetch('HELO_PORT', '2525'), + authentication: :plain + } + end +end diff --git a/config/initializers/mailtrap.rb b/config/initializers/mailtrap.rb new file mode 100644 index 000000000..6d1faa04b --- /dev/null +++ b/config/initializers/mailtrap.rb @@ -0,0 +1,17 @@ +if ENV.fetch('MAILTRAP_ENABLED') == 'enabled' + ActiveSupport.on_load(:action_mailer) do + module Mailtrap + class SMTP < ::Mail::SMTP; end + end + + ActionMailer::Base.add_delivery_method :mailtrap, Mailtrap::SMTP + ActionMailer::Base.mailtrap_settings = { + user_name: Rails.application.secrets.mailtrap[:username], + password: Rails.application.secrets.mailtrap[:password], + address: 'smtp.mailtrap.io', + domain: 'smtp.mailtrap.io', + port: '2525', + authentication: :cram_md5 + } + end +end diff --git a/config/initializers/sendinblue.rb b/config/initializers/sendinblue.rb index d523b2373..206b3905d 100644 --- a/config/initializers/sendinblue.rb +++ b/config/initializers/sendinblue.rb @@ -1,5 +1,23 @@ -require 'sib-api-v3-sdk' +if ENV.fetch('SENDINBLUE_ENABLED') == 'enabled' + require 'sib-api-v3-sdk' -SibApiV3Sdk.configure do |config| - config.api_key['api-key'] = ENV.fetch('SENDINBLUE_API_V3_KEY', '') + ActiveSupport.on_load(:action_mailer) do + module Sendinblue + class SMTP < ::Mail::SMTP; end + end + + ActionMailer::Base.add_delivery_method :sendinblue, Sendinblue::SMTP + ActionMailer::Base.sendinblue_settings = { + user_name: Rails.application.secrets.sendinblue[:username], + password: Rails.application.secrets.sendinblue[:smtp_key], + address: 'smtp-relay.sendinblue.com', + domain: 'smtp-relay.sendinblue.com', + port: '587', + authentication: :cram_md5 + } + end + + SibApiV3Sdk.configure do |config| + config.api_key['api-key'] = Rails.application.secrets.sendinblue[:api_v3_key] + end end diff --git a/lib/action_mailer/dynamic_smtp_settings_interceptor.rb b/lib/action_mailer/dynamic_smtp_settings_interceptor.rb deleted file mode 100644 index 2aeef5c5e..000000000 --- a/lib/action_mailer/dynamic_smtp_settings_interceptor.rb +++ /dev/null @@ -1,26 +0,0 @@ -# Note: this class is instanciated when being added as an interceptor -# during the app initialization. -# -# If you edit this file in development env, you will need to restart -# the app to see the changes. - -class DynamicSmtpSettingsInterceptor - def self.delivering_email(message) - if balance_to_sendinblue? - ApplicationMailer.wrap_delivery_behavior(message, :sendinblue) - end - # fallback to the default delivery method - end - - private - - def self.balance_to_sendinblue? - if ENV.fetch('SENDINBLUE_ENABLED') != 'enabled' - false - elsif ENV.fetch('SENDINBLUE_BALANCING') == 'enabled' - rand(0..99) < ENV.fetch('SENDINBLUE_BALANCING_VALUE').to_i - else - true - end - end -end diff --git a/spec/lib/balancer_delivery_method_spec.rb b/spec/lib/balancer_delivery_method_spec.rb new file mode 100644 index 000000000..b44fbdad8 --- /dev/null +++ b/spec/lib/balancer_delivery_method_spec.rb @@ -0,0 +1,93 @@ +RSpec.describe BalancerDeliveryMethod do + class ExampleMailer < ApplicationMailer + def greet(name) + mail(to: "smtp_to", from: "smtp_from", body: "Hello #{name}") + end + end + + class TestMail + def self.deliveries + @deliveries ||= [] + end + + def self.deliveries=(val) + @deliveries = val + end + + attr_accessor :settings + + def initialize(values) + @settings = values.dup + end + + def deliver!(mail) + Mail::CheckDeliveryParams.check(mail) + self.class.deliveries << mail + end + end + + class MockSmtp < TestMail; end + + class MockSendmail < TestMail; end + + class FixedSequence + def initialize(sequence) + @enumerator = sequence.each + end + + def rand(_) + @enumerator.next + end + end + + before do + ActionMailer::Base.add_delivery_method :mock_smtp, MockSmtp + ActionMailer::Base.add_delivery_method :mock_sendmail, MockSendmail + ActionMailer::Base.add_delivery_method :balancer, BalancerDeliveryMethod + + ExampleMailer.delivery_method = :balancer + end + + context 'when a single delivery method is provided' do + before do + ActionMailer::Base.balancer_settings = { mock_smtp: 10 } + end + + it 'sends emails to the selected delivery method' do + mail = ExampleMailer.greet('Joshua').deliver_now + expect(mail).to have_been_delivered_using(MockSmtp) + end + end + + context 'when multiple delivery methods are provided' do + before do + ActionMailer::Base.balancer_settings = { mock_smtp: 10, mock_sendmail: 5 } + + rng_sequence = [3, 14, 1] + BalancerDeliveryMethod.random = FixedSequence.new(rng_sequence) + end + + after do + BalancerDeliveryMethod.random = Random.new + end + + it 'sends emails randomly, given the provided weights' do + mail1 = ExampleMailer.greet('Lucia').deliver_now + expect(mail1).to have_been_delivered_using(MockSmtp) + + mail2 = ExampleMailer.greet('Damian').deliver_now + expect(mail2).to have_been_delivered_using(MockSendmail) + + mail3 = ExampleMailer.greet('Rahwa').deliver_now + expect(mail3).to have_been_delivered_using(MockSmtp) + end + end + + # Helpers + + def have_been_delivered_using(delivery_class) + satisfy("have been delivered using #{delivery_class}") do |mail| + delivery_class.deliveries.include?(mail) + end + end +end From bebc78b5874a3b2560dc2a779e1a811a70575259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Vantomme?= Date: Wed, 1 Dec 2021 10:24:32 +0100 Subject: [PATCH 10/17] chore(rubocop): replace deprecated rspec-focused by rubocop-rspec WARNING: We are no longer maintaining this gem, and will eventually archive this repo. We recommend you use rubocop-rspec instead. https://github.com/CarooDev/rubocop-rspec-focused --- .rubocop.yml | 4 ++-- Gemfile | 2 +- Gemfile.lock | 7 ++++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index ea5ad74b8..9a84c37d4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,7 +1,7 @@ require: - rubocop-performance - rubocop-rails - - rubocop/rspec/focused + - rubocop-rspec - ./lib/cops/add_concurrent_index.rb - ./lib/cops/application_name.rb - ./lib/cops/unscoped.rb @@ -877,7 +877,7 @@ Rails/WhereExists: Rails/WhereNot: Enabled: true -RSpec/Focused: +RSpec/Focus: Enabled: true Security/Eval: diff --git a/Gemfile b/Gemfile index e6ebfaf0a..b56231cea 100644 --- a/Gemfile +++ b/Gemfile @@ -113,7 +113,7 @@ group :development do gem 'rubocop', require: false gem 'rubocop-performance', require: false gem 'rubocop-rails', require: false - gem 'rubocop-rspec-focused', require: false + gem 'rubocop-rspec', require: false gem 'scss_lint', require: false gem 'web-console' end diff --git a/Gemfile.lock b/Gemfile.lock index 66ef40011..dad5cc90b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -627,8 +627,9 @@ GEM activesupport (>= 4.2.0) rack (>= 1.1) rubocop (>= 0.90.0, < 2.0) - rubocop-rspec-focused (1.0.0) - rubocop (>= 0.51) + rubocop-rspec (2.4.0) + rubocop (~> 1.0) + rubocop-ast (>= 1.1.0) ruby-graphviz (1.2.5) rexml ruby-progressbar (1.11.0) @@ -867,7 +868,7 @@ DEPENDENCIES rubocop rubocop-performance rubocop-rails - rubocop-rspec-focused + rubocop-rspec ruby-saml-idp sanitize-url sassc-rails From 8eaecd184a8d5df24303d1672eb2fdbf942fb678 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Vantomme?= Date: Wed, 2 Feb 2022 12:32:49 +0100 Subject: [PATCH 11/17] refactor(url): use env variables in content security policies --- config/initializers/{urls.rb => 02_urls.rb} | 4 ++- .../initializers/content_security_policy.rb | 31 ++++++++++++++----- 2 files changed, 26 insertions(+), 9 deletions(-) rename config/initializers/{urls.rb => 02_urls.rb} (93%) diff --git a/config/initializers/urls.rb b/config/initializers/02_urls.rb similarity index 93% rename from config/initializers/urls.rb rename to config/initializers/02_urls.rb index 732c583f2..6385e38e9 100644 --- a/config/initializers/urls.rb +++ b/config/initializers/02_urls.rb @@ -1,7 +1,9 @@ # rubocop:disable DS/ApplicationName # API URLs +API_ADRESSE_URL = ENV.fetch("API_ADRESSE_URL", "https://api-adresse.data.gouv.fr") API_ENTREPRISE_URL = ENV.fetch("API_ENTREPRISE_URL", "https://entreprise.api.gouv.fr/v2") API_EDUCATION_URL = ENV.fetch("API_EDUCATION_URL", "https://data.education.gouv.fr/api/records/1.0") +API_GEO_URL = ENV.fetch("API_GEO_URL", "https://geo.api.gouv.fr") API_PARTICULIER_URL = ENV.fetch("API_PARTICULIER_URL", "https://particulier.api.gouv.fr/api") HELPSCOUT_API_URL = ENV.fetch("HELPSCOUT_API_URL", "https://api.helpscout.net/v2") PIPEDRIVE_API_URL = ENV.fetch("PIPEDRIVE_API_URL", "https://api.pipedrive.com/v1") @@ -11,7 +13,7 @@ UNIVERSIGN_API_URL = ENV.fetch("UNIVERSIGN_API_URL", "https://ws.universign.eu/t FEATURE_UPVOTE_URL = ENV.fetch("FEATURE_UPVOTE_URL", "https://demarches-simplifiees.featureupvote.com") # Internal URLs -FOG_BASE_URL = "https://static.demarches-simplifiees.fr" +FOG_OPENSTACK_URL = ENV.fetch("FOG_OPENSTACK_URL", "https://static.demarches-simplifiees.fr") # External services URLs WEBINAIRE_URL = "https://app.livestorm.co/demarches-simplifiees" diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb index 17aa4aef0..73d7177a4 100644 --- a/config/initializers/content_security_policy.rb +++ b/config/initializers/content_security_policy.rb @@ -4,30 +4,45 @@ # For further information see the following documentation # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy -# rubocop:disable DS/ApplicationName Rails.application.config.content_security_policy do |policy| # Whitelist image - policy.img_src :self, "*.openstreetmap.org", "static.demarches-simplifiees.fr", "*.cloud.ovh.net", "stats.data.gouv.fr", "*", :data, :blob + images_whitelist = ["*.openstreetmap.org", "*.cloud.ovh.net", "*"] + images_whitelist << URI(FOG_OPENSTACK_URL).host if FOG_OPENSTACK_URL.present? + images_whitelist << URI(MATOMO_IFRAME_URL).host if MATOMO_IFRAME_URL.present? + policy.img_src(:self, :data, :blob, *images_whitelist) + # Whitelist JS: nous, sendinblue et matomo # miniprofiler et nous avons quelques boutons inline :( - policy.script_src :self, "stats.data.gouv.fr", "*.sendinblue.com", "*.crisp.chat", "crisp.chat", "*.sibautomation.com", "sibautomation.com", 'cdn.jsdelivr.net', 'maxcdn.bootstrapcdn.com', 'code.jquery.com', :unsafe_eval, :unsafe_inline, :blob + scripts_whitelist = ["*.sendinblue.com", "*.crisp.chat", "crisp.chat", "*.sibautomation.com", "sibautomation.com", "cdn.jsdelivr.net", "maxcdn.bootstrapcdn.com", "code.jquery.com"] + scripts_whitelist << URI(MATOMO_IFRAME_URL).host if MATOMO_IFRAME_URL.present? + policy.script_src(:self, :unsafe_eval, :unsafe_inline, :blob, *scripts_whitelist) + # Pour les CSS, on a beaucoup de style inline et quelques balises