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

9 files changed, 208 insertions, 28 deletions

diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb
index d3732d67622..5e9cf5e68b1 100644
--- a/lib/api/helpers/internal_helpers.rb
+++ b/lib/api/helpers/internal_helpers.rb
@@ -10,6 +10,10 @@ module API
         set_project unless defined?(@project)
         @project
       end
+      
+      def redirected_path
+        @redirected_path
+      end
 
       def ssh_authentication_abilities
         [
@@ -38,8 +42,9 @@ module API
       def set_project
         if params[:gl_repository]
           @project, @wiki = Gitlab::GlRepository.parse(params[:gl_repository])
+          @redirected_path = nil
         else
-          @project, @wiki = Gitlab::RepoPath.parse(params[:project])
+          @project, @wiki, @redirected_path = Gitlab::RepoPath.parse(params[:project])
         end
       end
 
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index ecd6d672cf7..9ec418edea4 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -34,7 +34,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)
+          .new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities, redirected_path: redirected_path)
 
         begin
           access_checker.check(params[:action], params[:changes])
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 0a19d24eb20..0b62911958d 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -22,12 +22,13 @@ module Gitlab
     PUSH_COMMANDS = %w{ git-receive-pack }.freeze
     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
       @project  = project
       @protocol = protocol
+      @redirected_path = redirected_path
       @authentication_abilities = authentication_abilities
     end
 
@@ -35,6 +36,7 @@ module Gitlab
       check_protocol!
       check_active_user!
       check_project_accessibility!
+      check_project_moved!
       check_command_disabled!(cmd)
       check_command_existence!(cmd)
       check_repository_existence!
@@ -87,6 +89,21 @@ module Gitlab
       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)
       if upload_pack?(cmd)
         check_upload_pack_disabled!
diff --git a/lib/gitlab/repo_path.rb b/lib/gitlab/repo_path.rb
index 878e03f61d7..3591fa9145e 100644
--- a/lib/gitlab/repo_path.rb
+++ b/lib/gitlab/repo_path.rb
@@ -3,16 +3,18 @@ module Gitlab
     NotFoundError = Class.new(StandardError)
 
     def self.parse(repo_path)
+      wiki = false
       project_path = strip_storage_path(repo_path.sub(/\.git\z/, ''), fail_on_not_found: false)
-      project = Project.find_by_full_path(project_path)
-      if project_path.end_with?('.wiki') && !project
-        project = Project.find_by_full_path(project_path.chomp('.wiki'))
+      project, was_redirected = find_project(project_path)
+
+      if project_path.end_with?('.wiki') && project.nil?
+        project, was_redirected = find_project(project_path.chomp('.wiki'))
         wiki = true
-      else
-        wiki = false
       end
 
-      [project, wiki]
+      redirected_path = project_path if was_redirected
+
+      [project, wiki, redirected_path]
     end
 
     def self.strip_storage_path(repo_path, fail_on_not_found: true)
@@ -30,5 +32,12 @@ module Gitlab
 
       result.sub(/\A\/*/, '')
     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
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 3dcc20c48e8..9a86cfa66e4 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -3,11 +3,12 @@ require 'spec_helper'
 describe Gitlab::GitAccess, lib: true do
   let(:pull_access_check) { access.check('git-upload-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(:user) { create(:user) }
   let(:actor) { user }
   let(:protocol) { 'ssh' }
+  let(:redirected_path) { nil }
   let(:authentication_abilities) do
     [
       :read_project,
@@ -162,6 +163,46 @@ describe Gitlab::GitAccess, lib: true do
     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
     before do
       project.team << [user, :master]
diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb
index a1eb95750ba..797ec8cb23e 100644
--- a/spec/lib/gitlab/git_access_wiki_spec.rb
+++ b/spec/lib/gitlab/git_access_wiki_spec.rb
@@ -1,9 +1,10 @@
 require 'spec_helper'
 
 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(:user) { create(:user) }
+  let(:redirected_path) { nil }
   let(:authentication_abilities) do
     [
       :read_project,
diff --git a/spec/lib/gitlab/repo_path_spec.rb b/spec/lib/gitlab/repo_path_spec.rb
index f9025397107..efea4f429bf 100644
--- a/spec/lib/gitlab/repo_path_spec.rb
+++ b/spec/lib/gitlab/repo_path_spec.rb
@@ -4,24 +4,44 @@ describe ::Gitlab::RepoPath do
   describe '.parse' do
     set(:project) { create(:project) }
 
-    it 'parses a full repository path' do
-      expect(described_class.parse(project.repository.path)).to eq([project, false])
-    end
+    context 'a repository storage path' do
+      it 'parses a full repository path' do
+        expect(described_class.parse(project.repository.path)).to eq([project, false, nil])
+      end
 
-    it 'parses a full wiki path' do
-      expect(described_class.parse(project.wiki.repository.path)).to eq([project, true])
+      it 'parses a full wiki path' do
+        expect(described_class.parse(project.wiki.repository.path)).to eq([project, true, nil])
+      end
     end
 
-    it 'parses a relative repository path' do
-      expect(described_class.parse(project.full_path + '.git')).to eq([project, false])
-    end
+    context 'a relative path' do
+      it 'parses a relative repository path' do
+        expect(described_class.parse(project.full_path + '.git')).to eq([project, false, nil])
+      end
 
-    it 'parses a relative wiki path' do
-      expect(described_class.parse(project.full_path + '.wiki.git')).to eq([project, true])
-    end
+      it 'parses a relative wiki path' do
+        expect(described_class.parse(project.full_path + '.wiki.git')).to eq([project, true, nil])
+      end
+
+      it 'parses a relative path starting with /' do
+        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('/' + project.full_path + '.git')).to eq([project, false])
+        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
 
@@ -43,4 +63,33 @@ describe ::Gitlab::RepoPath do
       )
     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
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 86e15d896df..6deaea956e0 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -321,8 +321,6 @@ describe API::Internal do
     end
 
     context "archived project" do
-      let(:personal_project) { create(:empty_project, namespace: user.namespace) }
-
       before do
         project.team << [user, :developer]
         project.archive!
@@ -445,6 +443,42 @@ describe API::Internal do
         expect(json_response['status']).to be_truthy
       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
 
   describe 'GET /internal/merge_request_urls' do
@@ -587,6 +621,17 @@ describe API::Internal do
     )
   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)
     post(
       api("/internal/allowed"),
@@ -600,6 +645,19 @@ describe API::Internal do
     )
   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)
     post(
       api("/internal/allowed"),
diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb
index 3c93da63f2e..6c4df7350ea 100644
--- a/spec/workers/post_receive_spec.rb
+++ b/spec/workers/post_receive_spec.rb
@@ -32,7 +32,7 @@ describe PostReceive do
 
   context "with an absolute path as the project identifier" 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)
     end