Commit f75b04be authored by Jarka Košanová's avatar Jarka Košanová

Merge branch '349845-enable-webhook_recursion_detection' into 'master'

Enable logging when recursive web hooks are detected

See merge request gitlab-org/gitlab!78343
parents 9b34f6cb 2c9dc9a4
---
name: webhook_recursion_detection
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75821
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/349845
milestone: '14.7'
type: development
group: group::integrations
default_enabled: false
......@@ -37,8 +37,6 @@ module Gitlab
# Adds the webhook ID to a cache (see `#cache_key_for_hook` for
# details of the cache).
def register!(hook)
return if disabled?(hook)
cache_key = cache_key_for_hook(hook)
::Gitlab::Redis::SharedState.with do |redis|
......@@ -53,8 +51,6 @@ module Gitlab
# number of IDs in the cache exceeds the limit (see
# `#cache_key_for_hook` for details of the cache).
def block?(hook)
return false if disabled?(hook)
# If a request UUID has not been set then we know the request was not
# made by a webhook, and no recursion is possible.
return false unless UUID.instance.request_uuid
......@@ -80,10 +76,6 @@ module Gitlab
private
def disabled?(hook)
Feature.disabled?(:webhook_recursion_detection, hook.parent)
end
# Returns a cache key scoped to a UUID.
#
# The particular UUID will be either:
......
......@@ -128,16 +128,6 @@ RSpec.describe Gitlab::WebHooks::RecursionDetection, :clean_gitlab_redis_shared_
end
end
end
it 'does not store anything if the feature flag is disabled' do
stub_feature_flags(webhook_recursion_detection: false)
described_class.register!(web_hook)
::Gitlab::Redis::SharedState.with do |redis|
expect(redis.exists(cache_key(web_hook))).to eq(false)
end
end
end
describe 'block?' do
......@@ -167,12 +157,6 @@ RSpec.describe Gitlab::WebHooks::RecursionDetection, :clean_gitlab_redis_shared_
is_expected.to eq(true)
end
it 'returns false if the feature flag is disabled' do
stub_feature_flags(webhook_recursion_detection: false)
is_expected.to eq(false)
end
context 'when the request UUID changes again' do
before do
uuid_class.instance.request_uuid = SecureRandom.uuid
......@@ -199,12 +183,6 @@ RSpec.describe Gitlab::WebHooks::RecursionDetection, :clean_gitlab_redis_shared_
is_expected.to eq(true)
end
it 'returns false if the feature flag is disabled' do
stub_feature_flags(webhook_recursion_detection: false)
is_expected.to eq(false)
end
context 'when the request UUID changes again' do
before do
uuid_class.instance.request_uuid = SecureRandom.uuid
......
......@@ -45,20 +45,6 @@ RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_red
.once
end
shared_examples 'when the feature flag is disabled' do
it 'executes and logs no errors' do
stub_feature_flags(webhook_recursion_detection: false)
stub_requests
expect(Gitlab::AuthLogger).not_to receive(:error)
trigger_web_hooks
expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)).once
expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)).once
end
end
context 'when one of the webhooks is recursive' do
before do
# Recreate the necessary state for the previous request to be
......@@ -87,8 +73,6 @@ RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_red
expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)).once
expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)).once
end
include_examples 'when the feature flag is disabled'
end
context 'when the count limit has been reached' do
......@@ -134,8 +118,6 @@ RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_red
expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)).once
end
end
include_examples 'when the feature flag is disabled'
end
context 'when the recursive webhook detection header is absent' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment