Commit a9891b4b authored by Michael Kozono's avatar Michael Kozono
Browse files

Render GitAccess error message if authenticated

parent 1050451b
......@@ -75,13 +75,16 @@ class Projects::GitHttpClientController < Projects::ApplicationController
def project
return @project if defined?(@project)
@project = Project.find_by_full_path(requested_path, follow_redirects: true)
end
def redirected_path
requested_path if project.full_path != requested_path
end
def requested_path
project_id, _ = project_id_with_suffix
@project =
if project_id.blank?
nil
else
Project.find_by_full_path("#{params[:namespace_id]}/#{project_id}")
end
"#{params[:namespace_id]}/#{project_id}"
end
# This method returns two values so that we can parse
......
......@@ -11,7 +11,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
elsif receive_pack? && receive_pack_allowed?
render_ok
elsif http_blocked?
render_http_not_allowed
render_git_access_error_message
else
render_denied
end
......@@ -62,23 +62,19 @@ class Projects::GitHttpController < Projects::GitHttpClientController
render json: Gitlab::Workhorse.git_http_ok(repository, wiki?, user, action_name)
end
def render_http_not_allowed
def render_git_access_error_message
render plain: access_check.message, status: :forbidden
end
def render_denied
if user && can?(user, :read_project, project)
render plain: access_denied_message, status: :forbidden
render_git_access_error_message
else
# Do not leak information about project existence
render_not_found
end
end
def access_denied_message
'Access denied'
end
def upload_pack_allowed?
return false unless Gitlab.config.gitlab_shell.upload_pack
......@@ -86,7 +82,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
end
def access
@access ||= access_klass.new(user, project, 'http', authentication_abilities: authentication_abilities)
@access ||= access_klass.new(user, project, 'http', authentication_abilities: authentication_abilities, redirected_path: redirected_path)
end
def access_check
......
---
title: Add "project was moved" messages when rejecting git push/pull
merge_request: 11259
author:
......@@ -141,30 +141,30 @@ describe 'Git HTTP requests', lib: true do
context 'when the repo is public' do
context 'but the repo is disabled' do
it 'does not allow to clone the repo' do
project = create(:project, :public, :repository_disabled)
let(:project) { create(:project, :public, :repository_disabled) }
download("#{project.path_with_namespace}.git", {}) do |response|
it 'does not allow to clone the repo' do
download(path, {}) do |response|
expect(response).to have_http_status(:unauthorized)
end
end
end
context 'but the repo is enabled' do
it 'allows to clone the repo' do
project = create(:project, :public, :repository_enabled)
let(:project) { create(:project, :public, :repository_enabled) }
download("#{project.path_with_namespace}.git", {}) do |response|
it 'allows to clone the repo' do
download(path, {}) do |response|
expect(response).to have_http_status(:ok)
end
end
end
context 'but only project members are allowed' do
it 'does not allow to clone the repo' do
project = create(:project, :public, :repository_private)
let(:project) { create(:project, :public, :repository_private) }
download("#{project.path_with_namespace}.git", {}) do |response|
it 'does not allow to clone the repo' do
download(path, {}) do |response|
expect(response).to have_http_status(:unauthorized)
end
end
......@@ -264,6 +264,26 @@ describe 'Git HTTP requests', lib: true do
expect(user_activity(user)).to be_present
end
end
context 'and the user requests a redirected path' do
let!(:redirect) { project.route.create_redirect('foo/bar') }
let(:path) { "#{redirect.path}.git" }
it 'downloads get status 403 with "project was moved" message' do
clone_get(path, env)
message = "Project '#{redirect.path}' was moved to '#{project.full_path}'.\n\nPlease update your Git remote and try again:\n\n git remote set-url origin #{project.http_url_to_repo}"
expect(response).to have_http_status(:forbidden)
expect(response.body).to match(message)
end
it 'uploads get status 403 with "project was moved" message' do
upload(path, env) do |response|
message = "Project '#{redirect.path}' was moved to '#{project.full_path}'.\n\nPlease update your Git remote and try again:\n\n git remote set-url origin #{project.http_url_to_repo}"
expect(response).to have_http_status(:forbidden)
expect(response.body).to match(message)
end
end
end
end
context "when an oauth token is provided" do
......@@ -508,7 +528,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
......
......@@ -29,7 +29,7 @@ describe PostReceive do
context "with an absolute path as the project identifier" do
it "searches the project by full path" do
expect(Project).to receive(:find_by_full_path).with(project.full_path).and_call_original
expect(Project).to receive(:find_by_full_path).with(project.full_path, follow_redirects: true).and_call_original
described_class.new.perform(pwd(project), key_id, base64_changes)
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