Commit b3647956 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch 'kassio/fix-project-import-from-remote-with-s3-urls' into 'master'

Fix project import from remote to import from S3

See merge request gitlab-org/gitlab!75170
parents 38b3f907 242c4c8f
......@@ -4,7 +4,10 @@ module Import
module GitlabProjects
class CreateProjectFromRemoteFileService < CreateProjectFromUploadedFileService
FILE_SIZE_LIMIT = 10.gigabytes
ALLOWED_CONTENT_TYPES = ['application/gzip'].freeze
ALLOWED_CONTENT_TYPES = [
'application/gzip', # most common content-type when fetching a tar.gz
'application/x-tar' # aws-s3 uses x-tar for tar.gz files
].freeze
validate :valid_remote_import_url?
validate :validate_file_size
......@@ -44,17 +47,27 @@ module Import
end
def validate_content_type
# AWS-S3 presigned URLs don't respond to HTTP HEAD requests,
# so file type cannot be validated
# https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75170#note_748059103
return if amazon_s3?
if headers['content-type'].blank?
errors.add(:base, "Missing 'ContentType' header")
elsif !ALLOWED_CONTENT_TYPES.include?(headers['content-type'])
errors.add(:base, "Remote file content type '%{content_type}' not allowed. (Allowed content types: %{allowed})" % {
content_type: headers['content-type'],
allowed: ALLOWED_CONTENT_TYPES.join(',')
allowed: ALLOWED_CONTENT_TYPES.join(', ')
})
end
end
def validate_file_size
# AWS-S3 presigned URLs don't respond to HTTP HEAD requests,
# so file size cannot be validated
# https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75170#note_748059103
return if amazon_s3?
if headers['content-length'].to_i == 0
errors.add(:base, "Missing 'ContentLength' header")
elsif headers['content-length'].to_i > FILE_SIZE_LIMIT
......@@ -64,6 +77,10 @@ module Import
end
end
def amazon_s3?
headers['Server'] == 'AmazonS3' && headers['x-amz-request-id'].present?
end
def headers
return {} if params[:remote_import_url].blank? || !valid_remote_import_url?
......
......@@ -18,24 +18,29 @@ RSpec.describe ::Import::GitlabProjects::CreateProjectFromRemoteFileService do
subject { described_class.new(user, params) }
it 'creates a project and returns a successful response' do
stub_headers_for(remote_url, {
'content-type' => 'application/gzip',
'content-length' => '10'
})
response = nil
expect { response = subject.execute }
.to change(Project, :count).by(1)
shared_examples 'successfully import' do |content_type|
it 'creates a project and returns a successful response' do
stub_headers_for(remote_url, {
'content-type' => content_type,
'content-length' => '10'
})
expect(response).to be_success
expect(response.http_status).to eq(:ok)
expect(response.payload).to be_instance_of(Project)
expect(response.payload.name).to eq('name')
expect(response.payload.path).to eq('path')
expect(response.payload.namespace).to eq(user.namespace)
response = nil
expect { response = subject.execute }
.to change(Project, :count).by(1)
expect(response).to be_success
expect(response.http_status).to eq(:ok)
expect(response.payload).to be_instance_of(Project)
expect(response.payload.name).to eq('name')
expect(response.payload.path).to eq('path')
expect(response.payload.namespace).to eq(user.namespace)
end
end
it_behaves_like 'successfully import', 'application/gzip'
it_behaves_like 'successfully import', 'application/x-tar'
context 'when the file url is invalid' do
it 'returns an erred response with the reason of the failure' do
stub_application_setting(allow_local_requests_from_web_hooks_and_services: false)
......@@ -79,7 +84,7 @@ RSpec.describe ::Import::GitlabProjects::CreateProjectFromRemoteFileService do
expect(response).not_to be_success
expect(response.http_status).to eq(:bad_request)
expect(response.message)
.to eq("Remote file content type 'application/js' not allowed. (Allowed content types: application/gzip)")
.to eq("Remote file content type 'application/js' not allowed. (Allowed content types: application/gzip, application/x-tar)")
end
end
......@@ -130,6 +135,20 @@ RSpec.describe ::Import::GitlabProjects::CreateProjectFromRemoteFileService do
end
end
it 'does not validate content-type or content-length when the file is stored in AWS-S3' do
stub_headers_for(remote_url, {
'Server' => 'AmazonS3',
'x-amz-request-id' => 'Something'
})
response = nil
expect { response = subject.execute }
.to change(Project, :count)
expect(response).to be_success
expect(response.http_status).to eq(:ok)
end
context 'when required parameters are not provided' do
let(:params) { {} }
......
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