Merge branch 'id-rate-limit-ssh-operations' into 'master'

Rate limit Gitlab Shell operations

See merge request gitlab-org/gitlab!78373

config/feature_flags/development/rate_limit_gitlab_shell.yml

+---
+name: rate_limit_gitlab_shell
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78373
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350465
+milestone: '14.7'
+type: development
+group: group::source code
+default_enabled: false

lib/api/internal/base.rb

@@ -43,6 +43,10 @@ module API
           # This is a separate method so that EE can alter its behaviour more
           # easily.
 
+          if Feature.enabled?(:rate_limit_gitlab_shell, default_enabled: :yaml)
+            check_rate_limit!(:gitlab_shell_operation, scope: [params[:action], params[:project], actor.key_or_user])
+          end
+
           # Stores some Git-specific env thread-safely
           env = parse_env
           Gitlab::Git::HookEnv.set(gl_repository, env) if container

lib/gitlab/application_rate_limiter.rb

@@ -56,7 +56,8 @@ module Gitlab
           profile_update_username:            { threshold: 10, interval: 1.minute },
           update_environment_canary_ingress:  { threshold: 1, interval: 1.minute },
           auto_rollback_deployment:           { threshold: 1, interval: 3.minutes },
-          user_email_lookup:            { threshold: -> { application_settings.user_email_lookup_limit }, interval: 1.minute }
+          user_email_lookup:            { threshold: -> { application_settings.user_email_lookup_limit }, interval: 1.minute },
+          gitlab_shell_operation:       { threshold: 600, interval: 1.minute }
         }.freeze
       end
 
@@ -64,7 +65,7 @@ module Gitlab
       # be throttled.
       #
       # @param key [Symbol] Key attribute registered in `.rate_limits`
-      # @param scope [Array<ActiveRecord>] Array of ActiveRecord models to scope throttling to a specific request (e.g. per user per project)
+      # @param scope [Array<ActiveRecord>] Array of ActiveRecord models, Strings or Symbols to scope throttling to a specific request (e.g. per user per project)
       # @param threshold [Integer] Optional threshold value to override default one registered in `.rate_limits`
       # @param users_allowlist [Array<String>] Optional list of usernames to exclude from the limit. This param will only be functional if Scope includes a current user.
       # @param peek [Boolean] Optional. When true the key will not be incremented but the current throttled state will be returned.

spec/requests/api/internal/base_spec.rb

@@ -372,7 +372,38 @@ RSpec.describe API::Internal::Base do
     end
   end
 
-  describe "POST /internal/allowed", :clean_gitlab_redis_shared_state do
+  describe "POST /internal/allowed", :clean_gitlab_redis_shared_state, :clean_gitlab_redis_rate_limiting do
+    shared_examples 'rate limited request' do
+      let(:action) { 'git-upload-pack' }
+      let(:actor) { key }
+
+      it 'is throttled by rate limiter' do
+        allow(::Gitlab::ApplicationRateLimiter).to receive(:threshold).and_return(1)
+        expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:gitlab_shell_operation, scope: [action, project.full_path, actor]).twice.and_call_original
+
+        request
+
+        expect(response).to have_gitlab_http_status(:ok)
+
+        request
+
+        expect(response).to have_gitlab_http_status(:too_many_requests)
+        expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.')
+      end
+
+      context 'when rate_limit_gitlab_shell feature flag is disabled' do
+        before do
+          stub_feature_flags(rate_limit_gitlab_shell: false)
+        end
+
+        it 'is not throttled by rate limiter' do
+          expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?)
+
+          subject
+        end
+      end
+    end
+
     context "access granted" do
       let(:env) { {} }
 
@@ -530,6 +561,32 @@ RSpec.describe API::Internal::Base do
             expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'true')
             expect(user.reload.last_activity_on).to eql(Date.today)
           end
+
+          it_behaves_like 'rate limited request' do
+            def request
+              pull(key, project)
+            end
+          end
+
+          context 'when user_id is passed' do
+            it_behaves_like 'rate limited request' do
+              let(:actor) { user }
+
+              def request
+                post(
+                  api("/internal/allowed"),
+                  params: {
+                    user_id: user.id,
+                    project: full_path_for(project),
+                    gl_repository: gl_repository_for(project),
+                    action: 'git-upload-pack',
+                    secret_token: secret_token,
+                    protocol: 'ssh'
+                  }
+                )
+              end
+            end
+          end
         end
 
         context "with a feature flag enabled for a project" do
@@ -576,6 +633,14 @@ RSpec.describe API::Internal::Base do
             expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage))
             expect(user.reload.last_activity_on).to be_nil
           end
+
+          it_behaves_like 'rate limited request' do
+            let(:action) { 'git-receive-pack' }
+
+            def request
+              push(key, project)
+            end
+          end
         end
 
         context 'when receive_max_input_size has been updated' do
@@ -838,6 +903,14 @@ RSpec.describe API::Internal::Base do
           expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage))
           expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage))
         end
+
+        it_behaves_like 'rate limited request' do
+          let(:action) { 'git-upload-archive' }
+
+          def request
+            archive(key, project)
+          end
+        end
       end
 
       context "not added to project" do