Merge pull request #5273 from consuldemocracy/lockable

ENS: Enable lockable devise module

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

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

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")

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
   #

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:

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

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
 

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

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