Merge branch '29952-remove-protected-paths-initializer' into 'master'

Remove protected paths settings from initializers

See merge request gitlab-org/gitlab!31221

config/initializers/1_settings.rb

@@ -701,7 +701,6 @@ Settings.rack_attack.git_basic_auth['ip_whitelist'] ||= %w{127.0.0.1}
 Settings.rack_attack.git_basic_auth['maxretry'] ||= 10
 Settings.rack_attack.git_basic_auth['findtime'] ||= 1.minute
 Settings.rack_attack.git_basic_auth['bantime'] ||= 1.hour
-Settings.rack_attack['admin_area_protected_paths_enabled'] ||= false
 
 #
 # Gitaly

config/initializers/rack_attack.rb.example

-# 1. Rename this file to rack_attack.rb
-# 2. Review the paths_to_be_protected and add any other path you need protecting
-#
-# If you change this file in a Merge Request, please also create a Merge Request on https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests
-
-paths_to_be_protected = [
-  "#{Rails.application.config.relative_url_root}/users/password",
-  "#{Rails.application.config.relative_url_root}/users/sign_in",
-  "#{Rails.application.config.relative_url_root}/api/#{API::API.version}/session.json",
-  "#{Rails.application.config.relative_url_root}/api/#{API::API.version}/session",
-  "#{Rails.application.config.relative_url_root}/users",
-  "#{Rails.application.config.relative_url_root}/users/confirmation",
-  "#{Rails.application.config.relative_url_root}/unsubscribes/",
-  "#{Rails.application.config.relative_url_root}/import/github/personal_access_token"
-
-]
-
-# Create one big regular expression that matches strings starting with any of
-# the paths_to_be_protected.
-paths_regex = Regexp.union(paths_to_be_protected.map { |path| /\A#{Regexp.escape(path)}/ })
-rack_attack_enabled = Gitlab.config.rack_attack.git_basic_auth['enabled']
-
-unless Rails.env.test? || !rack_attack_enabled
-  Rack::Attack.throttle('protected paths', limit: 10, period: 60.seconds) do |req|
-    if req.post? && req.path =~ paths_regex
-      req.ip
-    end
-  end
-end

config/initializers/rack_attack_new.rb

@@ -8,17 +8,9 @@ module Gitlab::Throttle
 
   # Returns true if we should use the Admin Area protected paths throttle
   def self.protected_paths_enabled?
-    return false if should_use_omnibus_protected_paths?
-
     self.settings.throttle_protected_paths_enabled?
   end
 
-  # To be removed in 13.0: https://gitlab.com/gitlab-org/gitlab/issues/29952
-  def self.should_use_omnibus_protected_paths?
-    !Settings.rack_attack.admin_area_protected_paths_enabled &&
-      self.omnibus_protected_paths_present?
-  end
-
   def self.omnibus_protected_paths_present?
     Rack::Attack.throttles.key?('protected paths')
   end

doc/security/rack_attack.md

@@ -36,27 +36,20 @@ will be enabled:
 
 ### Protected paths throttle
 
-NOTE: **Note:** Omnibus GitLab protected paths throttle is deprecated and is scheduled for removal in
-GitLab 13.0. Please refer to [Migrate settings from GitLab 12.3 and earlier](../user/admin_area/settings/protected_paths.md#migrate-settings-from-gitlab-123-and-earlier).
-
 GitLab responds with HTTP status code `429` to POST requests at protected paths
 that exceed 10 requests per minute per IP address.
 
 By default, protected paths are:
 
-```ruby
-default['gitlab']['gitlab-rails']['rack_attack_protected_paths'] = [
-  '/users/password',
-  '/users/sign_in',
-  '/api/#{API::API.version}/session.json',
-  '/api/#{API::API.version}/session',
-  '/users',
-  '/users/confirmation',
-  '/unsubscribes/',
-  '/import/github/personal_access_token',
-  '/admin/session'
-]
-```
+- `/users/password`
+- `/users/sign_in`
+- `/api/#{API::API.version}/session.json`
+- `/api/#{API::API.version}/session`
+- `/users`
+- `/users/confirmation`
+- `/unsubscribes/`
+- `/import/github/personal_access_token`
+- `/admin/session`
 
 This header is included in responses to blocked requests:
 
@@ -141,9 +134,6 @@ taken in order to enable protection for your GitLab instance:
    config.middleware.use Rack::Attack
    ```
 
-1. Copy `config/initializers/rack_attack.rb.example` to `config/initializers/rack_attack.rb`
-1. Open `config/initializers/rack_attack.rb`, review the
-   `paths_to_be_protected`, and add any other path you need protecting
 1. Restart GitLab:
 
    ```shell

doc/user/admin_area/settings/protected_paths.md

@@ -54,24 +54,3 @@ customized on **Admin > Network > Protected Paths**, along with these options:
 ![protected-paths](img/protected_paths.png)
 
 Requests over the rate limit are logged into `auth.log`.
-
-## Migrate settings from GitLab 12.3 and earlier
-
-Omnibus GitLab protected paths throttle is deprecated and is scheduled for removal in
-GitLab 13.0. Please see the [GitLab issue](https://gitlab.com/gitlab-org/gitlab/issues/29952) and the [Omnibus GitLab issue](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/4688) for more information.
-
-NOTE: **Note:** If Omnibus settings are present, applications settings will be automatically ignored to avoid generating multiple requests blocks.
-
-To migrate from Omnibus GitLab 12.3 and earlier settings:
-
-1. Customize and enable your protected paths settings by following [Configure using GitLab UI](#configure-using-gitlab-ui) section.
-
-1. SSH into your frontend nodes and add to `/etc/gitlab/gitlab.rb`:
-
-   ```ruby
-   gitlab_rails['rack_attack_admin_area_protected_paths_enabled'] = true
-   ```
-
-1. [Reconfigure GitLab](../../../administration/restart_gitlab.md#omnibus-gitlab-reconfigure) for the changes to take effect.
-
-That's it. Protected paths throttle are now managed by GitLab admin settings.

spec/lib/gitlab/throttle_spec.rb

@@ -6,82 +6,10 @@ describe Gitlab::Throttle do
   describe '.protected_paths_enabled?' do
     subject { described_class.protected_paths_enabled? }
 
-    context 'when omnibus protected paths throttle should be used' do
-      before do
-        expect(described_class).to receive(:should_use_omnibus_protected_paths?).and_return(true)
-      end
+    it 'returns Application Settings throttle_protected_paths_enabled?' do
+      expect(Gitlab::CurrentSettings.current_application_settings).to receive(:throttle_protected_paths_enabled?)
 
-      it { is_expected.to be_falsey }
-    end
-
-    context 'when omnibus protected paths throttle should not be used' do
-      before do
-        expect(described_class).to receive(:should_use_omnibus_protected_paths?).and_return(false)
-      end
-
-      it 'returns Application Settings throttle_protected_paths_enabled?' do
-        expect(Gitlab::CurrentSettings.current_application_settings).to receive(:throttle_protected_paths_enabled?)
-
-        subject
-      end
-    end
-  end
-
-  describe '.should_use_omnibus_protected_paths?' do
-    subject { described_class.should_use_omnibus_protected_paths? }
-
-    context 'when rack_attack.admin_area_protected_paths_enabled config is unspecified' do
-      context 'when the omnibus protected paths throttle has been recently used (it has data)' do
-        before do
-          expect(described_class).to receive(:omnibus_protected_paths_present?).and_return(true)
-        end
-
-        it { is_expected.to be_truthy }
-      end
-
-      context 'when the omnibus protected paths throttle has not been recently used' do
-        before do
-          expect(described_class).to receive(:omnibus_protected_paths_present?).and_return(false)
-        end
-
-        it { is_expected.to be_falsey }
-      end
-    end
-
-    context 'when rack_attack.admin_area_protected_paths_enabled config is false' do
-      before do
-        stub_config(rack_attack: {
-          admin_area_protected_paths_enabled: false
-        })
-      end
-
-      context 'when the omnibus protected paths throttle has been recently used (it has data)' do
-        before do
-          expect(described_class).to receive(:omnibus_protected_paths_present?).and_return(true)
-        end
-
-        it { is_expected.to be_truthy }
-      end
-
-      context 'when the omnibus protected paths throttle has not been recently used' do
-        before do
-          expect(described_class).to receive(:omnibus_protected_paths_present?).and_return(false)
-        end
-
-        it { is_expected.to be_falsey }
-      end
-    end
-
-    context 'when rack_attack.admin_area_protected_paths_enabled config is true' do
-      before do
-        stub_config(rack_attack: {
-          admin_area_protected_paths_enabled: true
-        })
-
-        expect(described_class).not_to receive(:omnibus_protected_paths_present?)
-      end
-
-      it { is_expected.to be_falsey }
+      subject
     end
   end
 end

spec/requests/rack_attack_global_spec.rb

@@ -262,20 +262,6 @@ describe 'Rack Attack global throttles' do
 
           expect_rejection { post protected_path_that_does_not_require_authentication, params: post_params }
         end
-
-        context 'when Omnibus throttle should be used' do
-          before do
-            allow(Gitlab::Throttle)
-              .to receive(:should_use_omnibus_protected_paths?).and_return(true)
-          end
-
-          it 'allows requests over the rate limit' do
-            (1 + requests_per_period).times do
-              post protected_path_that_does_not_require_authentication, params: post_params
-              expect(response).to have_gitlab_http_status(:ok)
-            end
-          end
-        end
       end
     end
 
@@ -311,28 +297,6 @@ describe 'Rack Attack global throttles' do
 
         it_behaves_like 'rate-limited token-authenticated requests'
       end
-
-      context 'when Omnibus throttle should be used' do
-        let(:request_args) { [api(api_partial_url, personal_access_token: token)] }
-        let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token)] }
-
-        before do
-          settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period
-          settings_to_set[:"#{throttle_setting_prefix}_period_in_seconds"] = period_in_seconds
-          settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true
-          stub_application_setting(settings_to_set)
-
-          allow(Gitlab::Throttle)
-            .to receive(:should_use_omnibus_protected_paths?).and_return(true)
-        end
-
-        it 'allows requests over the rate limit' do
-          (1 + requests_per_period).times do
-            post(*request_args)
-            expect(response).not_to have_gitlab_http_status(:too_many_requests)
-          end
-        end
-      end
     end
 
     describe 'web requests authenticated with regular login' do
@@ -352,27 +316,6 @@ describe 'Rack Attack global throttles' do
       end
 
       it_behaves_like 'rate-limited web authenticated requests'
-
-      context 'when Omnibus throttle should be used' do
-        before do
-          settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period
-          settings_to_set[:"#{throttle_setting_prefix}_period_in_seconds"] = period_in_seconds
-          settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true
-          stub_application_setting(settings_to_set)
-
-          allow(Gitlab::Throttle)
-            .to receive(:should_use_omnibus_protected_paths?).and_return(true)
-
-          login_as(user)
-        end
-
-        it 'allows requests over the rate limit' do
-          (1 + requests_per_period).times do
-            post url_that_requires_authentication
-            expect(response).not_to have_gitlab_http_status(:too_many_requests)
-          end
-        end
-      end
     end
   end
 end