Skip to content
Snippets Groups Projects
Commit fe347f91 authored by Illya Klymov's avatar Illya Klymov :rocket:
Browse files

Add import source checks to create project service

* ensure additional safety when creating projects
parent 41f2b59e
No related branches found
No related tags found
1 merge request!85262Move checks if import source is enabled to service layer
......@@ -21,6 +21,8 @@ def execute(credentials)
if project.persisted?
success(project)
elsif project.errors[:import_source_disabled].present?
error(project.errors[:import_source_disabled], :forbidden)
else
log_and_return_error(project_save_error(project), :unprocessable_entity)
end
......
......@@ -25,6 +25,8 @@ def execute(access_params, provider)
if project.persisted?
success(project)
elsif project.errors[:import_source_disabled].present?
error(project.errors[:import_source_disabled], :forbidden)
else
error(project_save_error(project), :unprocessable_entity)
end
......
......@@ -4,6 +4,9 @@ module Projects
class CreateService < BaseService
include ValidatesClassificationLabel
ImportSourceDisabledError = Class.new(StandardError)
INTERNAL_IMPORT_SOURCES = %w[bare_repository gitlab_custom_project_template gitlab_project_migration].freeze
def initialize(user, params)
@current_user = user
@params = params.dup
......@@ -25,6 +28,8 @@ def execute
@project = Project.new(params)
validate_import!
@project.visibility_level = @project.group.visibility_level unless @project.visibility_level_allowed_by_group?
# If a project is newly created it should have shared runners settings
......@@ -77,6 +82,9 @@ def execute
rescue ActiveRecord::RecordInvalid => e
message = "Unable to save #{e.inspect}: #{e.record.errors.full_messages.join(", ")}"
fail(error: message)
rescue ImportSourceDisabledError => e
@project.errors.add(:import_source_disabled, e.message) if @project
fail(error: e.message)
rescue StandardError => e
@project.errors.add(:base, e.message) if @project
fail(error: e.message)
......@@ -242,6 +250,14 @@ def extra_attributes_for_measurement
private
def validate_import!
return if INTERNAL_IMPORT_SOURCES.include?(@params[:import_type])
if @params[:import_type] && !::Gitlab::CurrentSettings.import_sources&.include?(@params[:import_type])
raise ImportSourceDisabledError, "#{@params[:import_type]} import source is disabled"
end
end
def parent_namespace
@parent_namespace ||= Namespace.find_by_id(@params[:namespace_id]) || current_user.namespace
end
......
......@@ -192,8 +192,6 @@ def filter_attributes_using_license!(attrs)
def validate_git_import_url!(import_url)
return if import_url.blank?
yield if block_given?
result = Import::ValidateRemoteGitEndpointService.new(url: import_url).execute # network call
if result.error?
......
......@@ -4,10 +4,6 @@ module API
class ImportBitbucketServer < ::API::Base
feature_category :importers
before do
forbidden! unless Gitlab::CurrentSettings.import_sources&.include?('bitbucket_server')
end
helpers do
def client
@client ||= BitbucketServer::Client.new(credentials)
......
......@@ -6,10 +6,6 @@ class ImportGithub < ::API::Base
rescue_from Octokit::Unauthorized, with: :provider_unauthorized
before do
forbidden! unless Gitlab::CurrentSettings.import_sources&.include?('github')
end
helpers do
def client
@client ||= if Feature.enabled?(:remove_legacy_github_client)
......
......@@ -90,10 +90,6 @@ def log_if_upload_exceed_max_size(user_project, file)
Gitlab::AppLogger.info({ message: "File exceeds maximum size", file_bytes: file.size, project_id: user_project.id, project_path: user_project.full_path, upload_allowed: allowed })
end
end
def check_import_by_url_is_enabled
Gitlab::CurrentSettings.import_sources&.include?('git') || forbidden!
end
end
helpers do
......@@ -202,6 +198,11 @@ def translate_params_for_compatibility(params)
params[:builds_enabled] = params.delete(:jobs_enabled) if params.key?(:jobs_enabled)
params
end
def add_import_params(params)
params[:import_type] ||= 'git' if params[:import_url]
params
end
end
resource :users, requirements: API::USER_REQUIREMENTS do
......@@ -271,9 +272,10 @@ def translate_params_for_compatibility(params)
Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/issues/21139')
attrs = declared_params(include_missing: false)
attrs = translate_params_for_compatibility(attrs)
attrs = add_import_params(attrs)
filter_attributes_using_license!(attrs)
validate_git_import_url!(params[:import_url]) { check_import_by_url_is_enabled }
validate_git_import_url!(params[:import_url])
project = ::Projects::CreateService.new(current_user, attrs).execute
......@@ -286,6 +288,8 @@ def translate_params_for_compatibility(params)
error!(project.errors[:limit_reached], 403)
end
forbidden! if project.errors[:import_source_disabled].present?
render_validation_error!(project)
end
end
......@@ -311,6 +315,7 @@ def translate_params_for_compatibility(params)
attrs = declared_params(include_missing: false)
attrs = translate_params_for_compatibility(attrs)
attrs = add_import_params(attrs)
filter_attributes_using_license!(attrs)
validate_git_import_url!(params[:import_url])
......@@ -321,6 +326,8 @@ def translate_params_for_compatibility(params)
user_can_admin_project: can?(current_user, :admin_project, project),
current_user: current_user
else
forbidden! if project.errors[:import_source_disabled].present?
render_validation_error!(project)
end
end
......@@ -441,6 +448,7 @@ def translate_params_for_compatibility(params)
authorize! :change_visibility_level, user_project if user_project.visibility_attribute_present?(attrs)
attrs = translate_params_for_compatibility(attrs)
attrs = add_import_params(attrs)
filter_attributes_using_license!(attrs)
verify_update_project_attrs!(user_project, attrs)
......
......@@ -9,7 +9,15 @@
let(:secret) { "sekrettt" }
let(:project_key) { 'TES' }
let(:repo_slug) { 'vim' }
let(:repo) { { name: 'vim' } }
let(:repo) do
double('repo',
name: repo_slug,
browse_url: "#{base_uri}/projects/#{project_key}/repos/#{repo_slug}/browse",
clone_url: "#{base_uri}/scm/#{project_key}/#{repo_slug}.git",
description: 'provider',
visibility_level: Gitlab::VisibilityLevel::PUBLIC
)
end
describe "POST /import/bitbucket_server" do
context 'with no optional parameters' do
......@@ -20,7 +28,7 @@
before do
Grape::Endpoint.before_each do |endpoint|
allow(endpoint).to receive(:client).and_return(client.as_null_object)
allow(client).to receive(:repo).with(project_key, repo_slug).and_return(double(name: repo_slug))
allow(client).to receive(:repo).with(project_key, repo_slug).and_return(repo)
end
end
......
......@@ -16,7 +16,11 @@
double('provider',
name: 'vim',
full_name: "#{provider_username}/vim",
owner: double('provider', login: provider_username)
owner: double('provider', login: provider_username),
description: 'provider',
private: false,
clone_url: 'https://fake.url/vim.git',
has_wiki?: true
)
end
......
......@@ -1180,9 +1180,15 @@
end
it 'disallows creating a project with an import_url when git import source is disabled' do
url = 'http://example.com'
stub_application_setting(import_sources: nil)
project_params = { import_url: 'http://example.com', path: 'path-project-Foo', name: 'Foo Project' }
endpoint_url = "#{url}/info/refs?service=git-upload-pack"
stub_full_request(endpoint_url, method: :get).to_return({ status: 200,
body: '001e# service=git-upload-pack',
headers: { 'Content-Type': 'application/x-git-upload-pack-advertisement' } })
project_params = { import_url: url, path: 'path-project-Foo', name: 'Foo Project' }
expect { post api('/projects', user), params: project_params }
.not_to change { Project.count }
......
......@@ -48,6 +48,23 @@
end
end
context 'when import source is disabled' do
before do
stub_application_setting(import_sources: nil)
allow(subject).to receive(:authorized?).and_return(true)
allow(client).to receive(:repo).with(project_key, repo_slug).and_return(double(repo))
end
it 'returns forbidden' do
result = subject.execute(credentials)
expect(result).to include(
status: :error,
http_status: :forbidden
)
end
end
context 'when user is unauthorized' do
before do
allow(subject).to receive(:authorized?).and_return(false)
......
......@@ -111,6 +111,33 @@
end
end
context 'when import source is disabled' do
let(:repository_double) do
double({
name: 'vim',
description: 'test',
full_name: 'test/vim',
clone_url: 'http://repo.com/repo/repo.git',
private: false,
has_wiki?: false
})
end
before do
stub_application_setting(import_sources: nil)
allow(client).to receive(:repository).and_return(repository_double)
end
it 'returns forbidden' do
result = subject.execute(access_params, :github)
expect(result).to include(
status: :error,
http_status: :forbidden
)
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}" }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment