Commit 90a74188 authored by Stan Hu's avatar Stan Hu

Merge branch 'fj-214714-fix-bug-ssh-git-create-project' into 'master'

Fix bug creating project from git push through ssh

Closes #214714

See merge request gitlab-org/gitlab!29771
parents 7dee6324 88b80c80
---
title: Fix bug creating project from git ssh
merge_request: 29771
author:
type: fixed
......@@ -43,12 +43,9 @@ module API
Gitlab::Git::HookEnv.set(gl_repository, env) if container
actor.update_last_used_at!
access_checker = access_checker_for(actor, params[:protocol])
check_result = begin
result = access_checker.check(params[:action], params[:changes])
@project ||= access_checker.project
result
access_check!(actor, params)
rescue Gitlab::GitAccess::ForbiddenError => e
# The return code needs to be 401. If we return 403
# the custom message we return won't be shown to the user
......@@ -92,6 +89,17 @@ module API
response_with_status(code: 500, success: false, message: UNKNOWN_CHECK_RESULT_ERROR)
end
end
def access_check!(actor, params)
access_checker = access_checker_for(actor, params[:protocol])
access_checker.check(params[:action], params[:changes]).tap do |result|
break result if @project || !repo_type.project?
# If we have created a project directly from a git push
# we have to assign its value to both @project and @container
@project = @container = access_checker.project
end
end
end
namespace 'internal' do
......
......@@ -766,29 +766,98 @@ describe API::Internal::Base do
end
context 'project does not exist' do
it 'returns a 200 response with status: false' do
project.destroy
context 'git pull' do
it 'returns a 200 response with status: false' do
project.destroy
pull(key, project)
pull(key, project)
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response["status"]).to be_falsey
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response["status"]).to be_falsey
end
it 'returns a 200 response when using a project path that does not exist' do
post(
api("/internal/allowed"),
params: {
key_id: key.id,
project: 'project/does-not-exist.git',
action: 'git-upload-pack',
secret_token: secret_token,
protocol: 'ssh'
}
)
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response["status"]).to be_falsey
end
end
it 'returns a 200 response when using a project path that does not exist' do
post(
api("/internal/allowed"),
params: {
key_id: key.id,
project: 'project/does-not-exist.git',
action: 'git-upload-pack',
secret_token: secret_token,
protocol: 'ssh'
}
)
context 'git push' do
before do
stub_const('Gitlab::QueryLimiting::Transaction::THRESHOLD', 120)
end
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response["status"]).to be_falsey
subject { push_with_path(key, full_path: path, changes: '_any') }
context 'from a user/group namespace' do
let!(:path) { "#{user.namespace.path}/notexist.git" }
it 'creates the project' do
expect do
subject
end.to change { Project.count }.by(1)
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['status']).to be_truthy
end
end
context 'from the personal snippet path' do
let!(:path) { 'snippets/notexist.git' }
it 'does not create snippet' do
expect do
subject
end.not_to change { Snippet.count }
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'from a project path' do
context 'from an non existent project path' do
let!(:path) { "#{user.namespace.path}/notexist/snippets/notexist.git" }
it 'does not create project' do
expect do
subject
end.not_to change { Project.count }
expect(response).to have_gitlab_http_status(:not_found)
end
it 'does not create snippet' do
expect do
subject
end.not_to change { Snippet.count }
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'from an existent project path' do
let!(:path) { "#{project.full_path}/notexist/snippets/notexist.git" }
it 'does not create snippet' do
expect do
subject
end.not_to change { Snippet.count }
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
end
end
......@@ -1062,18 +1131,27 @@ describe API::Internal::Base do
end
def push(key, container, protocol = 'ssh', env: nil, changes: nil)
push_with_path(key,
full_path: full_path_for(container),
gl_repository: gl_repository_for(container),
protocol: protocol,
env: env,
changes: changes)
end
def push_with_path(key, full_path:, gl_repository: nil, protocol: 'ssh', env: nil, changes: nil)
changes ||= 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master'
params = {
changes: changes,
key_id: key.id,
project: full_path_for(container),
gl_repository: gl_repository_for(container),
project: full_path,
action: 'git-receive-pack',
secret_token: secret_token,
protocol: protocol,
env: env
}
params[:gl_repository] = gl_repository if gl_repository
post(
api("/internal/allowed"),
......
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