From 02bdfef893d25a8432624dfb55ffa936a9b32c09 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 26 Aug 2021 15:04:13 -0500 Subject: [PATCH 1/9] devise: cleanup Users::PasswordController --- app/controllers/users/passwords_controller.rb | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index b756cc17e..a6921a188 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -37,16 +37,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? @@ -78,9 +78,4 @@ class Users::PasswordsController < Devise::PasswordsController 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 From e97b7164dd340df8495dd2185e385cdb4635c422 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 26 Aug 2021 17:23:01 +0000 Subject: [PATCH 2/9] models: extract password complexity to a concern --- .../concerns/password_complexity_concern.rb | 23 +++++++++++++++++++ app/models/super_admin.rb | 4 ++-- app/models/user.rb | 5 +++- 3 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 app/models/concerns/password_complexity_concern.rb diff --git a/app/models/concerns/password_complexity_concern.rb b/app/models/concerns/password_complexity_concern.rb new file mode 100644 index 000000000..bbcef17fc --- /dev/null +++ b/app/models/concerns/password_complexity_concern.rb @@ -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 diff --git a/app/models/super_admin.rb b/app/models/super_admin.rb index eeb69a255..e849ce3e6 100644 --- a/app/models/super_admin.rb +++ b/app/models/super_admin.rb @@ -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 diff --git a/app/models/user.rb b/app/models/user.rb index 9fde0c77d..32219d6b7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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 From 586f8ec5434c6b4eb6afeea522dd4e69e367422a Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 9 Sep 2021 09:40:23 -0500 Subject: [PATCH 3/9] models: improve password complexity specs --- spec/models/super_admin_spec.rb | 20 +++++++------ spec/models/user_spec.rb | 53 ++++++++++++++++++++------------- 2 files changed, 43 insertions(+), 30 deletions(-) diff --git a/spec/models/super_admin_spec.rb b/spec/models/super_admin_spec.rb index 19cf1f8d3..f75dbf86e 100644 --- a/spec/models/super_admin_spec.rb +++ b/spec/models/super_admin_spec.rb @@ -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 n’est 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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 15401ff14..8d4780268 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -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 n’est 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 'doesn’t enforce the password complexity' do + expect(subject).to be_empty end end end From 428ca8755fda8f8efc8de7f317655e5af8e132ea Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 31 Aug 2021 16:14:32 +0000 Subject: [PATCH 4/9] app: add a password_complexity component This component will replace the previous `password_field` component. --- .../stylesheets/password_complexity.scss | 40 +++++++++++++++++++ .../concerns/devise_populated_resource.rb | 13 ++++++ .../password_complexity_controller.rb | 15 +++++++ app/views/password_complexity/_bar.html.haml | 1 + .../password_complexity/_field.html.haml | 9 +++++ .../password_complexity/_label.html.haml | 16 ++++++++ app/views/password_complexity/show.js.erb | 3 ++ config/routes.rb | 2 + .../devise_populated_resource_spec.rb | 38 ++++++++++++++++++ .../password_complexity_controller_spec.rb | 35 ++++++++++++++++ 10 files changed, 172 insertions(+) create mode 100644 app/assets/stylesheets/password_complexity.scss create mode 100644 app/controllers/concerns/devise_populated_resource.rb create mode 100644 app/controllers/password_complexity_controller.rb create mode 100644 app/views/password_complexity/_bar.html.haml create mode 100644 app/views/password_complexity/_field.html.haml create mode 100644 app/views/password_complexity/_label.html.haml create mode 100644 app/views/password_complexity/show.js.erb create mode 100644 spec/controllers/concerns/devise_populated_resource_spec.rb create mode 100644 spec/controllers/password_complexity_controller_spec.rb diff --git a/app/assets/stylesheets/password_complexity.scss b/app/assets/stylesheets/password_complexity.scss new file mode 100644 index 000000000..24fc2ef77 --- /dev/null +++ b/app/assets/stylesheets/password_complexity.scss @@ -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; + } +} diff --git a/app/controllers/concerns/devise_populated_resource.rb b/app/controllers/concerns/devise_populated_resource.rb new file mode 100644 index 000000000..5d61113ae --- /dev/null +++ b/app/controllers/concerns/devise_populated_resource.rb @@ -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 diff --git a/app/controllers/password_complexity_controller.rb b/app/controllers/password_complexity_controller.rb new file mode 100644 index 000000000..f1572b124 --- /dev/null +++ b/app/controllers/password_complexity_controller.rb @@ -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 diff --git a/app/views/password_complexity/_bar.html.haml b/app/views/password_complexity/_bar.html.haml new file mode 100644 index 000000000..a9b8c8262 --- /dev/null +++ b/app/views/password_complexity/_bar.html.haml @@ -0,0 +1 @@ +#complexity-bar.password-complexity{ class: "complexity-#{@length < @min_length ? @score/2 : @score}" } diff --git a/app/views/password_complexity/_field.html.haml b/app/views/password_complexity/_field.html.haml new file mode 100644 index 000000000..eaf29e700 --- /dev/null +++ b/app/views/password_complexity/_field.html.haml @@ -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é. diff --git a/app/views/password_complexity/_label.html.haml b/app/views/password_complexity/_label.html.haml new file mode 100644 index 000000000..2e9bda1d0 --- /dev/null +++ b/app/views/password_complexity/_label.html.haml @@ -0,0 +1,16 @@ +#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. + - else + - case @score + - when 0..1 + Mot de passe très vulnérable. + - when 2...@min_complexity + Mot de passe vulnérable. + - when @min_complexity...4 + Mot de passe acceptable. Vous pouvez valider...
ou améliorer votre mot de passe. + - else + Félicitations ! Mot de passe suffisamment fort et sécurisé. + - else + Inscrivez un mot de passe. diff --git a/app/views/password_complexity/show.js.erb b/app/views/password_complexity/show.js.erb new file mode 100644 index 000000000..1d83ac45a --- /dev/null +++ b/app/views/password_complexity/show.js.erb @@ -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};") %> diff --git a/config/routes.rb b/config/routes.rb index 377c1c9a2..4b5de2c28 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -114,6 +114,8 @@ Rails.application.routes.draw do get '/administrateurs/password/test_strength' => 'administrateurs/passwords#test_strength' end + get 'password_complexity' => 'password_complexity#show', as: 'show_password_complexity' + # # Main routes # diff --git a/spec/controllers/concerns/devise_populated_resource_spec.rb b/spec/controllers/concerns/devise_populated_resource_spec.rb new file mode 100644 index 000000000..57f35b112 --- /dev/null +++ b/spec/controllers/concerns/devise_populated_resource_spec.rb @@ -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 diff --git a/spec/controllers/password_complexity_controller_spec.rb b/spec/controllers/password_complexity_controller_spec.rb new file mode 100644 index 000000000..7c60de3fc --- /dev/null +++ b/spec/controllers/password_complexity_controller_spec.rb @@ -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 From ed8b19d2eb445631b70dcec6edd1f508a92af17c Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 31 Aug 2021 16:15:08 +0000 Subject: [PATCH 5/9] app: use password_complexity in Administrateurs::ActivateController --- app/views/administrateurs/activate/new.html.haml | 3 +-- config/routes.rb | 4 ---- spec/features/admin/admin_creation_spec.rb | 13 +++++++++++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/app/views/administrateurs/activate/new.html.haml b/app/views/administrateurs/activate/new.html.haml index ae11eebdc..5d17e6aa8 100644 --- a/app/views/administrateurs/activate/new.html.haml +++ b/app/views/administrateurs/activate/new.html.haml @@ -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..." } diff --git a/config/routes.rb b/config/routes.rb index 4b5de2c28..bc29a6f95 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -110,10 +110,6 @@ 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' # diff --git a/spec/features/admin/admin_creation_spec.rb b/spec/features/admin/admin_creation_spec.rb index befcb152d..938bd1c56 100644 --- a/spec/features/admin/admin_creation_spec.rb +++ b/spec/features/admin/admin_creation_spec.rb @@ -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' From 62e4f7ee32b3705d23e6ba7ba21a9557d1a38cda Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 2 Sep 2021 19:20:30 +0000 Subject: [PATCH 6/9] devise: use password_complexity in User::PasswordsController This fixes the password strength meter no longer being displayed when an admin changes their password. --- app/controllers/users/passwords_controller.rb | 29 ++++--------------- app/views/users/passwords/edit.html.haml | 4 +-- spec/features/users/managing_password_spec.rb | 21 ++++++++++---- 3 files changed, 22 insertions(+), 32 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index a6921a188..945635d9e 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -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 @@ -67,15 +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 end diff --git a/app/views/users/passwords/edit.html.haml b/app/views/users/passwords/edit.html.haml index 33b8d466b..46d33dd55 100644 --- a/app/views/users/passwords/edit.html.haml +++ b/app/views/users/passwords/edit.html.haml @@ -14,9 +14,9 @@ = f.hidden_field :reset_password_token = f.label 'Nouveau mot de passe' - = f.password_field :password, autofocus: true, autocomplete: 'off' + = render 'password_complexity/field', { form: f, test_complexity: populated_resource.validate_password_complexity? } = 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' + = f.submit 'Changer le mot de passe', class: 'button large primary expand', id: "submit-password", data: { disable_with: "Envoi…" } diff --git a/spec/features/users/managing_password_spec.rb b/spec/features/users/managing_password_spec.rb index 16d16fe10..97d5c9946 100644 --- a/spec/features/users/managing_password_spec.rb +++ b/spec/features/users/managing_password_spec.rb @@ -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,16 @@ 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 From 80f9d4adc043252342bab1242dfa5a257b64bd38 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 2 Sep 2021 19:21:10 +0000 Subject: [PATCH 7/9] devise: use password_strength component in SuperAdmin::PasswordsController --- .../super_admins/passwords_controller.rb | 15 ++------- .../super_admins/passwords/edit.html.haml | 4 +-- config/routes.rb | 4 --- .../super_admins/passwords_controller_spec.rb | 12 ------- spec/features/users/managing_password_spec.rb | 33 +++++++++++++++++++ spec/support/feature_helpers.rb | 4 +-- 6 files changed, 38 insertions(+), 34 deletions(-) delete mode 100644 spec/controllers/super_admins/passwords_controller_spec.rb diff --git a/app/controllers/super_admins/passwords_controller.rb b/app/controllers/super_admins/passwords_controller.rb index 69c8de2d3..e532269b0 100644 --- a/app/controllers/super_admins/passwords_controller.rb +++ b/app/controllers/super_admins/passwords_controller.rb @@ -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 diff --git a/app/views/super_admins/passwords/edit.html.haml b/app/views/super_admins/passwords/edit.html.haml index d14e4ca7b..e134dc166 100644 --- a/app/views/super_admins/passwords/edit.html.haml +++ b/app/views/super_admins/passwords/edit.html.haml @@ -14,8 +14,6 @@ = f.hidden_field :reset_password_token = f.label 'Nouveau mot de passe' - - = render partial: 'shared/password/edit_password', locals: { form: f, controller: 'super_admins/passwords' } - + = render 'password_complexity/field', { form: f, test_complexity: populated_resource.validate_password_complexity? } = f.submit 'Changer le mot de passe', class: 'button large primary expand', id: "submit-password", data: { disable_with: "Envoi..." } diff --git a/config/routes.rb b/config/routes.rb index bc29a6f95..c5d388075 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -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' diff --git a/spec/controllers/super_admins/passwords_controller_spec.rb b/spec/controllers/super_admins/passwords_controller_spec.rb deleted file mode 100644 index d2e7c2b08..000000000 --- a/spec/controllers/super_admins/passwords_controller_spec.rb +++ /dev/null @@ -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 diff --git a/spec/features/users/managing_password_spec.rb b/spec/features/users/managing_password_spec.rb index 97d5c9946..aeabea7f8 100644 --- a/spec/features/users/managing_password_spec.rb +++ b/spec/features/users/managing_password_spec.rb @@ -63,4 +63,37 @@ feature 'Managing password:' do 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 + 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 + 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 end diff --git a/spec/support/feature_helpers.rb b/spec/support/feature_helpers.rb index 6e2302735..5b63e9386 100644 --- a/spec/support/feature_helpers.rb +++ b/spec/support/feature_helpers.rb @@ -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 From e5f449b5953387cac95d541f9831cdf92cbe480e Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Fri, 27 Aug 2021 04:29:18 +0000 Subject: [PATCH 8/9] devise: unify password reset views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By default, Devise will look for views: 1. First in `views/resource/passwords/…`, 2. Then in `views/devise/passwords/…` if not found. By moving the views to `views/devise`, we avoid having a partial in `views/shared` that we need to include manually, and instead let Devise do the job automatically. --- .../administrateurs/passwords_controller.rb | 14 -------------- .../administrateurs/passwords/edit.html.haml | 6 ------ .../passwords/edit.html.haml | 0 .../{users => devise}/passwords/new.html.haml | 0 .../super_admins/passwords/edit.html.haml | 19 ------------------- .../super_admins/passwords/new.html.haml | 17 ----------------- spec/features/users/managing_password_spec.rb | 2 ++ 7 files changed, 2 insertions(+), 56 deletions(-) delete mode 100644 app/controllers/administrateurs/passwords_controller.rb delete mode 100644 app/views/administrateurs/passwords/edit.html.haml rename app/views/{users => devise}/passwords/edit.html.haml (100%) rename app/views/{users => devise}/passwords/new.html.haml (100%) delete mode 100644 app/views/super_admins/passwords/edit.html.haml delete mode 100644 app/views/super_admins/passwords/new.html.haml diff --git a/app/controllers/administrateurs/passwords_controller.rb b/app/controllers/administrateurs/passwords_controller.rb deleted file mode 100644 index 00b8d93f2..000000000 --- a/app/controllers/administrateurs/passwords_controller.rb +++ /dev/null @@ -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 diff --git a/app/views/administrateurs/passwords/edit.html.haml b/app/views/administrateurs/passwords/edit.html.haml deleted file mode 100644 index e90a04120..000000000 --- a/app/views/administrateurs/passwords/edit.html.haml +++ /dev/null @@ -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 diff --git a/app/views/users/passwords/edit.html.haml b/app/views/devise/passwords/edit.html.haml similarity index 100% rename from app/views/users/passwords/edit.html.haml rename to app/views/devise/passwords/edit.html.haml diff --git a/app/views/users/passwords/new.html.haml b/app/views/devise/passwords/new.html.haml similarity index 100% rename from app/views/users/passwords/new.html.haml rename to app/views/devise/passwords/new.html.haml diff --git a/app/views/super_admins/passwords/edit.html.haml b/app/views/super_admins/passwords/edit.html.haml deleted file mode 100644 index e134dc166..000000000 --- a/app/views/super_admins/passwords/edit.html.haml +++ /dev/null @@ -1,19 +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' - = render 'password_complexity/field', { form: f, test_complexity: populated_resource.validate_password_complexity? } - - = f.submit 'Changer le mot de passe', class: 'button large primary expand', id: "submit-password", data: { disable_with: "Envoi..." } diff --git a/app/views/super_admins/passwords/new.html.haml b/app/views/super_admins/passwords/new.html.haml deleted file mode 100644 index 38dda419f..000000000 --- a/app/views/super_admins/passwords/new.html.haml +++ /dev/null @@ -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' diff --git a/spec/features/users/managing_password_spec.rb b/spec/features/users/managing_password_spec.rb index aeabea7f8..dba9af278 100644 --- a/spec/features/users/managing_password_spec.rb +++ b/spec/features/users/managing_password_spec.rb @@ -85,10 +85,12 @@ feature 'Managing password:' do 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) From 4a71b1d202d7ca235f9e1edd3617312e9484bcce Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 31 Aug 2021 16:21:17 +0000 Subject: [PATCH 9/9] app: remove former password complexity views The old component is now unused and ca be removed. --- app/assets/stylesheets/password_strength.scss | 40 ------------------- app/views/shared/password/_edit.html.haml | 21 ---------- .../shared/password/_edit_password.html.haml | 10 ----- .../password/_password_strength.html.haml | 1 - .../_password_strength_label.html.haml | 16 -------- .../shared/password/test_strength.js.erb | 3 -- 6 files changed, 91 deletions(-) delete mode 100644 app/assets/stylesheets/password_strength.scss delete mode 100644 app/views/shared/password/_edit.html.haml delete mode 100644 app/views/shared/password/_edit_password.html.haml delete mode 100644 app/views/shared/password/_password_strength.html.haml delete mode 100644 app/views/shared/password/_password_strength_label.html.haml delete mode 100644 app/views/shared/password/test_strength.js.erb diff --git a/app/assets/stylesheets/password_strength.scss b/app/assets/stylesheets/password_strength.scss deleted file mode 100644 index e5e0426c1..000000000 --- a/app/assets/stylesheets/password_strength.scss +++ /dev/null @@ -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; - } -} diff --git a/app/views/shared/password/_edit.html.haml b/app/views/shared/password/_edit.html.haml deleted file mode 100644 index d556d0ece..000000000 --- a/app/views/shared/password/_edit.html.haml +++ /dev/null @@ -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..." } - diff --git a/app/views/shared/password/_edit_password.html.haml b/app/views/shared/password/_edit_password.html.haml deleted file mode 100644 index 520288d48..000000000 --- a/app/views/shared/password/_edit_password.html.haml +++ /dev/null @@ -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 -   - -.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é. - diff --git a/app/views/shared/password/_password_strength.html.haml b/app/views/shared/password/_password_strength.html.haml deleted file mode 100644 index b98c2a8dc..000000000 --- a/app/views/shared/password/_password_strength.html.haml +++ /dev/null @@ -1 +0,0 @@ -#strength-bar.password-strength{ class: "strength-#{@length < @min_length ? @score/2 : @score}" } diff --git a/app/views/shared/password/_password_strength_label.html.haml b/app/views/shared/password/_password_strength_label.html.haml deleted file mode 100644 index fd196fd45..000000000 --- a/app/views/shared/password/_password_strength_label.html.haml +++ /dev/null @@ -1,16 +0,0 @@ -#strength-label{ style: 'font-weight: bold' } - - if @length > 0 - - if @length < @min_length - Le mot de passe doit faire au moins #{@min_length} caractères. - - else - - case @score - - when 0..1 - Mot de passe très vulnérable. - - when 2...@min_complexity - Mot de passe vulnérable. - - when @min_complexity...4 - Mot de passe acceptable. Vous pouvez valider...
ou améliorer votre mot de passe. - - else - Félicitations ! Mot de passe suffisamment fort et sécurisé. - - else - Inscrivez un mot de passe. diff --git a/app/views/shared/password/test_strength.js.erb b/app/views/shared/password/test_strength.js.erb deleted file mode 100644 index 24ae6c8e2..000000000 --- a/app/views/shared/password/test_strength.js.erb +++ /dev/null @@ -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};") %>