diff options
Diffstat (limited to 'spec/models/hooks/web_hook_spec.rb')
-rw-r--r-- | spec/models/hooks/web_hook_spec.rb | 402 |
1 files changed, 87 insertions, 315 deletions
diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 75ff917c036..72958a54e10 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -258,7 +258,7 @@ RSpec.describe WebHook, feature_category: :integrations do end describe 'encrypted attributes' do - subject { described_class.encrypted_attributes.keys } + subject { described_class.attr_encrypted_attributes.keys } it { is_expected.to contain_exactly(:token, :url, :url_variables) } end @@ -311,88 +311,6 @@ RSpec.describe WebHook, feature_category: :integrations do end end - describe '.executable/.disabled' do - let!(:not_executable) do - [ - [0, Time.current], - [0, 1.minute.from_now], - [1, 1.minute.from_now], - [3, 1.minute.from_now], - [4, nil], - [4, 1.day.ago], - [4, 1.minute.from_now] - ].map do |(recent_failures, disabled_until)| - create(:project_hook, project: project, recent_failures: recent_failures, disabled_until: disabled_until) - end - end - - let!(:executables) do - [ - [0, nil], - [0, 1.day.ago], - [1, nil], - [1, 1.day.ago], - [3, nil], - [3, 1.day.ago] - ].map do |(recent_failures, disabled_until)| - create(:project_hook, project: project, recent_failures: recent_failures, disabled_until: disabled_until) - end - end - - it 'finds the correct set of project hooks' do - expect(described_class.where(project_id: project.id).executable).to match_array executables - expect(described_class.where(project_id: project.id).disabled).to match_array not_executable - end - end - - describe '#executable?' do - let_it_be_with_reload(:web_hook) { create(:project_hook, project: project) } - - where(:recent_failures, :not_until, :executable) do - [ - [0, :not_set, true], - [0, :past, true], - [0, :future, true], - [0, :now, true], - [1, :not_set, true], - [1, :past, true], - [1, :future, true], - [3, :not_set, true], - [3, :past, true], - [3, :future, true], - [4, :not_set, false], - [4, :past, true], # expired suspension - [4, :now, false], # active suspension - [4, :future, false] # active suspension - ] - end - - with_them do - # Phasing means we cannot put these values in the where block, - # which is not subject to the frozen time context. - let(:disabled_until) do - case not_until - when :not_set - nil - when :past - 1.minute.ago - when :future - 1.minute.from_now - when :now - Time.current - end - end - - before do - web_hook.update!(recent_failures: recent_failures, disabled_until: disabled_until) - end - - it 'has the correct state' do - expect(web_hook.executable?).to eq(executable) - end - end - end - describe '#next_backoff' do context 'when there was no last backoff' do before do @@ -435,50 +353,112 @@ RSpec.describe WebHook, feature_category: :integrations do end end - shared_examples 'is tolerant of invalid records' do - specify do - hook.url = nil + describe '#rate_limited?' do + it 'is false when hook has not been rate limited' do + expect_next_instance_of(Gitlab::WebHooks::RateLimiter) do |rate_limiter| + expect(rate_limiter).to receive(:rate_limited?).and_return(false) + end - expect(hook).to be_invalid - run_expectation + expect(hook).not_to be_rate_limited + end + + it 'is true when hook has been rate limited' do + expect_next_instance_of(Gitlab::WebHooks::RateLimiter) do |rate_limiter| + expect(rate_limiter).to receive(:rate_limited?).and_return(true) + end + + expect(hook).to be_rate_limited end end - describe '#enable!' do - it 'makes a hook executable if it was marked as failed' do - hook.recent_failures = 1000 + describe '#rate_limit' do + it 'returns the hook rate limit' do + expect_next_instance_of(Gitlab::WebHooks::RateLimiter) do |rate_limiter| + expect(rate_limiter).to receive(:limit).and_return(10) + end - expect { hook.enable! }.to change(hook, :executable?).from(false).to(true) + expect(hook.rate_limit).to eq(10) end + end - it 'makes a hook executable if it is currently backed off' do - hook.recent_failures = 1000 - hook.disabled_until = 1.hour.from_now + describe '#to_json' do + it 'does not error' do + expect { hook.to_json }.not_to raise_error + end - expect { hook.enable! }.to change(hook, :executable?).from(false).to(true) + it 'does not contain binary attributes' do + expect(hook.to_json).not_to include('encrypted_url_variables') end + end - it 'does not update hooks unless necessary' do - sql_count = ActiveRecord::QueryRecorder.new { hook.enable! }.count + describe '#interpolated_url' do + subject(:hook) { build(:project_hook, project: project) } - expect(sql_count).to eq(0) + context 'when the hook URL does not contain variables' do + before do + hook.url = 'http://example.com' + end + + it { is_expected.to have_attributes(interpolated_url: hook.url) } + end + + it 'is not vulnerable to malicious input' do + hook.url = 'something%{%<foo>2147483628G}' + hook.url_variables = { 'foo' => '1234567890.12345678' } + + expect(hook).to have_attributes(interpolated_url: hook.url) end - include_examples 'is tolerant of invalid records' do - def run_expectation - hook.recent_failures = 1000 + context 'when the hook URL contains variables' do + before do + hook.url = 'http://example.com/{path}/resource?token={token}' + hook.url_variables = { 'path' => 'abc', 'token' => 'xyz' } + end + + it { is_expected.to have_attributes(interpolated_url: 'http://example.com/abc/resource?token=xyz') } + + context 'when a variable is missing' do + before do + hook.url_variables = { 'path' => 'present' } + end + + it 'raises an error' do + # We expect validations to prevent this entirely - this is not user-error + expect { hook.interpolated_url } + .to raise_error(described_class::InterpolationError, include('Missing key token')) + end + end + + context 'when the URL appears to include percent formatting' do + before do + hook.url = 'http://example.com/%{path}/resource?token=%{token}' + end - expect { hook.enable! }.to change(hook, :executable?).from(false).to(true) + it 'succeeds, interpolates correctly' do + expect(hook.interpolated_url).to eq 'http://example.com/%abc/resource?token=%xyz' + end end end end + describe '#update_last_failure' do + it 'is a method of this class' do + expect { described_class.new(project: project).update_last_failure }.not_to raise_error + end + end + + describe '#masked_token' do + it { expect(hook.masked_token).to be_nil } + + context 'with a token' do + let(:hook) { build(:project_hook, :token, project: project) } + + it { expect(hook.masked_token).to eq described_class::SECRET_MASK } + end + end + describe '#backoff!' do context 'when we have not backed off before' do - it 'does not disable the hook' do - expect { hook.backoff! }.not_to change(hook, :executable?).from(true) - end - it 'increments the recent_failures count' do expect { hook.backoff! }.to change(hook, :recent_failures).by(1) end @@ -517,20 +497,6 @@ RSpec.describe WebHook, feature_category: :integrations do expect { hook.backoff! }.to change(hook, :backoff_count).by(1) end - context 'when the hook is permanently disabled' do - before do - allow(hook).to receive(:permanently_disabled?).and_return(true) - end - - it 'does not set disabled_until' do - expect { hook.backoff! }.not_to change(hook, :disabled_until) - end - - it 'does not increment the backoff count' do - expect { hook.backoff! }.not_to change(hook, :backoff_count) - end - end - context 'when we have backed off MAX_FAILURES times' do before do stub_const("#{described_class}::MAX_FAILURES", 5) @@ -554,12 +520,6 @@ RSpec.describe WebHook, feature_category: :integrations do end end end - - include_examples 'is tolerant of invalid records' do - def run_expectation - expect { hook.backoff! }.to change(hook, :backoff_count).by(1) - end - end end end @@ -585,193 +545,5 @@ RSpec.describe WebHook, feature_category: :integrations do expect(sql_count).to eq(0) end - - include_examples 'is tolerant of invalid records' do - def run_expectation - expect { hook.failed! }.to change(hook, :recent_failures).by(1) - end - end - end - - describe '#disable!' do - it 'disables a hook' do - expect { hook.disable! }.to change(hook, :executable?).from(true).to(false) - end - - include_examples 'is tolerant of invalid records' do - def run_expectation - expect { hook.disable! }.to change(hook, :executable?).from(true).to(false) - end - end - end - - describe '#temporarily_disabled?' do - it 'is false when not temporarily disabled' do - expect(hook).not_to be_temporarily_disabled - end - - it 'allows FAILURE_THRESHOLD initial failures before we back-off' do - described_class::FAILURE_THRESHOLD.times do - hook.backoff! - expect(hook).not_to be_temporarily_disabled - end - - hook.backoff! - expect(hook).to be_temporarily_disabled - end - - context 'when hook has been told to back off' do - before do - hook.update!(recent_failures: described_class::FAILURE_THRESHOLD) - hook.backoff! - end - - it 'is true' do - expect(hook).to be_temporarily_disabled - end - end - end - - describe '#permanently_disabled?' do - it 'is false when not disabled' do - expect(hook).not_to be_permanently_disabled - end - - context 'when hook has been disabled' do - before do - hook.disable! - end - - it 'is true' do - expect(hook).to be_permanently_disabled - end - end - end - - describe '#rate_limited?' do - it 'is false when hook has not been rate limited' do - expect_next_instance_of(Gitlab::WebHooks::RateLimiter) do |rate_limiter| - expect(rate_limiter).to receive(:rate_limited?).and_return(false) - end - - expect(hook).not_to be_rate_limited - end - - it 'is true when hook has been rate limited' do - expect_next_instance_of(Gitlab::WebHooks::RateLimiter) do |rate_limiter| - expect(rate_limiter).to receive(:rate_limited?).and_return(true) - end - - expect(hook).to be_rate_limited - end - end - - describe '#rate_limit' do - it 'returns the hook rate limit' do - expect_next_instance_of(Gitlab::WebHooks::RateLimiter) do |rate_limiter| - expect(rate_limiter).to receive(:limit).and_return(10) - end - - expect(hook.rate_limit).to eq(10) - end - end - - describe '#alert_status' do - subject(:status) { hook.alert_status } - - it { is_expected.to eq :executable } - - context 'when hook has been disabled' do - before do - hook.disable! - end - - it { is_expected.to eq :disabled } - end - - context 'when hook has been backed off' do - before do - hook.update!(recent_failures: described_class::FAILURE_THRESHOLD + 1) - hook.disabled_until = 1.hour.from_now - end - - it { is_expected.to eq :temporarily_disabled } - end - end - - describe '#to_json' do - it 'does not error' do - expect { hook.to_json }.not_to raise_error - end - - it 'does not contain binary attributes' do - expect(hook.to_json).not_to include('encrypted_url_variables') - end - end - - describe '#interpolated_url' do - subject(:hook) { build(:project_hook, project: project) } - - context 'when the hook URL does not contain variables' do - before do - hook.url = 'http://example.com' - end - - it { is_expected.to have_attributes(interpolated_url: hook.url) } - end - - it 'is not vulnerable to malicious input' do - hook.url = 'something%{%<foo>2147483628G}' - hook.url_variables = { 'foo' => '1234567890.12345678' } - - expect(hook).to have_attributes(interpolated_url: hook.url) - end - - context 'when the hook URL contains variables' do - before do - hook.url = 'http://example.com/{path}/resource?token={token}' - hook.url_variables = { 'path' => 'abc', 'token' => 'xyz' } - end - - it { is_expected.to have_attributes(interpolated_url: 'http://example.com/abc/resource?token=xyz') } - - context 'when a variable is missing' do - before do - hook.url_variables = { 'path' => 'present' } - end - - it 'raises an error' do - # We expect validations to prevent this entirely - this is not user-error - expect { hook.interpolated_url } - .to raise_error(described_class::InterpolationError, include('Missing key token')) - end - end - - context 'when the URL appears to include percent formatting' do - before do - hook.url = 'http://example.com/%{path}/resource?token=%{token}' - end - - it 'succeeds, interpolates correctly' do - expect(hook.interpolated_url).to eq 'http://example.com/%abc/resource?token=%xyz' - end - end - end - end - - describe '#update_last_failure' do - it 'is a method of this class' do - expect { described_class.new.update_last_failure }.not_to raise_error - end - end - - describe '#masked_token' do - it { expect(hook.masked_token).to be_nil } - - context 'with a token' do - let(:hook) { build(:project_hook, :token, project: project) } - - it { expect(hook.masked_token).to eq described_class::SECRET_MASK } - end end end |