Refactor SystemHookUrlValidator and specs

Simplify SystemHookUrlValidator to inherit from PublicUrlValidator
Refactor specs to move out shared examples to be used in both
system hooks and public url validators.

5 files changed, 64 insertions, 87 deletions

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/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb
index 115599bd1a4..50167a2e059 100644
--- a/spec/services/web_hook_service_spec.rb
+++ b/spec/services/web_hook_service_spec.rb
@@ -23,14 +23,14 @@ describe WebHookService do
       stub_application_setting(setting_name => setting)
     end
 
-    shared_examples_for 'respecting outbound network setting' do
-      context 'local requests are allowed' do
+    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 'local requests are not allowed' do
+      context 'when local requests are not allowed' do
         let(:setting) { false }
 
         it { expect(hook.request_options[:allow_local_requests]).to be_falsey }
@@ -41,14 +41,14 @@ describe WebHookService do
       let(:setting_name) { :allow_local_requests_from_system_hooks }
       let(:hook) { described_class.new(build(:system_hook), data, :system_hook) }
 
-      include_examples 'respecting outbound network setting'
+      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 'respecting outbound network setting'
+      include_examples 'respects outbound network setting'
     end
   end
 
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
index 78e95db2b47..02384bbd1ce 100644
--- a/spec/validators/system_hook_url_validator_spec.rb
+++ b/spec/validators/system_hook_url_validator_spec.rb
@@ -4,55 +4,5 @@ require 'spec_helper'
 
 describe SystemHookUrlValidator 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
-
-  context 'when local requests are allowed' do
-    let(:validator) { described_class.new(attributes: [:link_url]) }
-    let!(:badge) { build(:badge, link_url: 'http://www.example.com') }
-    let!(:settings) { create(:application_setting) }
-
-    subject { validator.validate(badge) }
-
-    before do
-      stub_application_setting(allow_local_requests_from_system_hooks: true)
-    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
+  include_examples 'public url validator examples', allow_local_requests_from_system_hooks: true
 end