Commit 406a452d authored by Nick Thomas's avatar Nick Thomas

Don't expose project existence by redirecting from its .git URL

If you visit /group/project.git in a web browser, you are redirected to
/group/project as long as the project exists. This is a good idea, but
we should only do it when a user is authorized to see the project.
Doing it unconditionally means that we leak the fact that the project
exists to unauthorized users.
parent 3ba3bf52
...@@ -502,13 +502,15 @@ class ProjectsController < Projects::ApplicationController ...@@ -502,13 +502,15 @@ class ProjectsController < Projects::ApplicationController
render_404 unless Gitlab::CurrentSettings.project_export_enabled? render_404 unless Gitlab::CurrentSettings.project_export_enabled?
end end
# Redirect from localhost/group/project.git to localhost/group/project
def redirect_git_extension def redirect_git_extension
# Redirect from return unless params[:format] == 'git'
# localhost/group/project.git
# to # `project` calls `find_routable!`, so this will trigger the usual not-found
# localhost/group/project # behaviour when the user isn't authorized to see the project
# return unless project
redirect_to request.original_url.sub(%r{\.git/?\Z}, '') if params[:format] == 'git'
redirect_to(request.original_url.sub(%r{\.git/?\Z}, ''))
end end
def whitelist_query_limiting def whitelist_query_limiting
......
---
title: Don't expose project existence by redirecting from its .git URL
merge_request: 52818
author:
type: fixed
...@@ -5,6 +5,7 @@ require('spec_helper') ...@@ -5,6 +5,7 @@ require('spec_helper')
RSpec.describe ProjectsController do RSpec.describe ProjectsController do
include ExternalAuthorizationServiceHelpers include ExternalAuthorizationServiceHelpers
include ProjectForksHelper include ProjectForksHelper
using RSpec::Parameterized::TableSyntax
let_it_be(:project, reload: true) { create(:project, service_desk_enabled: false) } let_it_be(:project, reload: true) { create(:project, service_desk_enabled: false) }
let_it_be(:public_project) { create(:project, :public) } let_it_be(:public_project) { create(:project, :public) }
...@@ -324,14 +325,39 @@ RSpec.describe ProjectsController do ...@@ -324,14 +325,39 @@ RSpec.describe ProjectsController do
end end
end end
context "redirection from http://someproject.git" do context 'redirection from http://someproject.git' do
it 'redirects to project page (format.html)' do where(:user_type, :project_visibility, :expected_redirect) do
project = create(:project, :public) :anonymous | :public | :redirect_to_project
:anonymous | :internal | :redirect_to_signup
:anonymous | :private | :redirect_to_signup
get :show, params: { namespace_id: project.namespace, id: project }, format: :git :signed_in | :public | :redirect_to_project
:signed_in | :internal | :redirect_to_project
:signed_in | :private | nil
expect(response).to have_gitlab_http_status(:found) :member | :public | :redirect_to_project
expect(response).to redirect_to(namespace_project_path) :member | :internal | :redirect_to_project
:member | :private | :redirect_to_project
end
with_them do
let(:redirect_to_signup) { new_user_session_path }
let(:redirect_to_project) { project_path(project) }
let(:expected_status) { expected_redirect ? :found : :not_found }
before do
project.update!(visibility: project_visibility.to_s)
project.team.add_user(user, :guest) if user_type == :member
sign_in(user) unless user_type == :anonymous
end
it 'returns the expected status' do
get :show, params: { namespace_id: project.namespace, id: project }, format: :git
expect(response).to have_gitlab_http_status(expected_status)
expect(response).to redirect_to(send(expected_redirect)) if expected_status == :found
end
end end
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