Merge pull request #5583 from betagouv/better-admin-passwords

Ensure admin passwords are complex enough

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

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

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!

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
+    %b
       Pour vous connecter, cliquez sur le lien suivant :
-    = link_to(new_user_session_url, new_user_session_url)
+      = 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 }

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:

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

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

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

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

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

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

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