Merge branch 'chore/admin-mode-rack-attack-default-paths-migration' into 'master'

Add admin mode controller path to Rack::Attack defaults

See merge request gitlab-org/gitlab!20735

app/models/application_setting_implementation.rb

@@ -26,7 +26,8 @@ module ApplicationSettingImplementation
     '/users',
     '/users/confirmation',
     '/unsubscribes/',
-    '/import/github/personal_access_token'
+    '/import/github/personal_access_token',
+    '/admin/session'
   ].freeze
 
   class_methods do

changelogs/unreleased/chore-admin-mode-rack-attack-default-paths-migration.yml

+---
+title: Add admin mode controller path to Rack::Attack defaults
+merge_request: 20735
+author: Diego Louzán
+type: changed

db/migrate/20191125114345_add_admin_mode_protected_path.rb

+# frozen_string_literal: true
+
+class AddAdminModeProtectedPath < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  ADMIN_MODE_ENDPOINT = '/admin/session'
+
+  OLD_DEFAULT_PROTECTED_PATHS = [
+      '/users/password',
+      '/users/sign_in',
+      '/api/v3/session.json',
+      '/api/v3/session',
+      '/api/v4/session.json',
+      '/api/v4/session',
+      '/users',
+      '/users/confirmation',
+      '/unsubscribes/',
+      '/import/github/personal_access_token'
+  ]
+
+  NEW_DEFAULT_PROTECTED_PATHS = OLD_DEFAULT_PROTECTED_PATHS.dup << ADMIN_MODE_ENDPOINT
+
+  class ApplicationSetting < ActiveRecord::Base
+    self.table_name = 'application_settings'
+  end
+
+  def up
+    change_column_default :application_settings, :protected_paths, NEW_DEFAULT_PROTECTED_PATHS
+
+    # schema allows nulls for protected_paths
+    ApplicationSetting.where.not(protected_paths: nil).each do |application_setting|
+      unless application_setting.protected_paths.include?(ADMIN_MODE_ENDPOINT)
+        updated_protected_paths = application_setting.protected_paths << ADMIN_MODE_ENDPOINT
+
+        application_setting.update(protected_paths: updated_protected_paths)
+      end
+    end
+  end
+
+  def down
+    change_column_default :application_settings, :protected_paths, OLD_DEFAULT_PROTECTED_PATHS
+
+    # schema allows nulls for protected_paths
+    ApplicationSetting.where.not(protected_paths: nil).each do |application_setting|
+      if application_setting.protected_paths.include?(ADMIN_MODE_ENDPOINT)
+        updated_protected_paths = application_setting.protected_paths - [ADMIN_MODE_ENDPOINT]
+
+        application_setting.update(protected_paths: updated_protected_paths)
+      end
+    end
+  end
+end

db/schema.rb

@@ -328,7 +328,7 @@ ActiveRecord::Schema.define(version: 2019_11_25_140458) do
     t.boolean "throttle_protected_paths_enabled", default: false, null: false
     t.integer "throttle_protected_paths_requests_per_period", default: 10, null: false
     t.integer "throttle_protected_paths_period_in_seconds", default: 60, null: false
-    t.string "protected_paths", limit: 255, default: ["/users/password", "/users/sign_in", "/api/v3/session.json", "/api/v3/session", "/api/v4/session.json", "/api/v4/session", "/users", "/users/confirmation", "/unsubscribes/", "/import/github/personal_access_token"], array: true
+    t.string "protected_paths", limit: 255, default: ["/users/password", "/users/sign_in", "/api/v3/session.json", "/api/v3/session", "/api/v4/session.json", "/api/v4/session", "/users", "/users/confirmation", "/unsubscribes/", "/import/github/personal_access_token", "/admin/session"], array: true
     t.boolean "throttle_incident_management_notification_enabled", default: false, null: false
     t.integer "throttle_incident_management_notification_period_in_seconds", default: 3600
     t.integer "throttle_incident_management_notification_per_period", default: 3600

doc/security/rack_attack.md

@@ -53,7 +53,8 @@ default['gitlab']['gitlab-rails']['rack_attack_protected_paths'] = [
   '/users',
   '/users/confirmation',
   '/unsubscribes/',
-  '/import/github/personal_access_token'
+  '/import/github/personal_access_token',
+  '/admin/session'
 ]
 ```
 

doc/user/admin_area/settings/protected_paths.md

@@ -14,7 +14,8 @@ GitLab protects the following paths with Rack Attack by default:
 '/users',
 '/users/confirmation',
 '/unsubscribes/',
-'/import/github/personal_access_token'
+'/import/github/personal_access_token',
+'/admin/session'
 ```
 
 GitLab responds with HTTP status code `429` to POST requests at protected paths

spec/migrations/20191125114345_add_admin_mode_protected_path_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20191125114345_add_admin_mode_protected_path.rb')
+
+describe AddAdminModeProtectedPath, :migration do
+  ADMIN_MODE_ENDPOINT = '/admin/session'
+
+  subject(:migration) { described_class.new }
+
+  let(:application_settings) { table(:application_settings) }
+
+  context 'no settings available' do
+    it 'makes no changes' do
+      expect { migrate! }.not_to change { application_settings.count }
+    end
+  end
+
+  context 'protected_paths is null' do
+    before do
+      application_settings.create!(protected_paths: nil)
+    end
+
+    it 'makes no changes' do
+      expect { migrate! }.not_to change { application_settings.first.protected_paths }
+    end
+  end
+
+  it 'appends admin mode endpoint' do
+    application_settings.create!(protected_paths: '{a,b,c}')
+
+    protected_paths_before = %w[a b c]
+    protected_paths_after = protected_paths_before.dup << ADMIN_MODE_ENDPOINT
+
+    expect { migrate! }.to change { application_settings.first.protected_paths }.from(protected_paths_before).to(protected_paths_after)
+  end
+
+  it 'new default includes admin mode endpoint' do
+    settings_before = application_settings.create!
+
+    expect(settings_before.protected_paths).not_to include(ADMIN_MODE_ENDPOINT)
+
+    migrate!
+
+    application_settings.reset_column_information
+    settings_after = application_settings.create!
+
+    expect(settings_after.protected_paths).to include(ADMIN_MODE_ENDPOINT)
+  end
+end