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.

8 files changed, 49 insertions, 18 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/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/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..87d7d776a69 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
 
diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb
index 37bafc0c002..8353fade5ea 100644
--- a/spec/services/web_hook_service_spec.rb
+++ b/spec/services/web_hook_service_spec.rb
@@ -19,15 +19,43 @@ 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
-    end
+    context 'when SystemHook' do
+      context 'when allow_local_requests_from_system_hooks application setting is true' do
+        it 'allows local requests' do
+          stub_application_setting(allow_local_requests_from_system_hooks: true)
+          instance = described_class.new(build(:system_hook), data, :system_hook)
+
+          expect(instance.request_options[:allow_local_requests]).to be_truthy
+        end
+      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
+      context 'when allow_local_requests_from_system_hooks application setting is false' do
+        it 'denies local requests' do
+          stub_application_setting(allow_local_requests_from_system_hooks: false)
+          instance = described_class.new(build(:system_hook), data, :system_hook)
+
+          expect(instance.request_options[:allow_local_requests]).to be_falsey
+        end
+      end
+
+      context 'when ProjectHook' do
+        context 'when allow_local_requests_from_web_hooks_and_services application setting is true' do
+          it 'allows local requests' do
+            stub_application_setting(allow_local_requests_from_web_hooks_and_services: true)
+            instance = described_class.new(build(:project_hook), data, :project_hook)
+
+            expect(instance.request_options[:allow_local_requests]).to be_truthy
+          end
+        end
+
+        context 'when allow_local_requests_from_system_hooks application setting is false' do
+          it 'denies local requests' do
+            stub_application_setting(allow_local_requests_from_web_hooks_and_services: false)
+            instance = described_class.new(build(:project_hook), data, :project_hook)
+
+            expect(instance.request_options[:allow_local_requests]).to be_falsey
+          end
+        end
       end
     end
   end