Treat API requests from the frontend as web traffic in the rate limiter

This will allow us to impose stricter rate limits for general API
traffic, without affecting interactive API requests made by the
frontend during normal GitLab usage.

The frontend requests are identified by the inclusion of a CSRF token
in the headers.

Other rate limits that only affect a subset of API requests (e.g. the
Files and Packages APIs, or protected paths) are affected by this too,
i.e. requests for these paths with a CSRF token will always be counted
as web traffic, if the web rate limit is enabled.

Changelog: changed

doc/user/admin_area/settings/user_and_ip_rate_limits.md

@@ -22,6 +22,10 @@ NOTE:
 By default, all Git operations are first tried unauthenticated. Because of this, HTTP Git operations
 may trigger the rate limits configured for unauthenticated requests.
 
+NOTE:
+The rate limits for API requests don't affect requests made by the frontend, as these are always
+counted as web traffic.
+
 ## Enable unauthenticated API request rate limit
 
 To enable the unauthenticated request rate limit:

lib/gitlab/rack_attack/request.rb

@@ -3,6 +3,8 @@
 module Gitlab
   module RackAttack
     module Request
+      include ::Gitlab::Utils::StrongMemoize
+
       FILES_PATH_REGEX = %r{^/api/v\d+/projects/[^/]+/repository/files/.+}.freeze
       GROUP_PATH_REGEX = %r{^/api/v\d+/groups/[^/]+/?$}.freeze
 
@@ -30,15 +32,15 @@ module Gitlab
       end
 
       def api_internal_request?
-        path =~ %r{^/api/v\d+/internal/}
+        path.match?(%r{^/api/v\d+/internal/})
       end
 
       def health_check_request?
-        path =~ %r{^/-/(health|liveness|readiness|metrics)}
+        path.match?(%r{^/-/(health|liveness|readiness|metrics)})
       end
 
       def container_registry_event?
-        path =~ %r{^/api/v\d+/container_registry_event/}
+        path.match?(%r{^/api/v\d+/container_registry_event/})
       end
 
       def product_analytics_collector_request?
@@ -58,7 +60,7 @@ module Gitlab
       end
 
       def protected_path_regex
-        path =~ protected_paths_regex
+        path.match?(protected_paths_regex)
       end
 
       def throttle?(throttle, authenticated:)
@@ -70,6 +72,7 @@ module Gitlab
       def throttle_unauthenticated_api?
         api_request? &&
         !should_be_skipped? &&
+        !frontend_request? &&
         !throttle_unauthenticated_packages_api? &&
         !throttle_unauthenticated_files_api? &&
         !throttle_unauthenticated_deprecated_api? &&
@@ -78,7 +81,7 @@ module Gitlab
       end
 
       def throttle_unauthenticated_web?
-        web_request? &&
+        (web_request? || frontend_request?) &&
         !should_be_skipped? &&
         # TODO: Column will be renamed in https://gitlab.com/gitlab-org/gitlab/-/issues/340031
         Gitlab::Throttle.settings.throttle_unauthenticated_enabled &&
@@ -87,6 +90,7 @@ module Gitlab
 
       def throttle_authenticated_api?
         api_request? &&
+        !frontend_request? &&
         !throttle_authenticated_packages_api? &&
         !throttle_authenticated_files_api? &&
         !throttle_authenticated_deprecated_api? &&
@@ -94,7 +98,7 @@ module Gitlab
       end
 
       def throttle_authenticated_web?
-        web_request? &&
+        (web_request? || frontend_request?) &&
         !throttle_authenticated_git_lfs? &&
         Gitlab::Throttle.settings.throttle_authenticated_web_enabled
       end
@@ -178,15 +182,24 @@ module Gitlab
       end
 
       def packages_api_path?
-        path =~ ::Gitlab::Regex::Packages::API_PATH_REGEX
+        path.match?(::Gitlab::Regex::Packages::API_PATH_REGEX)
       end
 
       def git_lfs_path?
-        path =~ Gitlab::PathRegex.repository_git_lfs_route_regex
+        path.match?(Gitlab::PathRegex.repository_git_lfs_route_regex)
       end
 
       def files_api_path?
-        path =~ FILES_PATH_REGEX
+        path.match?(FILES_PATH_REGEX)
+      end
+
+      def frontend_request?
+        strong_memoize(:frontend_request) do
+          next false unless env.include?('HTTP_X_CSRF_TOKEN') && session.include?(:_csrf_token)
+
+          # CSRF tokens are not verified for GET/HEAD requests, so we pretend that we always have a POST request.
+          Gitlab::RequestForgeryProtection.verified?(env.merge('REQUEST_METHOD' => 'POST'))
+        end
       end
 
       def deprecated_api_request?
@@ -195,7 +208,7 @@ module Gitlab
         with_projects = params['with_projects']
         with_projects = true if with_projects.blank?
 
-        path =~ GROUP_PATH_REGEX && Gitlab::Utils.to_boolean(with_projects)
+        path.match?(GROUP_PATH_REGEX) && Gitlab::Utils.to_boolean(with_projects)
       end
     end
   end

spec/channels/application_cable/connection_spec.rb

@@ -3,15 +3,11 @@
 require 'spec_helper'
 
 RSpec.describe ApplicationCable::Connection, :clean_gitlab_redis_sessions do
-  let(:session_id) { Rack::Session::SessionId.new('6919a6f1bb119dd7396fadc38fd18d0d') }
+  include SessionHelpers
 
   context 'when session cookie is set' do
     before do
-      Gitlab::Redis::Sessions.with do |redis|
-        redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash))
-      end
-
-      cookies[Gitlab::Application.config.session_options[:key]] = session_id.public_id
+      stub_session(session_hash)
     end
 
     context 'when user is logged in' do

spec/lib/gitlab/rack_attack/request_spec.rb

@@ -5,6 +5,19 @@ require 'spec_helper'
 RSpec.describe Gitlab::RackAttack::Request do
   using RSpec::Parameterized::TableSyntax
 
+  let(:env) { {} }
+  let(:session) { {} }
+  let(:request) do
+    ::Rack::Attack::Request.new(
+      env.reverse_merge(
+        'REQUEST_METHOD' => 'GET',
+        'PATH_INFO' => path,
+        'rack.input' => StringIO.new,
+        'rack.session' => session
+      )
+    )
+  end
+
   describe 'FILES_PATH_REGEX' do
     subject { described_class::FILES_PATH_REGEX }
 
@@ -16,11 +29,63 @@ RSpec.describe Gitlab::RackAttack::Request do
     it { is_expected.not_to match('/api/v4/projects/some/nested/repo/repository/files/README') }
   end
 
+  describe '#api_request?' do
+    subject { request.api_request? }
+
+    where(:path, :env, :expected) do
+      '/'       | {} | false
+      '/groups' | {} | false
+
+      '/api'             | {} | true
+      '/api/v4/groups/1' | {} | true
+    end
+
+    with_them do
+      it { is_expected.to eq(expected) }
+    end
+  end
+
+  describe '#web_request?' do
+    subject { request.web_request? }
+
+    where(:path, :env, :expected) do
+      '/'       | {} | true
+      '/groups' | {} | true
+
+      '/api'             | {} | false
+      '/api/v4/groups/1' | {} | false
+    end
+
+    with_them do
+      it { is_expected.to eq(expected) }
+    end
+  end
+
+  describe '#frontend_request?', :allow_forgery_protection do
+    subject { request.send(:frontend_request?) }
+
+    let(:path) { '/' }
+
+    # Define these as local variables so we can use them in the `where` block.
+    valid_token = SecureRandom.base64(ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH)
+    other_token = SecureRandom.base64(ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH)
+
+    where(:session, :env, :expected) do
+      {}                           | {}                                     | false # rubocop:disable Lint/BinaryOperatorWithIdenticalOperands
+      {}                           | { 'HTTP_X_CSRF_TOKEN' => valid_token } | false
+      { _csrf_token: valid_token } | { 'HTTP_X_CSRF_TOKEN' => other_token } | false
+      { _csrf_token: valid_token } | { 'HTTP_X_CSRF_TOKEN' => valid_token } | true
+    end
+
+    with_them do
+      it { is_expected.to eq(expected) }
+    end
+  end
+
   describe '#deprecated_api_request?' do
-    let(:env) { { 'REQUEST_METHOD' => 'GET', 'rack.input' => StringIO.new, 'PATH_INFO' => path, 'QUERY_STRING' => query } }
-    let(:request) { ::Rack::Attack::Request.new(env) }
+    subject { request.send(:deprecated_api_request?) }
 
-    subject { !!request.__send__(:deprecated_api_request?) }
+    let(:env) { { 'QUERY_STRING' => query } }
 
     where(:path, :query, :expected) do
       '/' | '' | false

spec/requests/api/commits_spec.rb

@@ -5,6 +5,7 @@ require 'mime/types'
 
 RSpec.describe API::Commits do
   include ProjectForksHelper
+  include SessionHelpers
 
   let(:user) { create(:user) }
   let(:guest) { create(:user).tap { |u| project.add_guest(u) } }
@@ -378,14 +379,7 @@ RSpec.describe API::Commits do
 
       context 'when using warden' do
         it 'increments usage counters', :clean_gitlab_redis_sessions do
-          session_id = Rack::Session::SessionId.new('6919a6f1bb119dd7396fadc38fd18d0d')
-          session_hash = { 'warden.user.user.key' => [[user.id], user.encrypted_password[0, 29]] }
-
-          Gitlab::Redis::Sessions.with do |redis|
-            redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash))
-          end
-
-          cookies[Gitlab::Application.config.session_options[:key]] = session_id.public_id
+          stub_session('warden.user.user.key' => [[user.id], user.encrypted_password[0, 29]])
 
           expect(::Gitlab::UsageDataCounters::WebIdeCounter).to receive(:increment_commits_count)
           expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).to receive(:track_web_ide_edit_action)

spec/requests/rack_attack_global_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_caching do
   include RackAttackSpecHelpers
+  include SessionHelpers
 
   let(:settings) { Gitlab::CurrentSettings.current_application_settings }
 
@@ -63,6 +64,22 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac
     end
   end
 
+  describe 'API requests from the frontend', :api, :clean_gitlab_redis_sessions do
+    context 'when unauthenticated' do
+      it_behaves_like 'rate-limited frontend API requests' do
+        let(:throttle_setting_prefix) { 'throttle_unauthenticated' }
+      end
+    end
+
+    context 'when authenticated' do
+      it_behaves_like 'rate-limited frontend API requests' do
+        let_it_be(:personal_access_token) { create(:personal_access_token) }
+
+        let(:throttle_setting_prefix) { 'throttle_authenticated' }
+      end
+    end
+  end
+
   describe 'API requests authenticated with personal access token', :api do
     let_it_be(:user) { create(:user) }
     let_it_be(:token) { create(:personal_access_token, user: user) }

spec/support/helpers/rack_attack_spec_helpers.rb

@@ -26,14 +26,14 @@ module RackAttackSpecHelpers
     { 'AUTHORIZATION' => "Basic #{encoded_login}" }
   end
 
-  def expect_rejection(&block)
+  def expect_rejection(name = nil, &block)
     yield
 
     expect(response).to have_gitlab_http_status(:too_many_requests)
 
     expect(response.headers.to_h).to include(
       'RateLimit-Limit' => a_string_matching(/^\d+$/),
-      'RateLimit-Name' => a_string_matching(/^throttle_.*$/),
+      'RateLimit-Name' => name || a_string_matching(/^throttle_.*$/),
       'RateLimit-Observed' => a_string_matching(/^\d+$/),
       'RateLimit-Remaining' => a_string_matching(/^\d+$/),
       'Retry-After' => a_string_matching(/^\d+$/)

spec/support/helpers/session_helpers.rb

 # frozen_string_literal: true
 
 module SessionHelpers
+  # Stub a session in Redis, for use in request specs where we can't mock the session directly.
+  # This also needs the :clean_gitlab_redis_sessions tag on the spec.
+  def stub_session(session_hash)
+    unless RSpec.current_example.metadata[:clean_gitlab_redis_sessions]
+      raise 'Add :clean_gitlab_redis_sessions to your spec!'
+    end
+
+    session_id = Rack::Session::SessionId.new(SecureRandom.hex)
+
+    Gitlab::Redis::Sessions.with do |redis|
+      redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash))
+    end
+
+    cookies[Gitlab::Application.config.session_options[:key]] = session_id.public_id
+  end
+
   def expect_single_session_with_authenticated_ttl
     expect_single_session_with_expiration(Settings.gitlab['session_expire_delay'] * 60)
   end

spec/support/shared_examples/requests/api/graphql/mutations/snippets_shared_examples.rb

@@ -10,6 +10,8 @@ RSpec.shared_examples 'when the snippet is not found' do
 end
 
 RSpec.shared_examples 'snippet edit usage data counters' do
+  include SessionHelpers
+
   context 'when user is sessionless' do
     it 'does not track usage data actions' do
       expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).not_to receive(:track_snippet_editor_edit_action)
@@ -20,14 +22,7 @@ RSpec.shared_examples 'snippet edit usage data counters' do
 
   context 'when user is not sessionless', :clean_gitlab_redis_sessions do
     before do
-      session_id = Rack::Session::SessionId.new('6919a6f1bb119dd7396fadc38fd18d0d')
-      session_hash = { 'warden.user.user.key' => [[current_user.id], current_user.encrypted_password[0, 29]] }
-
-      Gitlab::Redis::Sessions.with do |redis|
-        redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash))
-      end
-
-      cookies[Gitlab::Application.config.session_options[:key]] = session_id.public_id
+      stub_session('warden.user.user.key' => [[current_user.id], current_user.encrypted_password[0, 29]])
     end
 
     it 'tracks usage data actions', :clean_gitlab_redis_sessions do

spec/support/shared_examples/requests/rack_attack_shared_examples.rb

@@ -580,3 +580,88 @@ RSpec.shared_examples 'rate-limited unauthenticated requests' do
     end
   end
 end
+
+# Requires let variables:
+# * throttle_setting_prefix: "throttle_authenticated", "throttle_unauthenticated"
+RSpec.shared_examples 'rate-limited frontend API requests' do
+  let(:requests_per_period) { 1 }
+  let(:csrf_token) { SecureRandom.base64(ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH) }
+  let(:csrf_session) { { _csrf_token: csrf_token } }
+  let(:personal_access_token) { nil }
+
+  let(:api_path) { '/projects' }
+
+  # These don't actually exist, so a 404 is the expected response.
+  let(:files_api_path) { '/projects/1/repository/files/ref/path' }
+  let(:packages_api_path) { '/projects/1/packages/foo' }
+  let(:deprecated_api_path) { '/groups/1?with_projects=true' }
+
+  def get_api(path: api_path, csrf: false)
+    headers = csrf ? { 'X-CSRF-Token' => csrf_token } : nil
+    get api(path, personal_access_token: personal_access_token), headers: headers
+  end
+
+  def expect_not_found(&block)
+    yield
+
+    expect(response).to have_gitlab_http_status(:not_found)
+  end
+
+  before do
+    stub_application_setting(
+      "#{throttle_setting_prefix}_enabled" => true,
+      "#{throttle_setting_prefix}_requests_per_period" => requests_per_period,
+      "#{throttle_setting_prefix}_api_enabled" => true,
+      "#{throttle_setting_prefix}_api_requests_per_period" => requests_per_period,
+      "#{throttle_setting_prefix}_web_enabled" => true,
+      "#{throttle_setting_prefix}_web_requests_per_period" => requests_per_period,
+      "#{throttle_setting_prefix}_files_api_enabled" => true,
+      "#{throttle_setting_prefix}_packages_api_enabled" => true,
+      "#{throttle_setting_prefix}_deprecated_api_enabled" => true
+    )
+
+    stub_session(csrf_session)
+  end
+
+  context 'with a CSRF token' do
+    it 'uses the rate limit for web requests' do
+      requests_per_period.times { get_api csrf: true }
+
+      expect_rejection("#{throttle_setting_prefix}_web") { get_api csrf: true }
+      expect_rejection("#{throttle_setting_prefix}_web") { get_api csrf: true, path: files_api_path }
+      expect_rejection("#{throttle_setting_prefix}_web") { get_api csrf: true, path: packages_api_path }
+      expect_rejection("#{throttle_setting_prefix}_web") { get_api csrf: true, path: deprecated_api_path }
+
+      # API rate limit is not triggered yet
+      expect_ok { get_api }
+      expect_not_found { get_api path: files_api_path }
+      expect_not_found { get_api path: packages_api_path }
+      expect_not_found { get_api path: deprecated_api_path }
+    end
+
+    context 'without a CSRF session' do
+      let(:csrf_session) { nil }
+
+      it 'always uses the rate limit for API requests' do
+        requests_per_period.times { get_api csrf: true }
+
+        expect_rejection("#{throttle_setting_prefix}_api") { get_api csrf: true }
+        expect_rejection("#{throttle_setting_prefix}_api") { get_api }
+      end
+    end
+  end
+
+  context 'without a CSRF token' do
+    it 'uses the rate limit for API requests' do
+      requests_per_period.times { get_api }
+
+      expect_rejection("#{throttle_setting_prefix}_api") { get_api }
+
+      # Web and custom API rate limits are not triggered yet
+      expect_ok { get_api csrf: true }
+      expect_not_found { get_api path: files_api_path }
+      expect_not_found { get_api path: packages_api_path }
+      expect_not_found { get_api path: deprecated_api_path }
+    end
+  end
+end