Commit 248ab5c8 authored by Paul Slaughter's avatar Paul Slaughter Committed by Luke Duncalfe

Fix sourcegraph breaking on projects/:id

- https://gitlab.com/gitlab-org/gitlab/-/issues/351058
- It does this by moving projects#resolve to its own
  controller.
- Now the ProjectsController can assume :namespace_id
  exists in the route.

Changelog: fixed
parent aa3c1bef
# 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
......@@ -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
......
......@@ -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
......
......@@ -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)
......
# 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
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment