summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/projects/git_http_controller.rb2
-rw-r--r--changelogs/unreleased/26388-push-to-create-a-new-project.yml5
-rw-r--r--lib/api/helpers/internal_helpers.rb17
-rw-r--r--lib/api/internal.rb9
-rw-r--r--lib/gitlab/checks/new_project.rb2
-rw-r--r--lib/gitlab/git_access.rb11
-rw-r--r--spec/lib/gitlab/checks/new_project_spec.rb46
-rw-r--r--spec/lib/gitlab/checks/project_moved_spec.rb18
-rw-r--r--spec/lib/gitlab/git_access_spec.rb79
-rw-r--r--spec/models/project_spec.rb6
-rw-r--r--spec/requests/api/internal_spec.rb36
-rw-r--r--spec/requests/git_http_spec.rb38
12 files changed, 224 insertions, 45 deletions
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index 6010423c243..5660a9027d5 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -31,6 +31,8 @@ class Projects::GitHttpController < Projects::GitHttpClientController
# POST /foo/bar.git/git-receive-pack" (git push)
def git_receive_pack
+ raise Gitlab::GitAccess::NotFoundError, 'Could not create project' unless project
+
render_ok
end
diff --git a/changelogs/unreleased/26388-push-to-create-a-new-project.yml b/changelogs/unreleased/26388-push-to-create-a-new-project.yml
new file mode 100644
index 00000000000..f641fcced37
--- /dev/null
+++ b/changelogs/unreleased/26388-push-to-create-a-new-project.yml
@@ -0,0 +1,5 @@
+---
+title: User can now git push to create a new project
+merge_request: 16547
+author:
+type: added
diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb
index c0fcae43638..fd568c5ef30 100644
--- a/lib/api/helpers/internal_helpers.rb
+++ b/lib/api/helpers/internal_helpers.rb
@@ -64,6 +64,15 @@ module API
false
end
+ def project_params
+ {
+ description: "",
+ path: Project.parse_project_id(project_match[:project_id]),
+ namespace_id: project_namespace&.id,
+ visibility_level: Gitlab::VisibilityLevel::PRIVATE.to_s
+ }
+ end
+
private
def project_path_regex
@@ -71,11 +80,13 @@ module API
end
def project_match
- @match ||= params[:project].match(project_path_regex).captures
+ @project_match ||= params[:project].match(project_path_regex)
end
- def namespace
- @namespace ||= Namespace.find_by_path_or_name(project_match[:namespace_id])
+ def project_namespace
+ return unless project_match
+
+ @project_namespace ||= Namespace.find_by_path_or_name(project_match[:namespace_id])
end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index f641ef457a3..b7475af2044 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -43,7 +43,7 @@ module API
access_checker_klass = wiki? ? Gitlab::GitAccessWiki : Gitlab::GitAccess
access_checker = access_checker_klass
- .new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities, redirected_path: redirected_path, target_namespace: namespace)
+ .new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities, redirected_path: redirected_path, target_namespace: project_namespace)
begin
access_checker.check(params[:action], params[:changes])
@@ -52,13 +52,6 @@ module API
end
if user && project.blank? && receive_pack?
- project_params = {
- description: "",
- path: Project.parse_project_id(project_match[:project_name]),
- namespace_id: namespace&.id,
- visibility_level: Gitlab::VisibilityLevel::PRIVATE.to_s
- }
-
@project = ::Projects::CreateService.new(user, project_params).execute
if @project.saved?
diff --git a/lib/gitlab/checks/new_project.rb b/lib/gitlab/checks/new_project.rb
index 40d0acefaba..488c5c03c32 100644
--- a/lib/gitlab/checks/new_project.rb
+++ b/lib/gitlab/checks/new_project.rb
@@ -20,6 +20,8 @@ module Gitlab
end
def add_new_project_message
+ return unless user.present? && project.present?
+
Gitlab::Redis::SharedState.with do |redis|
key = self.class.new_project_message_key(user.id, project.id)
redis.setex(key, 5.minutes, new_project_message)
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 598506aa418..38649a4fdef 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -19,8 +19,7 @@ module Gitlab
upload_pack_disabled_over_http: 'Pulling over HTTP is not allowed.',
receive_pack_disabled_over_http: 'Pushing over HTTP is not allowed.',
read_only: 'The repository is temporarily read-only. Please try again later.',
- cannot_push_to_read_only: "You can't push code to a read-only GitLab instance.",
- create: "Creating a repository to that namespace is not allowed."
+ cannot_push_to_read_only: "You can't push code to a read-only GitLab instance."
}.freeze
DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }.freeze
@@ -53,7 +52,7 @@ module Gitlab
check_download_access!
when *PUSH_COMMANDS
check_push_access!(cmd, changes)
- check_repository_creation!(cmd)
+ check_namespace_accessibility!(cmd)
end
true
@@ -149,16 +148,12 @@ module Gitlab
end
end
- def check_repository_creation!(cmd)
+ def check_namespace_accessibility!(cmd)
return unless project.blank?
unless target_namespace
raise NotFoundError, ERROR_MESSAGES[:namespace_not_found]
end
-
- unless can_create_project_in_namespace?(cmd)
- raise UnauthorizedError, ERROR_MESSAGES[:create]
- end
end
def check_download_access!
diff --git a/spec/lib/gitlab/checks/new_project_spec.rb b/spec/lib/gitlab/checks/new_project_spec.rb
new file mode 100644
index 00000000000..c696e02e41a
--- /dev/null
+++ b/spec/lib/gitlab/checks/new_project_spec.rb
@@ -0,0 +1,46 @@
+require 'rails_helper'
+
+describe Gitlab::Checks::NewProject, :clean_gitlab_redis_shared_state do
+ let(:user) { create(:user) }
+ let(:project) { create(:project) }
+
+ describe '.fetch_new_project_message' do
+ context 'with a new project message queue' do
+ let(:new_project) { described_class.new(user, project, 'http') }
+
+ before do
+ new_project.add_new_project_message
+ end
+
+ it 'returns new project message' do
+ expect(described_class.fetch_new_project_message(user.id, project.id)).to eq(new_project.new_project_message)
+ end
+
+ it 'deletes the new project message from redis' do
+ expect(Gitlab::Redis::SharedState.with { |redis| redis.get("new_project:#{user.id}:#{project.id}") }).not_to be_nil
+ described_class.fetch_new_project_message(user.id, project.id)
+ expect(Gitlab::Redis::SharedState.with { |redis| redis.get("new_project:#{user.id}:#{project.id}") }).to be_nil
+ end
+ end
+
+ context 'with no new project message queue' do
+ it 'returns nil' do
+ expect(described_class.fetch_new_project_message(1, 2)).to be_nil
+ end
+ end
+ end
+
+ describe '#add_new_project_message' do
+ it 'queues a new project message' do
+ new_project = described_class.new(user, project, 'http')
+
+ expect(new_project.add_new_project_message).to eq('OK')
+ end
+
+ it 'handles anonymous push' do
+ new_project = described_class.new(user, nil, 'http')
+
+ expect(new_project.add_new_project_message).to be_nil
+ end
+ end
+end
diff --git a/spec/lib/gitlab/checks/project_moved_spec.rb b/spec/lib/gitlab/checks/project_moved_spec.rb
index f90c2d6aded..b03a598edd8 100644
--- a/spec/lib/gitlab/checks/project_moved_spec.rb
+++ b/spec/lib/gitlab/checks/project_moved_spec.rb
@@ -6,14 +6,14 @@ describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do
describe '.fetch_redirct_message' do
context 'with a redirect message queue' do
- it 'should return the redirect message' do
+ it 'returns the redirect message' do
project_moved = described_class.new(project, user, 'foo/bar', 'http')
project_moved.add_redirect_message
expect(described_class.fetch_redirect_message(user.id, project.id)).to eq(project_moved.redirect_message)
end
- it 'should delete the redirect message from redis' do
+ it 'deletes the redirect message from redis' do
project_moved = described_class.new(project, user, 'foo/bar', 'http')
project_moved.add_redirect_message
@@ -24,19 +24,19 @@ describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do
end
context 'with no redirect message queue' do
- it 'should return nil' do
+ it 'returns nil' do
expect(described_class.fetch_redirect_message(1, 2)).to be_nil
end
end
end
describe '#add_redirect_message' do
- it 'should queue a redirect message' do
+ it 'queues a redirect message' do
project_moved = described_class.new(project, user, 'foo/bar', 'http')
expect(project_moved.add_redirect_message).to eq("OK")
end
- it 'should handle anonymous clones' do
+ it 'handles anonymous clones' do
project_moved = described_class.new(project, nil, 'foo/bar', 'http')
expect(project_moved.add_redirect_message).to eq(nil)
@@ -45,7 +45,7 @@ describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do
describe '#redirect_message' do
context 'when the push is rejected' do
- it 'should return a redirect message telling the user to try again' do
+ it 'returns a redirect message telling the user to try again' do
project_moved = described_class.new(project, user, 'foo/bar', 'http')
message = "Project 'foo/bar' was moved to '#{project.full_path}'." +
"\n\nPlease update your Git remote:" +
@@ -56,7 +56,7 @@ describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do
end
context 'when the push is not rejected' do
- it 'should return a redirect message' do
+ it 'returns a redirect message' do
project_moved = described_class.new(project, user, 'foo/bar', 'http')
message = "Project 'foo/bar' was moved to '#{project.full_path}'." +
"\n\nPlease update your Git remote:" +
@@ -69,7 +69,7 @@ describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do
describe '#permanent_redirect?' do
context 'with a permanent RedirectRoute' do
- it 'should return true' do
+ it 'returns true' do
project.route.create_redirect('foo/bar', permanent: true)
project_moved = described_class.new(project, user, 'foo/bar', 'http')
expect(project_moved.permanent_redirect?).to be_truthy
@@ -77,7 +77,7 @@ describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do
end
context 'without a permanent RedirectRoute' do
- it 'should return false' do
+ it 'returns false' do
project.route.create_redirect('foo/bar')
project_moved = described_class.new(project, user, 'foo/bar', 'http')
expect(project_moved.permanent_redirect?).to be_falsy
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 457e219c1a5..3c98c95e301 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -152,6 +152,30 @@ describe Gitlab::GitAccess do
expect { push_access_check }.to raise_not_found
end
end
+
+ context 'when user is allowed to create project in namespace' do
+ let(:access) { described_class.new(actor, nil, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path, target_namespace: user.namespace) }
+
+ it 'blocks pull access with "not found"' do
+ expect { pull_access_check }.to raise_not_found
+ end
+
+ it 'allows push access' do
+ expect { push_access_check }.not_to raise_error
+ end
+ end
+
+ context 'when user is not allowed to create project in namespace' do
+ let(:user2) { create(:user) }
+ let(:access) { described_class.new(actor, nil, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path, target_namespace: user2.namespace) }
+
+ it 'blocks push and pull with "not found"' do
+ aggregate_failures do
+ expect { pull_access_check }.to raise_not_found
+ expect { push_access_check }.to raise_not_found
+ end
+ end
+ end
end
end
@@ -311,6 +335,51 @@ describe Gitlab::GitAccess do
end
end
+ describe '#check_namespace_accessibility!' do
+ context 'when project exists' do
+ context 'when user can pull or push' do
+ before do
+ project.add_master(user)
+ end
+
+ it 'does not block pull or push' do
+ aggregate_failures do
+ expect { push_access_check }.not_to raise_error
+ expect { pull_access_check }.not_to raise_error
+ end
+ end
+ end
+ end
+
+ context 'when project does not exist' do
+ context 'when namespace does not exist' do
+ let(:access) { described_class.new(actor, nil, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path, target_namespace: nil) }
+
+ it 'blocks push and pull' do
+ aggregate_failures do
+ expect { push_access_check }.not_to raise_namespace_not_found
+ expect { pull_access_check }.not_to raise_namespace_not_found
+ end
+ end
+ end
+
+ context 'when namespace exists' do
+ context 'when user is unable to push to namespace' do
+ let(:user2) { create(:user) }
+ let(:access) { described_class.new(actor, nil, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path, target_namespace: user2.namespace) }
+
+ it 'blocks push' do
+ expect { push_access_check }.to raise_project_create
+ end
+
+ it 'does not block pull' do
+ expect { push_access_check }.to raise_error
+ end
+ end
+ end
+ end
+ end
+
describe '#check_download_access!' do
it 'allows masters to pull' do
project.add_master(user)
@@ -773,6 +842,16 @@ describe Gitlab::GitAccess do
Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found])
end
+ def raise_namespace_not_found
+ raise_error(Gitlab::GitAccess::NotFoundError,
+ Gitlab::GitAccess::ERROR_MESSAGES[:namespace_not_found])
+ end
+
+ def raise_project_create
+ raise_error(Gitlab::GitAccess::NotFoundError,
+ Gitlab::GitAccess::ERROR_MESSAGES[:create])
+ end
+
def build_authentication_abilities
[
:read_project,
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index da940571bc1..a502a798368 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1079,6 +1079,12 @@ describe Project do
end
end
+ describe '.parse_project_id' do
+ it 'removes .git from the project_id' do
+ expect(described_class.parse_project_id('new-project.git')).to eq('new-project')
+ end
+ end
+
context 'repository storage by default' do
let(:project) { create(:project) }
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 884a258fd12..5f6790312e4 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -366,25 +366,28 @@ describe API::Internal do
end
end
- context 'project as /namespace/project' do
+ context 'project as namespace/project' do
it do
- pull(key, project_with_repo_path('/' + project.full_path))
+ push(key, project_with_repo_path(project.full_path))
expect(response).to have_gitlab_http_status(200)
expect(json_response["status"]).to be_truthy
expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
expect(json_response["gl_repository"]).to eq("project-#{project.id}")
end
- end
- context 'project as namespace/project' do
- it do
- pull(key, project_with_repo_path(project.full_path))
+ context 'when project does not exist' do
+ it 'creates a new project' do
+ path = "#{user.namespace.path}/notexist.git"
- expect(response).to have_gitlab_http_status(200)
- expect(json_response["status"]).to be_truthy
- expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
- expect(json_response["gl_repository"]).to eq("project-#{project.id}")
+ expect do
+ push_with_path(key, path)
+ end.to change { Project.count }.by(1)
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response["status"]).to be_truthy
+ expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(path)
+ end
end
end
end
@@ -818,6 +821,19 @@ describe API::Internal do
end
end
+ context 'with new project data' do
+ it 'returns new project message on the response' do
+ new_project = Gitlab::Checks::NewProject.new(user, project, 'http')
+ new_project.add_new_project_message
+
+ post api("/internal/post_receive"), valid_params
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response["new_project_message"]).to be_present
+ expect(json_response["new_project_message"]).to eq(new_project.new_project_message)
+ end
+ end
+
context 'with an orphaned write deploy key' do
it 'does not try to notify that project moved' do
allow_any_instance_of(Gitlab::Identifier).to receive(:identify).and_return(nil)
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 27bd22d6bca..c73db257b05 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -107,15 +107,39 @@ describe 'Git HTTP requests' do
let(:user) { create(:user) }
context "when the project doesn't exist" do
- let(:path) { 'doesnt/exist.git' }
+ context "when namespace doesn't exist" do
+ let(:path) { 'doesnt/exist.git' }
- it_behaves_like 'pulls require Basic HTTP Authentication'
- it_behaves_like 'pushes require Basic HTTP Authentication'
+ it_behaves_like 'pulls require Basic HTTP Authentication'
+ it_behaves_like 'pushes require Basic HTTP Authentication'
- context 'when authenticated' do
- it 'rejects downloads and uploads with 404 Not Found' do
- download_or_upload(path, user: user.username, password: user.password) do |response|
- expect(response).to have_gitlab_http_status(:not_found)
+ context 'when authenticated' do
+ it 'rejects downloads and uploads with 404 Not Found' do
+ download_or_upload(path, user: user.username, password: user.password) do |response|
+ expect(response).to have_gitlab_http_status(:not_found)
+ end
+ end
+ end
+ end
+
+ context 'when namespace exists' do
+ let(:path) { "#{user.namespace.path}/new-project.git"}
+
+ context 'when authenticated' do
+ it 'creates a new project under the existing namespace' do
+ expect do
+ upload(path, user: user.username, password: user.password) do |response|
+ expect(response).to have_gitlab_http_status(:ok)
+ end
+ end.to change { user.projects.count }.by(1)
+ end
+
+ it 'rejects upload with 404 Not Found when project is invalid' do
+ path = "#{user.namespace.path}/new.git"
+
+ upload(path, user: user.username, password: user.password) do |response|
+ expect(response).to have_gitlab_http_status(:not_found)
+ end
end
end
end
View Lorry Controller Status