Render GitAccess error message if authenticatedmk-add-project-moved-errors-for-git

5 files changed, 48 insertions, 25 deletions

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 9a1bf037a95..4d897db5906 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -75,13 +75,16 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   def project
     return @project if defined?(@project)
 
+    @project = Project.find_by_full_path(requested_path, follow_redirects: true)
+  end
+
+  def redirected_path
+    requested_path if project.full_path != requested_path
+  end
+
+  def requested_path
     project_id, _ = project_id_with_suffix
-    @project =
-      if project_id.blank?
-        nil
-      else
-        Project.find_by_full_path("#{params[:namespace_id]}/#{project_id}")
-      end
+    "#{params[:namespace_id]}/#{project_id}"
   end
 
   # This method returns two values so that we can parse
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index 9e4edcae101..b586e1d5ecb 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -11,7 +11,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
     elsif receive_pack? && receive_pack_allowed?
       render_ok
     elsif http_blocked?
-      render_http_not_allowed
+      render_git_access_error_message
     else
       render_denied
     end
@@ -62,23 +62,19 @@ class Projects::GitHttpController < Projects::GitHttpClientController
     render json: Gitlab::Workhorse.git_http_ok(repository, wiki?, user, action_name)
   end
 
-  def render_http_not_allowed
+  def render_git_access_error_message
     render plain: access_check.message, status: :forbidden
   end
 
   def render_denied
     if user && can?(user, :read_project, project)
-      render plain: access_denied_message, status: :forbidden
+      render_git_access_error_message
     else
       # Do not leak information about project existence
       render_not_found
     end
   end
 
-  def access_denied_message
-    'Access denied'
-  end
-
   def upload_pack_allowed?
     return false unless Gitlab.config.gitlab_shell.upload_pack
 
@@ -86,7 +82,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
 
   def access
-    @access ||= access_klass.new(user, project, 'http', authentication_abilities: authentication_abilities)
+    @access ||= access_klass.new(user, project, 'http', authentication_abilities: authentication_abilities, redirected_path: redirected_path)
   end
 
   def access_check
diff --git a/changelogs/unreleased/mk-add-project-moved-errors-for-git.yml b/changelogs/unreleased/mk-add-project-moved-errors-for-git.yml
new file mode 100644
index 00000000000..8ab4f0a4c11
--- /dev/null
+++ b/changelogs/unreleased/mk-add-project-moved-errors-for-git.yml
@@ -0,0 +1,4 @@
+---
+title: Add "project was moved" messages when rejecting git push/pull
+merge_request: 11259
+author:
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 6ca3ef18fe6..a5edb5247e8 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -141,30 +141,30 @@ describe 'Git HTTP requests', lib: true do
 
         context 'when the repo is public' do
           context 'but the repo is disabled' do
-            it 'does not allow to clone the repo' do
-              project = create(:project, :public, :repository_disabled)
+            let(:project) { create(:project, :public, :repository_disabled) }
 
-              download("#{project.path_with_namespace}.git", {}) do |response|
+            it 'does not allow to clone the repo' do
+              download(path, {}) do |response|
                 expect(response).to have_http_status(:unauthorized)
               end
             end
           end
 
           context 'but the repo is enabled' do
-            it 'allows to clone the repo' do
-              project = create(:project, :public, :repository_enabled)
+            let(:project) { create(:project, :public, :repository_enabled) }
 
-              download("#{project.path_with_namespace}.git", {}) do |response|
+            it 'allows to clone the repo' do
+              download(path, {}) do |response|
                 expect(response).to have_http_status(:ok)
               end
             end
           end
 
           context 'but only project members are allowed' do
-            it 'does not allow to clone the repo' do
-              project = create(:project, :public, :repository_private)
+            let(:project) { create(:project, :public, :repository_private) }
 
-              download("#{project.path_with_namespace}.git", {}) do |response|
+            it 'does not allow to clone the repo' do
+              download(path, {}) do |response|
                 expect(response).to have_http_status(:unauthorized)
               end
             end
@@ -264,6 +264,26 @@ describe 'Git HTTP requests', lib: true do
                     expect(user_activity(user)).to be_present
                   end
                 end
+
+                context 'and the user requests a redirected path' do
+                  let!(:redirect) { project.route.create_redirect('foo/bar') }
+                  let(:path) { "#{redirect.path}.git" }
+
+                  it 'downloads get status 403 with "project was moved" message' do
+                    clone_get(path, env)
+                    message = "Project '#{redirect.path}' was moved to '#{project.full_path}'.\n\nPlease update your Git remote and try again:\n\n  git remote set-url origin #{project.http_url_to_repo}"
+                    expect(response).to have_http_status(:forbidden)
+                    expect(response.body).to match(message)
+                  end
+
+                  it 'uploads get status 403 with "project was moved" message' do
+                    upload(path, env) do |response|
+                      message = "Project '#{redirect.path}' was moved to '#{project.full_path}'.\n\nPlease update your Git remote and try again:\n\n  git remote set-url origin #{project.http_url_to_repo}"
+                      expect(response).to have_http_status(:forbidden)
+                      expect(response.body).to match(message)
+                    end
+                  end
+                end
               end
 
               context "when an oauth token is provided" do
@@ -508,7 +528,7 @@ describe 'Git HTTP requests', lib: true do
       end
 
       context "POST git-receive-pack" do
-        it "failes to find a route" do
+        it "fails to find a route" do
           expect { push_post(project.path_with_namespace) }.to raise_error(ActionController::RoutingError)
         end
       end
diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb
index 0260416dbe2..2fb9f2d5045 100644
--- a/spec/workers/post_receive_spec.rb
+++ b/spec/workers/post_receive_spec.rb
@@ -29,7 +29,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