diff options
author | George Koltsov <gkoltsov@gitlab.com> | 2019-07-26 11:21:52 +0100 |
---|---|---|
committer | George Koltsov <gkoltsov@gitlab.com> | 2019-08-02 15:39:18 +0100 |
commit | e5e1c907c01b53194f77e8d8de53554ba1824e7c (patch) | |
tree | 5f9602f3abf48056d4258a749cd9c756981d5abd /app | |
parent | eb2d4adf38726da62f62e850d181cedf12c64c5e (diff) | |
download | gitlab-ce-e5e1c907c01b53194f77e8d8de53554ba1824e7c.tar.gz |
Add outbound requests setting for system hooks
This MR adds new application setting to network section
`allow_local_requests_from_system_hooks`. Prior to this change
system hooks were allowed to do local network requests by default
and we are adding an ability for admins to control it.
Diffstat (limited to 'app')
-rw-r--r-- | app/helpers/application_settings_helper.rb | 3 | ||||
-rw-r--r-- | app/models/application_setting_implementation.rb | 3 | ||||
-rw-r--r-- | app/models/hooks/system_hook.rb | 5 | ||||
-rw-r--r-- | app/models/hooks/web_hook.rb | 2 | ||||
-rw-r--r-- | app/services/web_hook_service.rb | 6 | ||||
-rw-r--r-- | app/validators/addressable_url_validator.rb | 2 | ||||
-rw-r--r-- | app/validators/system_hook_url_validator.rb | 30 | ||||
-rw-r--r-- | app/views/admin/application_settings/_outbound.html.haml | 10 |
8 files changed, 51 insertions, 10 deletions
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 3847a35fbab..a646b62027a 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -159,7 +159,8 @@ module ApplicationSettingsHelper :after_sign_up_text, :akismet_api_key, :akismet_enabled, - :allow_local_requests_from_hooks_and_services, + :allow_local_requests_from_web_hooks_and_services, + :allow_local_requests_from_system_hooks, :dns_rebinding_protection_enabled, :archive_builds_in_human_readable, :authorized_keys_enabled, diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 4bb09bf3b53..b7a4d7aa803 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -21,7 +21,8 @@ module ApplicationSettingImplementation { after_sign_up_text: nil, akismet_enabled: false, - allow_local_requests_from_hooks_and_services: false, + allow_local_requests_from_web_hooks_and_services: false, + allow_local_requests_from_system_hooks: true, dns_rebinding_protection_enabled: true, authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand container_registry_token_expire_delay: 5, diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb index 90b4588a325..b83913c845f 100644 --- a/app/models/hooks/system_hook.rb +++ b/app/models/hooks/system_hook.rb @@ -14,8 +14,11 @@ class SystemHook < WebHook default_value_for :repository_update_events, true default_value_for :merge_requests_events, false + validates :url, presence: true, public_url: false, system_hook_url: { allow_localhost: lambda(&:allow_local_requests?), + allow_local_network: lambda(&:allow_local_requests?) } + # Allow urls pointing localhost and the local network def allow_local_requests? - true + Gitlab::CurrentSettings.allow_local_requests_from_system_hooks? end end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index daf7ff4b771..3203bb024ab 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -35,6 +35,6 @@ class WebHook < ApplicationRecord # Allow urls pointing localhost and the local network def allow_local_requests? - false + Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? end end diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 6d675c026bb..8c294218708 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -17,8 +17,10 @@ class WebHookService @hook = hook @data = data @hook_name = hook_name.to_s - @request_options = { timeout: Gitlab.config.gitlab.webhook_timeout } - @request_options.merge!(allow_local_requests: true) if @hook.is_a?(SystemHook) + @request_options = { + timeout: Gitlab.config.gitlab.webhook_timeout, + allow_local_requests: hook.allow_local_requests? + } end def execute diff --git a/app/validators/addressable_url_validator.rb b/app/validators/addressable_url_validator.rb index 273e15ef925..bb445499cee 100644 --- a/app/validators/addressable_url_validator.rb +++ b/app/validators/addressable_url_validator.rb @@ -107,6 +107,6 @@ class AddressableUrlValidator < ActiveModel::EachValidator # calls this validator. # # See https://gitlab.com/gitlab-org/gitlab-ee/issues/9833 - ApplicationSetting.current&.allow_local_requests_from_hooks_and_services? + ApplicationSetting.current&.allow_local_requests_from_web_hooks_and_services? end end diff --git a/app/validators/system_hook_url_validator.rb b/app/validators/system_hook_url_validator.rb new file mode 100644 index 00000000000..c8c0007e35b --- /dev/null +++ b/app/validators/system_hook_url_validator.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +# SystemHookUrlValidator +# +# Custom validator specifically for SystemHook URLs. This validator works like AddressableUrlValidator but +# it blocks urls pointing to localhost or the local network depending on +# ApplicationSetting.allow_local_requests_from_system_hooks +# +# Example: +# +# class SystemHook < WebHook +# validates :url, system_hook_url: { allow_localhost: true, allow_local_network: true } +# end +# +class SystemHookUrlValidator < AddressableUrlValidator + DEFAULT_OPTIONS = { + allow_localhost: true, + allow_local_network: true + }.freeze + + def initialize(options) + options.reverse_merge!(DEFAULT_OPTIONS) + + super(options) + end + + def self.allow_setting_local_requests? + ApplicationSetting.current&.allow_local_requests_from_system_hooks? + end +end diff --git a/app/views/admin/application_settings/_outbound.html.haml b/app/views/admin/application_settings/_outbound.html.haml index 4fecdb59e1d..d39e192d604 100644 --- a/app/views/admin/application_settings/_outbound.html.haml +++ b/app/views/admin/application_settings/_outbound.html.haml @@ -4,9 +4,13 @@ %fieldset .form-group .form-check - = f.check_box :allow_local_requests_from_hooks_and_services, class: 'form-check-input' - = f.label :allow_local_requests_from_hooks_and_services, class: 'form-check-label' do - Allow requests to the local network from hooks and services + = f.check_box :allow_local_requests_from_web_hooks_and_services, class: 'form-check-input' + = f.label :allow_local_requests_from_web_hooks_and_services, class: 'form-check-label' do + Allow requests to the local network from web hooks and services + .form-check + = f.check_box :allow_local_requests_from_system_hooks, class: 'form-check-input' + = f.label :allow_local_requests_from_system_hooks, class: 'form-check-label' do + Allow requests to the local network from system hooks .form-group = f.label :outbound_local_requests_whitelist_raw, class: 'label-bold' do |