Skip to content

Commit

Permalink
add better password complexity rules
Browse files Browse the repository at this point in the history
  • Loading branch information
tsubik committed Sep 26, 2024
1 parent 976afdf commit eb0b69a
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 9 deletions.
5 changes: 3 additions & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ class User < ApplicationRecord
validates :email, uniqueness: true
validates :name, presence: true
validates :locale, inclusion: {in: I18n.available_locales.map(&:to_s), allow_blank: true}
validates :password, confirmation: true, length: {within: 8..128}, on: :create
validates :password_confirmation, presence: true, on: :create
# password length and confirmation validation is handled by Devise
validates :password, format: {with: /\A(?=.*[a-z])(?=.*[A-Z])(?=.*\d).+\z/, message: :password_complexity}, if: -> { password.present? }

validates :user_permission, presence: true
validates :operator, presence: true, if: -> { user_permission&.operator? }
validates :observer, presence: true, if: -> { user_permission&.ngo? || user_permission&.ngo_manager? }
Expand Down
2 changes: 1 addition & 1 deletion config/initializers/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@

# ==> Configuration for :validatable
# Range for password length.
config.password_length = 6..128
config.password_length = 10..128

# Email regex used to validate email formats. It simply asserts that
# one (and only one) @ exists in the given string. This is mainly
Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1475,6 +1475,7 @@ en:

errors:
messages:
password_complexity: "must contain at least one uppercase letter, one lowercase letter, and one digit"
qc_in_progress: "QC must be in progress"
record_invalid: "Validation failed: %{errors}"
restrict_dependent_destroy:
Expand Down
1 change: 1 addition & 0 deletions config/locales/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1394,6 +1394,7 @@ fr:
government: 'Gouvernement'
errors:
messages:
password_complexity: "doit contenir au moins une lettre majuscule, une lettre minuscule et un chiffre"
qc_in_progress: "Le contrôle qualité doit être en cours"
record_invalid: 'La validation a échoué : %{errors}'
restrict_dependent_destroy:
Expand Down
2 changes: 1 addition & 1 deletion db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

$stdout.puts "Creating test users..."

common_fields = {password: "password", password_confirmation: "password", locale: :en, last_name: "User"}
common_fields = {password: "Supersecret1", password_confirmation: "Supersecret1", locale: :en, last_name: "User"}
admin = User.create_with(**common_fields, first_name: "Admin").find_or_create_by!(email: "admin@example.com") do |user|
user.build_user_permission(user_role: "admin")
end
Expand Down
4 changes: 2 additions & 2 deletions lib/tasks/db.rake
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ namespace :db do
desc "Prepare database for dev enviroment"
task prepare_for_dev: :environment do
ActiveRecord::Base.connection.reconnect! # make sure connection is open
puts "Changing all users passwords to secret"
User.update_all(encrypted_password: User.new(password: "secret").encrypted_password)
puts "Changing all users passwords to Supersecret1"
User.update_all(encrypted_password: User.new(password: "Supersecret1").encrypted_password)
end
end
2 changes: 1 addition & 1 deletion spec/factories/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
factory :user do
sequence(:email) { |n| "pepe#{n}@example.com" }

password { "password" }
password { "Supersecret1" }
password_confirmation { |u| u.password }
first_name { "Test" }
last_name { "user" }
Expand Down
24 changes: 22 additions & 2 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
describe "Validations" do
# TODO: reenable later when validating first/last names
# it { is_expected.to validate_presence_of(:name) }
it { is_expected.to validate_presence_of(:password_confirmation) }
it { is_expected.to validate_presence_of(:user_permission) }

it { is_expected.to validate_uniqueness_of(:email).ignoring_case_sensitivity }
Expand All @@ -57,7 +56,28 @@

it { is_expected.to validate_confirmation_of(:password) }

it { is_expected.to validate_length_of(:password).is_at_least(8).is_at_most(128).on(:create) }
it { is_expected.to validate_length_of(:password).is_at_least(10).is_at_most(128) }

it "is invalid when password does not contain lowercase letter" do
subject.password = "PASSWORD1234"
subject.password_confirmation = "PASSWORD1234"
expect(subject.valid?).to eq(false)
expect(subject.errors[:password]).to include("must contain at least one uppercase letter, one lowercase letter, and one digit")
end

it "is invalid when password does not contain uppercase letter" do
subject.password = "password1234"
subject.password_confirmation = "password1234"
expect(subject.valid?).to eq(false)
expect(subject.errors[:password]).to include("must contain at least one uppercase letter, one lowercase letter, and one digit")
end

it "is invalid when password does not contain digit" do
subject.password = "SuperPassword"
subject.password_confirmation = "SuperPassword"
expect(subject.valid?).to eq(false)
expect(subject.errors[:password]).to include("must contain at least one uppercase letter, one lowercase letter, and one digit")
end

describe "user permissions" do
let(:user) { build(:user, user_role: user_role) }
Expand Down

0 comments on commit eb0b69a

Please sign in to comment.