Merge branch '345554-oauth-protected-paths' into 'master'

Remove OAuth paths from protected paths rate limit

See merge request gitlab-org/gitlab!76882

db/migrate/20211215182006_update_application_settings_protected_paths.rb

+# frozen_string_literal: true
+
+class UpdateApplicationSettingsProtectedPaths < Gitlab::Database::Migration[1.0]
+  REMOVE_PROTECTED_PATHS = [
+    '/oauth/authorize',
+    '/oauth/token'
+  ].freeze
+
+  NEW_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',
+    '/admin/session'
+  ].freeze
+
+  OLD_DEFAULT_PROTECTED_PATHS = (NEW_DEFAULT_PROTECTED_PATHS + REMOVE_PROTECTED_PATHS).freeze
+
+  class ApplicationSetting < ActiveRecord::Base
+    self.table_name = 'application_settings'
+  end
+
+  def up
+    change_column_default(:application_settings, :protected_paths, NEW_DEFAULT_PROTECTED_PATHS)
+
+    ApplicationSetting.reset_column_information
+
+    ApplicationSetting.where.not(protected_paths: nil).each do |application_setting|
+      paths_to_remove = application_setting.protected_paths & REMOVE_PROTECTED_PATHS
+
+      next if paths_to_remove.empty?
+
+      updated_protected_paths = application_setting.protected_paths - paths_to_remove
+      application_setting.update!(protected_paths: updated_protected_paths)
+    end
+  end
+
+  def down
+    change_column_default(:application_settings, :protected_paths, OLD_DEFAULT_PROTECTED_PATHS)
+
+    ApplicationSetting.reset_column_information
+
+    ApplicationSetting.where.not(protected_paths: nil).each do |application_setting|
+      paths_to_add = REMOVE_PROTECTED_PATHS - application_setting.protected_paths
+
+      next if paths_to_add.empty?
+
+      updated_protected_paths = application_setting.protected_paths + paths_to_add
+      application_setting.update!(protected_paths: updated_protected_paths)
+    end
+  end
+end

db/schema_migrations/20211215182006

+ead2a1b13438514bb97bea3f1656f9bac352a8c733d9f808b2405685bce91e00
\ No newline at end of file

db/structure.sql

@@ -10298,7 +10298,7 @@ CREATE TABLE application_settings (
     throttle_protected_paths_enabled boolean DEFAULT false NOT NULL,
     throttle_protected_paths_requests_per_period integer DEFAULT 10 NOT NULL,
     throttle_protected_paths_period_in_seconds integer DEFAULT 60 NOT NULL,
-    protected_paths character varying(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,/oauth/authorize,/oauth/token}'::character varying[],
+    protected_paths character varying(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}'::character varying[],
     throttle_incident_management_notification_enabled boolean DEFAULT false NOT NULL,
     throttle_incident_management_notification_period_in_seconds integer DEFAULT 3600,
     throttle_incident_management_notification_per_period integer DEFAULT 3600,

spec/migrations/update_application_settings_protected_paths_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe UpdateApplicationSettingsProtectedPaths, :aggregate_failures do
+  subject(:migration) { described_class.new }
+
+  let_it_be(:application_settings) { table(:application_settings) }
+  let_it_be(:oauth_paths) { %w[/oauth/authorize /oauth/token] }
+  let_it_be(:custom_paths) { %w[/foo /bar] }
+
+  let(:default_paths) { application_settings.column_defaults.fetch('protected_paths') }
+
+  before do
+    application_settings.create!(protected_paths: custom_paths)
+    application_settings.create!(protected_paths: custom_paths + oauth_paths)
+    application_settings.create!(protected_paths: custom_paths + oauth_paths.take(1))
+  end
+
+  describe '#up' do
+    before do
+      migrate!
+      application_settings.reset_column_information
+    end
+
+    it 'removes the OAuth paths from the default value and persisted records' do
+      expect(default_paths).not_to include(*oauth_paths)
+      expect(default_paths).to eq(described_class::NEW_DEFAULT_PROTECTED_PATHS)
+      expect(application_settings.all).to all(have_attributes(protected_paths: custom_paths))
+    end
+  end
+
+  describe '#down' do
+    before do
+      migrate!
+      schema_migrate_down!
+    end
+
+    it 'adds the OAuth paths to the default value and persisted records' do
+      expect(default_paths).to include(*oauth_paths)
+      expect(default_paths).to eq(described_class::OLD_DEFAULT_PROTECTED_PATHS)
+      expect(application_settings.all).to all(have_attributes(protected_paths: custom_paths + oauth_paths))
+    end
+  end
+end