From 186f9161923f05cbc1999167caa9a2cf168749e9 Mon Sep 17 00:00:00 2001
From: Emelia Smith <ThisIsMissEm@users.noreply.github.com>
Date: Wed, 29 May 2024 16:00:05 +0200
Subject: [PATCH] Fix: remove broken OAuth Application vacuuming & throttle
 OAuth Application registrations (#30316)

Co-authored-by: Claire <claire.github-309c@sitedethib.com>
---
 app/lib/vacuum/applications_vacuum.rb        | 10 ----
 app/workers/scheduler/vacuum_scheduler.rb    |  5 --
 config/initializers/rack_attack.rb           |  4 ++
 spec/config/initializers/rack_attack_spec.rb | 18 ++++++++
 spec/lib/vacuum/applications_vacuum_spec.rb  | 48 --------------------
 5 files changed, 22 insertions(+), 63 deletions(-)
 delete mode 100644 app/lib/vacuum/applications_vacuum.rb
 delete mode 100644 spec/lib/vacuum/applications_vacuum_spec.rb

diff --git a/app/lib/vacuum/applications_vacuum.rb b/app/lib/vacuum/applications_vacuum.rb
deleted file mode 100644
index ba88655f1..000000000
--- a/app/lib/vacuum/applications_vacuum.rb
+++ /dev/null
@@ -1,10 +0,0 @@
-# frozen_string_literal: true
-
-class Vacuum::ApplicationsVacuum
-  def perform
-    Doorkeeper::Application.where(owner_id: nil)
-                           .where.missing(:created_users, :access_tokens, :access_grants)
-                           .where(created_at: ...1.day.ago)
-                           .in_batches.delete_all
-  end
-end
diff --git a/app/workers/scheduler/vacuum_scheduler.rb b/app/workers/scheduler/vacuum_scheduler.rb
index 1c9a2aabe..c22d6f5f8 100644
--- a/app/workers/scheduler/vacuum_scheduler.rb
+++ b/app/workers/scheduler/vacuum_scheduler.rb
@@ -22,7 +22,6 @@ class Scheduler::VacuumScheduler
       preview_cards_vacuum,
       backups_vacuum,
       access_tokens_vacuum,
-      applications_vacuum,
       feeds_vacuum,
       imports_vacuum,
     ]
@@ -56,10 +55,6 @@ class Scheduler::VacuumScheduler
     Vacuum::ImportsVacuum.new
   end
 
-  def applications_vacuum
-    Vacuum::ApplicationsVacuum.new
-  end
-
   def content_retention_policy
     ContentRetentionPolicy.current
   end
diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb
index d0af0fe94..482f4b95a 100644
--- a/config/initializers/rack_attack.rb
+++ b/config/initializers/rack_attack.rb
@@ -105,6 +105,10 @@ class Rack::Attack
     req.authenticated_user_id if (req.post? && req.path.match?(API_DELETE_REBLOG_REGEX)) || (req.delete? && req.path.match?(API_DELETE_STATUS_REGEX))
   end
 
+  throttle('throttle_oauth_application_registrations/ip', limit: 5, period: 10.minutes) do |req|
+    req.throttleable_remote_ip if req.post? && req.path == '/api/v1/apps'
+  end
+
   throttle('throttle_sign_up_attempts/ip', limit: 25, period: 5.minutes) do |req|
     req.throttleable_remote_ip if req.post? && req.path_matches?('/auth')
   end
diff --git a/spec/config/initializers/rack_attack_spec.rb b/spec/config/initializers/rack_attack_spec.rb
index 7cd4ac76b..ddbd74705 100644
--- a/spec/config/initializers/rack_attack_spec.rb
+++ b/spec/config/initializers/rack_attack_spec.rb
@@ -103,4 +103,22 @@ describe Rack::Attack, type: :request do
       it_behaves_like 'throttled endpoint'
     end
   end
+
+  describe 'throttle excessive oauth application registration requests by IP address' do
+    let(:throttle) { 'throttle_oauth_application_registrations/ip' }
+    let(:limit)  { 5 }
+    let(:period) { 10.minutes }
+    let(:path)   { '/api/v1/apps' }
+    let(:params) do
+      {
+        client_name: 'Throttle Test',
+        redirect_uris: 'urn:ietf:wg:oauth:2.0:oob',
+        scopes: 'read',
+      }
+    end
+
+    let(:request) { -> { post path, params: params, headers: { 'REMOTE_ADDR' => remote_ip } } }
+
+    it_behaves_like 'throttled endpoint'
+  end
 end
diff --git a/spec/lib/vacuum/applications_vacuum_spec.rb b/spec/lib/vacuum/applications_vacuum_spec.rb
deleted file mode 100644
index 57a222aaf..000000000
--- a/spec/lib/vacuum/applications_vacuum_spec.rb
+++ /dev/null
@@ -1,48 +0,0 @@
-# frozen_string_literal: true
-
-require 'rails_helper'
-
-RSpec.describe Vacuum::ApplicationsVacuum do
-  subject { described_class.new }
-
-  describe '#perform' do
-    let!(:app_with_token)  { Fabricate(:application, created_at: 1.month.ago) }
-    let!(:app_with_grant)  { Fabricate(:application, created_at: 1.month.ago) }
-    let!(:app_with_signup) { Fabricate(:application, created_at: 1.month.ago) }
-    let!(:app_with_owner)  { Fabricate(:application, created_at: 1.month.ago, owner: Fabricate(:user)) }
-    let!(:unused_app)      { Fabricate(:application, created_at: 1.month.ago) }
-    let!(:recent_app)      { Fabricate(:application, created_at: 1.hour.ago) }
-
-    let!(:active_access_token) { Fabricate(:access_token, application: app_with_token) }
-    let!(:active_access_grant) { Fabricate(:access_grant, application: app_with_grant) }
-    let!(:user) { Fabricate(:user, created_by_application: app_with_signup) }
-
-    before do
-      subject.perform
-    end
-
-    it 'does not delete applications with valid access tokens' do
-      expect { app_with_token.reload }.to_not raise_error
-    end
-
-    it 'does not delete applications with valid access grants' do
-      expect { app_with_grant.reload }.to_not raise_error
-    end
-
-    it 'does not delete applications that were used to create users' do
-      expect { app_with_signup.reload }.to_not raise_error
-    end
-
-    it 'does not delete owned applications' do
-      expect { app_with_owner.reload }.to_not raise_error
-    end
-
-    it 'does not delete applications registered less than a day ago' do
-      expect { recent_app.reload }.to_not raise_error
-    end
-
-    it 'deletes unused applications' do
-      expect { unused_app.reload }.to raise_error ActiveRecord::RecordNotFound
-    end
-  end
-end