diff --git a/app/controllers/discourse_solved/answer_controller.rb b/app/controllers/discourse_solved/answer_controller.rb index ced07b0..fa1e750 100644 --- a/app/controllers/discourse_solved/answer_controller.rb +++ b/app/controllers/discourse_solved/answer_controller.rb @@ -35,6 +35,14 @@ class DiscourseSolved::AnswerController < ::ApplicationController def limit_accepts return if current_user.staff? + run_rate_limiter = + DiscoursePluginRegistry.apply_modifier( + :solved_answers_controller_run_rate_limiter, + true, + current_user, + ) + return if !run_rate_limiter + RateLimiter.new(nil, "accept-hr-#{current_user.id}", 20, 1.hour).performed! RateLimiter.new(nil, "accept-min-#{current_user.id}", 4, 30.seconds).performed! end diff --git a/config/settings.yml b/config/settings.yml index ef73431..fe42b4e 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -59,4 +59,13 @@ discourse_solved: enable_solved_tags: type: tag_list default: "" - + solved_bypass_rate_limit: + default: false + client: false + description: "Allow high trust level users to bypass rate limiting for accepting solutions" + solved_min_trust_level_for_bypass: + default: 3 + client: false + min: 0 + max: 4 + description: "Minimum trust level required to bypass rate limiting" diff --git a/plugin.rb b/plugin.rb index 13f51a0..271681b 100644 --- a/plugin.rb +++ b/plugin.rb @@ -352,4 +352,26 @@ after_initialize do DiscourseDev::DiscourseSolved.populate(self) DiscourseAutomation::EntryPoint.inject(self) if defined?(DiscourseAutomation) DiscourseAssign::EntryPoint.inject(self) if defined?(DiscourseAssign) + + # Register a modifier to control rate limiting for solution acceptance + # Parameters: + # - default_value: The default rate limiting state (true to apply rate limiting, false to skip it) + # - user: The current user object, used to determine whether to apply rate limiting + register_modifier(:solved_answers_controller_run_rate_limiter) do |default_value, user| + get_setting = ->(name, default) do + SiteSetting.respond_to?(name) ? SiteSetting.public_send(name) : default + end + + user_level = user.respond_to?(:trust_level) ? user.trust_level : 0 + + min_bypass_level = get_setting.call(:solved_min_trust_level_for_bypass, 3) + + bypass_enabled = get_setting.call(:solved_bypass_rate_limit, false) + + if bypass_enabled && user_level >= min_bypass_level + false + else + default_value + end + end end diff --git a/spec/requests/answer_controller_spec.rb b/spec/requests/answer_controller_spec.rb new file mode 100644 index 0000000..c91eb10 --- /dev/null +++ b/spec/requests/answer_controller_spec.rb @@ -0,0 +1,176 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe DiscourseSolved::AnswerController do + fab!(:user) + fab!(:high_trust_user) { Fabricate(:user, trust_level: 3) } + fab!(:staff_user) { Fabricate(:admin) } + fab!(:category) + fab!(:topic) { Fabricate(:topic, category: category) } + fab!(:post) { Fabricate(:post, topic: topic) } + fab!(:solution_post) { Fabricate(:post, topic: topic) } + + before do + SiteSetting.solved_enabled = true + SiteSetting.allow_solved_on_all_topics = true + category.custom_fields[DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD] = "true" + category.save_custom_fields + + # Make the topic's first post's user the topic creator + # This ensures the guardian will allow the acceptance + topic.user_id = user.id + topic.save! + + # Make solution_post's user different from the topic creator + solution_post.user_id = Fabricate(:user).id + solution_post.save! + + # Give permission to accept solutions + user.update!(trust_level: 1) + high_trust_user.update!(trust_level: 3) + end + + describe "#accept" do + context "with rate limiting enabled" do + it "applies rate limits to regular users" do + sign_in(user) + + # First request should succeed + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(200) + + # Try to make too many requests in a short time + RateLimiter.any_instance.expects(:performed!).raises(RateLimiter::LimitExceeded.new(60)) + + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(429) # Rate limited status + end + + it "does not apply rate limits to staff" do + sign_in(staff_user) + + # Staff users bypass rate limiting by default + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(200) + + # Can make multiple requests without hitting rate limits + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(200) + end + end + + context "with bypass settings" do + before do + SiteSetting.solved_bypass_rate_limit = true + SiteSetting.solved_min_trust_level_for_bypass = 3 + end + + it "applies rate limits to low trust users" do + sign_in(user) + + # First request should succeed + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(200) + + # Try to make too many requests in a short time + RateLimiter.any_instance.expects(:performed!).raises(RateLimiter::LimitExceeded.new(60)) + + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(429) # Rate limited status + end + + it "does not apply rate limits to high trust users" do + sign_in(high_trust_user) + + # First request should succeed without rate limiting + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(200) + + # Should be able to make another request without rate limiting + RateLimiter.any_instance.expects(:performed!).never + + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(200) + end + + it "respects min trust level setting changes" do + SiteSetting.solved_min_trust_level_for_bypass = 4 + + sign_in(high_trust_user) # TL3 user + + # First request should succeed + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(200) + + # Now rate limiting should apply since TL3 < TL4 requirement + RateLimiter.any_instance.expects(:performed!).raises(RateLimiter::LimitExceeded.new(60)) + + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(429) # Rate limited status + end + end + + context "with bypass disabled" do + before do + SiteSetting.solved_bypass_rate_limit = false + SiteSetting.solved_min_trust_level_for_bypass = 3 + end + + it "applies rate limits to all non-staff users" do + sign_in(high_trust_user) # TL3 user + + # First request should succeed + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(200) + + # Rate limiting should apply despite high trust level because bypass is disabled + RateLimiter.any_instance.expects(:performed!).raises(RateLimiter::LimitExceeded.new(60)) + + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(429) # Rate limited status + end + end + end + + describe "#unaccept" do + before do + # Set up an accepted solution first + sign_in(user) + post "/solution/accept.json", params: { id: solution_post.id } + expect(response.status).to eq(200) + sign_out + end + + context "with bypass settings" do + before do + SiteSetting.solved_bypass_rate_limit = true + SiteSetting.solved_min_trust_level_for_bypass = 3 + end + + it "applies rate limits to low trust users" do + sign_in(user) + + # Try to make too many requests in a short time + RateLimiter.any_instance.expects(:performed!).raises(RateLimiter::LimitExceeded.new(60)) + + post "/solution/unaccept.json", params: { id: solution_post.id } + expect(response.status).to eq(429) # Rate limited status + end + + it "does not apply rate limits to high trust users" do + # Give topic ownership to high trust user so they can unaccept + topic.user_id = high_trust_user.id + topic.save! + + sign_in(high_trust_user) + + # Should be able to unaccept without rate limiting + RateLimiter.any_instance.expects(:performed!).never + + post "/solution/unaccept.json", params: { id: solution_post.id } + expect(response.status).to eq(200) + end + end + end +end