diff options
author | Rémy Coutable <remy@rymai.me> | 2019-08-05 11:55:16 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2019-08-05 11:55:16 +0300 |
commit | f74387d2982845b0fb5219ccf3fd128d75400472 (patch) | |
tree | 3cfbc60f1a62a1d308f1d1c98ad4a969721b873b /spec | |
parent | e6a05e5a63b3b336179a4ed67df76ba7ad08fadb (diff) | |
parent | 260225eb5753a18e9e33ec2bb9c1bfc76d613448 (diff) |
Merge branch 'georgekoltsov/55474-outbound-setting-system-hooks' into 'master'
Add outbound setting for system hooks
See merge request gitlab-org/gitlab-ce!31177
Diffstat (limited to 'spec')
-rw-r--r-- | spec/features/admin/admin_settings_spec.rb | 7 | ||||
-rw-r--r-- | spec/lib/gitlab/http_spec.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/kubernetes/kube_client_spec.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/octokit/middleware_spec.rb | 8 | ||||
-rw-r--r-- | spec/models/clusters/platforms/kubernetes_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/lfs_download_object_spec.rb | 2 | ||||
-rw-r--r-- | spec/requests/api/settings_spec.rb | 17 | ||||
-rw-r--r-- | spec/services/projects/lfs_pointers/lfs_download_service_spec.rb | 2 | ||||
-rw-r--r-- | spec/services/self_monitoring/project/create_service_spec.rb | 4 | ||||
-rw-r--r-- | spec/services/web_hook_service_spec.rb | 34 | ||||
-rw-r--r-- | spec/support/shared_examples/url_validator_examples.rb | 57 | ||||
-rw-r--r-- | spec/validators/public_url_validator_spec.rb | 24 | ||||
-rw-r--r-- | spec/validators/system_hook_url_validator_spec.rb | 8 |
13 files changed, 123 insertions, 50 deletions
diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index c77605f3869..ddd87404003 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -338,14 +338,17 @@ describe 'Admin updates settings' do visit network_admin_application_settings_path page.within('.as-outbound') do - check 'Allow requests to the local network from hooks and services' + check 'Allow requests to the local network from web hooks and services' + # Enabled by default + uncheck 'Allow requests to the local network from system hooks' # Enabled by default uncheck 'Enforce DNS rebinding attack protection' click_button 'Save changes' end expect(page).to have_content "Application settings saved successfully" - expect(current_settings.allow_local_requests_from_hooks_and_services).to be true + expect(current_settings.allow_local_requests_from_web_hooks_and_services).to be true + expect(current_settings.allow_local_requests_from_system_hooks).to be false expect(current_settings.dns_rebinding_protection_enabled).to be false end end diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb index 158f77cab2c..d3f9be845dd 100644 --- a/spec/lib/gitlab/http_spec.rb +++ b/spec/lib/gitlab/http_spec.rb @@ -23,14 +23,14 @@ describe Gitlab::HTTP do end end - describe 'allow_local_requests_from_hooks_and_services is' do + describe 'allow_local_requests_from_web_hooks_and_services is' do before do WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success') end context 'disabled' do before do - allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(false) + allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(false) end it 'deny requests to localhost' do @@ -52,7 +52,7 @@ describe Gitlab::HTTP do context 'enabled' do before do - allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(true) + allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(true) end it 'allow requests to localhost' do diff --git a/spec/lib/gitlab/kubernetes/kube_client_spec.rb b/spec/lib/gitlab/kubernetes/kube_client_spec.rb index 97ebb5f1554..f49d4e23e39 100644 --- a/spec/lib/gitlab/kubernetes/kube_client_spec.rb +++ b/spec/lib/gitlab/kubernetes/kube_client_spec.rb @@ -58,7 +58,7 @@ describe Gitlab::Kubernetes::KubeClient do context 'when local requests are allowed' do before do - stub_application_setting(allow_local_requests_from_hooks_and_services: true) + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) end it 'allows local addresses' do diff --git a/spec/lib/gitlab/octokit/middleware_spec.rb b/spec/lib/gitlab/octokit/middleware_spec.rb index 7f2b523f5b7..43f6d13f7ba 100644 --- a/spec/lib/gitlab/octokit/middleware_spec.rb +++ b/spec/lib/gitlab/octokit/middleware_spec.rb @@ -30,7 +30,7 @@ describe Gitlab::Octokit::Middleware do context 'when localhost requests are not allowed' do before do - stub_application_setting(allow_local_requests_from_hooks_and_services: false) + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) end it_behaves_like 'Local URL' @@ -38,7 +38,7 @@ describe Gitlab::Octokit::Middleware do context 'when localhost requests are allowed' do before do - stub_application_setting(allow_local_requests_from_hooks_and_services: true) + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) end it_behaves_like 'Public URL' @@ -50,7 +50,7 @@ describe Gitlab::Octokit::Middleware do context 'when local network requests are not allowed' do before do - stub_application_setting(allow_local_requests_from_hooks_and_services: false) + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) end it_behaves_like 'Local URL' @@ -58,7 +58,7 @@ describe Gitlab::Octokit::Middleware do context 'when local network requests are allowed' do before do - stub_application_setting(allow_local_requests_from_hooks_and_services: true) + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) end it_behaves_like 'Public URL' diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index 471769e4aab..5811016ea4d 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -106,7 +106,7 @@ describe Clusters::Platforms::Kubernetes do before do allow(ApplicationSetting) .to receive(:current) - .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true)) + .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: true)) end it { expect(kubernetes.save).to be_truthy } diff --git a/spec/models/lfs_download_object_spec.rb b/spec/models/lfs_download_object_spec.rb index effd8b08124..8b53effe98f 100644 --- a/spec/models/lfs_download_object_spec.rb +++ b/spec/models/lfs_download_object_spec.rb @@ -50,7 +50,7 @@ describe LfsDownloadObject do before do allow(ApplicationSetting) .to receive(:current) - .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: setting)) + .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: setting)) end context 'are allowed' do diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 8a60980fe80..184c00a356a 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -25,6 +25,9 @@ describe API::Settings, 'Settings' do expect(json_response['ed25519_key_restriction']).to eq(0) expect(json_response['performance_bar_allowed_group_id']).to be_nil expect(json_response['instance_statistics_visibility_private']).to be(false) + expect(json_response['allow_local_requests_from_hooks_and_services']).to be(false) + expect(json_response['allow_local_requests_from_web_hooks_and_services']).to be(false) + expect(json_response['allow_local_requests_from_system_hooks']).to be(true) expect(json_response).not_to have_key('performance_bar_allowed_group_path') expect(json_response).not_to have_key('performance_bar_enabled') end @@ -67,7 +70,9 @@ describe API::Settings, 'Settings' do instance_statistics_visibility_private: true, diff_max_patch_bytes: 150_000, default_branch_protection: ::Gitlab::Access::PROTECTION_DEV_CAN_MERGE, - local_markdown_version: 3 + local_markdown_version: 3, + allow_local_requests_from_web_hooks_and_services: true, + allow_local_requests_from_system_hooks: false } expect(response).to have_gitlab_http_status(200) @@ -95,6 +100,8 @@ describe API::Settings, 'Settings' do expect(json_response['diff_max_patch_bytes']).to eq(150_000) expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) expect(json_response['local_markdown_version']).to eq(3) + expect(json_response['allow_local_requests_from_web_hooks_and_services']).to eq(true) + expect(json_response['allow_local_requests_from_system_hooks']).to eq(false) end end @@ -117,6 +124,14 @@ describe API::Settings, 'Settings' do expect(json_response['performance_bar_allowed_group_id']).to be_nil end + it 'supports legacy allow_local_requests_from_hooks_and_services' do + put api("/application/settings", admin), + params: { allow_local_requests_from_hooks_and_services: true } + + expect(response).to have_gitlab_http_status(200) + expect(json_response['allow_local_requests_from_hooks_and_services']).to eq(true) + end + context 'external policy classification settings' do let(:settings) do { diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb index 75d534c59bf..970e82e7107 100644 --- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb @@ -17,7 +17,7 @@ describe Projects::LfsPointers::LfsDownloadService do before do ApplicationSetting.create_from_defaults - stub_application_setting(allow_local_requests_from_hooks_and_services: local_request_setting) + stub_application_setting(allow_local_requests_from_web_hooks_and_services: local_request_setting) allow(project).to receive(:lfs_enabled?).and_return(true) end diff --git a/spec/services/self_monitoring/project/create_service_spec.rb b/spec/services/self_monitoring/project/create_service_spec.rb index a1e7aaf45f2..7d4faba526b 100644 --- a/spec/services/self_monitoring/project/create_service_spec.rb +++ b/spec/services/self_monitoring/project/create_service_spec.rb @@ -37,7 +37,7 @@ describe SelfMonitoring::Project::CreateService do allow(ApplicationSetting) .to receive(:current) .and_return( - ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true) + ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: true) ) end @@ -95,7 +95,7 @@ describe SelfMonitoring::Project::CreateService do allow(ApplicationSetting) .to receive(:current) .and_return( - ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: false) + ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: false) ) end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 37bafc0c002..50167a2e059 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -19,17 +19,37 @@ describe WebHookService do let(:service_instance) { described_class.new(project_hook, data, :push_hooks) } describe '#initialize' do - it 'allow_local_requests is true if hook is a SystemHook' do - instance = described_class.new(build(:system_hook), data, :system_hook) - expect(instance.request_options[:allow_local_requests]).to be_truthy + before do + stub_application_setting(setting_name => setting) end - it 'allow_local_requests is false if hook is not a SystemHook' do - %i(project_hook service_hook web_hook_log).each do |hook| - instance = described_class.new(build(hook), data, hook) - expect(instance.request_options[:allow_local_requests]).to be_falsey + shared_examples_for 'respects outbound network setting' do + context 'when local requests are allowed' do + let(:setting) { true } + + it { expect(hook.request_options[:allow_local_requests]).to be_truthy } + end + + context 'when local requests are not allowed' do + let(:setting) { false } + + it { expect(hook.request_options[:allow_local_requests]).to be_falsey } end end + + context 'when SystemHook' do + let(:setting_name) { :allow_local_requests_from_system_hooks } + let(:hook) { described_class.new(build(:system_hook), data, :system_hook) } + + include_examples 'respects outbound network setting' + end + + context 'when ProjectHook' do + let(:setting_name) { :allow_local_requests_from_web_hooks_and_services } + let(:hook) { described_class.new(build(:project_hook), data, :project_hook) } + + include_examples 'respects outbound network setting' + end end describe '#execute' do diff --git a/spec/support/shared_examples/url_validator_examples.rb b/spec/support/shared_examples/url_validator_examples.rb index 16fceddb605..c5a775fefb6 100644 --- a/spec/support/shared_examples/url_validator_examples.rb +++ b/spec/support/shared_examples/url_validator_examples.rb @@ -1,12 +1,12 @@ # frozen_string_literal: true RSpec.shared_examples 'url validator examples' do |schemes| - let(:validator) { described_class.new(attributes: [:link_url], **options) } - let!(:badge) { build(:badge, link_url: 'http://www.example.com') } + describe '#validate' do + let(:validator) { described_class.new(attributes: [:link_url], **options) } + let(:badge) { build(:badge, link_url: 'http://www.example.com') } - subject { validator.validate(badge) } + subject { validator.validate(badge) } - describe '#validate' do context 'with no options' do let(:options) { {} } @@ -42,3 +42,52 @@ RSpec.shared_examples 'url validator examples' do |schemes| end end end + +RSpec.shared_examples 'public url validator examples' do |setting| + let(:validator) { described_class.new(attributes: [:link_url]) } + let(:badge) { build(:badge, link_url: 'http://www.example.com') } + + subject { validator.validate(badge) } + + context 'by default' do + it 'blocks urls pointing to localhost' do + badge.link_url = 'https://127.0.0.1' + + subject + + expect(badge.errors).to be_present + end + + it 'blocks urls pointing to the local network' do + badge.link_url = 'https://192.168.1.1' + + subject + + expect(badge.errors).to be_present + end + end + + context 'when local requests are allowed' do + let!(:settings) { create(:application_setting) } + + before do + stub_application_setting(setting) + end + + it 'does not block urls pointing to localhost' do + badge.link_url = 'https://127.0.0.1' + + subject + + expect(badge.errors).not_to be_present + end + + it 'does not block urls pointing to the local network' do + badge.link_url = 'https://192.168.1.1' + + subject + + expect(badge.errors).not_to be_present + end + end +end diff --git a/spec/validators/public_url_validator_spec.rb b/spec/validators/public_url_validator_spec.rb index f6364fb1dd5..3cbf1002730 100644 --- a/spec/validators/public_url_validator_spec.rb +++ b/spec/validators/public_url_validator_spec.rb @@ -2,27 +2,5 @@ require 'spec_helper' describe PublicUrlValidator do include_examples 'url validator examples', AddressableUrlValidator::DEFAULT_OPTIONS[:schemes] - - context 'by default' do - let(:validator) { described_class.new(attributes: [:link_url]) } - let!(:badge) { build(:badge, link_url: 'http://www.example.com') } - - subject { validator.validate(badge) } - - it 'blocks urls pointing to localhost' do - badge.link_url = 'https://127.0.0.1' - - subject - - expect(badge.errors).to be_present - end - - it 'blocks urls pointing to the local network' do - badge.link_url = 'https://192.168.1.1' - - subject - - expect(badge.errors).to be_present - end - end + include_examples 'public url validator examples', allow_local_requests_from_web_hooks_and_services: true end diff --git a/spec/validators/system_hook_url_validator_spec.rb b/spec/validators/system_hook_url_validator_spec.rb new file mode 100644 index 00000000000..02384bbd1ce --- /dev/null +++ b/spec/validators/system_hook_url_validator_spec.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SystemHookUrlValidator do + include_examples 'url validator examples', AddressableUrlValidator::DEFAULT_OPTIONS[:schemes] + include_examples 'public url validator examples', allow_local_requests_from_system_hooks: true +end |