From 1d6a59dd67e0e6200c855e3478ffe65a18a6e5bc Mon Sep 17 00:00:00 2001 From: Nicolas Florentin Date: Fri, 29 Jul 2022 17:37:42 +0200 Subject: [PATCH] improvement: add complexity check of the user password --- .../templates/shared/_member_form.html | 4 ++-- .../templates/shared/passwordEditModal.html | 4 ++-- app/frontend/templates/shared/signupModal.html | 2 +- app/models/concerns/single_sign_on_concern.rb | 2 +- app/models/user.rb | 7 +++++++ app/services/members/members_service.rb | 2 +- app/services/secure_password.rb | 18 ++++++++++++++++++ app/services/user_service.rb | 6 +++--- config/initializers/devise.rb | 2 +- config/locales/app.public.en.yml | 4 +++- test/models/user_test.rb | 2 +- 11 files changed, 40 insertions(+), 13 deletions(-) create mode 100644 app/services/secure_password.rb diff --git a/app/frontend/templates/shared/_member_form.html b/app/frontend/templates/shared/_member_form.html index 0dc2e53fe..0b0690626 100644 --- a/app/frontend/templates/shared/_member_form.html +++ b/app/frontend/templates/shared/_member_form.html @@ -142,7 +142,7 @@ class="form-control" id="user_password" placeholder="{{ 'app.shared.user.new_password' | translate }}" - ng-minlength="8" + ng-minlength="12" required/> {{ 'app.shared.user.password_is_required' }} @@ -158,7 +158,7 @@ class="form-control" id="user_password_confirmation" placeholder="{{ 'app.shared.user.confirmation_of_new_password' | translate }}" - ng-minlength="8" + ng-minlength="12" required match="user.password"/> diff --git a/app/frontend/templates/shared/passwordEditModal.html b/app/frontend/templates/shared/passwordEditModal.html index 00149b9a9..ac94f38a5 100644 --- a/app/frontend/templates/shared/passwordEditModal.html +++ b/app/frontend/templates/shared/passwordEditModal.html @@ -18,7 +18,7 @@ class="form-control" placeholder="{{ 'app.public.common.your_new_password' | translate }}" required - ng-minlength="8"> + ng-minlength="12"> {{ 'app.public.common.password_is_required' }} {{ 'app.public.common.password_is_too_short' }} @@ -35,7 +35,7 @@ class="form-control" placeholder="{{ 'app.public.common.type_your_password_again' | translate }}" required - ng-minlength="8" + ng-minlength="12" match="user.password"> {{ 'app.public.common.password_confirmation_is_required' }} diff --git a/app/frontend/templates/shared/signupModal.html b/app/frontend/templates/shared/signupModal.html index e9594d948..60ead0663 100644 --- a/app/frontend/templates/shared/signupModal.html +++ b/app/frontend/templates/shared/signupModal.html @@ -96,7 +96,7 @@ class="form-control" placeholder="{{ 'app.public.common.your_password' | translate }}" required - ng-minlength="8"> + ng-minlength="12"> {{ 'app.public.common.password_is_required' }} diff --git a/app/models/concerns/single_sign_on_concern.rb b/app/models/concerns/single_sign_on_concern.rb index 4e851eb4f..f23a6985c 100644 --- a/app/models/concerns/single_sign_on_concern.rb +++ b/app/models/concerns/single_sign_on_concern.rb @@ -161,7 +161,7 @@ module SingleSignOnConcern user.set_data_from_sso_mapping(key, value) end logger.debug 'generating a new password' - user.password = Devise.friendly_token[0, 20] + user.password = SecurePassword.generate end end end diff --git a/app/models/user.rb b/app/models/user.rb index 4050a2793..94721c2e9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -78,6 +78,7 @@ class User < ApplicationRecord validate :cgu_must_accept, if: :new_record? validates :username, presence: true, uniqueness: true, length: { maximum: 30 } + validate :password_complexity scope :active, -> { where(is_active: true) } scope :without_subscription, -> { includes(statistic_profile: [:subscriptions]).where(subscriptions: { statistic_profile_id: nil }) } @@ -347,4 +348,10 @@ class User < ApplicationRecord last_name: last_name ) end + + def password_complexity + return if password.blank? || SecurePassword.is_secured?(password) + + errors.add I18n.t("app.public.common.password_is_too_weak"), I18n.t("app.public.common.password_is_too_weak_explanations") + end end diff --git a/app/services/members/members_service.rb b/app/services/members/members_service.rb index f89a2c459..48ca5101e 100644 --- a/app/services/members/members_service.rb +++ b/app/services/members/members_service.rb @@ -128,7 +128,7 @@ class Members::MembersService def password(params) if !params[:password] && !params[:password_confirmation] - Devise.friendly_token.first(8) + SecurePassword.generate else params[:password] end diff --git a/app/services/secure_password.rb b/app/services/secure_password.rb new file mode 100644 index 000000000..cbbfc13e5 --- /dev/null +++ b/app/services/secure_password.rb @@ -0,0 +1,18 @@ +class SecurePassword + LOWER_LETTERS = ('a'..'z').to_a + UPPER_LETTERS = ('A'..'Z').to_a + DIGITS = ('0'..'9').to_a + SPECIAL_CHARS = ["!", "#", "$", "%", "&", "(", ")", "*", "+", ",", "-", ".", "/", ":", ";", "<", "=", ">", "?", "@", "[", "]", "^", "_", "{", "|", "}", "~", "'", "`", '"'] + + def self.generate + (LOWER_LETTERS.shuffle.first(4) + UPPER_LETTERS.shuffle.first(4) + DIGITS.shuffle.first(4) + SPECIAL_CHARS.shuffle.first(4)).shuffle.join + end + + def self.is_secured?(password) + password_as_array = password.split("") + password_as_array.any? {|c| c.in? LOWER_LETTERS } && + password_as_array.any? {|c| c.in? UPPER_LETTERS } && + password_as_array.any? {|c| c.in? DIGITS } && + password_as_array.any? {|c| c.in? SPECIAL_CHARS } + end +end \ No newline at end of file diff --git a/app/services/user_service.rb b/app/services/user_service.rb index 83b1cf6e6..d117193fb 100644 --- a/app/services/user_service.rb +++ b/app/services/user_service.rb @@ -3,7 +3,7 @@ # helpers for managing users with special roles class UserService def self.create_partner(params) - generated_password = Devise.friendly_token.first(8) + generated_password = SecurePassword.generate group_id = Group.first.id user = User.new( email: params[:email], @@ -31,7 +31,7 @@ class UserService end def self.create_admin(params) - generated_password = Devise.friendly_token.first(8) + generated_password = SecurePassword.generate admin = User.new(params.merge(password: generated_password)) admin.send :set_slug @@ -52,7 +52,7 @@ class UserService end def self.create_manager(params) - generated_password = Devise.friendly_token.first(8) + generated_password = SecurePassword.generate manager = User.new(params.merge(password: generated_password)) manager.send :set_slug diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 3e9b6ec3c..0afb3b735 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -134,7 +134,7 @@ Devise.setup do |config| # ==> Configuration for :validatable # Range for password length. - config.password_length = 8..128 + config.password_length = 12..128 # Email regex used to validate email formats. It simply asserts that # one (and only one) @ exists in the given string. This is mainly diff --git a/config/locales/app.public.en.yml b/config/locales/app.public.en.yml index 2949bb1e8..e063b3f10 100644 --- a/config/locales/app.public.en.yml +++ b/config/locales/app.public.en.yml @@ -71,7 +71,9 @@ en: email_is_required: "E-mail address is required." your_password: "Your password" password_is_required: "Password is required." - password_is_too_short: "Password is too short (minimum 8 characters)" + password_is_too_short: "Password is too short (minimum 12 characters)" + password_is_too_weak: "Password is too weak:" + password_is_too_weak_explanations: "minimum 12 characters, at least one uppercase letter, one lowercase letter, one number and one special character" type_your_password_again: "Type your password again" password_confirmation_is_required: "Password confirmation is required." password_does_not_match_with_confirmation: "Password does not match with confirmation." diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 88f34b1a7..a68b7f367 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -4,7 +4,7 @@ require 'test_helper' class UserTest < ActiveSupport::TestCase test 'must create wallet and profiles after create user' do - u = User.create(username: 'user', email: 'userwallet@fabmanager.com', password: 'testpassword', password_confirmation: 'testpassword', + u = User.create(username: 'user', email: 'userwallet@fabmanager.com', password: 'Testpassword1$', password_confirmation: 'Testpassword1$', profile_attributes: { first_name: 'user', last_name: 'wallet', phone: '0123456789' }, statistic_profile_attributes: { gender: true, birthday: 18.years.ago }) assert u.wallet.present?