diff options
author | Stan Hu <stanhu@gmail.com> | 2018-10-16 00:09:27 +0000 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2018-10-16 00:09:27 +0000 |
commit | c09de611ea9d8cbff7a1696ee63262ef65972daa (patch) | |
tree | 13f9af851fdf29c53256abdeaf954d6f5d512c56 | |
parent | a88004c876b94d44ce61c19d8c4c42e8de636f58 (diff) | |
parent | 2e75e93c31848df37cb85043cc440ed8e1cce28b (diff) | |
download | gitlab-ce-c09de611ea9d8cbff7a1696ee63262ef65972daa.tar.gz |
Merge branch 'da-fix-does-not-import-projects-over-ssh' into 'master'
Does not allow a SSH URI when importing a project
See merge request gitlab-org/gitlab-ce!22309
-rw-r--r-- | app/models/project.rb | 13 | ||||
-rw-r--r-- | changelogs/unreleased/da-fix-does-not-import-projects-over-ssh.yml | 5 | ||||
-rw-r--r-- | spec/lib/gitlab/bitbucket_import/project_creator_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 61 | ||||
-rw-r--r-- | spec/services/projects/import_service_spec.rb | 2 |
5 files changed, 57 insertions, 28 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index c7ca322853f..b80e41e4a96 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -49,8 +49,11 @@ class Project < ActiveRecord::Base attachments: 2 }.freeze - # Valids ports to import from - VALID_IMPORT_PORTS = [22, 80, 443].freeze + VALID_IMPORT_PORTS = [80, 443].freeze + VALID_IMPORT_PROTOCOLS = %w(http https git).freeze + + VALID_MIRROR_PORTS = [22, 80, 443].freeze + VALID_MIRROR_PROTOCOLS = %w(http https ssh git).freeze cache_markdown_field :description, pipeline: :description @@ -305,10 +308,10 @@ class Project < ActiveRecord::Base validates :namespace, presence: true validates :name, uniqueness: { scope: :namespace_id } - validates :import_url, url: { protocols: %w(http https ssh git), + validates :import_url, url: { protocols: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS }, + ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS }, allow_localhost: false, - enforce_user: true, - ports: VALID_IMPORT_PORTS }, if: [:external_import?, :import_url_changed?] + enforce_user: true }, if: [:external_import?, :import_url_changed?] validates :star_count, numericality: { greater_than_or_equal_to: 0 } validate :check_limit, on: :create validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? } diff --git a/changelogs/unreleased/da-fix-does-not-import-projects-over-ssh.yml b/changelogs/unreleased/da-fix-does-not-import-projects-over-ssh.yml new file mode 100644 index 00000000000..5867b1f0981 --- /dev/null +++ b/changelogs/unreleased/da-fix-does-not-import-projects-over-ssh.yml @@ -0,0 +1,5 @@ +--- +title: Does not allow a SSH URI when importing new projects +merge_request: 22309 +author: +type: fixed diff --git a/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb b/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb index ed6fa3d229f..e2bee22cf1f 100644 --- a/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb @@ -11,7 +11,7 @@ describe Gitlab::BitbucketImport::ProjectCreator do owner: "asd", full_name: 'Vim repo', visibility_level: Gitlab::VisibilityLevel::PRIVATE, - clone_url: 'ssh://git@bitbucket.org/asd/vim.git', + clone_url: 'http://bitbucket.org/asd/vim.git', has_wiki?: false) end @@ -32,7 +32,7 @@ describe Gitlab::BitbucketImport::ProjectCreator do project_creator = described_class.new(repo, 'vim', namespace, user, access_params) project = project_creator.execute - expect(project.import_url).to eq("ssh://git@bitbucket.org/asd/vim.git") + expect(project.import_url).to eq("http://bitbucket.org/asd/vim.git") expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 3fecddefff2..a807c336165 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -229,54 +229,75 @@ describe Project do end it 'does not allow an invalid URI as import_url' do - project2 = build(:project, import_url: 'invalid://') + project = build(:project, import_url: 'invalid://') - expect(project2).not_to be_valid + expect(project).not_to be_valid + end + + it 'does allow a SSH URI as import_url for persisted projects' do + project = create(:project) + project.import_url = 'ssh://test@gitlab.com/project.git' + + expect(project).to be_valid + end + + it 'does not allow a SSH URI as import_url for new projects' do + project = build(:project, import_url: 'ssh://test@gitlab.com/project.git') + + expect(project).not_to be_valid end it 'does allow a valid URI as import_url' do - project2 = build(:project, import_url: 'ssh://test@gitlab.com/project.git') + project = build(:project, import_url: 'http://gitlab.com/project.git') - expect(project2).to be_valid + expect(project).to be_valid end it 'allows an empty URI' do - project2 = build(:project, import_url: '') + project = build(:project, import_url: '') - expect(project2).to be_valid + expect(project).to be_valid end it 'does not produce import data on an empty URI' do - project2 = build(:project, import_url: '') + project = build(:project, import_url: '') - expect(project2.import_data).to be_nil + expect(project.import_data).to be_nil end it 'does not produce import data on an invalid URI' do - project2 = build(:project, import_url: 'test://') + project = build(:project, import_url: 'test://') - expect(project2.import_data).to be_nil + expect(project.import_data).to be_nil end it "does not allow import_url pointing to localhost" do - project2 = build(:project, import_url: 'http://localhost:9000/t.git') + project = build(:project, import_url: 'http://localhost:9000/t.git') + + expect(project).to be_invalid + expect(project.errors[:import_url].first).to include('Requests to localhost are not allowed') + end + + it "does not allow import_url with invalid ports for new projects" do + project = build(:project, import_url: 'http://github.com:25/t.git') - expect(project2).to be_invalid - expect(project2.errors[:import_url].first).to include('Requests to localhost are not allowed') + expect(project).to be_invalid + expect(project.errors[:import_url].first).to include('Only allowed ports are 80, 443') end - it "does not allow import_url with invalid ports" do - project2 = build(:project, import_url: 'http://github.com:25/t.git') + it "does not allow import_url with invalid ports for persisted projects" do + project = create(:project) + project.import_url = 'http://github.com:25/t.git' - expect(project2).to be_invalid - expect(project2.errors[:import_url].first).to include('Only allowed ports are 22, 80, 443') + expect(project).to be_invalid + expect(project.errors[:import_url].first).to include('Only allowed ports are 22, 80, 443') end it "does not allow import_url with invalid user" do - project2 = build(:project, import_url: 'http://$user:password@github.com/t.git') + project = build(:project, import_url: 'http://$user:password@github.com/t.git') - expect(project2).to be_invalid - expect(project2.errors[:import_url].first).to include('Username needs to start with an alphanumeric character') + expect(project).to be_invalid + expect(project.errors[:import_url].first).to include('Username needs to start with an alphanumeric character') end describe 'project pending deletion' do diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index e2a600d12d1..e6ffa2b957b 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -235,7 +235,7 @@ describe Projects::ImportService do result = described_class.new(project, user).execute expect(result[:status]).to eq :error - expect(result[:message]).to include('Only allowed ports are 22, 80, 443') + expect(result[:message]).to include('Only allowed ports are 80, 443') end end |