Commit e10c6951 authored by Colin Barr's avatar Colin Barr Committed by Doug Stull

Fixed NoMethodError on import from GitHub Enterprise on RFC1918 IP

With allow_local_requests_from_web_hooks_and_services set to false in
GitLab settings, an attempt to import from GitHub Enterprise on a
local/RFC1918 IP address would have resulted in a NoMethodError. This
change fixes that bug to correctly handle, log, and return the error.

Changelog: fixed
parent f28991ef
...@@ -10,7 +10,7 @@ module Import ...@@ -10,7 +10,7 @@ module Import
def execute(access_params, provider) def execute(access_params, provider)
if blocked_url? if blocked_url?
return log_and_return_error("Invalid URL: #{url}", :bad_request) return log_and_return_error("Invalid URL: #{url}", _("Invalid URL: %{url}") % { url: url }, :bad_request)
end end
unless authorized? unless authorized?
...@@ -119,6 +119,15 @@ module Import ...@@ -119,6 +119,15 @@ module Import
error(_('Import failed due to a GitHub error: %{original}') % { original: exception.response_body }, :unprocessable_entity) error(_('Import failed due to a GitHub error: %{original}') % { original: exception.response_body }, :unprocessable_entity)
end end
def log_and_return_error(message, translated_message, http_status)
Gitlab::GithubImport::Logger.error(
message: 'Error while attempting to import from GitHub',
error: message
)
error(translated_message, http_status)
end
end end
end end
......
...@@ -19065,6 +19065,9 @@ msgstr "" ...@@ -19065,6 +19065,9 @@ msgstr ""
msgid "Invalid URL" msgid "Invalid URL"
msgstr "" msgstr ""
msgid "Invalid URL: %{url}"
msgstr ""
msgid "Invalid container_name" msgid "Invalid container_name"
msgstr "" msgstr ""
......
...@@ -8,7 +8,7 @@ RSpec.describe Import::GithubService do ...@@ -8,7 +8,7 @@ RSpec.describe Import::GithubService do
let_it_be(:access_params) { { github_access_token: 'github-complex-token' } } let_it_be(:access_params) { { github_access_token: 'github-complex-token' } }
let_it_be(:params) { { repo_id: 123, new_name: 'new_repo', target_namespace: 'root' } } let_it_be(:params) { { repo_id: 123, new_name: 'new_repo', target_namespace: 'root' } }
let(:subject) { described_class.new(client, user, params) } subject(:github_importer) { described_class.new(client, user, params) }
before do before do
allow(subject).to receive(:authorized?).and_return(true) allow(subject).to receive(:authorized?).and_return(true)
...@@ -110,6 +110,29 @@ RSpec.describe Import::GithubService do ...@@ -110,6 +110,29 @@ RSpec.describe Import::GithubService do
end end
end end
end end
context 'when a blocked/local URL is used as github_hostname' do
let(:message) { 'Error while attempting to import from GitHub' }
let(:error) { "Invalid URL: #{url}" }
before do
stub_application_setting(allow_local_requests_from_web_hooks_and_services: false)
end
where(url: %w[https://localhost https://10.0.0.1])
with_them do
it 'returns and logs an error' do
allow(github_importer).to receive(:url).and_return(url)
expect(Gitlab::Import::Logger).to receive(:error).with({
message: message,
error: error
}).and_call_original
expect(github_importer.execute(access_params, :github)).to include(blocked_url_error(url))
end
end
end
end end
context 'when remove_legacy_github_client feature flag is enabled' do context 'when remove_legacy_github_client feature flag is enabled' do
...@@ -135,4 +158,12 @@ RSpec.describe Import::GithubService do ...@@ -135,4 +158,12 @@ RSpec.describe Import::GithubService do
message: '"repository" size (101 Bytes) is larger than the limit of 100 Bytes.' message: '"repository" size (101 Bytes) is larger than the limit of 100 Bytes.'
} }
end end
def blocked_url_error(url)
{
status: :error,
http_status: :bad_request,
message: "Invalid URL: #{url}"
}
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment