Commit af784cc6 authored by Michael Kozono's avatar Michael Kozono

Add “Project moved” error to Git-over-SSH

parent 3a38e5f1
...@@ -11,6 +11,10 @@ module API ...@@ -11,6 +11,10 @@ module API
@project @project
end end
def redirected_path
@redirected_path
end
def ssh_authentication_abilities def ssh_authentication_abilities
[ [
:read_project, :read_project,
...@@ -38,8 +42,9 @@ module API ...@@ -38,8 +42,9 @@ module API
def set_project def set_project
if params[:gl_repository] if params[:gl_repository]
@project, @wiki = Gitlab::GlRepository.parse(params[:gl_repository]) @project, @wiki = Gitlab::GlRepository.parse(params[:gl_repository])
@redirected_path = nil
else else
@project, @wiki = Gitlab::RepoPath.parse(params[:project]) @project, @wiki, @redirected_path = Gitlab::RepoPath.parse(params[:project])
end end
end end
......
...@@ -34,7 +34,7 @@ module API ...@@ -34,7 +34,7 @@ module API
access_checker_klass = wiki? ? Gitlab::GitAccessWiki : Gitlab::GitAccess access_checker_klass = wiki? ? Gitlab::GitAccessWiki : Gitlab::GitAccess
access_checker = access_checker_klass access_checker = access_checker_klass
.new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities) .new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities, redirected_path: redirected_path)
begin begin
access_checker.check(params[:action], params[:changes]) access_checker.check(params[:action], params[:changes])
......
...@@ -22,12 +22,13 @@ module Gitlab ...@@ -22,12 +22,13 @@ module Gitlab
PUSH_COMMANDS = %w{ git-receive-pack }.freeze PUSH_COMMANDS = %w{ git-receive-pack }.freeze
ALL_COMMANDS = DOWNLOAD_COMMANDS + PUSH_COMMANDS ALL_COMMANDS = DOWNLOAD_COMMANDS + PUSH_COMMANDS
attr_reader :actor, :project, :protocol, :authentication_abilities attr_reader :actor, :project, :protocol, :authentication_abilities, :redirected_path
def initialize(actor, project, protocol, authentication_abilities:) def initialize(actor, project, protocol, authentication_abilities:, redirected_path: nil)
@actor = actor @actor = actor
@project = project @project = project
@protocol = protocol @protocol = protocol
@redirected_path = redirected_path
@authentication_abilities = authentication_abilities @authentication_abilities = authentication_abilities
end end
...@@ -35,6 +36,7 @@ module Gitlab ...@@ -35,6 +36,7 @@ module Gitlab
check_protocol! check_protocol!
check_active_user! check_active_user!
check_project_accessibility! check_project_accessibility!
check_project_moved!
check_command_disabled!(cmd) check_command_disabled!(cmd)
check_command_existence!(cmd) check_command_existence!(cmd)
check_repository_existence! check_repository_existence!
...@@ -87,6 +89,21 @@ module Gitlab ...@@ -87,6 +89,21 @@ module Gitlab
end end
end end
def check_project_moved!
if redirected_path
url = protocol == 'ssh' ? project.ssh_url_to_repo : project.http_url_to_repo
message = <<-MESSAGE.strip_heredoc
Project '#{redirected_path}' was moved to '#{project.full_path}'.
Please update your Git remote and try again:
git remote set-url origin #{url}
MESSAGE
raise NotFoundError, message
end
end
def check_command_disabled!(cmd) def check_command_disabled!(cmd)
if upload_pack?(cmd) if upload_pack?(cmd)
check_upload_pack_disabled! check_upload_pack_disabled!
......
...@@ -3,16 +3,18 @@ module Gitlab ...@@ -3,16 +3,18 @@ module Gitlab
NotFoundError = Class.new(StandardError) NotFoundError = Class.new(StandardError)
def self.parse(repo_path) def self.parse(repo_path)
wiki = false
project_path = strip_storage_path(repo_path.sub(/\.git\z/, ''), fail_on_not_found: false) project_path = strip_storage_path(repo_path.sub(/\.git\z/, ''), fail_on_not_found: false)
project = Project.find_by_full_path(project_path) project, was_redirected = find_project(project_path)
if project_path.end_with?('.wiki') && !project
project = Project.find_by_full_path(project_path.chomp('.wiki')) if project_path.end_with?('.wiki') && project.nil?
project, was_redirected = find_project(project_path.chomp('.wiki'))
wiki = true wiki = true
else
wiki = false
end end
[project, wiki] redirected_path = project_path if was_redirected
[project, wiki, redirected_path]
end end
def self.strip_storage_path(repo_path, fail_on_not_found: true) def self.strip_storage_path(repo_path, fail_on_not_found: true)
...@@ -30,5 +32,12 @@ module Gitlab ...@@ -30,5 +32,12 @@ module Gitlab
result.sub(/\A\/*/, '') result.sub(/\A\/*/, '')
end end
def self.find_project(project_path)
project = Project.find_by_full_path(project_path, follow_redirects: true)
was_redirected = project && project.full_path.casecmp(project_path) != 0
[project, was_redirected]
end
end end
end end
...@@ -3,11 +3,12 @@ require 'spec_helper' ...@@ -3,11 +3,12 @@ require 'spec_helper'
describe Gitlab::GitAccess, lib: true do describe Gitlab::GitAccess, lib: true do
let(:pull_access_check) { access.check('git-upload-pack', '_any') } let(:pull_access_check) { access.check('git-upload-pack', '_any') }
let(:push_access_check) { access.check('git-receive-pack', '_any') } let(:push_access_check) { access.check('git-receive-pack', '_any') }
let(:access) { Gitlab::GitAccess.new(actor, project, protocol, authentication_abilities: authentication_abilities) } let(:access) { Gitlab::GitAccess.new(actor, project, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path) }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:actor) { user } let(:actor) { user }
let(:protocol) { 'ssh' } let(:protocol) { 'ssh' }
let(:redirected_path) { nil }
let(:authentication_abilities) do let(:authentication_abilities) do
[ [
:read_project, :read_project,
...@@ -162,6 +163,46 @@ describe Gitlab::GitAccess, lib: true do ...@@ -162,6 +163,46 @@ describe Gitlab::GitAccess, lib: true do
end end
end end
describe '#check_project_moved!' do
before do
project.team << [user, :master]
end
context 'when a redirect was not followed to find the project' do
context 'pull code' do
it { expect { pull_access_check }.not_to raise_error }
end
context 'push code' do
it { expect { push_access_check }.not_to raise_error }
end
end
context 'when a redirect was followed to find the project' do
let(:redirected_path) { 'some/other-path' }
context 'pull code' do
it { expect { pull_access_check }.to raise_not_found(/Project '#{redirected_path}' was moved to '#{project.full_path}'/) }
it { expect { pull_access_check }.to raise_not_found(/git remote set-url origin #{project.ssh_url_to_repo}/) }
context 'http protocol' do
let(:protocol) { 'http' }
it { expect { pull_access_check }.to raise_not_found(/git remote set-url origin #{project.http_url_to_repo}/) }
end
end
context 'push code' do
it { expect { push_access_check }.to raise_not_found(/Project '#{redirected_path}' was moved to '#{project.full_path}'/) }
it { expect { push_access_check }.to raise_not_found(/git remote set-url origin #{project.ssh_url_to_repo}/) }
context 'http protocol' do
let(:protocol) { 'http' }
it { expect { push_access_check }.to raise_not_found(/git remote set-url origin #{project.http_url_to_repo}/) }
end
end
end
end
describe '#check_command_disabled!' do describe '#check_command_disabled!' do
before do before do
project.team << [user, :master] project.team << [user, :master]
......
require 'spec_helper' require 'spec_helper'
describe Gitlab::GitAccessWiki, lib: true do describe Gitlab::GitAccessWiki, lib: true do
let(:access) { Gitlab::GitAccessWiki.new(user, project, 'web', authentication_abilities: authentication_abilities) } let(:access) { Gitlab::GitAccessWiki.new(user, project, 'web', authentication_abilities: authentication_abilities, redirected_path: redirected_path) }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:redirected_path) { nil }
let(:authentication_abilities) do let(:authentication_abilities) do
[ [
:read_project, :read_project,
......
...@@ -4,24 +4,44 @@ describe ::Gitlab::RepoPath do ...@@ -4,24 +4,44 @@ describe ::Gitlab::RepoPath do
describe '.parse' do describe '.parse' do
set(:project) { create(:project) } set(:project) { create(:project) }
context 'a repository storage path' do
it 'parses a full repository path' do it 'parses a full repository path' do
expect(described_class.parse(project.repository.path)).to eq([project, false]) expect(described_class.parse(project.repository.path)).to eq([project, false, nil])
end end
it 'parses a full wiki path' do it 'parses a full wiki path' do
expect(described_class.parse(project.wiki.repository.path)).to eq([project, true]) expect(described_class.parse(project.wiki.repository.path)).to eq([project, true, nil])
end
end end
context 'a relative path' do
it 'parses a relative repository path' do it 'parses a relative repository path' do
expect(described_class.parse(project.full_path + '.git')).to eq([project, false]) expect(described_class.parse(project.full_path + '.git')).to eq([project, false, nil])
end end
it 'parses a relative wiki path' do it 'parses a relative wiki path' do
expect(described_class.parse(project.full_path + '.wiki.git')).to eq([project, true]) expect(described_class.parse(project.full_path + '.wiki.git')).to eq([project, true, nil])
end end
it 'parses a relative path starting with /' do it 'parses a relative path starting with /' do
expect(described_class.parse('/' + project.full_path + '.git')).to eq([project, false]) expect(described_class.parse('/' + project.full_path + '.git')).to eq([project, false, nil])
end
context 'of a redirected project' do
let(:redirect) { project.route.create_redirect('foo/bar') }
it 'parses a relative repository path' do
expect(described_class.parse(redirect.path + '.git')).to eq([project, false, 'foo/bar'])
end
it 'parses a relative wiki path' do
expect(described_class.parse(redirect.path + '.wiki.git')).to eq([project, true, 'foo/bar.wiki'])
end
it 'parses a relative path starting with /' do
expect(described_class.parse('/' + redirect.path + '.git')).to eq([project, false, 'foo/bar'])
end
end
end end
end end
...@@ -43,4 +63,33 @@ describe ::Gitlab::RepoPath do ...@@ -43,4 +63,33 @@ describe ::Gitlab::RepoPath do
) )
end end
end end
describe '.find_project' do
let(:project) { create(:empty_project) }
let(:redirect) { project.route.create_redirect('foo/bar/baz') }
context 'when finding a project by its canonical path' do
context 'when the cases match' do
it 'returns the project and false' do
expect(described_class.find_project(project.full_path)).to eq([project, false])
end
end
context 'when the cases do not match' do
# This is slightly different than web behavior because on the web it is
# easy and safe to redirect someone to the correctly-cased URL. For git
# requests, we should accept wrongly-cased URLs because it is a pain to
# block people's git operations and force them to update remote URLs.
it 'returns the project and false' do
expect(described_class.find_project(project.full_path.upcase)).to eq([project, false])
end
end
end
context 'when finding a project via a redirect' do
it 'returns the project and true' do
expect(described_class.find_project(redirect.path)).to eq([project, true])
end
end
end
end end
...@@ -321,8 +321,6 @@ describe API::Internal do ...@@ -321,8 +321,6 @@ describe API::Internal do
end end
context "archived project" do context "archived project" do
let(:personal_project) { create(:empty_project, namespace: user.namespace) }
before do before do
project.team << [user, :developer] project.team << [user, :developer]
project.archive! project.archive!
...@@ -445,6 +443,42 @@ describe API::Internal do ...@@ -445,6 +443,42 @@ describe API::Internal do
expect(json_response['status']).to be_truthy expect(json_response['status']).to be_truthy
end end
end end
context 'the project path was changed' do
let!(:old_path_to_repo) { project.repository.path_to_repo }
let!(:old_full_path) { project.full_path }
let(:project_moved_message) do
<<-MSG.strip_heredoc
Project '#{old_full_path}' was moved to '#{project.full_path}'.
Please update your Git remote and try again:
git remote set-url origin #{project.ssh_url_to_repo}
MSG
end
before do
project.team << [user, :developer]
project.path = 'new_path'
project.save!
end
it 'rejects the push' do
push_with_path(key, old_path_to_repo)
expect(response).to have_http_status(200)
expect(json_response['status']).to be_falsey
expect(json_response['message']).to eq(project_moved_message)
end
it 'rejects the SSH pull' do
pull_with_path(key, old_path_to_repo)
expect(response).to have_http_status(200)
expect(json_response['status']).to be_falsey
expect(json_response['message']).to eq(project_moved_message)
end
end
end end
describe 'GET /internal/merge_request_urls' do describe 'GET /internal/merge_request_urls' do
...@@ -587,6 +621,17 @@ describe API::Internal do ...@@ -587,6 +621,17 @@ describe API::Internal do
) )
end end
def pull_with_path(key, path_to_repo, protocol = 'ssh')
post(
api("/internal/allowed"),
key_id: key.id,
project: path_to_repo,
action: 'git-upload-pack',
secret_token: secret_token,
protocol: protocol
)
end
def push(key, project, protocol = 'ssh', env: nil) def push(key, project, protocol = 'ssh', env: nil)
post( post(
api("/internal/allowed"), api("/internal/allowed"),
...@@ -600,6 +645,19 @@ describe API::Internal do ...@@ -600,6 +645,19 @@ describe API::Internal do
) )
end end
def push_with_path(key, path_to_repo, protocol = 'ssh', env: nil)
post(
api("/internal/allowed"),
changes: 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master',
key_id: key.id,
project: path_to_repo,
action: 'git-receive-pack',
secret_token: secret_token,
protocol: protocol,
env: env
)
end
def archive(key, project) def archive(key, project)
post( post(
api("/internal/allowed"), api("/internal/allowed"),
......
...@@ -32,7 +32,7 @@ describe PostReceive do ...@@ -32,7 +32,7 @@ describe PostReceive do
context "with an absolute path as the project identifier" do context "with an absolute path as the project identifier" do
it "searches the project by full path" do it "searches the project by full path" do
expect(Project).to receive(:find_by_full_path).with(project.full_path).and_call_original expect(Project).to receive(:find_by_full_path).with(project.full_path, follow_redirects: true).and_call_original
described_class.new.perform(pwd(project), key_id, base64_changes) described_class.new.perform(pwd(project), key_id, base64_changes)
end end
......
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