From 6bdcbf933ed0f32928c701de03836b3c31c74a6a Mon Sep 17 00:00:00 2001
From: Martin Gruner <mg@zammad.com>
Date: Thu, 31 Mar 2022 10:35:43 +0200
Subject: [PATCH] Maintenance: Improved password reset handling.

(cherry picked from commit 8ea7b6630951b603a97bd38128de3f915a549b4d)
---
 Gemfile                                   |  3 ++
 Gemfile.lock                              |  3 ++
 config/initializers/rack_attack.rb        | 17 +++++++++
 spec/requests/user/password_reset_spec.rb | 42 +++++++++++++++++++++++
 4 files changed, 65 insertions(+)
 create mode 100644 config/initializers/rack_attack.rb
 create mode 100644 spec/requests/user/password_reset_spec.rb

diff --git a/Gemfile b/Gemfile
index cca5e1229..3bb4a2cfb 100644
--- a/Gemfile
+++ b/Gemfile
@@ -94,6 +94,9 @@ gem 'omniauth-saml'
 gem 'omniauth-twitter'
 gem 'omniauth-weibo-oauth2'
 
+# Rate limiting
+gem 'rack-attack'
+
 # channels
 gem 'gmail_xoauth'
 gem 'koala'
diff --git a/Gemfile.lock b/Gemfile.lock
index 9f1f07063..11b431dca 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -391,6 +391,8 @@ GEM
       rspec-rails (>= 3.0.0)
     racc (1.6.0)
     rack (2.2.3)
+    rack-attack (6.6.0)
+      rack (>= 1.0, < 3)
     rack-livereload (0.3.17)
       rack
     rack-test (1.1.0)
@@ -663,6 +665,7 @@ DEPENDENCIES
   puma (~> 4)
   pundit
   pundit-matchers
+  rack-attack
   rack-livereload
   rails (~> 6.0.0)
   rails-controller-testing
diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb
new file mode 100644
index 000000000..58727995c
--- /dev/null
+++ b/config/initializers/rack_attack.rb
@@ -0,0 +1,17 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+#
+# Throttle password reset requests
+#
+API_V1_USERS__PASSWORD_RESET_PATH = '/api/v1/users/password_reset'.freeze
+Rack::Attack.throttle('limit password reset requests per username', limit: 3, period: 60) do |req|
+  if req.path == API_V1_USERS__PASSWORD_RESET_PATH && req.post?
+    # Normalize to protect against rate limit bypasses.
+    req.params['username'].to_s.downcase.gsub(%r{\s+}, '')
+  end
+end
+Rack::Attack.throttle('limit password reset requests per source IP address', limit: 3, period: 60) do |req|
+  if req.path == API_V1_USERS__PASSWORD_RESET_PATH && req.post?
+    req.ip
+  end
+end
diff --git a/spec/requests/user/password_reset_spec.rb b/spec/requests/user/password_reset_spec.rb
new file mode 100644
index 000000000..2dd16b672
--- /dev/null
+++ b/spec/requests/user/password_reset_spec.rb
@@ -0,0 +1,42 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'User endpoint', type: :request, authenticated_as: false do
+
+  context 'when user resets password once' do
+    it 'creates a token' do
+      expect do
+        post api_v1_users_password_reset_path, params: { username: create(:user).login }
+      end.to change(Token, :count)
+    end
+
+    it 'returns success' do
+      post api_v1_users_password_reset_path, params: { username: create(:user).login }
+      expect(response).to have_http_status(:ok)
+    end
+  end
+
+  # For the throttling, see config/initializers/rack_attack.rb.
+  context 'when user resets password more than throttle allows' do
+
+    let(:static_username) { create(:user).login }
+    let(:static_ipv4) { Faker::Internet.ip_v4_address }
+
+    it 'blocks due to username throttling (multiple IPs)' do
+      5.times do
+        post api_v1_users_password_reset_path, params: { username: static_username }, headers: { 'X-Forwarded-For': Faker::Internet.ip_v4_address }
+      end
+
+      expect(response).to have_http_status(:too_many_requests)
+    end
+
+    it 'blocks due to source IP address throttling (multiple usernames)' do
+      5.times do
+        post api_v1_users_password_reset_path, params: { username: create(:user).login }, headers: { 'X-Forwarded-For': static_ipv4 }
+      end
+
+      expect(response).to have_http_status(:too_many_requests)
+    end
+  end
+end