Merge pull request #1367 from betagouv/fix-1285

Ensure password strength

Gemfile

@@ -113,6 +113,8 @@ gem 'webpacker', '>= 4.0.x'
 
 gem 'after_party'
 
+gem 'zxcvbn-ruby', require: 'zxcvbn'
+
 # Cron jobs
 gem 'delayed_job_active_record'
 gem "daemons"

Gemfile.lock

@@ -799,6 +799,7 @@ GEM
       nokogiri (~> 1.8)
     xray-rails (0.3.1)
       rails (>= 3.1.0)
+    zxcvbn-ruby (0.1.2)
 
 PLATFORMS
   ruby
@@ -901,6 +902,7 @@ DEPENDENCIES
   webmock
   webpacker (>= 4.0.x)
   xray-rails
+  zxcvbn-ruby
 
 BUNDLED WITH
    1.16.4

app/assets/stylesheets/new_design/administrateurs_activate.scss

@@ -0,0 +1,4 @@
+.administrateurs-activate.container {
+  margin-top: 60px;
+  margin-bottom: 60px;
+}

app/assets/stylesheets/new_design/common.scss

@@ -45,6 +45,14 @@ a {
   color: $blue;
 }
 
+em {
+  font-style: italic;
+}
+
+strong {
+  font-weight: bold;
+}
+
 .container {
   @include horizontal-padding($default-padding);
   max-width: $page-width + 2 * $default-padding;

app/assets/stylesheets/new_design/layouts.scss

@@ -40,3 +40,8 @@
     }
   }
 }
+
+.one-column-centered {
+  margin: auto;
+  max-width: $page-width / 2;
+}

app/assets/stylesheets/new_design/password_strength.scss

@@ -0,0 +1,34 @@
+@import "colors";
+@import "constants";
+
+$strength-bg: #EEEEEE;
+$weak-strength-color: $lighter-red;
+$medium-strength-color: $orange;
+$strong-strength-color: $green;
+
+.password-strength {
+  margin-top: -24px;
+  width: 100%;
+  min-height: 22px;
+  background: $strength-bg;
+  display: block;
+  margin-bottom: $default-spacer;
+  text-align: center;
+
+  &.strength-1 {
+    background: linear-gradient(to right, $weak-strength-color 0%, $weak-strength-color 25%, $strength-bg 25%, $strength-bg 100%);
+  }
+
+  &.strength-2 {
+    background: linear-gradient(to right, $medium-strength-color 0%, $medium-strength-color 50%, $strength-bg 50%, $strength-bg 100%);
+  }
+
+  &.strength-3 {
+    background: linear-gradient(to right, $medium-strength-color 0%, $medium-strength-color 75%, $strength-bg 75%, $strength-bg 100%);
+  }
+
+  &.strength-4 {
+    background: $strong-strength-color;
+    color: #FFFFFF;
+  }
+}

app/controllers/administrateurs/activate_controller.rb

@@ -1,3 +1,5 @@
+require 'zxcvbn'
+
 class Administrateurs::ActivateController < ApplicationController
   layout "new_application"
 
@@ -29,6 +31,11 @@ class Administrateurs::ActivateController < ApplicationController
     end
   end
 
+  def test_password_strength
+    score = Zxcvbn.test(params[:password], [], ZXCVBN_DICTIONNARIES).score
+    render json: { score: score }
+  end
+
   private
 
   def update_administrateur_params

app/javascript/new_design/password-strength.js

@@ -0,0 +1,34 @@
+import $ from 'jquery';
+
+export function displayPasswordStrength(strengthBarId, score) {
+  var $bar = $('#' + strengthBarId),
+    passwordMessage;
+
+  $bar.removeClass('strength-1 strength-2 strength-3 strength-4');
+
+  if (score < 4) {
+    passwordMessage = 'Mot de passe pas assez complexe';
+  } else {
+    passwordMessage = 'Mot de passe suffisamment complexe';
+  }
+
+  $bar.text(passwordMessage);
+  $bar.addClass('strength-' + score);
+}
+
+export function checkPasswordStrength(event, strengthBarId) {
+  var $target = $(event.target),
+    password = $target.val();
+
+  if (password.length > 2) {
+    $.post(
+      '/admin/activate/test_password_strength',
+      { password: password },
+      function(data) {
+        displayPasswordStrength(strengthBarId, data.score);
+      }
+    );
+  } else {
+    displayPasswordStrength(strengthBarId, 0);
+  }
+}

app/javascript/packs/application.js

@@ -24,6 +24,10 @@ import { toggleCondidentielExplanation } from '../new_design/avis';
 import { togglePrintMenu } from '../new_design/dossier';
 import { toggleHeaderMenu } from '../new_design/header';
 import { scrollMessagerie } from '../new_design/messagerie';
+import {
+  checkPasswordStrength,
+  displayPasswordStrength
+} from '../new_design/password-strength';
 import { showMotivation, motivationCancel } from '../new_design/state-button';
 import { toggleChart } from '../new_design/toggle-chart';
 
@@ -33,6 +37,8 @@ const DS = {
   togglePrintMenu,
   toggleHeaderMenu,
   scrollMessagerie,
+  checkPasswordStrength,
+  displayPasswordStrength,
   showMotivation,
   motivationCancel,
   toggleChart

app/models/administrateur.rb

@@ -17,6 +17,17 @@ class Administrateur < ApplicationRecord
 
   scope :inactive, -> { where(active: false) }
 
+  validate :password_complexity, if: Proc.new { |a| Devise.password_length.include?(a.password.try(:size)) }
+
+  def password_complexity
+    if password.present?
+      score = Zxcvbn.test(password, [], ZXCVBN_DICTIONNARIES).score
+      if score < 4
+        errors.add(:password, :not_strength)
+      end
+    end
+  end
+
   def self.find_inactive_by_token(reset_password_token)
     self.inactive.with_reset_password_token(reset_password_token)
   end

app/views/administrateurs/activate/new.html.haml

@@ -1,8 +1,29 @@
-.container
+- content_for(:title, "Choix du mot de passe")
+
+- content_for :footer do
+  = render partial: "root/footer"
+
+.administrateurs-activate.container
+  .one-column-centered
     = form_for @administrateur, url: { controller: 'administrateurs/activate', action: :create }, html: { class: "form" } do |f|
       %br
       %h1
-      = @administrateur.email
-    = f.password_field :password, placeholder: 'Mot de passe'
+        Choix du mot de passe
+
+      = f.label :email, "Email"
+      = f.text_field :email, disabled: true
+
+      = f.label :password do
+        Mot de passe
+
+      = f.password_field :password, placeholder: 'Mot de passe', onKeyUp: "javascript:DS.checkPasswordStrength(event, 'strength-bar', 'submit-password');"
+
+      #strength-bar.password-strength
+         
+
+      .explication
+        %strong Aide :
+        Une courte phrase peut être un mot de passe très sécurisé.
+
       = f.hidden_field :reset_password_token, value: params[:token]
-    = f.submit 'Définir le mot de passe', class: 'button large primary expand'
+      = f.submit 'Continuer', class: 'button large primary expand', id: "submit-password", data: { disable_with: "Envoi..." }

config/initializers/zxcvbn.rb

@@ -0,0 +1 @@
+ZXCVBN_DICTIONNARIES = YAML.safe_load(File.read(Rails.root.join("config", "initializers", "zxcvbn_dictionnaries.yaml")))

config/locales/models/administrateur/fr.yml

@@ -0,0 +1,15 @@
+fr:
+  activerecord:
+    attributes:
+      administrateur:
+        password: Le mot de passe
+    errors:
+      models:
+        administrateur:
+          attributes:
+            email:
+              blank: 'doit être rempli'
+            password:
+              too_short: 'est trop court'
+              blank: 'doit être rempli'
+              not_strength: "n'est pas assez complexe"

config/routes.rb

@@ -175,6 +175,7 @@ Rails.application.routes.draw do
   namespace :admin do
     get 'activate' => '/administrateurs/activate#new'
     patch 'activate' => '/administrateurs/activate#create'
+    post 'activate/test_password_strength' => '/administrateurs/activate#test_password_strength'
     get 'sign_in' => '/administrateurs/sessions#new'
     get 'procedures/archived' => 'procedures#archived'
     get 'procedures/draft' => 'procedures#draft'

spec/controllers/gestionnaires/passwords_controller_spec.rb

@@ -7,9 +7,9 @@ describe Gestionnaires::PasswordsController, type: :controller do
 
   describe "update" do
     context "unified login" do
-      let(:gestionnaire) { create(:gestionnaire, email: 'unique@plop.com', password: 'password') }
-      let(:user) { create(:user, email: 'unique@plop.com', password: 'password') }
-      let(:administrateur) { create(:administrateur, email: 'unique@plop.com', password: 'password') }
+      let(:gestionnaire) { create(:gestionnaire, email: 'unique@plop.com', password: 'un super mot de passe') }
+      let(:user) { create(:user, email: 'unique@plop.com', password: 'un super mot de passe') }
+      let(:administrateur) { create(:administrateur, email: 'unique@plop.com', password: 'un super mot de passe') }
 
       before do
         @token = gestionnaire.send(:set_reset_password_token)

spec/controllers/users/passwords_controller_spec.rb

@@ -7,9 +7,9 @@ describe Users::PasswordsController, type: :controller do
 
   describe "update" do
     context "unified login" do
-      let(:user) { create(:user, email: 'unique@plop.com', password: 'password') }
-      let(:gestionnaire) { create(:gestionnaire, email: 'unique@plop.com', password: 'password') }
-      let(:administrateur) { create(:administrateur, email: 'unique@plop.com', password: 'password') }
+      let(:user) { create(:user, email: 'unique@plop.com', password: 'mot de passe complexe') }
+      let(:gestionnaire) { create(:gestionnaire, email: 'unique@plop.com', password: 'mot de passe complexe') }
+      let(:administrateur) { create(:administrateur, email: 'unique@plop.com', password: 'mot de passe complexe') }
 
       before do
         @token = user.send(:set_reset_password_token)
@@ -21,8 +21,8 @@ describe Users::PasswordsController, type: :controller do
         put :update, params: {
           user: {
             reset_password_token: @token,
-            password: "supersecret",
-            password_confirmation: "supersecret",
+            password: "mot de passe super secret",
+            password_confirmation: "mot de passe super secret",
           }
         }
         expect(subject.current_user).to eq(user)
@@ -33,8 +33,8 @@ describe Users::PasswordsController, type: :controller do
         put :update, params: {
           user: {
             reset_password_token: @token,
-            password: "supersecret",
-            password_confirmation: "supersecret",
+            password: "mot de passe super secret",
+            password_confirmation: "mot de passe super secret",
           }
         }
         expect(subject.current_user).to eq(user)

spec/controllers/users/sessions_controller_spec.rb

@@ -24,7 +24,7 @@ describe Users::SessionsController, type: :controller do
 
     context "unified login" do
       let(:email) { 'unique@plop.com' }
-      let(:password) { 'password' }
+      let(:password) { 'un super mot de passe' }
 
       let(:user) { create(:user, email: email, password: password) }
       let(:gestionnaire) { create(:gestionnaire, email: email, password: password) }
@@ -80,8 +80,8 @@ describe Users::SessionsController, type: :controller do
       end
 
       context 'with different passwords' do
-        let!(:gestionnaire) { create(:gestionnaire, email: email, password: 'another_password') }
-        let!(:administrateur) { create(:administrateur, email: email, password: 'another_password') }
+        let!(:gestionnaire) { create(:gestionnaire, email: email, password: 'mot de passe complexe') }
+        let!(:administrateur) { create(:administrateur, email: email, password: 'mot de passe complexe') }
 
         before do
           user
@@ -165,7 +165,7 @@ describe Users::SessionsController, type: :controller do
       end
 
       context "when associated administrateur" do
-        let(:administrateur) { create(:administrateur, email: 'unique@plop.com', password: 'password') }
+        let(:administrateur) { create(:administrateur, email: 'unique@plop.com') }
 
         it 'signs user + gestionnaire + administrateur out' do
           sign_in user

spec/factories/administrateur.rb

@@ -2,6 +2,6 @@ FactoryBot.define do
   sequence(:administrateur_email) { |n| "admin#{n}@admin.com" }
   factory :administrateur do
     email { generate(:administrateur_email) }
-    password { 'password' }
+    password { 'mon chien aime les bananes' }
   end
 end

spec/models/administrateur_spec.rb

@@ -9,7 +9,7 @@ describe Administrateur, type: :model do
   end
 
   describe 'after_save' do
-    subject { described_class.new(email: 'toto@tps.com', password: 'password') }
+    subject { create(:administrateur) }
     before do
       subject.save
     end
@@ -19,7 +19,7 @@ describe Administrateur, type: :model do
   describe 'generate_api_token' do
     let(:token) { 'bullshit' }
     let(:new_token) { 'pocket_master' }
-    let!(:admin_1) { create(:administrateur, email: 'toto@tps.com', password: 'password', api_token: token) }
+    let!(:admin_1) { create(:administrateur, api_token: token) }
     before do
       allow(SecureRandom).to receive(:hex).and_return(token, new_token)
       admin_1.renew_api_token
@@ -34,22 +34,22 @@ describe Administrateur, type: :model do
       administrateur = create(:administrateur)
       user = create(:user, email: administrateur.email)
 
-      administrateur.update(email: 'whoami@plop.com', password: 'super secret')
+      administrateur.update(email: 'whoami@plop.com', password: 'voilà un super mdp')
 
       user.reload
       expect(user.email).to eq('whoami@plop.com')
-      expect(user.valid_password?('super secret')).to be(true)
+      expect(user.valid_password?('voilà un super mdp')).to be(true)
     end
 
     it 'syncs credentials to associated administrateur' do
       administrateur = create(:administrateur)
       gestionnaire = create(:gestionnaire, email: administrateur.email)
 
-      administrateur.update(email: 'whoami@plop.com', password: 'super secret')
+      administrateur.update(email: 'whoami@plop.com', password: 'et encore un autre mdp')
 
       gestionnaire.reload
       expect(gestionnaire.email).to eq('whoami@plop.com')
-      expect(gestionnaire.valid_password?('super secret')).to be(true)
+      expect(gestionnaire.valid_password?('et encore un autre mdp')).to be(true)
     end
   end
 
@@ -64,8 +64,8 @@ describe Administrateur, type: :model do
     let(:administrateur) { create(:administration).invite_admin('paul@tps.fr') }
     let(:reset_password_token) { administrateur.invite!(administration.id) }
 
-    it { expect(Administrateur.reset_password(reset_password_token, '12345678').errors).to be_empty }
-    it { expect(Administrateur.reset_password('123', '12345678').errors).not_to be_empty }
+    it { expect(Administrateur.reset_password(reset_password_token, "j'aime manger des radis").errors).to be_empty }
+    it { expect(Administrateur.reset_password('123', "j'aime manger des radis").errors).not_to be_empty }
     it { expect(Administrateur.reset_password(reset_password_token, '').errors).not_to be_empty }
   end
 
@@ -79,4 +79,30 @@ describe Administrateur, type: :model do
     it { expect(administrateur.feature_enabled?(:champ_siret)).to be_falsey }
     it { expect(administrateur.feature_enabled?(:champ_pj)).to be_truthy }
   end
+
+  describe "#password_complexity" do
+    let(:administrateur) { build(:administrateur, password: password) }
+
+    subject do
+      administrateur.save
+      administrateur.errors[:password]
+    end
+
+    context "with a strong password" do
+      let(:password) { "la démat c'est simple" }
+      it { expect(subject).to eq([]) }
+    end
+
+    context "with a weak password" do
+      let(:password) { "12345678" }
+      it { expect(subject).to include "n'est pas assez complexe" }
+      it { expect(subject).not_to include "est trop court" }
+    end
+
+    context "with a short password" do
+      let(:password) { "1" }
+      it { expect(subject).to include "est trop court" }
+      it { expect(subject).not_to include "n'est pas assez complexe" }
+    end
+  end
 end