Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/persistent_user_callout.js

+import { parseBoolean } from './lib/utils/common_utils';
 import axios from './lib/utils/axios_utils';
 import { __ } from './locale';
 import Flash from './flash';
 
+const DEFERRED_LINK_CLASS = 'deferred-link';
+
 export default class PersistentUserCallout {
   constructor(container) {
-    const { dismissEndpoint, featureId } = container.dataset;
+    const { dismissEndpoint, featureId, deferLinks } = container.dataset;
     this.container = container;
     this.dismissEndpoint = dismissEndpoint;
     this.featureId = featureId;
+    this.deferLinks = parseBoolean(deferLinks);
 
     this.init();
   }
@@ -15,9 +19,21 @@ export default class PersistentUserCallout {
   init() {
     const closeButton = this.container.querySelector('.js-close');
     closeButton.addEventListener('click', event => this.dismiss(event));
+
+    if (this.deferLinks) {
+      this.container.addEventListener('click', event => {
+        const isDeferredLink = event.target.classList.contains(DEFERRED_LINK_CLASS);
+
+        if (isDeferredLink) {
+          const { href, target } = event.target;
+
+          this.dismiss(event, { href, target });
+        }
+      });
+    }
   }
 
-  dismiss(event) {
+  dismiss(event, deferredLinkOptions = null) {
     event.preventDefault();
 
     axios
@@ -26,6 +42,11 @@ export default class PersistentUserCallout {
       })
       .then(() => {
         this.container.remove();
+
+        if (deferredLinkOptions) {
+          const { href, target } = deferredLinkOptions;
+          window.open(href, target);
+        }
       })
       .catch(() => {
         Flash(__('An error occurred while dismissing the alert. Refresh the page and try again.'));

app/assets/javascripts/privacy_policy_update_callout.js

+import PersistentUserCallout from '~/persistent_user_callout';
+
+function initPrivacyPolicyUpdateCallout() {
+  const callout = document.querySelector('.privacy-policy-update-64341');
+  PersistentUserCallout.factory(callout);
+}
+
+export default initPrivacyPolicyUpdateCallout;

app/helpers/application_settings_helper.rb

@@ -160,6 +160,8 @@ module ApplicationSettingsHelper
       :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,

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,

app/models/hooks/system_hook.rb

@@ -14,8 +14,10 @@ class SystemHook < WebHook
   default_value_for :repository_update_events, true
   default_value_for :merge_requests_events, false
 
+  validates :url, system_hook_url: true
+
   # Allow urls pointing localhost and the local network
   def allow_local_requests?
-    true
+    Gitlab::CurrentSettings.allow_local_requests_from_system_hooks?
   end
 end

app/models/hooks/web_hook.rb

@@ -15,8 +15,8 @@ class WebHook < ApplicationRecord
 
   has_many :web_hook_logs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
 
-  validates :url, presence: true, public_url: { allow_localhost: lambda(&:allow_local_requests?),
-                                                allow_local_network: lambda(&:allow_local_requests?) }
+  validates :url, presence: true
+  validates :url, public_url: true, unless: ->(hook) { hook.is_a?(SystemHook) }
 
   validates :token, format: { without: /\n/ }
   validates :push_events_branch_filter, branch_filter: true
@@ -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

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

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

app/validators/system_hook_url_validator.rb

+# frozen_string_literal: true
+
+# SystemHookUrlValidator
+#
+# Custom validator specific to 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: true
+#   end
+#
+class SystemHookUrlValidator < PublicUrlValidator
+  def self.allow_setting_local_requests?
+    ApplicationSetting.current&.allow_local_requests_from_system_hooks?
+  end
+end

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

app/views/help/_shortcuts.html.haml

@@ -24,11 +24,11 @@
                   %td.shortcut
                     %kbd f
                   %td Focus Filter
-                - if performance_bar_enabled?
                 %tr
                   %td.shortcut
-                      %kbd p b
-                    %td Show/hide the Performance Bar
+                    %kbd p
+                    %kbd b
+                  %td Toggle the Performance Bar
                 %tr
                   %td.shortcut
                     %kbd ?

changelogs/unreleased/64341-user-callout-deferred-link-support.yml

+---
+title: Add support for deferred links in persistent user callouts.
+merge_request: 30818
+author:
+type: added

changelogs/unreleased/georgekoltsov-55474-outbound-setting-system-hooks.yml

+---
+title: Add new outbound network requests application setting for system hooks
+merge_request: 31177
+author:
+type: added

db/migrate/20190726101050_rename_allow_local_requests_from_hooks_and_services_application_setting.rb

+# frozen_string_literal: true
+
+class RenameAllowLocalRequestsFromHooksAndServicesApplicationSetting < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    rename_column_concurrently :application_settings, :allow_local_requests_from_hooks_and_services, :allow_local_requests_from_web_hooks_and_services
+  end
+
+  def down
+    cleanup_concurrent_column_rename :application_settings, :allow_local_requests_from_web_hooks_and_services, :allow_local_requests_from_hooks_and_services
+  end
+end

db/migrate/20190726101133_add_allow_local_requests_from_system_hooks_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddAllowLocalRequestsFromSystemHooksToApplicationSettings < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    add_column(:application_settings, :allow_local_requests_from_system_hooks,
+               :boolean,
+               default: true,
+               null: false)
+  end
+
+  def down
+    remove_column(:application_settings, :allow_local_requests_from_system_hooks)
+  end
+end

db/post_migrate/20190801114109_cleanup_allow_local_requests_from_hooks_and_services_application_setting_rename.rb

+# frozen_string_literal: true
+
+class CleanupAllowLocalRequestsFromHooksAndServicesApplicationSettingRename < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    cleanup_concurrent_column_rename :application_settings, :allow_local_requests_from_hooks_and_services, :allow_local_requests_from_web_hooks_and_services
+  end
+
+  def down
+    rename_column_concurrently :application_settings, :allow_local_requests_from_web_hooks_and_services, :allow_local_requests_from_hooks_and_services
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_07_31_084415) do
+ActiveRecord::Schema.define(version: 2019_08_01_114109) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -183,7 +183,6 @@ ActiveRecord::Schema.define(version: 2019_07_31_084415) do
     t.string "external_authorization_service_default_label"
     t.boolean "pages_domain_verification_enabled", default: true, null: false
     t.string "user_default_internal_regex"
-    t.boolean "allow_local_requests_from_hooks_and_services", default: false, null: false
     t.float "external_authorization_service_timeout", default: 0.5
     t.text "external_auth_client_cert"
     t.text "encrypted_external_auth_client_key"
@@ -230,6 +229,8 @@ ActiveRecord::Schema.define(version: 2019_07_31_084415) do
     t.string "grafana_url", default: "/-/grafana", null: false
     t.string "outbound_local_requests_whitelist", limit: 255, default: [], null: false, array: true
     t.integer "raw_blob_request_limit", default: 300, null: false
+    t.boolean "allow_local_requests_from_web_hooks_and_services", default: false, null: false
+    t.boolean "allow_local_requests_from_system_hooks", default: true, null: false
     t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
     t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
     t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id"

doc/api/settings.md

@@ -64,7 +64,10 @@ Example response:
    "performance_bar_allowed_group_id": 42,
    "instance_statistics_visibility_private": false,
    "user_show_add_ssh_key_message": true,
-   "local_markdown_version": 0
+   "local_markdown_version": 0,
+   "allow_local_requests_from_hooks_and_services": true,
+   "allow_local_requests_from_web_hooks_and_services": true,
+   "allow_local_requests_from_system_hooks": false
 }
 ```
 
@@ -138,7 +141,10 @@ Example response:
   "user_show_add_ssh_key_message": true,
   "file_template_project_id": 1,
   "local_markdown_version": 0,
-  "geo_node_allowed_ips": "0.0.0.0/0, ::/0"
+  "geo_node_allowed_ips": "0.0.0.0/0, ::/0",
+  "allow_local_requests_from_hooks_and_services": true,
+  "allow_local_requests_from_web_hooks_and_services": true,
+  "allow_local_requests_from_system_hooks": false
 }
 ```
 
@@ -177,7 +183,9 @@ are listed in the descriptions of the relevant settings.
 | `akismet_api_key`                        | string           | required by: `akismet_enabled`       | API key for akismet spam protection. |
 | `akismet_enabled`                        | boolean          | no                                   | (**If enabled, requires:** `akismet_api_key`) Enable or disable akismet spam protection. |
 | `allow_group_owners_to_manage_ldap`      | boolean          | no                                   | **(PREMIUM)** Set to `true` to allow group owners to manage LDAP |
-| `allow_local_requests_from_hooks_and_services` | boolean    | no                                   | Allow requests to the local network from hooks and services. |
+| `allow_local_requests_from_hooks_and_services` | boolean    | no                                   | (Deprecated: Use `allow_local_requests_from_web_hooks_and_services` instead) Allow requests to the local network from hooks and services. |
+| `allow_local_requests_from_web_hooks_and_services` | boolean    | no                                   | Allow requests to the local network from web hooks and services. |
+| `allow_local_requests_from_system_hooks` | boolean    | no                                   | Allow requests to the local network from system hooks. |
 | `authorized_keys_enabled`                | boolean          | no                                   | By default, we write to the `authorized_keys` file to support Git over SSH without additional configuration. GitLab can be optimized to authenticate SSH keys via the database file. Only disable this if you have configured your OpenSSH server to use the AuthorizedKeysCommand. |
 | `auto_devops_domain`                     | string           | no                                   | Specify a domain to use by default for every project's Auto Review Apps and Auto Deploy stages. |
 | `auto_devops_enabled`                    | boolean          | no                                   | Enable Auto DevOps for projects by default. It will automatically build, test, and deploy applications based on a predefined CI/CD configuration. |

doc/security/webhooks.md

@@ -34,15 +34,16 @@ to 127.0.0.1, ::1 and 0.0.0.0, as well as IPv4 10.0.0.0/8, 172.16.0.0/12,
 192.168.0.0/16 and IPv6 site-local (ffc0::/10) addresses won't be allowed.
 
 This behavior can be overridden by enabling the option *"Allow requests to the
-local network from hooks and services"* in the *"Outbound requests"* section
+local network from web hooks and services"* in the *"Outbound requests"* section
 inside the Admin area under **Settings**
 (`/admin/application_settings/network`):
 
-![Outbound requests admin settings](img/outbound_requests_section.png)
+![Outbound requests admin settings](img/outbound_requests_section_v12_2.png)
 
->**Note:**
-*System hooks* are exempt from this protection because they are set up by
-admins.
+NOTE: **Note:**
+*System hooks* are enabled to make requests to local network by default since they are
+set up by administrators. However, you can turn this off by disabling the
+**Allow requests to the local network from system hooks** option.
 
 <!-- ## Troubleshooting
 

lib/api/entities.rb

@@ -1162,6 +1162,7 @@ module API
         attributes = ::ApplicationSettingsHelper.visible_attributes
         attributes.delete(:performance_bar_allowed_group_path)
         attributes.delete(:performance_bar_enabled)
+        attributes.delete(:allow_local_requests_from_hooks_and_services)
 
         attributes
       end
@@ -1180,6 +1181,7 @@ module API
       # support legacy names, can be removed in v5
       expose :password_authentication_enabled_for_web, as: :password_authentication_enabled
       expose :password_authentication_enabled_for_web, as: :signin_enabled
+      expose :allow_local_requests_from_web_hooks_and_services, as: :allow_local_requests_from_hooks_and_services
     end
 
     # deprecated old Release representation

lib/api/settings.rb

@@ -124,6 +124,7 @@ module API
       optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'
       optional :instance_statistics_visibility_private, type: Boolean, desc: 'When set to `true` Instance statistics will only be available to admins'
       optional :local_markdown_version, type: Integer, desc: "Local markdown version, increase this value when any cached markdown should be invalidated"
+      optional :allow_local_requests_from_hooks_and_services, type: Boolean, desc: 'Deprecated: Use :allow_local_requests_from_web_hooks_and_services instead. Allow requests to the local network from hooks and services.' # support legacy names, can be removed in v5
 
       ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
         optional :"#{type}_key_restriction",
@@ -158,6 +159,11 @@ module API
         attrs[:password_authentication_enabled_for_web] = attrs.delete(:password_authentication_enabled)
       end
 
+      # support legacy names, can be removed in v5
+      if attrs.has_key?(:allow_local_requests_from_hooks_and_services)
+        attrs[:allow_local_requests_from_web_hooks_and_services] = attrs.delete(:allow_local_requests_from_hooks_and_services)
+      end
+
       attrs = filter_attributes_using_license(attrs)
 
       if ApplicationSettings::UpdateService.new(current_settings, current_user, attrs).execute

lib/gitlab/http_connection_adapter.rb

 # frozen_string_literal: true
 
 # This class is part of the Gitlab::HTTP wrapper. Depending on the value
-# of the global setting allow_local_requests_from_hooks_and_services this adapter
+# of the global setting allow_local_requests_from_web_hooks_and_services this adapter
 # will allow/block connection to internal IPs and/or urls.
 #
 # This functionality can be overridden by providing the setting the option
@@ -38,7 +38,7 @@ module Gitlab
     end
 
     def allow_settings_local_requests?
-      Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
+      Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
     end
   end
 end

lib/gitlab/kubernetes/kube_client.rb

@@ -128,7 +128,7 @@ module Gitlab
       private
 
       def validate_url!
-        return if Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
+        return if Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
 
         Gitlab::UrlBlocker.validate!(api_prefix, allow_local_network: false)
       end

lib/gitlab/octokit/middleware.rb

@@ -16,7 +16,7 @@ module Gitlab
       private
 
       def allow_local_requests?
-        Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
+        Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
       end
     end
   end

locale/gitlab.pot

@@ -1207,6 +1207,12 @@ msgstr ""
 msgid "Allow requests to the local network from hooks and services."
 msgstr ""
 
+msgid "Allow requests to the local network from system hooks"
+msgstr ""
+
+msgid "Allow requests to the local network from web hooks and services"
+msgstr ""
+
 msgid "Allow this key to push to repository as well? (Default only allows pull access.)"
 msgstr ""
 

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

spec/javascripts/persistent_user_callout_spec.js

@@ -22,6 +22,24 @@ describe('PersistentUserCallout', () => {
     return fixture;
   }
 
+  function createDeferredLinkFixture() {
+    const fixture = document.createElement('div');
+    fixture.innerHTML = `
+      <div
+        class="container"
+        data-dismiss-endpoint="${dismissEndpoint}"
+        data-feature-id="${featureName}"
+        data-defer-links="true"
+      >
+        <button type="button" class="js-close"></button>
+        <a href="/somewhere-pleasant" target="_blank" class="deferred-link">A link</a>
+        <a href="/somewhere-else" target="_blank" class="normal-link">Another link</a>
+      </div>
+    `;
+
+    return fixture;
+  }
+
   describe('dismiss', () => {
     let button;
     let mockAxios;
@@ -74,6 +92,75 @@ describe('PersistentUserCallout', () => {
     });
   });
 
+  describe('deferred links', () => {
+    let button;
+    let deferredLink;
+    let normalLink;
+    let mockAxios;
+    let persistentUserCallout;
+    let windowSpy;
+
+    beforeEach(() => {
+      const fixture = createDeferredLinkFixture();
+      const container = fixture.querySelector('.container');
+      button = fixture.querySelector('.js-close');
+      deferredLink = fixture.querySelector('.deferred-link');
+      normalLink = fixture.querySelector('.normal-link');
+      mockAxios = new MockAdapter(axios);
+      persistentUserCallout = new PersistentUserCallout(container);
+      spyOn(persistentUserCallout.container, 'remove');
+      windowSpy = spyOn(window, 'open').and.callFake(() => {});
+    });
+
+    afterEach(() => {
+      mockAxios.restore();
+    });
+
+    it('defers loading of a link until callout is dismissed', done => {
+      const { href, target } = deferredLink;
+      mockAxios.onPost(dismissEndpoint).replyOnce(200);
+
+      deferredLink.click();
+
+      setTimeoutPromise()
+        .then(() => {
+          expect(windowSpy).toHaveBeenCalledWith(href, target);
+          expect(persistentUserCallout.container.remove).toHaveBeenCalled();
+          expect(mockAxios.history.post[0].data).toBe(
+            JSON.stringify({ feature_name: featureName }),
+          );
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('does not dismiss callout on non-deferred links', done => {
+      normalLink.click();
+
+      setTimeoutPromise()
+        .then(() => {
+          expect(windowSpy).not.toHaveBeenCalled();
+          expect(persistentUserCallout.container.remove).not.toHaveBeenCalled();
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('does not follow link when notification is closed', done => {
+      mockAxios.onPost(dismissEndpoint).replyOnce(200);
+
+      button.click();
+
+      setTimeoutPromise()
+        .then(() => {
+          expect(windowSpy).not.toHaveBeenCalled();
+          expect(persistentUserCallout.container.remove).toHaveBeenCalled();
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+
   describe('factory', () => {
     it('returns an instance of PersistentUserCallout with the provided container property', () => {
       const fixture = createFixture();

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

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

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'

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 }

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

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
         {

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
 

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
 

spec/services/web_hook_service_spec.rb

@@ -19,16 +19,36 @@ 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
+
+    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
 
-    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 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
 

spec/spec_helper.rb

@@ -150,9 +150,9 @@ RSpec.configure do |config|
     Gitlab::ThreadMemoryCache.cache_backend.clear
   end
 
-  config.around(:example, :quarantine) do
+  config.around(:example, :quarantine) do |example|
     # Skip tests in quarantine unless we explicitly focus on them.
-    skip('In quarantine') unless config.inclusion_filter[:quarantine]
+    example.run if config.inclusion_filter[:quarantine]
   end
 
   config.before(:example, :request_store) do

spec/support/shared_examples/url_validator_examples.rb

 # frozen_string_literal: true
 
 RSpec.shared_examples 'url validator examples' do |schemes|
+  describe '#validate' do
     let(:validator) { described_class.new(attributes: [:link_url], **options) }
-  let!(:badge) { build(:badge, link_url: 'http://www.example.com') }
+    let(:badge) { build(:badge, link_url: 'http://www.example.com') }
 
     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

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

spec/validators/system_hook_url_validator_spec.rb

+# 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