Remove existence check from ProjectUrlConstrainer

This was causing non-existent projects to fall back to our
wildcard not found route which then behaves differently
for repository ref paths.

For repository ref paths like blobs, we ignore the format in the
URL because these are rendered as HTML. But when it falls back to the
not found route, the format is parsed causing Devise to use the
wrong status code for unauthenticated requests.

With this change, valid URLs whether the project exists or not will
be handled by the same controller so that behavior will be the same.

Only paths that have invalid project paths would fall back to
the wildcard catch-all route.

app/controllers/application_controller.rb

@@ -95,11 +95,11 @@ class ApplicationController < ActionController::Base
   end
 
   def route_not_found
-    if current_user
-      not_found
-    else
-      authenticate_user!
-    end
+    # We need to call #authenticate_user! here because sometimes this is called from another action
+    # and not from our wildcard fallback route
+    authenticate_user!
+
+    not_found
   end
 
   def render(*args)

config/routes/git_http.rb

@@ -52,7 +52,7 @@ scope(path: '*namespace_id/:project_id',
     # /info/refs?service=git-receive-pack, but nothing else.
     #
     git_http_handshake = lambda do |request|
-      ::Constraints::ProjectUrlConstrainer.new.matches?(request, existence_check: false) &&
+      ::Constraints::ProjectUrlConstrainer.new.matches?(request) &&
         (request.query_string.blank? ||
          request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/))
     end

lib/constraints/project_url_constrainer.rb

@@ -2,17 +2,12 @@
 
 module Constraints
   class ProjectUrlConstrainer
-    def matches?(request, existence_check: true)
+    def matches?(request)
       namespace_path = request.params[:namespace_id]
       project_path = request.params[:project_id] || request.params[:id]
       full_path = [namespace_path, project_path].join('/')
 
-      return false unless ProjectPathValidator.valid_path?(full_path)
-      return true unless existence_check
-
-      # We intentionally allow SELECT(*) here so result of this query can be used
-      # as cache for further Project.find_by_full_path calls within request
-      Project.find_by_full_path(full_path, follow_redirects: request.get?).present?
+      ProjectPathValidator.valid_path?(full_path)
     end
   end
 end

spec/controllers/projects/releases_controller_spec.rb

@@ -111,8 +111,8 @@ describe Projects::ReleasesController do
       context 'when the project is private and the user is not logged in' do
         let(:project) { private_project }
 
-        it 'returns a redirect' do
-          expect(response).to have_gitlab_http_status(:redirect)
+        it 'returns a 401' do
+          expect(response).to have_gitlab_http_status(:unauthorized)
         end
       end
     end

spec/lib/constraints/project_url_constrainer_spec.rb

@@ -14,42 +14,15 @@ describe Constraints::ProjectUrlConstrainer do
     end
 
     context 'invalid request' do
-      context "non-existing project" do
-        let(:request) { build_request('foo', 'bar') }
-
-        it { expect(subject.matches?(request)).to be_falsey }
-
-        context 'existence_check is false' do
-          it { expect(subject.matches?(request, existence_check: false)).to be_truthy }
-        end
-      end
-
       context "project id ending with .git" do
         let(:request) { build_request(namespace.full_path, project.path + '.git') }
 
         it { expect(subject.matches?(request)).to be_falsey }
       end
     end
-
-    context 'when the request matches a redirect route' do
-      let(:old_project_path) { 'old_project_path' }
-      let!(:redirect_route) { project.redirect_routes.create!(path: "#{namespace.full_path}/#{old_project_path}") }
-
-      context 'and is a GET request' do
-        let(:request) { build_request(namespace.full_path, old_project_path) }
-        it { expect(subject.matches?(request)).to be_truthy }
-      end
-
-      context 'and is NOT a GET request' do
-        let(:request) { build_request(namespace.full_path, old_project_path, 'POST') }
-        it { expect(subject.matches?(request)).to be_falsey }
-      end
-    end
   end
 
-  def build_request(namespace, project, method = 'GET')
-    double(:request,
-      'get?': (method == 'GET'),
-      params: { namespace_id: namespace, id: project })
+  def build_request(namespace, project)
+    double(:request, params: { namespace_id: namespace, id: project })
   end
 end