diff --git a/app/mailers/administration_mailer.rb b/app/mailers/administration_mailer.rb index efd2c573a..4718af99e 100644 --- a/app/mailers/administration_mailer.rb +++ b/app/mailers/administration_mailer.rb @@ -2,13 +2,13 @@ class AdministrationMailer < ApplicationMailer layout 'mailers/layout' - def invite_admin(admin, reset_password_token, administration_id) + def invite_admin(user, reset_password_token, administration_id) @reset_password_token = reset_password_token - @admin = admin + @user = user @author_name = BizDev.full_name(administration_id) subject = "Activez votre compte administrateur" - mail(to: admin.email, + mail(to: user.email, subject: subject, reply_to: CONTACT_EMAIL) end diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index d32135baa..b78428aef 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -31,14 +31,6 @@ class Administrateur < ApplicationRecord user&.email end - # validate :password_complexity, if: Proc.new { |a| Devise.password_length.include?(a.password.try(:size)) } - - def password_complexity - if password.present? && ZxcvbnService.new(password).score < PASSWORD_COMPLEXITY_FOR_ADMIN - errors.add(:password, :not_strong) - end - end - def self.find_inactive_by_token(reset_password_token) self.inactive.with_reset_password_token(reset_password_token) end diff --git a/app/models/user.rb b/app/models/user.rb index e713006f0..1582b6a6c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -54,6 +54,14 @@ class User < ApplicationRecord before_validation -> { sanitize_email(:email) } + validate :password_complexity, if: -> (u) { u.administrateur.present? && Devise.password_length.include?(u.password.try(:size)) } + + def password_complexity + if password.present? && ZxcvbnService.new(password).score < PASSWORD_COMPLEXITY_FOR_ADMIN + errors.add(:password, :not_strong) + end + end + # Override of Devise::Models::Confirmable#send_confirmation_instructions def send_confirmation_instructions unless @raw_confirmation_token @@ -90,13 +98,7 @@ class User < ApplicationRecord end def invite_administrateur!(administration_id) - reset_password_token = nil - - if !active? - reset_password_token = set_reset_password_token - end - - AdministrationMailer.invite_admin(self, reset_password_token, administration_id).deliver_later + AdministrationMailer.invite_admin(self, set_reset_password_token, administration_id).deliver_later end def remind_invitation! diff --git a/app/views/administration_mailer/invite_admin.html.haml b/app/views/administration_mailer/invite_admin.html.haml index ee56b73a0..456c76654 100644 --- a/app/views/administration_mailer/invite_admin.html.haml +++ b/app/views/administration_mailer/invite_admin.html.haml @@ -7,17 +7,18 @@ Je vous remercie de l’intérêt que vous portez à notre outil de dématérialisation de démarches. %p - Votre compte administrateur a été créé pour l'adresse email #{@admin.email}. + Votre compte administrateur a été créé pour l'adresse email #{@user.email}. -- if @reset_password_token.present? +- if !@user.active? %p %b Pour l’activer, cliquez sur le lien suivant : = link_to(admin_activate_url(token: @reset_password_token), admin_activate_url(token: @reset_password_token)) - else %p - Pour vous connecter, cliquez sur le lien suivant : - = link_to(new_user_session_url, new_user_session_url) + %b + Pour vous connecter, cliquez sur le lien suivant : + = link_to edit_user_password_url(@user, reset_password_token: @reset_password_token), edit_user_password_url(@user, reset_password_token: @reset_password_token) %p = render partial: "layouts/mailers/bizdev_signature", locals: { author_name: @author_name } diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 465afd67b..42641fdd3 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -82,6 +82,7 @@ fr: taken: déjà utilisé password: too_short: 'est trop court' + not_strong: 'n’est pas assez complexe' password_confirmation: confirmation: ': Les deux mots de passe ne correspondent pas' invite: diff --git a/spec/controllers/webhook_controller_spec.rb b/spec/controllers/webhook_controller_spec.rb index 534983c84..b5548e224 100644 --- a/spec/controllers/webhook_controller_spec.rb +++ b/spec/controllers/webhook_controller_spec.rb @@ -25,7 +25,7 @@ describe WebhookController, type: :controller do end context 'when there is a matching user' do - let(:user) { create(:user) } + let(:user) { create(:user, :with_strong_password) } let(:customer_email) { user.email } it 'returns a 200 response' do diff --git a/spec/factories/administrateur.rb b/spec/factories/administrateur.rb index c82404e2c..b665f9249 100644 --- a/spec/factories/administrateur.rb +++ b/spec/factories/administrateur.rb @@ -3,7 +3,7 @@ FactoryBot.define do factory :administrateur do transient do email { generate(:administrateur_email) } - password { 'mon chien aime les bananes' } + password { 'Mon [hien 4im3 {es banane$' } end initialize_with do diff --git a/spec/factories/user.rb b/spec/factories/user.rb index 55c6d258e..a5bcfa1a4 100644 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -8,5 +8,9 @@ FactoryBot.define do trait :unconfirmed do confirmed_at { nil } end + + trait :with_strong_password do + password { '{my-%s3cure[]-p4$$w0rd' } + end end end diff --git a/spec/mailers/administration_mailer_spec.rb b/spec/mailers/administration_mailer_spec.rb index 0646b7a12..e2e8e875a 100644 --- a/spec/mailers/administration_mailer_spec.rb +++ b/spec/mailers/administration_mailer_spec.rb @@ -1,12 +1,24 @@ RSpec.describe AdministrationMailer, type: :mailer do describe '#invite_admin' do - let(:admin) { create(:administrateur) } - let(:token) { "Toc toc toc" } + let(:admin_user) { create(:user, last_sign_in_at: last_sign_in_at) } + let(:token) { "some_token" } let(:administration_id) { BizDev::PIPEDRIVE_ID } + let(:last_sign_in_at) { nil } - subject { described_class.invite_admin(admin, token, administration_id) } + subject { described_class.invite_admin(admin_user, token, administration_id) } it { expect(subject.subject).not_to be_empty } + + describe "when the user has not been activated" do + it { expect(subject.body).to include(admin_activate_path(token: token)) } + it { expect(subject.body).not_to include(edit_user_password_url(admin_user, reset_password_token: token)) } + end + + describe "when the user is already active" do + let(:last_sign_in_at) { Time.zone.now } + it { expect(subject.body).not_to include(admin_activate_path(token: token)) } + it { expect(subject.body).to include(edit_user_password_url(admin_user, reset_password_token: token)) } + end end describe '#refuse_admin' do diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb index 6b449d900..f8c3e9a85 100644 --- a/spec/models/administrateur_spec.rb +++ b/spec/models/administrateur_spec.rb @@ -75,34 +75,4 @@ describe Administrateur, type: :model do expect(Administrateur.find_by(id: administrateur.id)).to be_nil end end - - # describe '#password_complexity' do - # let(:email) { 'mail@beta.gouv.fr' } - # let(:passwords) { ['pass', '12pass23', 'démarches ', 'démarches-simple', 'my-s3cure-p4ssword'] } - # let(:administrateur) { build(:administrateur, email: email, password: password) } - # let(:min_complexity) { PASSWORD_COMPLEXITY_FOR_ADMIN } - - # subject do - # administrateur.save - # administrateur.errors.full_messages - # end - - # context 'when password is too short' do - # let(:password) { 's' * (PASSWORD_MIN_LENGTH - 1) } - - # it { expect(subject).to eq(["Le mot de passe est trop court"]) } - # end - - # context 'when password is too simple' do - # let(:password) { passwords[min_complexity - 1] } - - # it { expect(subject).to eq(["Le mot de passe n'est pas assez complexe"]) } - # end - - # context 'when password is acceptable' do - # let(:password) { passwords[min_complexity] } - - # it { expect(subject).to eq([]) } - # end - # end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 54b6a856e..8dd086276 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -186,7 +186,9 @@ describe User, type: :model do subject end - it { expect(AdministrationMailer).to have_received(:invite_admin).with(user, nil, administration.id) } + it 'receives an invitation to update its password' do + expect(AdministrationMailer).to have_received(:invite_admin).with(user, kind_of(String), administration.id) + end end end @@ -293,4 +295,55 @@ describe User, type: :model do end end end + + describe '#password_complexity' do + # This password list is sorted by password complexity, according to zxcvbn (used for complexity evaluation) + # 0 - too guessable: risky password. (guesses < 10^3) + # 1 - very guessable: protection from throttled online attacks. (guesses < 10^6) + # 2 - somewhat guessable: protection from unthrottled online attacks. (guesses < 10^8) + # 3 - safely unguessable: moderate protection from offline slow-hash scenario. (guesses < 10^10) + # 4 - very unguessable: strong protection from offline slow-hash scenario. (guesses >= 10^10) + passwords = ['pass', '12pass23', 'démarches ', 'démarches-simple', '{My-$3cure-p4ssWord}'] + min_complexity = PASSWORD_COMPLEXITY_FOR_ADMIN + + context 'administrateurs' do + let(:email) { 'mail@beta.gouv.fr' } + let(:administrateur) { build(:user, email: email, password: password, administrateur: build(:administrateur)) } + + subject do + administrateur.save + administrateur.errors.full_messages + end + + context 'when password is too short' do + let(:password) { 's' * (PASSWORD_MIN_LENGTH - 1) } + + it { expect(subject).to eq(["Le mot de passe est trop court"]) } + end + + context 'when password is too simple' do + passwords[0..(min_complexity - 1)].each do |password| + let(:password) { password } + + it { expect(subject).to eq(["Le mot de passe n’est pas assez complexe"]) } + end + end + + context 'when password is acceptable' do + let(:password) { passwords[min_complexity] } + + it { expect(subject).to eq([]) } + end + end + + context 'simple users' do + passwords.each do |password| + let(:user) { build(:user, email: 'some@email.fr', password: password) } + it 'has no complexity validation' do + user.save + expect(user.errors.full_messages).to eq([]) + end + end + end + end end diff --git a/spec/services/administrateur_usage_statistics_service_spec.rb b/spec/services/administrateur_usage_statistics_service_spec.rb index 835206df7..17749a21c 100644 --- a/spec/services/administrateur_usage_statistics_service_spec.rb +++ b/spec/services/administrateur_usage_statistics_service_spec.rb @@ -36,7 +36,7 @@ describe AdministrateurUsageStatisticsService do context 'for an administrateur that has plenty of things' do let(:administrateur) do create(:administrateur, - user: create(:user, sign_in_count: 17, current_sign_in_at: Time.zone.local(2019, 3, 7), last_sign_in_at: Time.zone.local(2019, 2, 27)), + user: create(:user, :with_strong_password, sign_in_count: 17, current_sign_in_at: Time.zone.local(2019, 3, 7), last_sign_in_at: Time.zone.local(2019, 2, 27)), services: [create(:service)], instructeurs: [create(:instructeur)]) end