diff --git a/Gemfile b/Gemfile index ce4173db7e..e49d01b418 100644 --- a/Gemfile +++ b/Gemfile @@ -62,6 +62,7 @@ gem 'graphiql-rails', git: 'https://github.com/meedan/graphiql-rails.git', ref: gem 'graphql-formatter' gem 'nokogiri', '1.14.3' gem 'puma' +gem 'rack-attack' gem 'rack-cors', '1.0.6', require: 'rack/cors' gem 'sidekiq', '5.2.10' gem 'sidekiq-cloudwatchmetrics' diff --git a/Gemfile.lock b/Gemfile.lock index 876aec8c6a..1f1facb61d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -637,6 +637,8 @@ GEM raabro (1.4.0) racc (1.7.1) rack (2.2.6.4) + rack-attack (6.7.0) + rack (>= 1.0, < 4) rack-cors (1.0.6) rack (>= 1.6.0) rack-protection (2.2.0) @@ -983,6 +985,7 @@ DEPENDENCIES puma pusher rack (= 2.2.6.4) + rack-attack rack-cors (= 1.0.6) rack-protection (= 2.2.0) railroady diff --git a/app/models/bot/smooch.rb b/app/models/bot/smooch.rb index c0c0e6ebc2..7f0ac223ae 100644 --- a/app/models/bot/smooch.rb +++ b/app/models/bot/smooch.rb @@ -302,7 +302,7 @@ def self.run(body) when 'message:delivery:failure' self.resend_message(json) true - when 'conversation:start' + when 'conversation:start', 'conversation:referral' message = { '_id': json['conversation']['_id'], authorId: json['appUser']['_id'], diff --git a/app/models/user.rb b/app/models/user.rb index 6384fac4aa..64bcc7b4b3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -30,7 +30,7 @@ class ToSOrPrivacyPolicyReadError < StandardError; end devise :registerable, :recoverable, :rememberable, :trackable, :validatable, :confirmable, - :omniauthable, omniauth_providers: [:twitter, :facebook, :slack, :google_oauth2] + :omniauthable, :lockable, omniauth_providers: [:twitter, :facebook, :slack, :google_oauth2] before_create :skip_confirmation_for_non_email_provider after_create :create_source_and_account, :set_source_image, :send_welcome_email diff --git a/config/application.rb b/config/application.rb index 8862b01770..b3a6dac1e3 100644 --- a/config/application.rb +++ b/config/application.rb @@ -99,5 +99,8 @@ class Application < Rails::Application }) config.active_record.yaml_column_permitted_classes = [Time, Symbol] + + # Rack Attack Configuration + config.middleware.use Rack::Attack end end diff --git a/config/config.yml.example b/config/config.yml.example index 873865aebc..e34a778e2c 100644 --- a/config/config.yml.example +++ b/config/config.yml.example @@ -268,6 +268,10 @@ development: &default nlu_global_rate_limit: 100 nlu_user_rate_limit: 30 + devise_maximum_attempts: 5 + devise_unlock_accounts_after: 1 + login_rate_limit: 10 + api_rate_limit: 100 test: <<: *default checkdesk_base_url_private: http://api:3000 diff --git a/config/environments/test.rb b/config/environments/test.rb index 592c08505d..6e70ec23e3 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -23,7 +23,8 @@ # Show full error reports and disable caching. config.consider_all_requests_local = true - config.action_controller.perform_caching = false + config.action_controller.perform_caching = true + config.cache_store = :memory_store # Raise exceptions instead of rendering exception templates. config.action_dispatch.show_exceptions = false diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 0c43abae93..72f1482708 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -49,6 +49,13 @@ def http_auth_body end config.mailer = 'DeviseMailer' config.invite_for = 1.month + + # Account lockout + config.lock_strategy = :failed_attempts + config.unlock_strategy = :both + config.unlock_keys = [ :time ] + config.maximum_attempts = CheckConfig.get('devise_maximum_attempts', 5) + config.unlock_in = CheckConfig.get('devise_unlock_accounts_after', 1).hour end AuthTrail.geocode = false diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb new file mode 100644 index 0000000000..f1971262b4 --- /dev/null +++ b/config/initializers/rack_attack.rb @@ -0,0 +1,21 @@ +class Rack::Attack + # Throttle login attempts by IP address + throttle('logins/ip', limit: proc { CheckConfig.get('login_rate_limit', 10, :integer) }, period: 60.seconds) do |req| + if req.path == '/api/users/sign_in' && req.post? + req.ip + end + end + + # Throttle login attempts by email address + throttle('logins/email', limit: proc { CheckConfig.get('login_rate_limit', 10, :integer) }, period: 60.seconds) do |req| + if req.path == '/api/users/sign_in' && req.post? + # Return the email if present, nil otherwise + req.params['user']['email'].presence if req.params['user'] + end + end + + # Throttle all graphql requests by IP address + throttle('api/graphql', limit: proc { CheckConfig.get('api_rate_limit', 100, :integer) }, period: 60.seconds) do |req| + req.ip if req.path == '/api/graphql' + end +end diff --git a/db/migrate/20240115101312_add_lockable_to_devise.rb b/db/migrate/20240115101312_add_lockable_to_devise.rb new file mode 100644 index 0000000000..2e49cd2a9c --- /dev/null +++ b/db/migrate/20240115101312_add_lockable_to_devise.rb @@ -0,0 +1,7 @@ +class AddLockableToDevise < ActiveRecord::Migration[6.1] + def change + add_column :users, :failed_attempts, :integer, default: 0, null: false + add_column :users, :unlock_token, :string + add_column :users, :locked_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index b3db55b598..9839702263 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2024_01_14_024701) do +ActiveRecord::Schema.define(version: 2024_01_15_101312) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -669,7 +669,6 @@ t.datetime "updated_at", null: false t.string "state" t.index ["external_id", "state"], name: "index_tipline_messages_on_external_id_and_state", unique: true - t.index ["external_id"], name: "index_tipline_messages_on_external_id" t.index ["team_id"], name: "index_tipline_messages_on_team_id" t.index ["uid"], name: "index_tipline_messages_on_uid" end @@ -799,6 +798,9 @@ t.integer "consumed_timestep" t.boolean "otp_required_for_login" t.string "otp_backup_codes", array: true + t.integer "failed_attempts", default: 0, null: false + t.string "unlock_token" + t.datetime "locked_at" t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["email"], name: "index_users_on_email", unique: true, where: "((email IS NOT NULL) AND ((email)::text <> ''::text))" t.index ["invitation_token"], name: "index_users_on_invitation_token", unique: true @@ -810,7 +812,8 @@ end create_table "versions", id: :serial, force: :cascade do |t| - t.string "item_type", null: false + t.string "item_type" + t.string "{:null=>false}" t.string "item_id", null: false t.string "event", null: false t.string "whodunnit" diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 76a9ccf4a6..ce63e1c7c0 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -71,5 +71,35 @@ def setup assert_not_nil @controller.current_api_user end + test "should lock user after excessive login requests" do + u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: 'test@test.com' + u.confirm + Devise.maximum_attempts = 2 + + 2.times do + post :create, params: { api_user: { email: 'test@test.com', password: '12345679' } } + end + + u.reload + assert u.access_locked? + assert_not_nil u.locked_at + end + + test "should unlock locked user accounts after specified time" do + u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: 'test@test.com' + u.confirm + maximum_attempts = Devise.maximum_attempts + + maximum_attempts.times do + post :create, params: { api_user: { email: 'test@test.com', password: '12345679' } } + end + + travel_to CheckConfig.get('devise_unlock_accounts_after', 5, :integer).hours.from_now + 1.hour do + u.reload + assert !u.access_locked? + assert_nil u.locked_at + end + end + end diff --git a/test/lib/check_rack_attack_test.rb b/test/lib/check_rack_attack_test.rb new file mode 100644 index 0000000000..ce32666574 --- /dev/null +++ b/test/lib/check_rack_attack_test.rb @@ -0,0 +1,32 @@ +require 'test_helper' + +class ThrottlingTest < ActionDispatch::IntegrationTest + setup do + Rails.cache.clear + end + + test "should throttle excessive requests to /api/graphql" do + stub_configs({ 'api_rate_limit' => 2 }) do + 2.times do + post api_graphql_path + assert_response :unauthorized + end + + post api_graphql_path + assert_response :too_many_requests + end + end + + test "should throttle excessive requests to /api/users/sign_in" do + stub_configs({ 'login_rate_limit' => 2 }) do + user_params = { api_user: { email: 'user@example.com', password: 'password' } } + + 2.times do + post api_user_session_path, params: user_params, as: :json + end + + post api_user_session_path, params: user_params, as: :json + assert_response :too_many_requests + end + end +end