summaryrefslogtreecommitdiff
path: root/spec
diff options
context:
space:
mode:
authorGitLab Release Tools Bot <robert+release-tools@gitlab.com>2019-05-28 13:15:55 +0000
committerGitLab Release Tools Bot <robert+release-tools@gitlab.com>2019-05-28 13:15:55 +0000
commit112776b29fa0109cce88812104eaa29691dd8998 (patch)
treede504357d54a9292ba3d4986555935fe2764f11e /spec
parent701914e1d9f6e5f56533c277fccdfc110d047a77 (diff)
parent1de0a03355fe3f165a4352ba381da975b9b37ad7 (diff)
downloadgitlab-ce-112776b29fa0109cce88812104eaa29691dd8998.tar.gz
Merge branch 'security-http-hostname-override-11-10' into '11-10-stable'
Protect Gitlab::HTTP against DNS rebinding attack See merge request gitlab/gitlabhq!3114
Diffstat (limited to 'spec')
-rw-r--r--spec/controllers/projects/ci/lints_controller_spec.rb4
-rw-r--r--spec/lib/gitlab/ci/config/external/file/remote_spec.rb14
-rw-r--r--spec/lib/gitlab/ci/config/external/mapper_spec.rb4
-rw-r--r--spec/lib/gitlab/ci/config/external/processor_spec.rb14
-rw-r--r--spec/lib/gitlab/ci/config_spec.rb5
-rw-r--r--spec/lib/gitlab/http_connection_adapter_spec.rb88
-rw-r--r--spec/lib/gitlab/http_spec.rb28
-rw-r--r--spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb4
-rw-r--r--spec/lib/gitlab/url_blocker_spec.rb48
-rw-r--r--spec/lib/mattermost/session_spec.rb7
-rw-r--r--spec/models/project_services/assembla_service_spec.rb6
-rw-r--r--spec/models/project_services/bamboo_service_spec.rb3
-rw-r--r--spec/models/project_services/buildkite_service_spec.rb10
-rw-r--r--spec/models/project_services/campfire_service_spec.rb24
-rw-r--r--spec/models/project_services/pivotaltracker_service_spec.rb10
-rw-r--r--spec/models/project_services/pushover_service_spec.rb6
-rw-r--r--spec/models/project_services/teamcity_service_spec.rb3
-rw-r--r--spec/requests/api/system_hooks_spec.rb8
-rw-r--r--spec/services/projects/lfs_pointers/lfs_download_service_spec.rb19
-rw-r--r--spec/services/submit_usage_ping_service_spec.rb4
-rw-r--r--spec/services/web_hook_service_spec.rb10
-rw-r--r--spec/support/helpers/stub_requests.rb40
22 files changed, 297 insertions, 62 deletions
diff --git a/spec/controllers/projects/ci/lints_controller_spec.rb b/spec/controllers/projects/ci/lints_controller_spec.rb
index 0b79484bbfa..992a0e0c030 100644
--- a/spec/controllers/projects/ci/lints_controller_spec.rb
+++ b/spec/controllers/projects/ci/lints_controller_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe Projects::Ci::LintsController do
+ include StubRequests
+
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
@@ -68,7 +70,7 @@ describe Projects::Ci::LintsController do
context 'with a valid gitlab-ci.yml' do
before do
- WebMock.stub_request(:get, remote_file_path).to_return(body: remote_file_content)
+ stub_full_request(remote_file_path).to_return(body: remote_file_content)
project.add_developer(user)
post :create, params: { namespace_id: project.namespace, project_id: project, content: content }
diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb
index d8a61618e77..46d68097fff 100644
--- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb
+++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
describe Gitlab::Ci::Config::External::File::Remote do
+ include StubRequests
+
let(:context) { described_class::Context.new(nil, '12345', nil, Set.new) }
let(:params) { { remote: location } }
let(:remote_file) { described_class.new(params, context) }
@@ -46,7 +48,7 @@ describe Gitlab::Ci::Config::External::File::Remote do
describe "#valid?" do
context 'when is a valid remote url' do
before do
- WebMock.stub_request(:get, location).to_return(body: remote_file_content)
+ stub_full_request(location).to_return(body: remote_file_content)
end
it 'returns true' do
@@ -92,7 +94,7 @@ describe Gitlab::Ci::Config::External::File::Remote do
describe "#content" do
context 'with a valid remote file' do
before do
- WebMock.stub_request(:get, location).to_return(body: remote_file_content)
+ stub_full_request(location).to_return(body: remote_file_content)
end
it 'returns the content of the file' do
@@ -114,7 +116,7 @@ describe Gitlab::Ci::Config::External::File::Remote do
let(:location) { 'https://asdasdasdaj48ggerexample.com' }
before do
- WebMock.stub_request(:get, location).to_raise(SocketError.new('Some HTTP error'))
+ stub_full_request(location).to_raise(SocketError.new('Some HTTP error'))
end
it 'is nil' do
@@ -144,7 +146,7 @@ describe Gitlab::Ci::Config::External::File::Remote do
context 'when timeout error has been raised' do
before do
- WebMock.stub_request(:get, location).to_timeout
+ stub_full_request(location).to_timeout
end
it 'returns error message about a timeout' do
@@ -154,7 +156,7 @@ describe Gitlab::Ci::Config::External::File::Remote do
context 'when HTTP error has been raised' do
before do
- WebMock.stub_request(:get, location).to_raise(Gitlab::HTTP::Error)
+ stub_full_request(location).to_raise(Gitlab::HTTP::Error)
end
it 'returns error message about a HTTP error' do
@@ -164,7 +166,7 @@ describe Gitlab::Ci::Config::External::File::Remote do
context 'when response has 404 status' do
before do
- WebMock.stub_request(:get, location).to_return(body: remote_file_content, status: 404)
+ stub_full_request(location).to_return(body: remote_file_content, status: 404)
end
it 'returns error message about a timeout' do
diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb
index 136974569de..e068b786b02 100644
--- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb
+++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
describe Gitlab::Ci::Config::External::Mapper do
+ include StubRequests
+
set(:project) { create(:project, :repository) }
set(:user) { create(:user) }
@@ -18,7 +20,7 @@ describe Gitlab::Ci::Config::External::Mapper do
end
before do
- WebMock.stub_request(:get, remote_url).to_return(body: file_content)
+ stub_full_request(remote_url).to_return(body: file_content)
end
describe '#process' do
diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb
index e94bb44f990..f813806e4fb 100644
--- a/spec/lib/gitlab/ci/config/external/processor_spec.rb
+++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
describe Gitlab::Ci::Config::External::Processor do
+ include StubRequests
+
set(:project) { create(:project, :repository) }
set(:another_project) { create(:project, :repository) }
set(:user) { create(:user) }
@@ -42,7 +44,7 @@ describe Gitlab::Ci::Config::External::Processor do
let(:values) { { include: remote_file, image: 'ruby:2.2' } }
before do
- WebMock.stub_request(:get, remote_file).to_raise(SocketError.new('Some HTTP error'))
+ stub_full_request(remote_file).and_raise(SocketError.new('Some HTTP error'))
end
it 'raises an error' do
@@ -75,7 +77,7 @@ describe Gitlab::Ci::Config::External::Processor do
end
before do
- WebMock.stub_request(:get, remote_file).to_return(body: external_file_content)
+ stub_full_request(remote_file).to_return(body: external_file_content)
end
it 'appends the file to the values' do
@@ -145,7 +147,7 @@ describe Gitlab::Ci::Config::External::Processor do
allow_any_instance_of(Gitlab::Ci::Config::External::File::Local)
.to receive(:fetch_local_content).and_return(local_file_content)
- WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content)
+ stub_full_request(remote_file).to_return(body: remote_file_content)
end
it 'appends the files to the values' do
@@ -191,7 +193,8 @@ describe Gitlab::Ci::Config::External::Processor do
end
it 'takes precedence' do
- WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content)
+ stub_full_request(remote_file).to_return(body: remote_file_content)
+
expect(processor.perform[:image]).to eq('ruby:2.2')
end
end
@@ -231,7 +234,8 @@ describe Gitlab::Ci::Config::External::Processor do
HEREDOC
end
- WebMock.stub_request(:get, 'http://my.domain.com/config.yml').to_return(body: 'remote_build: { script: echo Hello World }')
+ stub_full_request('http://my.domain.com/config.yml')
+ .to_return(body: 'remote_build: { script: echo Hello World }')
end
context 'when project is public' do
diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb
index fd2a29e4ddb..64396a746ee 100644
--- a/spec/lib/gitlab/ci/config_spec.rb
+++ b/spec/lib/gitlab/ci/config_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe Gitlab::Ci::Config do
+ include StubRequests
+
set(:user) { create(:user) }
let(:config) do
@@ -217,8 +219,7 @@ describe Gitlab::Ci::Config do
end
before do
- WebMock.stub_request(:get, remote_location)
- .to_return(body: remote_file_content)
+ stub_full_request(remote_location).to_return(body: remote_file_content)
allow(project.repository)
.to receive(:blob_data_at).and_return(local_file_content)
diff --git a/spec/lib/gitlab/http_connection_adapter_spec.rb b/spec/lib/gitlab/http_connection_adapter_spec.rb
new file mode 100644
index 00000000000..af033be8e5b
--- /dev/null
+++ b/spec/lib/gitlab/http_connection_adapter_spec.rb
@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::HTTPConnectionAdapter do
+ describe '#connection' do
+ context 'when local requests are not allowed' do
+ it 'sets up the connection' do
+ uri = URI('https://example.org')
+
+ connection = described_class.new(uri).connection
+
+ expect(connection).to be_a(Net::HTTP)
+ expect(connection.address).to eq('93.184.216.34')
+ expect(connection.hostname_override).to eq('example.org')
+ expect(connection.addr_port).to eq('example.org')
+ expect(connection.port).to eq(443)
+ end
+
+ it 'raises error when it is a request to local address' do
+ uri = URI('http://172.16.0.0/12')
+
+ expect { described_class.new(uri).connection }
+ .to raise_error(Gitlab::HTTP::BlockedUrlError,
+ "URL 'http://172.16.0.0/12' is blocked: Requests to the local network are not allowed")
+ end
+
+ it 'raises error when it is a request to localhost address' do
+ uri = URI('http://127.0.0.1')
+
+ expect { described_class.new(uri).connection }
+ .to raise_error(Gitlab::HTTP::BlockedUrlError,
+ "URL 'http://127.0.0.1' is blocked: Requests to localhost are not allowed")
+ end
+
+ context 'when port different from URL scheme is used' do
+ it 'sets up the addr_port accordingly' do
+ uri = URI('https://example.org:8080')
+
+ connection = described_class.new(uri).connection
+
+ expect(connection.address).to eq('93.184.216.34')
+ expect(connection.hostname_override).to eq('example.org')
+ expect(connection.addr_port).to eq('example.org:8080')
+ expect(connection.port).to eq(8080)
+ end
+ end
+ end
+
+ context 'when local requests are allowed' do
+ it 'sets up the connection' do
+ uri = URI('https://example.org')
+
+ connection = described_class.new(uri, allow_local_requests: true).connection
+
+ expect(connection).to be_a(Net::HTTP)
+ expect(connection.address).to eq('93.184.216.34')
+ expect(connection.hostname_override).to eq('example.org')
+ expect(connection.addr_port).to eq('example.org')
+ expect(connection.port).to eq(443)
+ end
+
+ it 'sets up the connection when it is a local network' do
+ uri = URI('http://172.16.0.0/12')
+
+ connection = described_class.new(uri, allow_local_requests: true).connection
+
+ expect(connection).to be_a(Net::HTTP)
+ expect(connection.address).to eq('172.16.0.0')
+ expect(connection.hostname_override).to be(nil)
+ expect(connection.addr_port).to eq('172.16.0.0')
+ expect(connection.port).to eq(80)
+ end
+
+ it 'sets up the connection when it is localhost' do
+ uri = URI('http://127.0.0.1')
+
+ connection = described_class.new(uri, allow_local_requests: true).connection
+
+ expect(connection).to be_a(Net::HTTP)
+ expect(connection.address).to eq('127.0.0.1')
+ expect(connection.hostname_override).to be(nil)
+ expect(connection.addr_port).to eq('127.0.0.1')
+ expect(connection.port).to eq(80)
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb
index 6c37c157f5d..158f77cab2c 100644
--- a/spec/lib/gitlab/http_spec.rb
+++ b/spec/lib/gitlab/http_spec.rb
@@ -1,6 +1,28 @@
require 'spec_helper'
describe Gitlab::HTTP do
+ include StubRequests
+
+ context 'when allow_local_requests' do
+ it 'sends the request to the correct URI' do
+ stub_full_request('https://example.org:8080', ip_address: '8.8.8.8').to_return(status: 200)
+
+ described_class.get('https://example.org:8080', allow_local_requests: false)
+
+ expect(WebMock).to have_requested(:get, 'https://8.8.8.8:8080').once
+ end
+ end
+
+ context 'when not allow_local_requests' do
+ it 'sends the request to the correct URI' do
+ stub_full_request('https://example.org:8080')
+
+ described_class.get('https://example.org:8080', allow_local_requests: true)
+
+ expect(WebMock).to have_requested(:get, 'https://8.8.8.9:8080').once
+ end
+ end
+
describe 'allow_local_requests_from_hooks_and_services is' do
before do
WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success')
@@ -21,6 +43,8 @@ describe Gitlab::HTTP do
context 'if allow_local_requests set to true' do
it 'override the global value and allow requests to localhost or private network' do
+ stub_full_request('http://localhost:3003')
+
expect { described_class.get('http://localhost:3003', allow_local_requests: true) }.not_to raise_error
end
end
@@ -32,6 +56,8 @@ describe Gitlab::HTTP do
end
it 'allow requests to localhost' do
+ stub_full_request('http://localhost:3003')
+
expect { described_class.get('http://localhost:3003') }.not_to raise_error
end
@@ -49,7 +75,7 @@ describe Gitlab::HTTP do
describe 'handle redirect loops' do
before do
- WebMock.stub_request(:any, "http://example.org").to_raise(HTTParty::RedirectionTooDeep.new("Redirection Too Deep"))
+ stub_full_request("http://example.org", method: :any).to_raise(HTTParty::RedirectionTooDeep.new("Redirection Too Deep"))
end
it 'handles GET requests' do
diff --git a/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb b/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb
index 7c4ac62790e..21a227335cd 100644
--- a/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb
+++ b/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe Gitlab::ImportExport::AfterExportStrategies::WebUploadStrategy do
+ include StubRequests
+
let(:example_url) { 'http://www.example.com' }
let(:strategy) { subject.new(url: example_url, http_method: 'post') }
let!(:project) { create(:project, :with_export) }
@@ -35,7 +37,7 @@ describe Gitlab::ImportExport::AfterExportStrategies::WebUploadStrategy do
context 'when upload fails' do
it 'stores the export error' do
- stub_request(:post, example_url).to_return(status: [404, 'Page not found'])
+ stub_full_request(example_url, method: :post).to_return(status: [404, 'Page not found'])
strategy.execute(user, project)
diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb
index 62970bd8cb6..9c5c7602d05 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -2,6 +2,52 @@
require 'spec_helper'
describe Gitlab::UrlBlocker do
+ describe '#validate!' do
+ context 'when URI is nil' do
+ let(:import_url) { nil }
+
+ it 'returns no URI and hostname' do
+ uri, hostname = described_class.validate!(import_url)
+
+ expect(uri).to be(nil)
+ expect(hostname).to be(nil)
+ end
+ end
+
+ context 'when URI is internal' do
+ let(:import_url) { 'http://localhost' }
+
+ it 'returns URI and no hostname' do
+ uri, hostname = described_class.validate!(import_url)
+
+ expect(uri).to eq(Addressable::URI.parse('http://[::1]'))
+ expect(hostname).to eq('localhost')
+ end
+ end
+
+ context 'when the URL hostname is a domain' do
+ let(:import_url) { 'https://example.org' }
+
+ it 'returns URI and hostname' do
+ uri, hostname = described_class.validate!(import_url)
+
+ expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34'))
+ expect(hostname).to eq('example.org')
+ end
+ end
+
+ context 'when the URL hostname is an IP address' do
+ let(:import_url) { 'https://93.184.216.34' }
+
+ it 'returns URI and no hostname' do
+ uri, hostname = described_class.validate!(import_url)
+
+ expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34'))
+ expect(hostname).to be(nil)
+ end
+ end
+ end
+
describe '#blocked_url?' do
let(:ports) { Project::VALID_IMPORT_PORTS }
@@ -208,7 +254,7 @@ describe Gitlab::UrlBlocker do
end
def stub_domain_resolv(domain, ip)
- address = double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false)
+ address = double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false, ipv4?: false)
allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([address])
allow(address).to receive(:ipv6_v4mapped?).and_return(false)
end
diff --git a/spec/lib/mattermost/session_spec.rb b/spec/lib/mattermost/session_spec.rb
index 77fea5b2d24..346455067a7 100644
--- a/spec/lib/mattermost/session_spec.rb
+++ b/spec/lib/mattermost/session_spec.rb
@@ -2,6 +2,7 @@ require 'spec_helper'
describe Mattermost::Session, type: :request do
include ExclusiveLeaseHelpers
+ include StubRequests
let(:user) { create(:user) }
@@ -24,7 +25,7 @@ describe Mattermost::Session, type: :request do
let(:location) { 'http://location.tld' }
let(:cookie_header) {'MMOAUTH=taskik8az7rq8k6rkpuas7htia; Path=/;'}
let!(:stub) do
- WebMock.stub_request(:get, "#{mattermost_url}/oauth/gitlab/login")
+ stub_full_request("#{mattermost_url}/oauth/gitlab/login")
.to_return(headers: { 'location' => location, 'Set-Cookie' => cookie_header }, status: 302)
end
@@ -63,7 +64,7 @@ describe Mattermost::Session, type: :request do
end
before do
- WebMock.stub_request(:get, "#{mattermost_url}/signup/gitlab/complete")
+ stub_full_request("#{mattermost_url}/signup/gitlab/complete")
.with(query: hash_including({ 'state' => state }))
.to_return do |request|
post "/oauth/token",
@@ -80,7 +81,7 @@ describe Mattermost::Session, type: :request do
end
end
- WebMock.stub_request(:post, "#{mattermost_url}/api/v4/users/logout")
+ stub_full_request("#{mattermost_url}/api/v4/users/logout", method: :post)
.to_return(headers: { Authorization: 'token thisworksnow' }, status: 200)
end
diff --git a/spec/models/project_services/assembla_service_spec.rb b/spec/models/project_services/assembla_service_spec.rb
index 7742e33e901..2c86c0ec7be 100644
--- a/spec/models/project_services/assembla_service_spec.rb
+++ b/spec/models/project_services/assembla_service_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
describe AssemblaService do
+ include StubRequests
+
describe "Associations" do
it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook }
@@ -23,12 +25,12 @@ describe AssemblaService do
)
@sample_data = Gitlab::DataBuilder::Push.build_sample(project, user)
@api_url = 'https://atlas.assembla.com/spaces/project_name/github_tool?secret_key=verySecret'
- WebMock.stub_request(:post, @api_url)
+ stub_full_request(@api_url, method: :post)
end
it "calls Assembla API" do
@assembla_service.execute(@sample_data)
- expect(WebMock).to have_requested(:post, @api_url).with(
+ expect(WebMock).to have_requested(:post, stubbed_hostname(@api_url)).with(
body: /#{@sample_data[:before]}.*#{@sample_data[:after]}.*#{project.path}/
).once
end
diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb
index 08c510f09df..65d227a17f9 100644
--- a/spec/models/project_services/bamboo_service_spec.rb
+++ b/spec/models/project_services/bamboo_service_spec.rb
@@ -4,6 +4,7 @@ require 'spec_helper'
describe BambooService, :use_clean_rails_memory_store_caching do
include ReactiveCachingHelpers
+ include StubRequests
let(:bamboo_url) { 'http://gitlab.com/bamboo' }
@@ -257,7 +258,7 @@ describe BambooService, :use_clean_rails_memory_store_caching do
end
def stub_bamboo_request(url, status, body)
- WebMock.stub_request(:get, url).to_return(
+ stub_full_request(url).to_return(
status: status,
headers: { 'Content-Type' => 'application/json' },
body: body
diff --git a/spec/models/project_services/buildkite_service_spec.rb b/spec/models/project_services/buildkite_service_spec.rb
index 091d4d8f695..ca196069055 100644
--- a/spec/models/project_services/buildkite_service_spec.rb
+++ b/spec/models/project_services/buildkite_service_spec.rb
@@ -4,6 +4,7 @@ require 'spec_helper'
describe BuildkiteService, :use_clean_rails_memory_store_caching do
include ReactiveCachingHelpers
+ include StubRequests
let(:project) { create(:project) }
@@ -110,10 +111,9 @@ describe BuildkiteService, :use_clean_rails_memory_store_caching do
body ||= %q({"status":"success"})
buildkite_full_url = 'https://gitlab.buildkite.com/status/secret-sauce-status-token.json?commit=123'
- WebMock.stub_request(:get, buildkite_full_url).to_return(
- status: status,
- headers: { 'Content-Type' => 'application/json' },
- body: body
- )
+ stub_full_request(buildkite_full_url)
+ .to_return(status: status,
+ headers: { 'Content-Type' => 'application/json' },
+ body: body)
end
end
diff --git a/spec/models/project_services/campfire_service_spec.rb b/spec/models/project_services/campfire_service_spec.rb
index bf4c52fc7ab..0d3dd89e93b 100644
--- a/spec/models/project_services/campfire_service_spec.rb
+++ b/spec/models/project_services/campfire_service_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
describe CampfireService do
+ include StubRequests
+
describe 'Associations' do
it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook }
@@ -49,39 +51,37 @@ describe CampfireService do
it "calls Campfire API to get a list of rooms and speak in a room" do
# make sure a valid list of rooms is returned
body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms.json')
- WebMock.stub_request(:get, @rooms_url).with(basic_auth: @auth).to_return(
+
+ stub_full_request(@rooms_url).with(basic_auth: @auth).to_return(
body: body,
status: 200,
headers: @headers
)
+
# stub the speak request with the room id found in the previous request's response
speak_url = 'https://project-name.campfirenow.com/room/123/speak.json'
- WebMock.stub_request(:post, speak_url).with(basic_auth: @auth)
+ stub_full_request(speak_url, method: :post).with(basic_auth: @auth)
@campfire_service.execute(@sample_data)
- expect(WebMock).to have_requested(:get, @rooms_url).once
- expect(WebMock).to have_requested(:post, speak_url).with(
- body: /#{project.path}.*#{@sample_data[:before]}.*#{@sample_data[:after]}/
- ).once
+ expect(WebMock).to have_requested(:get, stubbed_hostname(@rooms_url)).once
+ expect(WebMock).to have_requested(:post, stubbed_hostname(speak_url))
+ .with(body: /#{project.path}.*#{@sample_data[:before]}.*#{@sample_data[:after]}/).once
end
it "calls Campfire API to get a list of rooms but shouldn't speak in a room" do
# return a list of rooms that do not contain a room named 'test-room'
body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms2.json')
- WebMock.stub_request(:get, @rooms_url).with(basic_auth: @auth).to_return(
+ stub_full_request(@rooms_url).with(basic_auth: @auth).to_return(
body: body,
status: 200,
headers: @headers
)
- # we want to make sure no request is sent to the /speak endpoint, here is a basic
- # regexp that matches this endpoint
- speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/.*/speak.json'
@campfire_service.execute(@sample_data)
- expect(WebMock).to have_requested(:get, @rooms_url).once
- expect(WebMock).not_to have_requested(:post, /#{speak_url}/)
+ expect(WebMock).to have_requested(:get, 'https://8.8.8.9/rooms.json').once
+ expect(WebMock).not_to have_requested(:post, '*/room/.*/speak.json')
end
end
end
diff --git a/spec/models/project_services/pivotaltracker_service_spec.rb b/spec/models/project_services/pivotaltracker_service_spec.rb
index 773b8b7890f..dde46c82df6 100644
--- a/spec/models/project_services/pivotaltracker_service_spec.rb
+++ b/spec/models/project_services/pivotaltracker_service_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
describe PivotaltrackerService do
+ include StubRequests
+
describe 'Associations' do
it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook }
@@ -53,12 +55,12 @@ describe PivotaltrackerService do
end
before do
- WebMock.stub_request(:post, url)
+ stub_full_request(url, method: :post)
end
it 'posts correct message' do
service.execute(push_data)
- expect(WebMock).to have_requested(:post, url).with(
+ expect(WebMock).to have_requested(:post, stubbed_hostname(url)).with(
body: {
'source_commit' => {
'commit_id' => '21c12ea',
@@ -85,14 +87,14 @@ describe PivotaltrackerService do
service.execute(push_data(branch: 'master'))
service.execute(push_data(branch: 'v10'))
- expect(WebMock).to have_requested(:post, url).twice
+ expect(WebMock).to have_requested(:post, stubbed_hostname(url)).twice
end
it 'does not post message if branch is not in the list' do
service.execute(push_data(branch: 'mas'))
service.execute(push_data(branch: 'v11'))
- expect(WebMock).not_to have_requested(:post, url)
+ expect(WebMock).not_to have_requested(:post, stubbed_hostname(url))
end
end
end
diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb
index d2a45f48705..380f02739bc 100644
--- a/spec/models/project_services/pushover_service_spec.rb
+++ b/spec/models/project_services/pushover_service_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
describe PushoverService do
+ include StubRequests
+
describe 'Associations' do
it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook }
@@ -57,13 +59,13 @@ describe PushoverService do
sound: sound
)
- WebMock.stub_request(:post, api_url)
+ stub_full_request(api_url, method: :post, ip_address: '8.8.8.8')
end
it 'calls Pushover API' do
pushover.execute(sample_data)
- expect(WebMock).to have_requested(:post, api_url).once
+ expect(WebMock).to have_requested(:post, 'https://8.8.8.8/1/messages.json').once
end
end
end
diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb
index 96dccae733b..1c434b25205 100644
--- a/spec/models/project_services/teamcity_service_spec.rb
+++ b/spec/models/project_services/teamcity_service_spec.rb
@@ -4,6 +4,7 @@ require 'spec_helper'
describe TeamcityService, :use_clean_rails_memory_store_caching do
include ReactiveCachingHelpers
+ include StubRequests
let(:teamcity_url) { 'http://gitlab.com/teamcity' }
@@ -212,7 +213,7 @@ describe TeamcityService, :use_clean_rails_memory_store_caching do
body ||= %Q({"build":{"status":"#{build_status}","id":"666"}})
- WebMock.stub_request(:get, teamcity_full_url).with(basic_auth: auth).to_return(
+ stub_full_request(teamcity_full_url).with(basic_auth: auth).to_return(
status: status,
headers: { 'Content-Type' => 'application/json' },
body: body
diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb
index b6e8d74c2e9..0e2f3face71 100644
--- a/spec/requests/api/system_hooks_spec.rb
+++ b/spec/requests/api/system_hooks_spec.rb
@@ -1,12 +1,14 @@
require 'spec_helper'
describe API::SystemHooks do
+ include StubRequests
+
let(:user) { create(:user) }
let(:admin) { create(:admin) }
let!(:hook) { create(:system_hook, url: "http://example.com") }
before do
- stub_request(:post, hook.url)
+ stub_full_request(hook.url, method: :post)
end
describe "GET /hooks" do
@@ -68,6 +70,8 @@ describe API::SystemHooks do
end
it 'sets default values for events' do
+ stub_full_request('http://mep.mep', method: :post)
+
post api('/hooks', admin), params: { url: 'http://mep.mep' }
expect(response).to have_gitlab_http_status(201)
@@ -78,6 +82,8 @@ describe API::SystemHooks do
end
it 'sets explicit values for events' do
+ stub_full_request('http://mep.mep', method: :post)
+
post api('/hooks', admin),
params: {
url: 'http://mep.mep',
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 876beb39801..bcfdbe51476 100644
--- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb
+++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe Projects::LfsPointers::LfsDownloadService do
+ include StubRequests
+
let(:project) { create(:project) }
let(:lfs_content) { SecureRandom.random_bytes(10) }
let(:oid) { Digest::SHA256.hexdigest(lfs_content) }
@@ -61,7 +63,7 @@ describe Projects::LfsPointers::LfsDownloadService do
describe '#execute' do
context 'when file download succeeds' do
before do
- WebMock.stub_request(:get, download_link).to_return(body: lfs_content)
+ stub_full_request(download_link).to_return(body: lfs_content)
end
it_behaves_like 'lfs object is created'
@@ -103,7 +105,7 @@ describe Projects::LfsPointers::LfsDownloadService do
let(:size) { 1 }
before do
- WebMock.stub_request(:get, download_link).to_return(body: lfs_content)
+ stub_full_request(download_link).to_return(body: lfs_content)
end
it_behaves_like 'no lfs object is created'
@@ -117,7 +119,7 @@ describe Projects::LfsPointers::LfsDownloadService do
context 'when downloaded lfs file has a different oid' do
before do
- WebMock.stub_request(:get, download_link).to_return(body: lfs_content)
+ stub_full_request(download_link).to_return(body: lfs_content)
allow_any_instance_of(Digest::SHA256).to receive(:hexdigest).and_return('foobar')
end
@@ -135,7 +137,7 @@ describe Projects::LfsPointers::LfsDownloadService do
let(:lfs_object) { LfsDownloadObject.new(oid: oid, size: size, link: download_link_with_credentials) }
before do
- WebMock.stub_request(:get, download_link).with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNzd29yZA==' }).to_return(body: lfs_content)
+ stub_full_request(download_link).with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNzd29yZA==' }).to_return(body: lfs_content)
end
it 'the request adds authorization headers' do
@@ -148,7 +150,7 @@ describe Projects::LfsPointers::LfsDownloadService do
let(:local_request_setting) { true }
before do
- WebMock.stub_request(:get, download_link).to_return(body: lfs_content)
+ stub_full_request(download_link, ip_address: '192.168.2.120').to_return(body: lfs_content)
end
it_behaves_like 'lfs object is created'
@@ -172,7 +174,8 @@ describe Projects::LfsPointers::LfsDownloadService do
with_them do
before do
- WebMock.stub_request(:get, download_link).to_return(status: 301, headers: { 'Location' => redirect_link })
+ stub_full_request(download_link, ip_address: '192.168.2.120')
+ .to_return(status: 301, headers: { 'Location' => redirect_link })
end
it_behaves_like 'no lfs object is created'
@@ -183,8 +186,8 @@ describe Projects::LfsPointers::LfsDownloadService do
let(:redirect_link) { "http://example.com/"}
before do
- WebMock.stub_request(:get, download_link).to_return(status: 301, headers: { 'Location' => redirect_link })
- WebMock.stub_request(:get, redirect_link).to_return(body: lfs_content)
+ stub_full_request(download_link).to_return(status: 301, headers: { 'Location' => redirect_link })
+ stub_full_request(redirect_link).to_return(body: lfs_content)
end
it_behaves_like 'lfs object is created'
diff --git a/spec/services/submit_usage_ping_service_spec.rb b/spec/services/submit_usage_ping_service_spec.rb
index c8a6fc1a99b..5dd97740594 100644
--- a/spec/services/submit_usage_ping_service_spec.rb
+++ b/spec/services/submit_usage_ping_service_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe SubmitUsagePingService do
+ include StubRequests
+
context 'when usage ping is disabled' do
before do
stub_application_setting(usage_ping_enabled: false)
@@ -97,7 +99,7 @@ describe SubmitUsagePingService do
end
def stub_response(body)
- stub_request(:post, 'https://version.gitlab.com/usage_data')
+ stub_full_request('https://version.gitlab.com/usage_data', method: :post)
.to_return(
headers: { 'Content-Type' => 'application/json' },
body: body.to_json
diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb
index 747e04fb18c..932df1fee4f 100644
--- a/spec/services/web_hook_service_spec.rb
+++ b/spec/services/web_hook_service_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe WebHookService do
+ include StubRequests
+
let(:project) { create(:project) }
let(:project_hook) { create(:project_hook) }
let(:headers) do
@@ -65,11 +67,11 @@ describe WebHookService do
let(:project_hook) { create(:project_hook, url: 'https://demo:demo@example.org/') }
it 'uses the credentials' do
- WebMock.stub_request(:post, url)
+ stub_full_request(url, method: :post)
service_instance.execute
- expect(WebMock).to have_requested(:post, url).with(
+ expect(WebMock).to have_requested(:post, stubbed_hostname(url)).with(
headers: headers.merge('Authorization' => 'Basic ZGVtbzpkZW1v')
).once
end
@@ -80,11 +82,11 @@ describe WebHookService do
let(:project_hook) { create(:project_hook, url: 'https://demo@example.org/') }
it 'uses the credentials anyways' do
- WebMock.stub_request(:post, url)
+ stub_full_request(url, method: :post)
service_instance.execute
- expect(WebMock).to have_requested(:post, url).with(
+ expect(WebMock).to have_requested(:post, stubbed_hostname(url)).with(
headers: headers.merge('Authorization' => 'Basic ZGVtbzo=')
).once
end
diff --git a/spec/support/helpers/stub_requests.rb b/spec/support/helpers/stub_requests.rb
new file mode 100644
index 00000000000..5cad35282c0
--- /dev/null
+++ b/spec/support/helpers/stub_requests.rb
@@ -0,0 +1,40 @@
+module StubRequests
+ IP_ADDRESS_STUB = '8.8.8.9'.freeze
+
+ # Fully stubs a request using WebMock class. This class also
+ # stubs the IP address the URL is translated to (DNS lookup).
+ #
+ # It expects the final request to go to the `ip_address` instead the given url.
+ # That's primarily a DNS rebind attack prevention of Gitlab::HTTP
+ # (see: Gitlab::UrlBlocker).
+ #
+ def stub_full_request(url, ip_address: IP_ADDRESS_STUB, port: 80, method: :get)
+ stub_dns(url, ip_address: ip_address, port: port)
+
+ url = stubbed_hostname(url, hostname: ip_address)
+ WebMock.stub_request(method, url)
+ end
+
+ def stub_dns(url, ip_address:, port: 80)
+ url = parse_url(url)
+ socket = Socket.sockaddr_in(port, ip_address)
+ addr = Addrinfo.new(socket)
+
+ # See Gitlab::UrlBlocker
+ allow(Addrinfo).to receive(:getaddrinfo)
+ .with(url.hostname, url.port, nil, :STREAM)
+ .and_return([addr])
+ end
+
+ def stubbed_hostname(url, hostname: IP_ADDRESS_STUB)
+ url = parse_url(url)
+ url.hostname = hostname
+ url.to_s
+ end
+
+ private
+
+ def parse_url(url)
+ url.is_a?(URI) ? url : URI(url)
+ end
+end