Merge branch 'sh-rate-limit-archive-endpoint' into 'master'

Rate limit archive endpoint by user

See merge request gitlab-org/gitlab!25750

app/controllers/projects/repositories_controller.rb

@@ -3,11 +3,13 @@
 class Projects::RepositoriesController < Projects::ApplicationController
   include ExtractsPath
   include StaticObjectExternalStorage
+  include Gitlab::RateLimitHelpers
 
   prepend_before_action(only: [:archive]) { authenticate_sessionless_user!(:archive) }
 
   # Authorize
   before_action :require_non_empty_project, except: :create
+  before_action :archive_rate_limit!, only: :archive
   before_action :assign_archive_vars, only: :archive
   before_action :assign_append_sha, only: :archive
   before_action :authorize_download_code!
@@ -34,6 +36,12 @@ class Projects::RepositoriesController < Projects::ApplicationController
 
   private
 
+  def archive_rate_limit!
+    if archive_rate_limit_reached?(current_user, @project)
+      render plain: ::Gitlab::RateLimitHelpers::ARCHIVE_RATE_LIMIT_REACHED_MESSAGE, status: :too_many_requests
+    end
+  end
+
   def repo_params
     @repo_params ||= { ref: @ref, path: params[:path], format: params[:format], append_sha: @append_sha }
   end

changelogs/unreleased/sh-rate-limit-archive-endpoint.yml

+---
+title: Rate limit archive endpoint by user
+merge_request: 25750
+author:
+type: changed

lib/api/repositories.rb

@@ -13,6 +13,8 @@ module API
     end
     resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
       helpers do
+        include ::Gitlab::RateLimitHelpers
+
         def handle_project_member_errors(errors)
           if errors[:project_access].any?
             error!(errors[:project_access], 422)
@@ -89,6 +91,10 @@ module API
         optional :format, type: String, desc: 'The archive format'
       end
       get ':id/repository/archive', requirements: { format: Gitlab::PathRegex.archive_formats_regex } do
+        if archive_rate_limit_reached?(current_user, user_project)
+          render_api_error!({ error: ::Gitlab::RateLimitHelpers::ARCHIVE_RATE_LIMIT_REACHED_MESSAGE }, 429)
+        end
+
         send_git_archive user_project.repository, ref: params[:sha], format: params[:format], append_sha: true
       rescue
         not_found!('File')

lib/gitlab/application_rate_limiter.rb

@@ -21,6 +21,7 @@ module Gitlab
         {
           project_export:              { threshold: 1, interval: 5.minutes },
           project_download_export:     { threshold: 10, interval: 10.minutes },
+          project_repositories_archive: { threshold: 5, interval: 1.minute },
           project_generate_new_export: { threshold: 1, interval: 5.minutes },
           project_import:              { threshold: 30, interval: 10.minutes },
           play_pipeline_schedule:      { threshold: 1, interval: 1.minute },

lib/gitlab/rate_limit_helpers.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module RateLimitHelpers
+    ARCHIVE_RATE_LIMIT_REACHED_MESSAGE = 'This archive has been requested too many times. Try again later.'
+    ARCHIVE_RATE_ANONYMOUS_THRESHOLD = 100 # Allow 100 requests/min for anonymous users
+    ARCHIVE_RATE_THROTTLE_KEY = :project_repositories_archive
+
+    def archive_rate_limit_reached?(user, project)
+      return false unless Feature.enabled?(:archive_rate_limit, default_enabled: true)
+
+      key = ARCHIVE_RATE_THROTTLE_KEY
+
+      if rate_limiter.throttled?(key, scope: [project], threshold: archive_rate_threshold_by_user(user))
+        rate_limiter.log_request(request, "#{key}_request_limit".to_sym, user)
+
+        return true
+      end
+
+      false
+    end
+
+    def archive_rate_threshold_by_user(user)
+      if user
+        nil # Use the defaults
+      else
+        ARCHIVE_RATE_ANONYMOUS_THRESHOLD
+      end
+    end
+
+    def rate_limiter
+      ::Gitlab::ApplicationRateLimiter
+    end
+  end
+end

spec/controllers/projects/repositories_controller_spec.rb

@@ -6,6 +6,10 @@ describe Projects::RepositoriesController do
   let(:project) { create(:project, :repository) }
 
   describe "GET archive" do
+    before do
+      allow(controller).to receive(:archive_rate_limit_reached?).and_return(false)
+    end
+
     context 'as a guest' do
       it 'responds with redirect in correct format' do
         get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master" }, format: "zip"
@@ -96,6 +100,16 @@ describe Projects::RepositoriesController do
         end
       end
 
+      describe 'rate limiting' do
+        it 'rate limits user when thresholds hit' do
+          expect(controller).to receive(:archive_rate_limit_reached?).and_return(true)
+
+          get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'master' }, format: "html"
+
+          expect(response).to have_gitlab_http_status(:too_many_requests)
+        end
+      end
+
       describe 'caching' do
         it 'sets appropriate caching headers' do
           get_archive

spec/lib/gitlab/rate_limit_helpers_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::RateLimitHelpers, :clean_gitlab_redis_shared_state do
+  let(:limiter_class) do
+    Class.new do
+      include ::Gitlab::RateLimitHelpers
+
+      attr_reader :request
+
+      def initialize(request)
+        @request = request
+      end
+    end
+  end
+
+  let(:request) { instance_double(ActionDispatch::Request, request_method: 'GET', ip: '127.0.0.1', fullpath: '/') }
+  let(:class_instance) { limiter_class.new(request) }
+
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+
+  describe '#archive_rate_limit_reached?' do
+    context 'with a user' do
+      it 'rate limits the user properly' do
+        5.times do
+          expect(class_instance.archive_rate_limit_reached?(user, project)).to be_falsey
+        end
+
+        expect(class_instance.archive_rate_limit_reached?(user, project)).to be_truthy
+      end
+    end
+
+    context 'with an anonymous user' do
+      before do
+        stub_const('Gitlab::RateLimitHelpers::ARCHIVE_RATE_ANONYMOUS_THRESHOLD', 2)
+      end
+
+      it 'rate limits with higher limits' do
+        2.times do
+          expect(class_instance.archive_rate_limit_reached?(nil, project)).to be_falsey
+        end
+
+        expect(class_instance.archive_rate_limit_reached?(nil, project)).to be_truthy
+      end
+    end
+  end
+end

spec/requests/api/repositories_spec.rb

@@ -223,6 +223,10 @@ describe API::Repositories do
   describe "GET /projects/:id/repository/archive(.:format)?:sha" do
     let(:route) { "/projects/#{project.id}/repository/archive" }
 
+    before do
+      allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(false)
+    end
+
     shared_examples_for 'repository archive' do
       it 'returns the repository archive' do
         get api(route, current_user)
@@ -263,6 +267,14 @@ describe API::Repositories do
           let(:message) { '404 File Not Found' }
         end
       end
+
+      it 'rate limits user when thresholds hit' do
+        allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
+
+        get api("/projects/#{project.id}/repository/archive.tar.bz2", user)
+
+        expect(response).to have_gitlab_http_status(:too_many_requests)
+      end
     end
 
     context 'when unauthenticated', 'and project is public' do