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

This commit is contained in:
Michael Kozono 2017-06-15 17:04:17 -07:00
parent af784cc6e2
commit 8ef3bc5d75
3 changed files with 36 additions and 39 deletions

View file

@ -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)
parse_repo_path unless 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
@project
end
# 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
# 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)
parse_repo_path unless defined?(@wiki)
_, suffix = project_id_with_suffix
@wiki = suffix == '.wiki.git'
end
def render_not_found
render plain: 'Not Found', status: :not_found
@wiki
end
def handle_basic_authentication(login, password)

View file

@ -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

View file

@ -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