diff --git a/app/models/admin_user.rb b/app/models/admin_user.rb index 05f368a16..da370e800 100644 --- a/app/models/admin_user.rb +++ b/app/models/admin_user.rb @@ -1,8 +1,8 @@ class AdminUser < ApplicationRecord # Include default devise modules. Others available are: - # :confirmable, :lockable, :timeoutable, :omniauthable + # :confirmable, :timeoutable, :omniauthable devise :two_factor_authenticatable, :database_authenticatable, :recoverable, - :rememberable, :validatable, :trackable + :rememberable, :validatable, :trackable, :lockable has_one_time_password(encrypted: true) diff --git a/app/models/user.rb b/app/models/user.rb index d37130c19..7a74f1121 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,8 +1,8 @@ class User < ApplicationRecord # Include default devise modules. Others available are: - # :confirmable, :lockable, :timeoutable and :omniauthable + # :confirmable, :timeoutable and :omniauthable devise :database_authenticatable, :recoverable, :rememberable, :validatable, - :trackable + :trackable, :lockable belongs_to :organisation has_many :owned_case_logs, through: :organisation diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index d377aacd0..4d43e7f3b 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -195,27 +195,27 @@ Devise.setup do |config| # Defines which strategy will be used to lock an account. # :failed_attempts = Locks an account after a number of failed attempts to sign in. # :none = No lock strategy. You should handle locking by yourself. - # config.lock_strategy = :failed_attempts + config.lock_strategy = :failed_attempts # Defines which key will be used when locking and unlocking an account - # config.unlock_keys = [:email] + config.unlock_keys = [:email] # Defines which strategy will be used to unlock an account. # :email = Sends an unlock link to the user email # :time = Re-enables login after a certain amount of time (see :unlock_in below) # :both = Enables both strategies # :none = No unlock strategy. You should handle unlocking by yourself. - # config.unlock_strategy = :both + config.unlock_strategy = :time # Number of authentication tries before locking an account if lock_strategy # is failed attempts. - # config.maximum_attempts = 20 + config.maximum_attempts = 5 # Time interval to unlock the account if :time is enabled as unlock_strategy. - # config.unlock_in = 1.hour + config.unlock_in = 1.hour # Warn on the last attempt before the account is locked. - # config.last_attempt_warning = true + config.last_attempt_warning = true # ==> Configuration for :recoverable # diff --git a/db/migrate/20220308164721_add_lockable_fields.rb b/db/migrate/20220308164721_add_lockable_fields.rb new file mode 100644 index 000000000..89316018e --- /dev/null +++ b/db/migrate/20220308164721_add_lockable_fields.rb @@ -0,0 +1,10 @@ +class AddLockableFields < ActiveRecord::Migration[7.0] + def change + change_table :users, bulk: true do |t| + t.column :failed_attempts, :integer, default: 0 + t.column :unlock_token, :string + t.column :locked_at, :datetime + end + add_index :users, :unlock_token, unique: true + end +end diff --git a/db/migrate/20220310120127_add_lockable_fields2.rb b/db/migrate/20220310120127_add_lockable_fields2.rb new file mode 100644 index 000000000..6d1315689 --- /dev/null +++ b/db/migrate/20220310120127_add_lockable_fields2.rb @@ -0,0 +1,10 @@ +class AddLockableFields2 < ActiveRecord::Migration[7.0] + def change + change_table :admin_users, bulk: true do |t| + t.column :failed_attempts, :integer, default: 0 + t.column :unlock_token, :string + t.column :locked_at, :datetime + end + add_index :admin_users, :unlock_token, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index c762b1a4e..3d485704b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -36,7 +36,11 @@ ActiveRecord::Schema[7.0].define(version: 202202071123100) do t.datetime "last_sign_in_at", precision: nil t.string "current_sign_in_ip" t.string "last_sign_in_ip" + t.integer "failed_attempts", default: 0 + t.string "unlock_token" + t.datetime "locked_at", precision: nil t.index ["encrypted_otp_secret_key"], name: "index_admin_users_on_encrypted_otp_secret_key", unique: true + t.index ["unlock_token"], name: "index_admin_users_on_unlock_token", unique: true end create_table "case_logs", force: :cascade do |t| @@ -268,9 +272,13 @@ ActiveRecord::Schema[7.0].define(version: 202202071123100) do t.integer "role" t.string "old_user_id" t.string "phone" + t.integer "failed_attempts", default: 0 + t.string "unlock_token" + t.datetime "locked_at", precision: nil t.index ["email"], name: "index_users_on_email", unique: true t.index ["organisation_id"], name: "index_users_on_organisation_id" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true + t.index ["unlock_token"], name: "index_users_on_unlock_token", unique: true end create_table "versions", force: :cascade do |t| diff --git a/spec/features/auth/user_lockout_spec.rb b/spec/features/auth/user_lockout_spec.rb new file mode 100644 index 000000000..871dee26a --- /dev/null +++ b/spec/features/auth/user_lockout_spec.rb @@ -0,0 +1,75 @@ +require "rails_helper" + +RSpec.describe "User Lockout" do + let(:user) { FactoryBot.create(:user) } + let(:admin) { FactoryBot.create(:admin_user) } + let(:max_login_attempts) { Devise.maximum_attempts } + let(:max_2fa_attempts) { Devise.max_login_attempts } + let(:notify_client) { instance_double(Notifications::Client) } + + context "when login-in with the wrong user password up to a maximum number of attempts" do + before do + visit("/users/sign-in") + max_login_attempts.times do + fill_in("user[email]", with: user.email) + fill_in("user[password]", with: "wrong_password") + click_button("Sign in") + end + end + + it "locks the user account" do + visit("/users/sign-in") + fill_in("user[email]", with: user.email) + fill_in("user[password]", with: user.password) + click_button("Sign in") + expect(page).to have_http_status(:unprocessable_entity) + expect(page).to have_content("Your account is locked.") + end + end + + context "when login-in with the wrong admin password up to a maximum number of attempts" do + before do + visit("/admin/sign-in") + max_login_attempts.times do + fill_in("admin_user[email]", with: admin.email) + fill_in("admin_user[password]", with: "wrong_password") + click_button("Sign in") + end + end + + it "locks the admin account" do + visit("/admin/sign-in") + fill_in("admin_user[email]", with: admin.email) + fill_in("admin_user[password]", with: admin.password) + click_button("Sign in") + expect(page).to have_http_status(:unprocessable_entity) + expect(page).to have_content("Your account is locked.") + end + end + + context "when login-in with the right admin password and incorrect 2FA token up to a maximum number of attempts" do + before do + allow(Sms).to receive(:notify_client).and_return(notify_client) + allow(notify_client).to receive(:send_sms).and_return(true) + + visit("/admin/sign-in") + fill_in("admin_user[email]", with: admin.email) + fill_in("admin_user[password]", with: admin.password) + click_button("Sign in") + + max_2fa_attempts.times do + fill_in("code", with: "random") + click_button("Submit") + end + end + + it "locks the admin account" do + visit("/admin/sign-in") + fill_in("admin_user[email]", with: admin.email) + fill_in("admin_user[password]", with: admin.password) + click_button("Sign in") + expect(page).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("devise.two_factor_authentication.account_locked")) + end + end +end