From 05193e1d1e5d55f6c222296173c972005b71d08d Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 12 Feb 2024 12:39:43 +0100 Subject: [PATCH] feat(User.validation): validates email when env var is present --- app/models/user.rb | 21 +++++- app/validators/strict_email_validator.rb | 16 +++-- config/locales/en.yml | 1 + config/locales/fr.yml | 1 + spec/models/user_spec.rb | 82 ++++++++++++++++++++++++ 5 files changed, 115 insertions(+), 6 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index f69f6a72d..fb9449717 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -33,10 +33,15 @@ class User < ApplicationRecord accepts_nested_attributes_for :france_connect_information default_scope { eager_load(:instructeur, :administrateur, :expert) } - before_validation -> { sanitize_email(:email) } + before_validation -> { sanitize_email(:email) } validate :does_not_merge_on_self, if: :requested_merge_into_id_changed? + with_options if: :elligible_to_new_validation? do + before_validation :remove_devise_email_validator + validates :email, strict_email: true + end + def validate_password_complexity? administrateur? end @@ -268,4 +273,18 @@ class User < ApplicationRecord def link_invites! Invite.where(email: email).update_all(user_id: id) end + + def elligible_to_new_validation? + StrictEmailValidator.elligible_to_new_validation?(self) + end + + def remove_devise_email_validator + _validators[:email]&.reject! { _1.is_a?(ActiveModel::Validations::FormatValidator) } + _validate_callbacks.each do |callback| + next if !callback.filter.is_a?(ActiveModel::Validations::FormatValidator) + next if !callback.filter.attributes.include? :email + + callback.filter.attributes.delete(:email) + end + end end diff --git a/app/validators/strict_email_validator.rb b/app/validators/strict_email_validator.rb index c2d6f4e97..d56d4005d 100644 --- a/app/validators/strict_email_validator.rb +++ b/app/validators/strict_email_validator.rb @@ -6,14 +6,22 @@ class StrictEmailValidator < ActiveModel::EachValidator REGEXP = /\A[^@\s]+@[^@\s\.]+\.[^@\s]{2,}\z/ def validate_each(record, attribute, value) - if value.present? && !REGEXP.match?(value) - record.errors.add(attribute, :format) + if value.present? && !regexp_for(record).match?(value) + record.errors.add(attribute, :invalid_email_format) + end + end + + def regexp_for(record) + if StrictEmailValidator.elligible_to_new_validation?(record) + REGEXP + else + Devise.email_regexp end end def self.elligible_to_new_validation?(record) return false if !strict_validation_enabled? - return false if (record.created_at || Time.now) < date_since_strict_email_validation + return false if (record.created_at || Time.zone.now) < date_since_strict_email_validation true end @@ -26,6 +34,4 @@ class StrictEmailValidator < ActiveModel::EachValidator rescue DateTime.new(1789, 5, 5, 0, 0) # french revolution, ds was not yet launched end - - end diff --git a/config/locales/en.yml b/config/locales/en.yml index e4ba5db91..ba8aefd6a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -605,6 +605,7 @@ en: not_a_phone: 'Invalid phone number' not_a_rna: 'Invalid RNA number' url: 'is not a valid link' + invalid_email_format: "is not a valid email" models: attestation_template: attributes: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 1a671e312..d37f9c1f9 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -608,6 +608,7 @@ fr: not_a_phone: 'Numéro de téléphone invalide' not_a_rna: 'Numéro RNA invalide' url: 'n’est pas un lien valide' + invalid_email_format: "n'est pas un email valide" models: attestation_template: attributes: diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index aa0f8486f..ba1e9e974 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -493,4 +493,86 @@ describe User, type: :model do end end end + + describe 'discard default devise validation when needed' do + let(:now) { Time.zone.now } + let(:before) { now - 1.day } + let(:after) { now + 1.day } + subject { user.valid? } + + shared_examples_for "validation of users.email was flacky" do + context 'when value is username' do + let(:email) { 'username' } + it { is_expected.to be_falsey } + end + + context 'when value does not contain extension' do + let(:email) { 'username@mailserver' } + # what we allowed but was a mistake + it { is_expected.to be_truthy } + end + + context 'when value include an alias' do + let(:email) { 'username+alias@mailserver.fr' } + it { is_expected.to be_truthy } + end + + context 'when value includes accents' do + let(:email) { 'tech@démarches.gouv.fr' } + it { is_expected.to be_falsey } + end + + context 'when value is the classic standard user@domain.ext' do + let(:email) { 'username@mailserver.domain' } + it { is_expected.to be_truthy } + end + end + + context 'when env var is not present' do + let(:user) { build(:user, email: email) } + before { allow(StrictEmailValidator).to receive(:strict_validation_enabled?).and_return(false).at_least(1) } + it_behaves_like "validation of users.email was flacky" + end + + context "record.created_at < ENV['STRICT_EMAIL_VALIDATION_STARTS_AT']" do + let(:user) { build(:user, email: email, created_at: before) } + before do + allow(StrictEmailValidator).to receive(:strict_validation_enabled?).and_return(true).at_least(1) + allow(StrictEmailValidator).to receive(:date_since_strict_email_validation).and_return(now).at_least(1) + end + it_behaves_like "validation of users.email was flacky" + end + + context "record.created_at > ENV['STRICT_EMAIL_VALIDATION_STARTS_AT']" do + let(:user) { build(:user, email: email, created_at: after) } + before do + allow(StrictEmailValidator).to receive(:strict_validation_enabled?).and_return(true).at_least(1) + allow(StrictEmailValidator).to receive(:date_since_strict_email_validation).and_return(now).at_least(1) + end + context 'when value is username' do + let(:email) { 'username' } + it { is_expected.to be_falsey } + end + + context 'when value does not contain extension' do + let(:email) { 'username@mailserver' } + it { is_expected.to be_falsey } + end + + context 'when value include an alias' do + let(:email) { 'username+alias@mailserver.fr' } + it { is_expected.to be_truthy } + end + + context 'when value includes accents' do + let(:email) { 'tech@démarches.gouv.fr' } + it { is_expected.to be_truthy } + end + + context 'when value is the classic standard user@domain.ext' do + let(:email) { 'username@mailserver.domain' } + it { is_expected.to be_truthy } + end + end + end end