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

3 files changed, 37 insertions, 40 deletions

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 928f17e6a8e..7d0e2b3e2ef 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -4,7 +4,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   include ActionController::HttpAuthentication::Basic
   include KerberosSpnegoHelper
 
-  attr_reader :authentication_result
+  attr_reader :authentication_result, :redirected_path
 
   delegate :actor, :authentication_abilities, to: :authentication_result, allow_nil: true
 
@@ -14,7 +14,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   skip_before_action :verify_authenticity_token
   skip_before_action :repository
   before_action :authenticate_user
-  before_action :ensure_project_found!
 
   private
 
@@ -68,38 +67,14 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     headers['Www-Authenticate'] = challenges.join("\n") if challenges.any?
   end
 
-  def ensure_project_found!
-    render_not_found if project.blank?
-  end
-
   def project
-    return @project if defined?(@project)
-
-    project_id, _ = project_id_with_suffix
-    @project =
-      if project_id.blank?
-        nil
-      else
-        Project.find_by_full_path("#{params[:namespace_id]}/#{project_id}")
-      end
-  end
+    parse_repo_path unless defined?(@project)
 
-  # This method returns two values so that we can parse
-  # params[:project_id] (untrusted input!) in exactly one place.
-  def project_id_with_suffix
-    id = params[:project_id] || ''
-
-    %w[.wiki.git .git].each do |suffix|
-      if id.end_with?(suffix)
-        # Be careful to only remove the suffix from the end of 'id'.
-        # Accidentally removing it from the middle is how security
-        # vulnerabilities happen!
-        return [id.slice(0, id.length - suffix.length), suffix]
-      end
-    end
+    @project
+  end
 
-    # Something is wrong with params[:project_id]; do not pass it on.
-    [nil, nil]
+  def parse_repo_path
+    @project, @wiki, @redirected_path = Gitlab::RepoPath.parse("#{params[:namespace_id]}/#{params[:project_id]}")
   end
 
   def render_missing_personal_token
@@ -114,14 +89,9 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   end
 
   def wiki?
-    return @wiki if defined?(@wiki)
-
-    _, suffix = project_id_with_suffix
-    @wiki = suffix == '.wiki.git'
-  end
+    parse_repo_path unless defined?(@wiki)
 
-  def render_not_found
-    render plain: 'Not Found', status: :not_found
+    @wiki
   end
 
   def handle_basic_authentication(login, password)
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index b6b62da7b60..71ae60cb8cd 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -56,7 +56,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
 
   def access
-    @access ||= access_klass.new(access_actor, project, 'http', authentication_abilities: authentication_abilities)
+    @access ||= access_klass.new(access_actor, project, 'http', authentication_abilities: authentication_abilities, redirected_path: redirected_path)
   end
 
   def access_actor
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index dce78faefc9..000d552bb75 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -505,6 +505,33 @@ describe 'Git HTTP requests', lib: true do
                   Rack::Attack::Allow2Ban.reset(ip, options)
                 end
               end
+
+              context 'and the user requests a redirected path' do
+                let!(:redirect) { project.route.create_redirect('foo/bar') }
+                let(:path) { "#{redirect.path}.git" }
+                let(:project_moved_message) do
+                  <<-MSG.strip_heredoc
+                    Project '#{redirect.path}' was moved to '#{project.full_path}'.
+
+                    Please update your Git remote and try again:
+
+                      git remote set-url origin #{project.http_url_to_repo}
+                  MSG
+                end
+
+                it 'downloads get status 404 with "project was moved" message' do
+                  clone_get(path, env)
+                  expect(response).to have_http_status(:not_found)
+                  expect(response.body).to match(project_moved_message)
+                end
+
+                it 'uploads get status 404 with "project was moved" message' do
+                  upload(path, env) do |response|
+                    expect(response).to have_http_status(:not_found)
+                    expect(response.body).to match(project_moved_message)
+                  end
+                end
+              end
             end
 
             context "when the user doesn't have access to the project" do
@@ -680,7 +707,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