From aceb4e80698daddcf9ade1b5aef5dc2783e12de8 Mon Sep 17 00:00:00 2001 From: "zhanfeng.zeng" Date: Mon, 19 May 2025 14:42:20 +0800 Subject: [PATCH] FEATURE: Provide generic modifier interface for rate limitin --- config/settings.yml | 10 --- plugin.rb | 22 ----- spec/requests/answer_controller_spec.rb | 113 ++++-------------------- 3 files changed, 17 insertions(+), 128 deletions(-) diff --git a/config/settings.yml b/config/settings.yml index fe42b4e..8c5a744 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -59,13 +59,3 @@ 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 271681b..13f51a0 100644 --- a/plugin.rb +++ b/plugin.rb @@ -352,26 +352,4 @@ 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 index 5c0c58e..226158a 100644 --- a/spec/requests/answer_controller_spec.rb +++ b/spec/requests/answer_controller_spec.rb @@ -4,7 +4,6 @@ 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) } @@ -19,7 +18,6 @@ describe DiscourseSolved::AnswerController do # Give permission to accept solutions user.update!(trust_level: 1) - high_trust_user.update!(trust_level: 3) end describe "#accept" do @@ -54,84 +52,27 @@ describe DiscourseSolved::AnswerController do 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 + context "with plugin modifier" do + it "allows plugins to bypass rate limiting via modifier" 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 - # 让high_trust_user成为话题创建者，这样他就有权限接受答案 - topic.update!(user_id: high_trust_user.id) - - 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 - # 让high_trust_user成为话题创建者，这样他就有权限接受答案 - topic.update!(user_id: high_trust_user.id) - - SiteSetting.solved_min_trust_level_for_bypass = 4 - - sign_in(high_trust_user) # TL3 user + # Example of how plugins can customize rate limiting behavior + DiscoursePluginRegistry.register_modifier( + :solved_answers_controller_run_rate_limiter, + ) do |_, _| + false # Skip rate limiting completely + end # 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 - - # 让high_trust_user成为话题创建者，这样他就有权限接受答案 - topic.update!(user_id: high_trust_user.id) - end - - it "applies rate limits to all non-staff users" do - sign_in(high_trust_user) # TL3 user - - # First request should succeed + # Second request should also succeed because rate limiting is bypassed 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 + # Clean up + DiscoursePluginRegistry.unregister_modifier(:solved_answers_controller_run_rate_limiter) end end end @@ -148,34 +89,14 @@ describe DiscourseSolved::AnswerController do 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 regular users" do + sign_in(user) - 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)) - # 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.update!(user_id: high_trust_user.id) - - 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 + post "/solution/unaccept.json", params: { id: solution_post.id } + expect(response.status).to eq(429) # Rate limited status end end end