diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-09 10:41:06 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-09 10:41:06 +0000 |
commit | 87a652a7dfff3a3e5ad4b877e08154c93ef1fb24 (patch) | |
tree | fc359df4f9f91da61f6ebcc852da247e33755f63 | |
parent | e79596882a08bc9ec27db1e0721a4e5c10d573da (diff) | |
download | gitlab-ce-87a652a7dfff3a3e5ad4b877e08154c93ef1fb24.tar.gz |
Add latest changes from gitlab-org/security/gitlab@15-6-stable-ee
-rw-r--r-- | app/controllers/concerns/page_limiter.rb | 2 | ||||
-rw-r--r-- | app/models/active_session.rb | 2 | ||||
-rw-r--r-- | app/models/hooks/web_hook.rb | 5 | ||||
-rw-r--r-- | lib/gitlab/safe_device_detector.rb | 16 | ||||
-rw-r--r-- | spec/lib/gitlab/safe_device_detector_spec.rb | 20 | ||||
-rw-r--r-- | spec/models/hooks/web_hook_spec.rb | 28 | ||||
-rw-r--r-- | spec/services/web_hook_service_spec.rb | 5 |
7 files changed, 74 insertions, 4 deletions
diff --git a/app/controllers/concerns/page_limiter.rb b/app/controllers/concerns/page_limiter.rb index 362b02e5856..3e75c428fdd 100644 --- a/app/controllers/concerns/page_limiter.rb +++ b/app/controllers/concerns/page_limiter.rb @@ -57,7 +57,7 @@ module PageLimiter # Record the page limit being hit in Prometheus def record_page_limit_interception - dd = DeviceDetector.new(request.user_agent) + dd = Gitlab::SafeDeviceDetector.new(request.user_agent) Gitlab::Metrics.counter(:gitlab_page_out_of_bounds, controller: params[:controller], diff --git a/app/models/active_session.rb b/app/models/active_session.rb index b16c4a2b353..2d1dec1977d 100644 --- a/app/models/active_session.rb +++ b/app/models/active_session.rb @@ -67,7 +67,7 @@ class ActiveSession def self.set(user, request) Gitlab::Redis::Sessions.with do |redis| session_private_id = request.session.id.private_id - client = DeviceDetector.new(request.user_agent) + client = Gitlab::SafeDeviceDetector.new(request.user_agent) timestamp = Time.current expiry = Settings.gitlab['session_expire_delay'] * 60 diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 946cdda2e75..639fe138a46 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -41,6 +41,7 @@ class WebHook < ApplicationRecord after_initialize :initialize_url_variables before_validation :reset_token + before_validation :reset_url_variables, unless: ->(hook) { hook.is_a?(ServiceHook) } before_validation :set_branch_filter_nil, \ if: -> { branch_filter_strategy_all_branches? && enhanced_webhook_support_regex? } validates :push_events_branch_filter, \ @@ -224,6 +225,10 @@ class WebHook < ApplicationRecord self.token = nil if url_changed? && !encrypted_token_changed? end + def reset_url_variables + self.url_variables = {} if url_changed? && !encrypted_url_variables_changed? + end + def next_failure_count recent_failures.succ.clamp(1, MAX_FAILURES) end diff --git a/lib/gitlab/safe_device_detector.rb b/lib/gitlab/safe_device_detector.rb new file mode 100644 index 00000000000..ba1010ae5e4 --- /dev/null +++ b/lib/gitlab/safe_device_detector.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true +# rubocop:disable Gitlab/NamespacedClass +require 'device_detector' + +module Gitlab + class SafeDeviceDetector < ::DeviceDetector + USER_AGENT_MAX_SIZE = 1024 + + def initialize(user_agent) + super(user_agent) + @user_agent = user_agent && user_agent[0..USER_AGENT_MAX_SIZE] + end + end +end + +# rubocop:enable Gitlab/NamespacedClass diff --git a/spec/lib/gitlab/safe_device_detector_spec.rb b/spec/lib/gitlab/safe_device_detector_spec.rb new file mode 100644 index 00000000000..c37dc1e1c7e --- /dev/null +++ b/spec/lib/gitlab/safe_device_detector_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'device_detector' +require_relative '../../../lib/gitlab/safe_device_detector' + +RSpec.describe Gitlab::SafeDeviceDetector, feature_category: :authentication_and_authorization do + it 'retains the behavior for normal user agents' do + chrome_user_agent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 \ + (KHTML, like Gecko) Chrome/108.0.0.0 Safari/537.36" + + expect(described_class.new(chrome_user_agent).user_agent).to be_eql(chrome_user_agent) + expect(described_class.new(chrome_user_agent).name).to be_eql('Chrome') + end + + it 'truncates big user agents' do + big_user_agent = "chrome #{'abc' * 1024}" + expect(described_class.new(big_user_agent).user_agent).not_to be_eql(big_user_agent) + end +end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 9b55db15f3b..54d81208860 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe WebHook do +RSpec.describe WebHook, feature_category: :integrations do include AfterNextHelpers let_it_be(:project) { create(:project) } @@ -225,6 +225,32 @@ RSpec.describe WebHook do end end + describe 'before_validation :reset_url_variables' do + subject(:hook) { build_stubbed(:project_hook, :url_variables, project: project, url: 'http://example.com/{abc}') } + + it 'resets url variables if url changed' do + hook.url = 'http://example.com/new-hook' + + expect(hook).to be_valid + expect(hook.url_variables).to eq({}) + end + + it 'resets url variables if url is changed but url variables stayed the same' do + hook.url = 'http://test.example.com/{abc}' + + expect(hook).not_to be_valid + expect(hook.url_variables).to eq({}) + end + + it 'does not reset url variables if both url and url variables are changed' do + hook.url = 'http://example.com/{one}/{two}' + hook.url_variables = { 'one' => 'foo', 'two' => 'bar' } + + expect(hook).to be_valid + expect(hook.url_variables).to eq({ 'one' => 'foo', 'two' => 'bar' }) + end + end + it "only consider these branch filter strategies are valid" do expected_valid_types = %w[all_branches regex wildcard] expect(described_class.branch_filter_strategies.keys).to contain_exactly(*expected_valid_types) diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index c081b20d95f..4b925a058e7 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -129,7 +129,10 @@ RSpec.describe WebHookService, :request_store, :clean_gitlab_redis_shared_state context 'there is userinfo' do before do - project_hook.update!(url: 'http://{one}:{two}@example.com') + project_hook.update!( + url: 'http://{one}:{two}@example.com', + url_variables: { 'one' => 'a', 'two' => 'b' } + ) stub_full_request('http://example.com', method: :post) end |