diff --git a/app/models/user.rb b/app/models/user.rb index 38eb7911e..b7aa66e3f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -5,6 +5,7 @@ class User < ApplicationRecord devise :database_authenticatable, :registerable, :confirmable, :recoverable, :rememberable, :trackable, :validatable, :omniauthable, :password_expirable, :secure_validatable, authentication_keys: [:login] + devise :lockable if Rails.application.config.devise_lockable acts_as_voter acts_as_paranoid column: :hidden_at @@ -424,6 +425,14 @@ class User < ApplicationRecord end end + def self.maximum_attempts + (Tenant.current_secrets.dig(:security, :lockable, :maximum_attempts) || 20).to_i + end + + def self.unlock_in + (Tenant.current_secrets.dig(:security, :lockable, :unlock_in) || 1).to_f.hours + end + private def clean_document_number diff --git a/config/application.rb b/config/application.rb index 9a37609a9..636d4552b 100644 --- a/config/application.rb +++ b/config/application.rb @@ -138,6 +138,9 @@ module Consul config.paths["app/views"].unshift(Rails.root.join("app", "views", "custom")) + # Set to true to enable devise user lockable feature + config.devise_lockable = Rails.application.secrets.devise_lockable + # Set to true to enable managing different tenants using the same application config.multitenancy = Rails.application.secrets.multitenancy end diff --git a/config/environments/test.rb b/config/environments/test.rb index 5787afca2..530286e6c 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -81,6 +81,8 @@ Rails.application.configure do # Allow managing different tenants using the same application config.multitenancy = true + + config.devise_lockable = true end require Rails.root.join("config", "environments", "custom", "test") diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index e297bfd48..d351f06ad 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -167,27 +167,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 = :both # Number of authentication tries before locking an account if lock_strategy # is failed attempts. - # config.maximum_attempts = 20 + config.maximum_attempts = 20 # Overwritten in User model # Time interval to unlock the account if :time is enabled as unlock_strategy. - # config.unlock_in = 1.hour + config.unlock_in = 1.hour # Overwritten in User model # Warn on the last attempt before the account is locked. - # config.last_attempt_warning = true + config.last_attempt_warning = false # ==> Configuration for :recoverable # diff --git a/config/secrets.yml.example b/config/secrets.yml.example index c2c736321..7b61e7a15 100644 --- a/config/secrets.yml.example +++ b/config/secrets.yml.example @@ -18,10 +18,14 @@ http_basic_auth: &http_basic_auth development: http_basic_username: "dev" http_basic_password: "pass" + devise_lockable: false multitenancy: false security: last_sign_in: false password_complexity: false + # lockable: + # maximum_attempts: 20 + # unlock_in: 1 # In hours secret_key_base: 56792feef405a59b18ea7db57b4777e855103882b926413d4afdfb8c0ea8aa86ea6649da4e729c5f5ae324c0ab9338f789174cf48c544173bc18fdc3b14262e4 <<: *maps @@ -50,12 +54,16 @@ staging: errbit_self_hosted_ssl: false http_basic_username: "" http_basic_password: "" + devise_lockable: false managers_url: "" managers_application_key: "" multitenancy: false security: last_sign_in: false password_complexity: false + # lockable: + # maximum_attempts: 20 + # unlock_in: 1 # In hours tenants: # If you've enabled multitenancy, you can overwrite secrets for a # specific tenant with: @@ -89,12 +97,16 @@ preproduction: errbit_self_hosted_ssl: false http_basic_username: "" http_basic_password: "" + devise_lockable: false managers_url: "" managers_application_key: "" multitenancy: false security: last_sign_in: false password_complexity: false + # lockable: + # maximum_attempts: 20 + # unlock_in: 1 # In hours tenants: # If you've enabled multitenancy, you can overwrite secrets for a # specific tenant with: @@ -133,12 +145,16 @@ production: errbit_self_hosted_ssl: false http_basic_username: "" http_basic_password: "" + devise_lockable: false managers_url: "" managers_application_key: "" multitenancy: false security: last_sign_in: false password_complexity: false + # lockable: + # maximum_attempts: 20 + # unlock_in: 1 # In hours tenants: # If you've enabled multitenancy, you can overwrite secrets for a # specific tenant with: diff --git a/db/migrate/20231009073912_add_lockable_to_devise.rb b/db/migrate/20231009073912_add_lockable_to_devise.rb new file mode 100644 index 000000000..35bd75d3e --- /dev/null +++ b/db/migrate/20231009073912_add_lockable_to_devise.rb @@ -0,0 +1,11 @@ +class AddLockableToDevise < ActiveRecord::Migration[6.1] + def change + # Only if lock strategy is :failed_attempts + add_column :users, :failed_attempts, :integer, default: 0, null: false + add_column :users, :locked_at, :datetime + + # Add these only if unlock strategy is :email or :both + add_column :users, :unlock_token, :string + add_index :users, :unlock_token, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 603deabd5..db35e4c6c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1644,6 +1644,9 @@ ActiveRecord::Schema.define(version: 2023_10_12_141318) do t.boolean "recommended_debates", default: true t.boolean "recommended_proposals", default: true t.string "subscriptions_token" + t.integer "failed_attempts", default: 0, null: false + t.datetime "locked_at" + t.string "unlock_token" t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["date_of_birth"], name: "index_users_on_date_of_birth" t.index ["email"], name: "index_users_on_email", unique: true @@ -1652,6 +1655,7 @@ ActiveRecord::Schema.define(version: 2023_10_12_141318) do t.index ["hidden_at"], name: "index_users_on_hidden_at" t.index ["password_changed_at"], name: "index_users_on_password_changed_at" 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 t.index ["username"], name: "index_users_on_username" end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb new file mode 100644 index 000000000..29f0ace2d --- /dev/null +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +describe Users::SessionsController do + before { request.env["devise.mapping"] = Devise.mappings[:user] } + + let!(:user) { create(:user, email: "citizen@consul.org", password: "12345678") } + + describe "Devise lock" do + context "when devise sign in maximum_attempts reached", :with_frozen_time do + it "locks the user account and sends an email to the account with an unlock link" do + allow(User).to receive(:maximum_attempts).and_return(1) + + expect do + post :create, params: { user: { login: "citizen@consul.org", password: "wrongpassword" }} + end.to change { user.reload.failed_attempts }.by(1) + .and change { user.reload.locked_at }.from(nil).to(Time.current) + + expect(ActionMailer::Base.deliveries.count).to eq(1) + body = ActionMailer::Base.deliveries.last.body + expect(body).to have_content "Your account has been locked" + expect(body).to have_link "Unlock my account" + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2ff3a9cb5..ed2bf468e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -882,4 +882,70 @@ describe User do end end end + + describe ".maximum_attempts" do + it "returns 20 as default when the secrets aren't configured" do + expect(User.maximum_attempts).to eq 20 + end + + context "when secrets are configured" do + before do + allow(Rails.application).to receive(:secrets).and_return(ActiveSupport::OrderedOptions.new.merge( + security: { + lockable: { maximum_attempts: "14" } + }, + tenants: { + superstrict: { + security: { + lockable: { maximum_attempts: "1" } + } + } + } + )) + end + + it "uses the general secrets for the main tenant" do + expect(User.maximum_attempts).to eq 14 + end + + it "uses the tenant secrets for a tenant" do + allow(Tenant).to receive(:current_schema).and_return("superstrict") + + expect(User.maximum_attempts).to eq 1 + end + end + end + + describe ".unlock_in" do + it "returns 1 as default when the secrets aren't configured" do + expect(User.unlock_in).to eq 1.hour + end + + context "when secrets are configured" do + before do + allow(Rails.application).to receive(:secrets).and_return(ActiveSupport::OrderedOptions.new.merge( + security: { + lockable: { unlock_in: "2" } + }, + tenants: { + superstrict: { + security: { + lockable: { unlock_in: "50" } + } + } + } + )) + end + + it "uses the general secrets for the main tenant" do + expect(User.unlock_in).to eq 2.hours + end + + it "uses the tenant secrets for a tenant" do + allow(Tenant).to receive(:current_schema).and_return("superstrict") + + expect(User.unlock_in).to eq 50.hours + end + end + end end