Commit e8972c11 authored by Michael Kozono's avatar Michael Kozono

Clarify error messages

And refactor to self-document a little better.
parent 23d37382
...@@ -14,8 +14,8 @@ module Gitlab ...@@ -14,8 +14,8 @@ module Gitlab
project_not_found: 'The project you were looking for could not be found.', project_not_found: 'The project you were looking for could not be found.',
account_blocked: 'Your account has been blocked.', account_blocked: 'Your account has been blocked.',
command_not_allowed: "The command you're trying to execute is not allowed.", command_not_allowed: "The command you're trying to execute is not allowed.",
upload_pack_disabled_in_config: 'The command "git-upload-pack" is not allowed.', upload_pack_disabled_over_http: 'Pulling over HTTP is not allowed.',
receive_pack_disabled_in_config: 'The command "git-receive-pack" is not allowed.' receive_pack_disabled_over_http: 'Pushing over HTTP is not allowed.'
}.freeze }.freeze
DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }.freeze DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }.freeze
...@@ -94,12 +94,22 @@ module Gitlab ...@@ -94,12 +94,22 @@ module Gitlab
end end
def check_command_disabled!(cmd) def check_command_disabled!(cmd)
if http? if upload_pack?(cmd)
if upload_pack?(cmd) && !Gitlab.config.gitlab_shell.upload_pack check_upload_pack_disabled!
raise UnauthorizedError, ERROR_MESSAGES[:upload_pack_disabled_in_config] elsif receive_pack?(cmd)
elsif receive_pack?(cmd) && !Gitlab.config.gitlab_shell.receive_pack check_receive_pack_disabled!
raise UnauthorizedError, ERROR_MESSAGES[:receive_pack_disabled_in_config] end
end end
def check_upload_pack_disabled!
if http? && upload_pack_disabled_over_http?
raise UnauthorizedError, ERROR_MESSAGES[:upload_pack_disabled_over_http]
end
end
def check_receive_pack_disabled!
if http? && receive_pack_disabled_over_http?
raise UnauthorizedError, ERROR_MESSAGES[:receive_pack_disabled_over_http]
end end
end end
...@@ -215,6 +225,14 @@ module Gitlab ...@@ -215,6 +225,14 @@ module Gitlab
command == 'git-receive-pack' command == 'git-receive-pack'
end end
def upload_pack_disabled_over_http?
!Gitlab.config.gitlab_shell.upload_pack
end
def receive_pack_disabled_over_http?
!Gitlab.config.gitlab_shell.receive_pack
end
protected protected
def user def user
......
...@@ -170,7 +170,7 @@ describe Gitlab::GitAccess, lib: true do ...@@ -170,7 +170,7 @@ describe Gitlab::GitAccess, lib: true do
end end
context 'when calling git-upload-pack' do context 'when calling git-upload-pack' do
it { expect { pull_access_check }.to raise_unauthorized('The command "git-upload-pack" is not allowed.') } it { expect { pull_access_check }.to raise_unauthorized('Pulling over HTTP is not allowed.') }
end end
context 'when calling git-receive-pack' do context 'when calling git-receive-pack' do
...@@ -184,7 +184,7 @@ describe Gitlab::GitAccess, lib: true do ...@@ -184,7 +184,7 @@ describe Gitlab::GitAccess, lib: true do
end end
context 'when calling git-receive-pack' do context 'when calling git-receive-pack' do
it { expect { push_access_check }.to raise_unauthorized('The command "git-receive-pack" is not allowed.') } it { expect { push_access_check }.to raise_unauthorized('Pushing over HTTP is not allowed.') }
end end
context 'when calling git-upload-pack' do context 'when calling git-upload-pack' do
......
...@@ -254,7 +254,7 @@ describe 'Git HTTP requests', lib: true do ...@@ -254,7 +254,7 @@ describe 'Git HTTP requests', lib: true do
it 'rejects pushes with 403 Forbidden' do it 'rejects pushes with 403 Forbidden' do
upload(path, env) do |response| upload(path, env) do |response|
expect(response).to have_http_status(:forbidden) expect(response).to have_http_status(:forbidden)
expect(response.body).to eq(git_access_error(:receive_pack_disabled_in_config)) expect(response.body).to eq(git_access_error(:receive_pack_disabled_over_http))
end end
end end
end end
...@@ -265,7 +265,7 @@ describe 'Git HTTP requests', lib: true do ...@@ -265,7 +265,7 @@ describe 'Git HTTP requests', lib: true do
download(path, env) do |response| download(path, env) do |response|
expect(response).to have_http_status(:forbidden) expect(response).to have_http_status(:forbidden)
expect(response.body).to eq(git_access_error(:upload_pack_disabled_in_config)) expect(response.body).to eq(git_access_error(:upload_pack_disabled_over_http))
end end
end end
end end
...@@ -541,7 +541,7 @@ describe 'Git HTTP requests', lib: true do ...@@ -541,7 +541,7 @@ describe 'Git HTTP requests', lib: true do
context 'when the repo does not exist' do context 'when the repo does not exist' do
let(:project) { create(:empty_project) } let(:project) { create(:empty_project) }
it 'rejects pulls with 403 Forbidden' do it 'rejects pulls with 403 Forbidden' do
clone_get path, env clone_get path, env
......
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