Merge pull request #4202 from tomhughes/changeset-comment-limit

Add rate limiting for changeset comments

app/assets/javascripts/index/changeset.js

@@ -29,6 +29,7 @@ OSM.Changeset = function (map) {
   function updateChangeset(form, method, url, include_data) {
     var data;
 
+    $(form).find("#comment-error").prop("hidden", true);
     $(form).find("input[type=submit]").prop("disabled", true);
 
     if (include_data) {
@@ -44,6 +45,11 @@ OSM.Changeset = function (map) {
       data: data,
       success: function () {
         OSM.loadSidebarContent(window.location.pathname, page.load);
+      },
+      error: function (xhr, xhr_status, http_status) {
+        $(form).find("#comment-error").text(http_status);
+        $(form).find("#comment-error").prop("hidden", false);
+        $(form).find("input[type=submit]").prop("disabled", false);
       }
     });
   }

app/controllers/api/changeset_comments_controller.rb

@@ -17,6 +17,7 @@ module Api
       # Check the arguments are sane
       raise OSM::APIBadUserInput, "No id was given" unless params[:id]
       raise OSM::APIBadUserInput, "No text was given" if params[:text].blank?
+      raise OSM::APIRateLimitExceeded if current_user.changeset_comments.where("created_at >= ?", Time.now.utc - 1.hour).count >= current_user.max_changeset_comments_per_hour
 
       # Extract the arguments
       id = params[:id].to_i

app/models/user.rb

@@ -395,6 +395,19 @@ class User < ApplicationRecord
     max_friends.clamp(0, Settings.max_friends_per_hour)
   end
 
+  def max_changeset_comments_per_hour
+    if moderator?
+      36000
+    else
+      previous_comments = changeset_comments.limit(200).count
+      active_reports = issues.with_status(:open).sum(:reports_count)
+      max_comments = previous_comments / 200.0 * Settings.max_changeset_comments_per_hour
+      max_comments = max_comments.floor.clamp(Settings.min_changeset_comments_per_hour, Settings.max_changeset_comments_per_hour)
+      max_comments /= 2**active_reports
+      max_comments.floor.clamp(1, Settings.max_changeset_comments_per_hour)
+    end
+  end
+
   private
 
   def encrypt_password

app/views/browse/changeset.html.erb

@@ -78,6 +78,8 @@
         <div class="mb-3">
           <textarea class="form-control" name="text" cols="40" rows="5"></textarea>
         </div>
+        <div id="comment-error" class="alert-danger p-2 mb-3" hidden>
+        </div>
         <div>
           <input type="submit" name="comment" value="<%= t("javascripts.changesets.show.comment") %>" data-changeset-id="<%= @changeset.id %>" data-method="POST" data-url="<%= changeset_comment_url(@changeset) %>" disabled="1" class="btn btn-sm btn-primary" />
         </div>

config/settings.yml

@@ -55,6 +55,9 @@ user_block_periods: [0, 1, 3, 6, 12, 24, 48, 96, 168, 336, 731, 4383, 8766, 8766
 max_messages_per_hour: 60
 # Rate limit for friending
 max_friends_per_hour: 60
+# Rate limit for changeset comments
+min_changeset_comments_per_hour: 6
+max_changeset_comments_per_hour: 60
 # Domain for handling message replies
 #messages_domain: "messages.openstreetmap.org"
 # MaxMind GeoIPv2 database

db/migrate/20230825162137_restore_author_index_to_changeset_comments.rb

@@ -0,0 +1,7 @@
+class RestoreAuthorIndexToChangesetComments < ActiveRecord::Migration[7.0]
+  disable_ddl_transaction!
+
+  def change
+    add_index :changeset_comments, [:author_id, :created_at], :algorithm => :concurrently
+  end
+end

db/structure.sql

@@ -2370,6 +2370,13 @@ CREATE UNIQUE INDEX index_active_storage_blobs_on_key ON public.active_storage_b
 CREATE UNIQUE INDEX index_active_storage_variant_records_uniqueness ON public.active_storage_variant_records USING btree (blob_id, variation_digest);
 
 
+--
+-- Name: index_changeset_comments_on_author_id_and_created_at; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_changeset_comments_on_author_id_and_created_at ON public.changeset_comments USING btree (author_id, created_at);
+
+
 --
 -- Name: index_changeset_comments_on_changeset_id_and_created_at; Type: INDEX; Schema: public; Owner: -
 --
@@ -3396,6 +3403,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20220201183346'),
 ('20220223140543'),
 ('20230816135800'),
+('20230825162137'),
 ('21'),
 ('22'),
 ('23'),

lib/osm.rb

@@ -353,6 +353,13 @@ module OSM
     end
   end
 
+  # Raised when a rate limit is exceeded
+  class APIRateLimitExceeded < APIError
+    def status
+      :too_many_requests
+    end
+  end
+
   # Helper methods for going to/from mercator and lat/lng.
   class Mercator
     include Math

test/controllers/api/changeset_comments_controller_test.rb

@@ -132,6 +132,27 @@ module Api
       assert_response :bad_request
     end
 
+    ##
+    # create comment rate limit
+    def test_create_comment_rate_limit
+      changeset = create(:changeset, :closed)
+      user = create(:user)
+
+      auth_header = basic_authorization_header user.email, "test"
+
+      assert_difference "ChangesetComment.count", Settings.min_changeset_comments_per_hour do
+        1.upto(Settings.min_changeset_comments_per_hour) do |count|
+          post changeset_comment_path(:id => changeset, :text => "Comment #{count}"), :headers => auth_header
+          assert_response :success
+        end
+      end
+
+      assert_no_difference "ChangesetComment.count" do
+        post changeset_comment_path(:id => changeset, :text => "One comment too many"), :headers => auth_header
+        assert_response :too_many_requests
+      end
+    end
+
     ##
     # test hide comment fail
     def test_destroy_comment_fail