Maintenance: Reimplement form submit throttling.

.eslintignore

@@ -5,5 +5,6 @@ app/frontend/**/graphql/**/*.api.ts
 app/frontend/shared/graphql/types.ts
 .storybook/storybook-static/**
 
-# Skip legacy qunit tests
+# Skip legacy files
 public/assets/tests/**/*.js
+public/assets/form/**/*.js

app/controllers/form_controller.rb

@@ -10,7 +10,6 @@ class FormController < ApplicationController
 
   def configuration
     return if !fingerprint_exists?
-    return if limit_reached?
 
     api_path  = Rails.configuration.api_path
     http_type = Setting.get('http_type')
@@ -34,7 +33,6 @@ class FormController < ApplicationController
   def submit
     return if !fingerprint_exists?
     return if !token_valid?(params[:token], params[:fingerprint])
-    return if limit_reached?
 
     # validate input
     errors = {}
@@ -199,28 +197,6 @@ class FormController < ApplicationController
     true
   end
 
-  def limit_reached?
-    return false if !SearchIndexBackend.enabled?
-
-    # quote ipv6 ip'
-    remote_ip = request.remote_ip.gsub(':', '\\:')
-
-    # in elasticsearch7 "created_at:>now-1h" is not working. Needed to catch -2h
-    form_limit_by_ip_per_hour = Setting.get('form_ticket_create_by_ip_per_hour') || 20
-    result = SearchIndexBackend.search("preferences.form.remote_ip:'#{remote_ip}' AND created_at:>now-2h", 'Ticket', limit: form_limit_by_ip_per_hour)
-    raise Exceptions::Forbidden if result.count >= form_limit_by_ip_per_hour.to_i
-
-    form_limit_by_ip_per_day = Setting.get('form_ticket_create_by_ip_per_day') || 240
-    result = SearchIndexBackend.search("preferences.form.remote_ip:'#{remote_ip}' AND created_at:>now-1d", 'Ticket', limit: form_limit_by_ip_per_day)
-    raise Exceptions::Forbidden if result.count >= form_limit_by_ip_per_day.to_i
-
-    form_limit_per_day = Setting.get('form_ticket_create_per_day') || 5000
-    result = SearchIndexBackend.search('preferences.form.remote_ip:* AND created_at:>now-1d', 'Ticket', limit: form_limit_per_day)
-    raise Exceptions::Forbidden if result.count >= form_limit_per_day.to_i
-
-    false
-  end
-
   def fingerprint_exists?
     return true if params[:fingerprint].present? && params[:fingerprint].length > 30
 

config/initializers/rack_attack.rb

@@ -4,14 +4,37 @@
 # 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|
+Rack::Attack.throttle('limit password reset requests per username', limit: 3, period: 1.minute.to_i) 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|
+Rack::Attack.throttle('limit password reset requests per source IP address', limit: 3, period: 1.minute.to_i) do |req|
   if req.path == API_V1_USERS__PASSWORD_RESET_PATH && req.post?
     req.ip
   end
 end
+
+#
+# Throttle form submit requests
+#
+API_V1_FORM_SUBMIT_PATH = '/api/v1/form_submit'.freeze
+form_limit_by_ip_per_hour_proc = proc { Setting.get('form_ticket_create_by_ip_per_hour') || 20 }
+Rack::Attack.throttle('form submits per IP and hour', limit: form_limit_by_ip_per_hour_proc, period: 1.hour.to_i) do |req|
+  if req.path == API_V1_FORM_SUBMIT_PATH
+    req.ip
+  end
+end
+form_limit_by_ip_per_day_proc = proc { Setting.get('form_ticket_create_by_ip_per_day') || 240 }
+Rack::Attack.throttle('form submits per IP and day', limit: form_limit_by_ip_per_day_proc, period: 1.day.to_i) do |req|
+  if req.path == API_V1_FORM_SUBMIT_PATH
+    req.ip
+  end
+end
+form_limit_per_day_proc = proc { Setting.get('form_ticket_create_per_day') || 5000 }
+Rack::Attack.throttle('form submits per day', limit: form_limit_per_day_proc, period: 1.day.to_i) do |req|
+  if req.path == API_V1_FORM_SUBMIT_PATH
+    req.path
+  end
+end

public/assets/form/form.js

@@ -316,7 +316,7 @@ $(function() {
       _this.log('debug', 'modalOpenTime', _this.modalOpenTime.getTime())
       _this.log('debug', 'diffTime', diff)
       if (diff < 1000*10) {
-        alert('Sorry, you look like an robot!')
+        alert('Sorry, you look like a robot!')
         return
       }
     }
@@ -357,7 +357,7 @@ $(function() {
 
     }).fail(function() {
       _this.$form.find('button').prop('disabled', false)
-      alert('Faild to submit form!')
+      alert('The form could not be submitted!')
     });
   }
 

spec/requests/form_spec.rb

@@ -2,12 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe 'Form', type: :request, searchindex: true do
-
-  before do
-    configure_elasticsearch
-    rebuild_searchindex
-  end
+RSpec.describe 'Form', type: :request do
 
   describe 'request handling' do
 
@@ -139,8 +134,6 @@ RSpec.describe 'Form', type: :request, searchindex: true do
     end
 
     it 'does limits' do
-      skip('No ES configured') if !SearchIndexBackend.enabled?
-
       Setting.set('form_ticket_create', true)
       fingerprint = SecureRandom.hex(40)
       post '/api/v1/form_config', params: { fingerprint: fingerprint }, as: :json
@@ -160,15 +153,10 @@ RSpec.describe 'Form', type: :request, searchindex: true do
         expect(json_response['errors']).to be_falsey
         expect(json_response['ticket']).to be_truthy
         expect(json_response['ticket']['id']).to be_truthy
-        Scheduler.worker(true)
       end
 
-      sleep 10 # wait until elasticsearch is index
-
       post '/api/v1/form_submit', params: { fingerprint: fingerprint, token: token, name: 'Bob Smith', email: 'discard@znuny.com', title: 'test-last', body: 'hello' }, as: :json
-      expect(response).to have_http_status(:forbidden)
-      expect(json_response).to be_a_kind_of(Hash)
-      expect(json_response['error']).to be_truthy
+      expect(response).to have_http_status(:too_many_requests)
 
       @headers = { 'ACCEPT' => 'application/json', 'CONTENT_TYPE' => 'application/json', 'REMOTE_ADDR' => '1.2.3.5' }
 
@@ -180,15 +168,10 @@ RSpec.describe 'Form', type: :request, searchindex: true do
         expect(json_response['errors']).to be_falsey
         expect(json_response['ticket']).to be_truthy
         expect(json_response['ticket']['id']).to be_truthy
-        Scheduler.worker(true)
       end
 
-      sleep 10 # wait until elasticsearch is index
-
       post '/api/v1/form_submit', params: { fingerprint: fingerprint, token: token, name: 'Bob Smith', email: 'discard@znuny.com', title: 'test-2-last', body: 'hello' }, as: :json
-      expect(response).to have_http_status(:forbidden)
-      expect(json_response).to be_a_kind_of(Hash)
-      expect(json_response['error']).to be_truthy
+      expect(response).to have_http_status(:too_many_requests)
 
       @headers = { 'ACCEPT' => 'application/json', 'CONTENT_TYPE' => 'application/json', 'REMOTE_ADDR' => '::1' }
 
@@ -200,15 +183,10 @@ RSpec.describe 'Form', type: :request, searchindex: true do
         expect(json_response['errors']).to be_falsey
         expect(json_response['ticket']).to be_truthy
         expect(json_response['ticket']['id']).to be_truthy
-        Scheduler.worker(true)
       end
 
-      sleep 10 # wait until elasticsearch is index
-
       post '/api/v1/form_submit', params: { fingerprint: fingerprint, token: token, name: 'Bob Smith', email: 'discard@znuny.com', title: 'test-2-last', body: 'hello' }, as: :json
-      expect(response).to have_http_status(:forbidden)
-      expect(json_response).to be_a_kind_of(Hash)
-      expect(json_response['error']).to be_truthy
+      expect(response).to have_http_status(:too_many_requests)
     end
 
     it 'does customer_ticket_create false disables form' do

spec/system/form_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe 'Form', type: :system, authenticated_as: true do
   shared_examples 'validating form fields' do
     it 'validate name input' do
       within form_context do
-        fill_in 'Email', with: 'discard@znuny.com'
+        fill_in 'Email', with: 'discard@discard.zammad.org'
         fill_in 'Message', with: 'message here'
         click_on 'Submit'
 
@@ -28,7 +28,7 @@ RSpec.describe 'Form', type: :system, authenticated_as: true do
     it 'validate message input' do
       within form_context do
         fill_in 'Name', with: 'some sender'
-        fill_in 'Email', with: 'discard@znuny.com'
+        fill_in 'Email', with: 'discard@discard.zammad.org'
         click_on 'Submit'
 
         expect(page).to have_validation_message_for(body_input)
@@ -66,7 +66,7 @@ RSpec.describe 'Form', type: :system, authenticated_as: true do
       within form_context do
         fill_in 'Name', with: 'some sender'
         fill_in 'Message', with: 'message here'
-        fill_in 'Email', with: 'discard@znuny.com'
+        fill_in 'Email', with: 'discard@discard.zammad.org'
         sleep 10
         click_on 'Submit'
 
@@ -78,9 +78,23 @@ RSpec.describe 'Form', type: :system, authenticated_as: true do
       within form_context do
         fill_in 'Name', with: 'some sender'
         fill_in 'Message', with: 'message here'
-        fill_in 'Email', with: 'discard@znuny.com'
+        fill_in 'Email', with: 'discard@discard.zammad.org'
         click_on 'Submit'
-        accept_alert('Sorry, you look like an robot!')
+        accept_alert('Sorry, you look like a robot!')
+      end
+    end
+  end
+
+  shared_examples 'submitting fails due to throttling' do
+    it 'rejects form submission due to throttling' do
+      within form_context do
+        fill_in 'Name', with: 'some sender'
+        fill_in 'Message', with: 'message here'
+        fill_in 'Email', with: 'discard@discard.zammad.org'
+        sleep 10
+        # Avoid await_empty_ajax_queue.
+        execute_script('$("button:submit").trigger("click")')
+        accept_alert('The form could not be submitted!')
       end
     end
   end
@@ -162,6 +176,20 @@ RSpec.describe 'Form', type: :system, authenticated_as: true do
         it_behaves_like 'validating form fields'
         it_behaves_like 'submitting valid form fields'
       end
+
+      context 'when form is throttled with :too_many_requests' do
+        before do
+          Setting.set('form_ticket_create_by_ip_per_hour', 0)
+          visit path
+        end
+
+        let(:form_context) { form_inline_selector }
+        let(:name_input) { '#zammad-form-name-inline' }
+        let(:body_input) { '#zammad-form-body-inline' }
+        let(:email_input) { '#zammad-form-email-inline' }
+
+        it_behaves_like 'submitting fails due to throttling'
+      end
     end
 
     context 'when feature is disabled' do