Merge branch '351058-fix-projects-id-redirect-500' into 'master'

Fix sourcegraph breaking on project resolve endpoint

See merge request gitlab-org/gitlab!82447

app/controllers/projects/redirect_controller.rb

+# frozen_string_literal: true
+
+# Projects::RedirectController is used to resolve the route projects/:id.
+# It's helpful for this to be in its own controller so that the
+# ProjectsController can assume that :namespace_id exists
+class Projects::RedirectController < ::ApplicationController
+  skip_before_action :authenticate_user!
+
+  feature_category :projects
+
+  def redirect_from_id
+    project = Project.find(params[:id])
+
+    if can?(current_user, :read_project, project)
+      redirect_to project
+    else
+      render_404
+    end
+  end
+end

app/controllers/projects_controller.rb

@@ -17,10 +17,10 @@ class ProjectsController < Projects::ApplicationController
   around_action :allow_gitaly_ref_name_caching, only: [:index, :show]
 
   before_action :disable_query_limiting, only: [:show, :create]
-  before_action :authenticate_user!, except: [:index, :show, :activity, :refs, :resolve, :unfoldered_environment_names]
+  before_action :authenticate_user!, except: [:index, :show, :activity, :refs, :unfoldered_environment_names]
   before_action :redirect_git_extension, only: [:show]
-  before_action :project, except: [:index, :new, :create, :resolve]
-  before_action :repository, except: [:index, :new, :create, :resolve]
+  before_action :project, except: [:index, :new, :create]
+  before_action :repository, except: [:index, :new, :create]
   before_action :verify_git_import_enabled, only: [:create]
   before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export]
   before_action :present_project, only: [:edit]
@@ -48,7 +48,7 @@ class ProjectsController < Projects::ApplicationController
 
   feature_category :projects, [
                      :index, :show, :new, :create, :edit, :update, :transfer,
-                     :destroy, :resolve, :archive, :unarchive, :toggle_star, :activity
+                     :destroy, :archive, :unarchive, :toggle_star, :activity
                    ]
 
   feature_category :source_code_management, [:remove_fork, :housekeeping, :refs]
@@ -324,16 +324,6 @@ class ProjectsController < Projects::ApplicationController
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
-  def resolve
-    @project = Project.find(params[:id])
-
-    if can?(current_user, :read_project, @project)
-      redirect_to @project
-    else
-      render_404
-    end
-  end
-
   def unfoldered_environment_names
     respond_to do |format|
       format.json do

config/routes.rb

@@ -269,7 +269,7 @@ Rails.application.routes.draw do
 
   resources :projects, only: [:index, :new, :create]
 
-  get '/projects/:id' => 'projects#resolve'
+  get '/projects/:id' => 'projects/redirect#redirect_from_id'
 
   draw :git_http
   draw :api

spec/controllers/projects_controller_spec.rb

@@ -1602,59 +1602,6 @@ RSpec.describe ProjectsController do
     end
   end
 
-  describe 'GET resolve' do
-    shared_examples 'resolvable endpoint' do
-      it 'redirects to the project page' do
-        get :resolve, params: { id: project.id }
-
-        expect(response).to have_gitlab_http_status(:found)
-        expect(response).to redirect_to(project_path(project))
-      end
-    end
-
-    context 'with an authenticated user' do
-      before do
-        sign_in(user)
-      end
-
-      context 'when user has access to the project' do
-        before do
-          project.add_developer(user)
-        end
-
-        it_behaves_like 'resolvable endpoint'
-      end
-
-      context 'when user has no access to the project' do
-        it 'gives 404 for existing project' do
-          get :resolve, params: { id: project.id }
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
-      end
-
-      it 'gives 404 for non-existing project' do
-        get :resolve, params: { id: '0' }
-
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
-    end
-
-    context 'non authenticated user' do
-      context 'with a public project' do
-        let(:project) { public_project }
-
-        it_behaves_like 'resolvable endpoint'
-      end
-
-      it 'gives 404 for private project' do
-        get :resolve, params: { id: project.id }
-
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
-    end
-  end
-
   it 'updates Service Desk attributes' do
     project.add_maintainer(user)
     sign_in(user)

spec/requests/projects/redirect_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe "Projects::RedirectController requests" do
+  using RSpec::Parameterized::TableSyntax
+
+  let_it_be(:private_project) { create(:project, :private) }
+  let_it_be(:public_project) { create(:project, :public) }
+  let_it_be(:user) { create(:user) }
+
+  before_all do
+    private_project.add_developer(user)
+  end
+
+  describe 'GET redirect_from_id' do
+    where(:authenticated, :project, :is_found) do
+      true  | ref(:private_project)        | true
+      false | ref(:private_project)        | false
+      true  | ref(:public_project)         | true
+      false | ref(:public_project)         | true
+      true  | build(:project, id: 0)       | false
+    end
+
+    with_them do
+      before do
+        sign_in(user) if authenticated
+
+        get "/projects/#{project.id}"
+      end
+
+      if params[:is_found]
+        it 'redirects to the project page' do
+          expect(response).to have_gitlab_http_status(:found)
+          expect(response).to redirect_to(project_path(project))
+        end
+      else
+        it 'gives 404' do
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+    end
+  end
+
+  # This is a regression test for https://gitlab.com/gitlab-org/gitlab/-/issues/351058
+  context 'with sourcegraph enabled' do
+    let_it_be(:sourcegraph_url) { 'https://sourcegraph.test' }
+
+    before do
+      allow(Gitlab::CurrentSettings).to receive(:sourcegraph_url).and_return(sourcegraph_url)
+      allow(Gitlab::CurrentSettings).to receive(:sourcegraph_enabled).and_return(true)
+
+      sign_in(user)
+    end
+
+    context 'with projects/:id route' do
+      subject { get "/projects/#{public_project.id}" }
+
+      it 'redirects successfully' do
+        subject
+
+        expect(response).to redirect_to(project_path(public_project))
+      end
+    end
+  end
+end

spec/routing/project_routing_spec.rb

@@ -70,9 +70,11 @@ RSpec.describe 'project routing' do
         route_to('projects#preview_markdown', namespace_id: 'gitlab', id: 'gitlabhq')
       )
     end
+  end
 
-    it 'to #resolve' do
-      expect(get('/projects/1')).to route_to('projects#resolve', id: '1')
+  describe Projects::RedirectController, 'routing' do
+    it 'to #redirect_from_id' do
+      expect(get('/projects/1')).to route_to('projects/redirect#redirect_from_id', id: '1')
     end
   end