From ce7a14104b50c65a3fd40b88887d2f8a53d73ffa Mon Sep 17 00:00:00 2001
From: Angus McLeod <angus@mcleod.org.au>
Date: Mon, 30 Dec 2024 17:10:48 +0100
Subject: [PATCH] Add user api key client rate limit settings (#30402)

---
 .../user_api_key_clients_controller.rb        | 16 +++++++-
 config/site_settings.yml                      |  9 +++++
 .../user_api_key_clients_controller_spec.rb   | 39 +++++++++++++++++++
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/app/controllers/user_api_key_clients_controller.rb b/app/controllers/user_api_key_clients_controller.rb
index 421ddfc0476..45638a650eb 100644
--- a/app/controllers/user_api_key_clients_controller.rb
+++ b/app/controllers/user_api_key_clients_controller.rb
@@ -12,7 +12,7 @@ class UserApiKeyClientsController < ApplicationController
   end
 
   def create
-    rate_limit
+    rate_limit unless skip_rate_limit?
     require_params
     validate_params
     ensure_new_client
@@ -34,8 +34,20 @@ class UserApiKeyClientsController < ApplicationController
     end
   end
 
+  def skip_rate_limit?
+    SiteSetting
+      .create_user_api_key_client_ip_rate_limit_override_ips
+      .split("|")
+      .include?(request.remote_ip)
+  end
+
   def rate_limit
-    RateLimiter.new(nil, "user-api-key-clients-#{request.remote_ip}", 1, 24.hours).performed!
+    RateLimiter.new(
+      nil,
+      "user-api-key-clients-#{request.remote_ip}",
+      SiteSetting.user_api_key_clients_create_per_day,
+      24.hours,
+    ).performed!
   end
 
   def require_params
diff --git a/config/site_settings.yml b/config/site_settings.yml
index ce57933fabf..8f70a0c0333 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -2381,6 +2381,11 @@ rate_limits:
   max_uploads_per_minute:
     default: 10
     hidden: true
+  user_api_key_clients_create_per_day:
+    default: 3
+    min: 1
+    max: 10
+    hidden: true
 
 developer:
   force_hostname:
@@ -3168,6 +3173,10 @@ user_api:
     default: 30
     max: 36500
     hidden: true
+  create_user_api_key_client_ip_rate_limit_override_ips:
+    default: ""
+    type: list
+    hidden: true
 
 tags:
   tagging_enabled:
diff --git a/spec/requests/user_api_key_clients_controller_spec.rb b/spec/requests/user_api_key_clients_controller_spec.rb
index 4340a776471..6983359e18e 100644
--- a/spec/requests/user_api_key_clients_controller_spec.rb
+++ b/spec/requests/user_api_key_clients_controller_spec.rb
@@ -84,6 +84,45 @@ RSpec.describe UserApiKeyClientsController do
             expect(response.status).to eq(403)
           end
         end
+
+        context "with rate limiting" do
+          before { RateLimiter.enable }
+
+          it "works" do
+            SiteSetting.user_api_key_clients_create_per_day = 1
+            post "/user-api-key-client.json", params: args_with_scopes
+            expect(response.status).to eq(200)
+            post "/user-api-key-client.json",
+                 params: args_with_scopes.merge(client_id: "another_client1")
+            expect(response.status).to eq(429)
+          end
+
+          it "can be changed via site setting" do
+            SiteSetting.user_api_key_clients_create_per_day = 2
+            post "/user-api-key-client.json", params: args_with_scopes
+            expect(response.status).to eq(200)
+            post "/user-api-key-client.json",
+                 params: args_with_scopes.merge(client_id: "another_client1")
+            expect(response.status).to eq(200)
+            post "/user-api-key-client.json",
+                 params: args_with_scopes.merge(client_id: "another_client2")
+            expect(response.status).to eq(429)
+          end
+
+          it "can be overriden by ip address set in a site setting" do
+            SiteSetting.user_api_key_clients_create_per_day = 1
+            SiteSetting.create_user_api_key_client_ip_rate_limit_override_ips = "1.2.3.4"
+
+            post "/user-api-key-client.json", params: args_with_scopes
+            expect(response.status).to eq(200)
+            post "/user-api-key-client.json",
+                 params: args_with_scopes.merge(client_id: "another_client1"),
+                 env: {
+                   REMOTE_ADDR: "1.2.3.4",
+                 }
+            expect(response.status).to eq(200)
+          end
+        end
       end
     end
   end