diff --git a/app/controllers/shopkeeper_auth/sessions_controller.rb b/app/controllers/shopkeeper_auth/sessions_controller.rb index 4739587..76cc772 100644 --- a/app/controllers/shopkeeper_auth/sessions_controller.rb +++ b/app/controllers/shopkeeper_auth/sessions_controller.rb @@ -1,4 +1,20 @@ class ShopkeeperAuth::SessionsController < DeviseTokenAuth::SessionsController + RENDER_LOGIN_THROTTLED = -> { + render json: {code: 429, error_message: I18n.t("errors.messages.too_many_logins")}, + status: :too_many_requests + } + private_constant :RENDER_LOGIN_THROTTLED + + rate_limit to: 5, within: 20.seconds, only: :create, + name: "logins/ip", + with: RENDER_LOGIN_THROTTLED + + rate_limit to: 5, within: 20.seconds, only: :create, + name: "logins/email", + by: -> { params[:email].to_s.downcase.gsub(/\s+/, "") }, + if: -> { params[:email].present? }, + with: RENDER_LOGIN_THROTTLED + def create super return if @resource.blank? diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index c349b3e..447d61d 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -32,39 +32,9 @@ class Rack::Attack req.ip # unless req.path.start_with?("/assets") end - ### Prevent Brute-Force Login Attacks ### - - # The most common brute-force login attack is a brute-force password - # attack where an attacker simply tries a large number of emails and - # passwords to see if any credentials match. - # - # Another common method of attack is to use a swarm of computers with - # different IPs to try brute-forcing a password for a specific account. - - # Throttle POST requests to /login by IP address - # - # Key: "rack::attack:#{Time.now.to_i/:period}:logins/ip:#{req.ip}" - throttle("logins/ip", limit: 5, period: 20.seconds) do |req| - if req.path == "/shopkeeper_auth/sign_in" && req.post? - req.ip - end - end - - # Throttle POST requests to /login by email param - # - # Key: "rack::attack:#{Time.now.to_i/:period}:logins/email:#{normalized_email}" - # - # Note: This creates a problem where a malicious user could intentionally - # throttle logins for another user and force their login requests to be - # denied, but that's not very common and shouldn't happen to you. (Knock - # on wood!) - throttle("logins/email", limit: 5, period: 20.seconds) do |req| - if req.path == "/shopkeeper_auth/sign_in" && req.post? - # Normalize the email, using the same logic as your authentication process, to - # protect against rate limit bypasses. Return the normalized email if present, nil otherwise. - req.params["email"].to_s.downcase.gsub(/\s+/, "").presence - end - end + # Per-endpoint throttles (sign-in, sign-up) live on the controllers + # themselves via Rails 8's ActionController::RateLimiting. See + # ShopkeeperAuth::SessionsController and ShopkeeperAuth::RegistrationsController. ### Custom Throttle Response ### diff --git a/config/locales/en.yml b/config/locales/en.yml index c4dcd6e..73af755 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -143,3 +143,4 @@ en: limit_count_accounts_shopkeeper: "Organization members can be created up to %{limit_count}. Please contact the organization admin user or owner." limit_count_item_tag: "You can create up to %{limit_count} item tags." too_many_signups: "Too many sign-up attempts. Please try again later." + too_many_logins: "Too many sign-in attempts. Please try again later." diff --git a/test/integration/sign_in_throttle_test.rb b/test/integration/sign_in_throttle_test.rb new file mode 100644 index 0000000..42de4d5 --- /dev/null +++ b/test/integration/sign_in_throttle_test.rb @@ -0,0 +1,40 @@ +require "test_helper" + +class SignInThrottleTest < ActionDispatch::IntegrationTest + setup do + @shopkeeper = shopkeepers(:one) + @shopkeeper.create_default_account + end + + def post_sign_in(email:, password: "password", ip: nil) + headers = {source: "ios"} + headers["REMOTE_ADDR"] = ip if ip + + post shopkeeper_session_url, + params: {email: email, password: password}, + headers: headers, + as: :json + end + + test "the sixth sign-in from the same IP within the window is rate-limited" do + 5.times { post_sign_in(email: "noone#{_1}@example.com", password: "wrong") } + + post_sign_in(email: "noone5@example.com", password: "wrong") + + assert_response :too_many_requests + assert_equal 429, response.parsed_body["code"] + assert_equal I18n.t("errors.messages.too_many_logins"), response.parsed_body["error_message"] + end + + test "the sixth sign-in attempt against the same email from different IPs is rate-limited" do + 5.times do |i| + post_sign_in(email: @shopkeeper.email, password: "wrong", ip: "10.0.0.#{i + 1}") + end + + post_sign_in(email: @shopkeeper.email, password: "wrong", ip: "10.0.0.99") + + assert_response :too_many_requests + assert_equal 429, response.parsed_body["code"] + assert_equal I18n.t("errors.messages.too_many_logins"), response.parsed_body["error_message"] + end +end