Merge pull request #6417 from betagouv/fix-password-strength-meter

Administrateur : rétablissement de l'indicateur de complexité lors d'un changement de mot de passe (#6417)
This commit is contained in:
Pierre de La Morinerie 2021-09-09 09:58:29 -05:00 committed by GitHub
commit 92ca41ee4b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
33 changed files with 308 additions and 249 deletions

View file

@ -0,0 +1,40 @@
@import "colors";
@import "constants";
$complexity-bg: #EEEEEE;
$complexity-color-0: $lighter-red;
$complexity-color-1: #FF5000;
$complexity-color-2: $orange;
$complexity-color-3: #FFD000;
$complexity-color-4: $green;
.password-complexity {
margin-top: -24px;
width: 100%;
height: 12px;
background: $complexity-bg;
display: block;
margin-bottom: $default-spacer;
text-align: center;
border-radius: 8px;
&.complexity-0 {
background: linear-gradient(to right, $complexity-color-0 00%, $complexity-bg 20%);
}
&.complexity-1 {
background: linear-gradient(to right, $complexity-color-1 20%, $complexity-bg 40%);
}
&.complexity-2 {
background: linear-gradient(to right, $complexity-color-2 40%, $complexity-bg 60%);
}
&.complexity-3 {
background: linear-gradient(to right, $complexity-color-3 60%, $complexity-bg 80%);
}
&.complexity-4 {
background: $complexity-color-4;
}
}

View file

@ -1,40 +0,0 @@
@import "colors";
@import "constants";
$strength-bg: #EEEEEE;
$strength-color-0: $lighter-red;
$strength-color-1: #FF5000;
$strength-color-2: $orange;
$strength-color-3: #FFD000;
$strength-color-4: $green;
.password-strength {
margin-top: -24px;
width: 100%;
height: 12px;
background: $strength-bg;
display: block;
margin-bottom: $default-spacer;
text-align: center;
border-radius: 8px;
&.strength-0 {
background: linear-gradient(to right, $strength-color-0 00%, $strength-bg 20%);
}
&.strength-1 {
background: linear-gradient(to right, $strength-color-1 20%, $strength-bg 40%);
}
&.strength-2 {
background: linear-gradient(to right, $strength-color-2 40%, $strength-bg 60%);
}
&.strength-3 {
background: linear-gradient(to right, $strength-color-3 60%, $strength-bg 80%);
}
&.strength-4 {
background: $strength-color-4;
}
}

View file

@ -1,14 +0,0 @@
class Administrateurs::PasswordsController < ApplicationController
def test_strength
@score, @words, @length = ZxcvbnService.new(password_params[:password]).complexity
@min_length = PASSWORD_MIN_LENGTH
@min_complexity = PASSWORD_COMPLEXITY_FOR_ADMIN
render 'shared/password/test_strength'
end
private
def password_params
params.require(:administrateur).permit(:password)
end
end

View file

@ -0,0 +1,13 @@
module DevisePopulatedResource
extend ActiveSupport::Concern
# During a GET /password/edit, the resource is a brand new object.
# This method gives access to the actual resource record, complete with email, relationships, etc.
def populated_resource
resource_class.with_reset_password_token(resource.reset_password_token)
end
included do
helper_method :populated_resource
end
end

View file

@ -0,0 +1,15 @@
class PasswordComplexityController < ApplicationController
def show
@score, @words, @length = ZxcvbnService.new(password_param).complexity
@min_length = PASSWORD_MIN_LENGTH
@min_complexity = PASSWORD_COMPLEXITY_FOR_ADMIN
end
private
def password_param
params
.transform_keys! { |k| params[k].try(:has_key?, :password) ? 'resource' : k }
.dig(:resource, :password)
end
end

View file

@ -1,19 +1,8 @@
class SuperAdmins::PasswordsController < Devise::PasswordsController
include DevisePopulatedResource
def update
super
self.resource.disable_otp!
end
def test_strength
@score, @words, @length = ZxcvbnService.new(password_params[:password]).complexity
@min_length = PASSWORD_MIN_LENGTH
@min_complexity = PASSWORD_COMPLEXITY_FOR_ADMIN
render 'shared/password/test_strength'
end
private
def password_params
params.require(:super_admin).permit(:password)
end
end

View file

@ -1,4 +1,6 @@
class Users::PasswordsController < Devise::PasswordsController
include DevisePopulatedResource
after_action :try_to_authenticate_instructeur, only: [:update]
after_action :try_to_authenticate_administrateur, only: [:update]
@ -8,19 +10,9 @@ class Users::PasswordsController < Devise::PasswordsController
# end
# POST /resource/password
def create
# Check the credentials associated to the mail to generate a correct reset link
email = params[:user][:email]
if Administrateur.by_email(email)
@devise_mapping = Devise.mappings[:administrateur]
params[:administrateur] = params[:user]
# uncomment to check password complexity for Instructeur
# elsif Instructeur.by_email(email)
# @devise_mapping = Devise.mappings[:instructeur]
# params[:instructeur] = params[:user]
end
super
end
# def create
# super
# end
# GET /resource/password/edit?reset_password_token=abcdef
# def edit
@ -37,16 +29,16 @@ class Users::PasswordsController < Devise::PasswordsController
@email = params[:email]
end
# protected
protected
# def after_resetting_password_path_for(resource)
# super(resource)
# end
# The path used after sending reset password instructions
# def after_sending_reset_password_instructions_path_for(resource_name)
# super(resource_name)
# end
def after_sending_reset_password_instructions_path_for(resource_name)
flash.discard(:notice)
users_password_reset_link_sent_path(email: resource.email)
end
def try_to_authenticate_instructeur
if user_signed_in?
@ -67,20 +59,4 @@ class Users::PasswordsController < Devise::PasswordsController
end
end
end
def test_strength
@score, @words, @length = ZxcvbnService.new(password_params[:password]).complexity
@min_length = PASSWORD_MIN_LENGTH
@min_complexity = PASSWORD_COMPLEXITY_FOR_USER
render 'shared/password/test_strength'
end
def password_params
params.require(:user).permit(:reset_password_token, :password)
end
def after_sending_reset_password_instructions_path_for(resource_name)
flash.discard(:notice)
users_password_reset_link_sent_path(email: resource.email)
end
end

View file

@ -0,0 +1,23 @@
module PasswordComplexityConcern
extend ActiveSupport::Concern
# Allows adding a condition to the password complexity validation.
# Default is yes. Can be overridden in included classes.
def validate_password_complexity?
true
end
included do
# Add a validator for password complexity.
#
# The validator triggers as soon as the password is long enough (to avoid presenting
# two errors when the password is too short, one about length and one about complexity).
validates :password, password_complexity: true, if: -> { password_has_minimum_length? && validate_password_complexity? }
end
private
def password_has_minimum_length?
self.class.password_length.include?(password.try(:size))
end
end

View file

@ -25,11 +25,11 @@
# updated_at :datetime
#
class SuperAdmin < ApplicationRecord
include PasswordComplexityConcern
devise :rememberable, :trackable, :validatable, :lockable, :async, :recoverable,
:two_factor_authenticatable, :otp_secret_encryption_key => Rails.application.secrets.otp_secret_key
validates :password, password_complexity: true, if: -> (u) { Devise.password_length.include?(u.password.try(:size)) }
def enable_otp!
self.otp_secret = SuperAdmin.generate_otp_secret
self.otp_required_for_login = true

View file

@ -31,6 +31,7 @@
#
class User < ApplicationRecord
include EmailSanitizableConcern
include PasswordComplexityConcern
enum loged_in_with_france_connect: {
particulier: 'particulier',
@ -57,7 +58,9 @@ class User < ApplicationRecord
before_validation -> { sanitize_email(:email) }
validates :password, password_complexity: true, if: -> (u) { u.administrateur.present? && Devise.password_length.include?(u.password.try(:size)) }
def validate_password_complexity?
administrateur?
end
# Override of Devise::Models::Confirmable#send_confirmation_instructions
def send_confirmation_instructions

View file

@ -16,7 +16,6 @@
= f.label :password do
Mot de passe
= render partial: 'shared/password/edit_password', locals: { form: f, controller: 'administrateurs/passwords' }
= render 'password_complexity/field', { form: f, test_complexity: true }
= f.submit 'Continuer', class: 'button large primary expand', id: "submit-password", data: { disable_with: "Envoi..." }

View file

@ -1,6 +0,0 @@
- content_for(:title, 'Changement de mot de passe')
- content_for :footer do
= render partial: 'root/footer'
= render 'shared/password/edit', test_password_strength: administrateurs_password_test_strength_path

View file

@ -14,8 +14,9 @@
= f.hidden_field :reset_password_token
= f.label 'Nouveau mot de passe'
= render 'password_complexity/field', { form: f, test_complexity: populated_resource.validate_password_complexity? }
= render partial: 'shared/password/edit_password', locals: { form: f, controller: 'super_admins/passwords' }
= f.label 'Confirmez le nouveau mot de passe'
= f.password_field :password_confirmation, autocomplete: 'off'
= f.submit 'Changer le mot de passe', class: 'button large primary expand', id: "submit-password", data: { disable_with: "Envoi..." }
= f.submit 'Changer le mot de passe', class: 'button large primary expand', id: "submit-password", data: { disable_with: "Envoi…" }

View file

@ -0,0 +1 @@
#complexity-bar.password-complexity{ class: "complexity-#{@length < @min_length ? @score/2 : @score}" }

View file

@ -0,0 +1,9 @@
= form.password_field :password, autofocus: true, autocomplete: 'off', placeholder: 'Mot de passe', data: { remote: test_complexity, url: show_password_complexity_path }
- if test_complexity
#complexity-bar.password-complexity
.explication
#complexity-label{ style: 'font-weight: bold' }
Inscrivez un mot de passe.
Une courte phrase avec ponctuation peut être un mot de passe très sécurisé.

View file

@ -1,4 +1,4 @@
#strength-label{ style: 'font-weight: bold' }
#complexity-label{ style: 'font-weight: bold' }
- if @length > 0
- if @length < @min_length
Le mot de passe doit faire au moins #{@min_length} caractères.

View file

@ -0,0 +1,3 @@
<%= render_to_element('#complexity-label', partial: 'label', outer: true) %>
<%= render_to_element('#complexity-bar', partial: 'bar', outer: true) %>
<%= raw("document.querySelector('#submit-password').disabled = #{@score < @min_complexity || @length < @min_length};") %>

View file

@ -1,21 +0,0 @@
- content_for(:title, 'Changement de mot de passe')
- content_for :footer do
= render partial: 'root/footer'
.container.devise-container
.one-column-centered
= devise_error_messages!
= form_for(resource, as: resource_name, url: password_path(resource_name), html: { method: :patch, class: 'form' }) do |f|
%h1 Changement de mot de passe
= f.hidden_field :reset_password_token, value: params[:token]
= f.label 'Nouveau mot de passe'
= render partial 'shared/password/edit_password', locals: { form: f, controller: controller }
= f.submit 'Changer le mot de passe', class: 'button large primary expand', id: "submit-password", data: { disable_with: "Envoi..." }

View file

@ -1,10 +0,0 @@
= form.password_field :password, autofocus: true, autocomplete: 'off', placeholder: 'Mot de passe', data: { remote: true, url: url_for(controller: controller, action: 'test_strength') }
#strength-bar.password-strength
&nbsp;
.explication
#strength-label{ style: 'font-weight: bold' }
Inscrivez un mot de passe.
Une courte phrase avec ponctuation peut être un mot de passe très sécurisé.

View file

@ -1 +0,0 @@
#strength-bar.password-strength{ class: "strength-#{@length < @min_length ? @score/2 : @score}" }

View file

@ -1,3 +0,0 @@
<%= render_to_element('#strength-label', partial: 'shared/password/password_strength_label', outer: true) %>
<%= render_to_element('#strength-bar', partial: 'shared/password/password_strength', outer: true) %>
<%= raw("document.querySelector('#submit-password').disabled = #{@score < @min_complexity || @length < @min_length};") %>

View file

@ -1,17 +0,0 @@
- content_for(:title, 'Mot de passe oublié')
- content_for :footer do
= render partial: 'root/footer'
.container.devise-container
.one-column-centered
= devise_error_messages!
= form_for(resource, as: resource_name, url: password_path(resource_name), html: { class: 'form' }) do |f|
%h1 Mot de passe oublié
= f.label :email, 'Email'
= f.email_field :email, autofocus: true
= f.submit 'Demander un nouveau mot de passe', class: 'button large expand primary'

View file

@ -1,22 +0,0 @@
- content_for(:title, 'Changement de mot de passe')
- content_for :footer do
= render partial: 'root/footer'
.container.devise-container
.one-column-centered
= devise_error_messages!
= form_for(resource, as: resource_name, url: password_path(resource_name), html: { method: :patch, class: 'form' }) do |f|
%h1 Changement de mot de passe
= f.hidden_field :reset_password_token
= f.label 'Nouveau mot de passe'
= f.password_field :password, autofocus: true, autocomplete: 'off'
= f.label 'Confirmez le nouveau mot de passe'
= f.password_field :password_confirmation, autocomplete: 'off'
= f.submit 'Changer le mot de passe', class: 'button primary'

View file

@ -88,10 +88,6 @@ Rails.application.routes.draw do
passwords: 'super_admins/passwords'
}
devise_scope :super_admin do
get '/super_admins/password/test_strength' => 'super_admins/passwords#test_strength'
end
get 'super_admins/edit_otp', to: 'super_admins#edit_otp', as: 'edit_super_admin_otp'
put 'super_admins/enable_otp', to: 'super_admins#enable_otp', as: 'enable_super_admin_otp'
@ -110,9 +106,7 @@ Rails.application.routes.draw do
get '/users/password/reset-link-sent' => 'users/passwords#reset_link_sent'
end
devise_scope :administrateur do
get '/administrateurs/password/test_strength' => 'administrateurs/passwords#test_strength'
end
get 'password_complexity' => 'password_complexity#show', as: 'show_password_complexity'
#
# Main routes

View file

@ -0,0 +1,38 @@
describe DevisePopulatedResource, type: :controller do
controller(Devise::PasswordsController) do
include DevisePopulatedResource
end
let(:user) { create(:user) }
before do
routes.draw do
get 'edit' => 'devise/passwords#edit'
put 'update' => 'devise/passwords#update'
end
@request.env["devise.mapping"] = Devise.mappings[:user]
@token = user.send_reset_password_instructions
end
context 'when initiating a password reset' do
subject { get :edit, params: { reset_password_token: @token } }
it 'returns the fully populated resource' do
subject
expect(controller.populated_resource.id).to eq(user.id)
expect(controller.populated_resource.email).to eq(user.email)
end
end
context 'when submitting a password reset' do
subject { put :update, params: { user: { reset_password_token: @token } } }
it 'returns the fully populated resource' do
subject
expect(controller.populated_resource.id).to eq(user.id)
expect(controller.populated_resource.email).to eq(user.email)
end
end
end

View file

@ -0,0 +1,35 @@
describe PasswordComplexityController, type: :controller do
describe '#show' do
let(:params) do
{ user: { password: 'moderately complex password' } }
end
subject { get :show, format: :js, params: params, xhr: true }
it 'computes a password score' do
subject
expect(assigns(:score)).to eq(3)
end
context 'with a different resource name' do
let(:params) do
{ super_admin: { password: 'moderately complex password' } }
end
it 'computes a password score' do
subject
expect(assigns(:score)).to eq(3)
end
end
context 'when rendering the view' do
render_views
it 'renders Javascript that updates the password complexity meter' do
subject
expect(response.body).to include('#complexity-label')
expect(response.body).to include('#complexity-bar')
end
end
end
end

View file

@ -1,12 +0,0 @@
describe SuperAdmins::PasswordsController, type: :controller do
describe '#test_strength' do
it 'calculate score' do
password = "bonjour"
@request.env["devise.mapping"] = Devise.mappings[:super_admin]
get 'test_strength', xhr: true, params: { super_admin: { password: password } }
expect(assigns(:score)).to be_present
end
end
end

View file

@ -2,6 +2,8 @@ feature 'As an administrateur', js: true do
let(:super_admin) { create(:super_admin) }
let(:admin_email) { 'new_admin@gouv.fr' }
let(:new_admin) { Administrateur.by_email(admin_email) }
let(:weak_password) { '12345678' }
let(:strong_password) { 'a new, long, and complicated password!' }
before do
perform_enqueued_jobs do
@ -9,14 +11,21 @@ feature 'As an administrateur', js: true do
end
end
scenario 'I can register' do
scenario 'I can register', js: true do
expect(new_admin.reload.user.active?).to be(false)
confirmation_email = open_email(admin_email)
token_params = confirmation_email.body.match(/token=[^"]+/)
visit "admin/activate?#{token_params}"
fill_in :administrateur_password, with: 'my-s3cure-p4ssword'
fill_in :administrateur_password, with: weak_password
expect(page).to have_text('Mot de passe très vulnérable')
expect(page).to have_button('Continuer', disabled: true)
fill_in :administrateur_password, with: strong_password
expect(page).to have_text('Mot de passe suffisamment fort et sécurisé')
expect(page).to have_button('Continuer', disabled: false)
click_button 'Continuer'

View file

@ -27,11 +27,12 @@ feature 'Managing password:' do
end
context 'for admins' do
let(:user) { create(:user) }
let(:administrateur) { create(:administrateur, user: user) }
let(:new_password) { 'a new, long, and complicated password!' }
let(:administrateur) { create(:administrateur) }
let(:user) { administrateur.user }
let(:weak_password) { '12345678' }
let(:strong_password) { 'a new, long, and complicated password!' }
scenario 'an admin can reset their password' do
scenario 'an admin can reset their password', js: true do
visit root_path
click_on 'Connexion'
click_on 'Mot de passe oublié ?'
@ -48,8 +49,51 @@ feature 'Managing password:' do
expect(page).to have_content 'Changement de mot de passe'
fill_in 'user_password', with: new_password
fill_in 'user_password_confirmation', with: new_password
fill_in 'user_password', with: weak_password
fill_in 'user_password_confirmation', with: weak_password
expect(page).to have_text('Mot de passe très vulnérable')
expect(page).to have_button('Changer le mot de passe', disabled: true)
fill_in 'user_password', with: strong_password
fill_in 'user_password_confirmation', with: strong_password
expect(page).to have_text('Mot de passe suffisamment fort et sécurisé')
expect(page).to have_button('Changer le mot de passe', disabled: false)
click_on 'Changer le mot de passe'
expect(page).to have_content('Votre mot de passe a bien été modifié.')
end
end
context 'for super-admins' do
let(:super_admin) { create(:super_admin) }
let(:weak_password) { '12345678' }
let(:strong_password) { 'a new, long, and complicated password!' }
scenario 'a super-admin can reset their password', js: true do
visit manager_root_path
click_on 'Mot de passe oublié'
expect(page).to have_current_path(new_super_admin_password_path)
fill_in 'Email', with: super_admin.email
perform_enqueued_jobs do
click_on 'Demander un nouveau mot de passe'
end
expect(page).to have_text 'vous recevrez un lien vous permettant de récupérer votre mot de passe'
click_reset_password_link_for super_admin.email
expect(page).to have_content 'Changement de mot de passe'
fill_in 'super_admin_password', with: weak_password
fill_in 'super_admin_password_confirmation', with: weak_password
expect(page).to have_text('Mot de passe très vulnérable')
expect(page).to have_button('Changer le mot de passe', disabled: true)
fill_in 'super_admin_password', with: strong_password
fill_in 'super_admin_password_confirmation', with: strong_password
expect(page).to have_text('Mot de passe suffisamment fort et sécurisé')
expect(page).to have_button('Changer le mot de passe', disabled: false)
click_on 'Changer le mot de passe'
expect(page).to have_content('Votre mot de passe a bien été modifié.')
end

View file

@ -69,35 +69,37 @@ describe SuperAdmin, type: :model do
# 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}']
passwords = ['password', '12pass23', 'démarches ', 'démarches-simple', '{My-$3cure-p4ssWord}']
min_complexity = PASSWORD_COMPLEXITY_FOR_ADMIN
let(:email) { 'mail@beta.gouv.fr' }
let(:super_admin) { build(:super_admin, email: email, password: password) }
subject do
super_admin.save
super_admin.valid?
super_admin.errors.full_messages
end
context 'when password is too short' do
context 'when the password is too short' do
let(:password) { 's' * (PASSWORD_MIN_LENGTH - 1) }
it { expect(subject).to eq(["Le mot de passe est trop court"]) }
it 'reports an error about password length (but not about complexity)' do
expect(subject).to eq(["Le mot de passe est trop court"])
end
end
context 'when password is too simple' do
passwords[0..(min_complexity - 1)].each do |password|
let(:password) { password }
passwords[0..(min_complexity - 1)].each do |simple_password|
context 'when the password is long enough, but too simple' do
let(:password) { simple_password }
it { expect(subject).to eq(["Le mot de passe nest pas assez complexe"]) }
end
end
context 'when password is acceptable' do
context 'when the password is long and complex' do
let(:password) { passwords[min_complexity] }
it { expect(subject).to eq([]) }
it { expect(subject).to be_empty }
end
end
end

View file

@ -363,45 +363,56 @@ describe User, type: :model do
# 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}']
passwords = ['password', '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
user.valid?
user.errors.full_messages
end
subject do
administrateur.save
administrateur.errors.full_messages
end
context 'for administrateurs' do
let(:user) { build(:user, email: 'admin@exemple.fr', password: password, administrateur: build(:administrateur)) }
context 'when password is too short' do
context 'when the password is too short' do
let(:password) { 's' * (PASSWORD_MIN_LENGTH - 1) }
it { expect(subject).to eq(["Le mot de passe est trop court"]) }
it 'reports an error about password length (but not about complexity)' do
expect(subject).to eq(["Le mot de passe est trop court"])
end
end
context 'when password is too simple' do
passwords[0..(min_complexity - 1)].each do |password|
let(:password) { password }
passwords[0..(min_complexity - 1)].each do |simple_password|
context 'when the password is long enough, but too simple' do
let(:password) { simple_password }
it { expect(subject).to eq(["Le mot de passe nest pas assez complexe"]) }
end
end
context 'when password is acceptable' do
context 'when the password is long and complex' do
let(:password) { passwords[min_complexity] }
it { expect(subject).to eq([]) }
it { expect(subject).to be_empty }
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([])
context 'for simple users' do
let(:user) { build(:user, email: 'user@exemple.fr', password: password) }
context 'when the password is too short' do
let(:password) { 's' * (PASSWORD_MIN_LENGTH - 1) }
it 'reports an error about password length (but not about complexity)' do
expect(subject).to eq(["Le mot de passe est trop court"])
end
end
context 'when the password is long enough, but simple' do
let(:password) { 'simple-password' }
it 'doesnt enforce the password complexity' do
expect(subject).to be_empty
end
end
end

View file

@ -64,9 +64,9 @@ module FeatureHelpers
def click_reset_password_link_for(email)
reset_password_email = open_email(email)
token_params = reset_password_email.body.match(/reset_password_token=[^"]+/)
reset_password_url = reset_password_email.body.match(/http[s]?:\/\/[^\/]+(\/[^\s]+reset_password_token=[^\s"]+)/)[1]
visit "/users/password/edit?#{token_params}"
visit reset_password_url
end
# Add a new type de champ in the procedure editor