Align git return error codes

In this commit we rename GitAccess::UnauthorizedError
to ForbiddenError to align it with the kind of returned
error 403.

Besides we also fix the error code returned through
the internal api and align it to use 403 as well.
parent ee354d8f
...@@ -7,7 +7,7 @@ module Repositories ...@@ -7,7 +7,7 @@ module Repositories
before_action :access_check before_action :access_check
prepend_before_action :deny_head_requests, only: [:info_refs] prepend_before_action :deny_head_requests, only: [:info_refs]
rescue_from Gitlab::GitAccess::UnauthorizedError, with: :render_403_with_exception rescue_from Gitlab::GitAccess::ForbiddenError, with: :render_403_with_exception
rescue_from Gitlab::GitAccess::NotFoundError, with: :render_404_with_exception rescue_from Gitlab::GitAccess::NotFoundError, with: :render_404_with_exception
rescue_from Gitlab::GitAccess::ProjectCreationError, with: :render_422_with_exception rescue_from Gitlab::GitAccess::ProjectCreationError, with: :render_422_with_exception
rescue_from Gitlab::GitAccess::TimeoutError, with: :render_503_with_exception rescue_from Gitlab::GitAccess::TimeoutError, with: :render_503_with_exception
......
...@@ -4,13 +4,13 @@ module Lfs ...@@ -4,13 +4,13 @@ module Lfs
class LockFileService < BaseService class LockFileService < BaseService
def execute def execute
unless can?(current_user, :push_code, project) unless can?(current_user, :push_code, project)
raise Gitlab::GitAccess::UnauthorizedError, 'You have no permissions' raise Gitlab::GitAccess::ForbiddenError, 'You have no permissions'
end end
create_lock! create_lock!
rescue ActiveRecord::RecordNotUnique rescue ActiveRecord::RecordNotUnique
error('already locked', 409, current_lock) error('already locked', 409, current_lock)
rescue Gitlab::GitAccess::UnauthorizedError => ex rescue Gitlab::GitAccess::ForbiddenError => ex
error(ex.message, 403) error(ex.message, 403)
rescue => ex rescue => ex
error(ex.message, 500) error(ex.message, 500)
......
...@@ -4,11 +4,11 @@ module Lfs ...@@ -4,11 +4,11 @@ module Lfs
class UnlockFileService < BaseService class UnlockFileService < BaseService
def execute def execute
unless can?(current_user, :push_code, project) unless can?(current_user, :push_code, project)
raise Gitlab::GitAccess::UnauthorizedError, _('You have no permissions') raise Gitlab::GitAccess::ForbiddenError, _('You have no permissions')
end end
unlock_file unlock_file
rescue Gitlab::GitAccess::UnauthorizedError => ex rescue Gitlab::GitAccess::ForbiddenError => ex
error(ex.message, 403) error(ex.message, 403)
rescue ActiveRecord::RecordNotFound rescue ActiveRecord::RecordNotFound
error(_('Lock not found'), 404) error(_('Lock not found'), 404)
......
---
title: Align git returned error codes
merge_request: 25936
author:
type: changed
...@@ -56,7 +56,7 @@ module EE ...@@ -56,7 +56,7 @@ module EE
return :geo unless geo_push_user_headers_provided? return :geo unless geo_push_user_headers_provided?
return geo_push_user.user if geo_push_user.user return geo_push_user.user if geo_push_user.user
raise ::Gitlab::GitAccess::UnauthorizedError, 'Geo push user is invalid.' raise ::Gitlab::GitAccess::ForbiddenError, 'Geo push user is invalid.'
end end
override :authenticate_user override :authenticate_user
......
...@@ -80,7 +80,7 @@ module EE ...@@ -80,7 +80,7 @@ module EE
"File name #{diff.new_path} was blacklisted by the pattern #{blacklisted_regex}." "File name #{diff.new_path} was blacklisted by the pattern #{blacklisted_regex}."
end end
rescue ::PushRule::MatchError => e rescue ::PushRule::MatchError => e
raise ::Gitlab::GitAccess::UnauthorizedError, e.message raise ::Gitlab::GitAccess::ForbiddenError, e.message
end end
end end
end end
......
...@@ -14,13 +14,13 @@ module EE ...@@ -14,13 +14,13 @@ module EE
logger.log_timed(LOG_MESSAGE) do logger.log_timed(LOG_MESSAGE) do
unless branch_name_allowed_by_push_rule? unless branch_name_allowed_by_push_rule?
message = ERROR_MESSAGE % { branch_name_regex: push_rule.branch_name_regex } message = ERROR_MESSAGE % { branch_name_regex: push_rule.branch_name_regex }
raise ::Gitlab::GitAccess::UnauthorizedError.new(message) raise ::Gitlab::GitAccess::ForbiddenError.new(message)
end end
end end
PushRules::CommitCheck.new(change_access).validate! PushRules::CommitCheck.new(change_access).validate!
rescue ::PushRule::MatchError => e rescue ::PushRule::MatchError => e
raise ::Gitlab::GitAccess::UnauthorizedError, e.message raise ::Gitlab::GitAccess::ForbiddenError, e.message
end end
private private
......
...@@ -27,14 +27,14 @@ module EE ...@@ -27,14 +27,14 @@ module EE
end end
end end
rescue ::PushRule::MatchError => e rescue ::PushRule::MatchError => e
raise ::Gitlab::GitAccess::UnauthorizedError, e.message raise ::Gitlab::GitAccess::ForbiddenError, e.message
end end
private private
def push_rule_commit_check(commit) def push_rule_commit_check(commit)
error = check_commit(commit) error = check_commit(commit)
raise ::Gitlab::GitAccess::UnauthorizedError, error if error raise ::Gitlab::GitAccess::ForbiddenError, error if error
end end
# If commit does not pass push rule validation the whole push should be rejected. # If commit does not pass push rule validation the whole push should be rejected.
......
...@@ -19,7 +19,7 @@ module EE ...@@ -19,7 +19,7 @@ module EE
end end
if large_blob if large_blob
raise ::Gitlab::GitAccess::UnauthorizedError, %Q{File "#{large_blob.path}" is larger than the allowed size of #{max_file_size} MB} raise ::Gitlab::GitAccess::ForbiddenError, %Q{File "#{large_blob.path}" is larger than the allowed size of #{max_file_size} MB}
end end
end end
end end
......
...@@ -10,7 +10,7 @@ module EE ...@@ -10,7 +10,7 @@ module EE
logger.log_timed("Checking if you are allowed to delete a tag...") do logger.log_timed("Checking if you are allowed to delete a tag...") do
if tag_deletion_denied_by_push_rule? if tag_deletion_denied_by_push_rule?
raise ::Gitlab::GitAccess::UnauthorizedError, 'You cannot delete a tag' raise ::Gitlab::GitAccess::ForbiddenError, 'You cannot delete a tag'
end end
end end
end end
......
...@@ -76,13 +76,13 @@ module EE ...@@ -76,13 +76,13 @@ module EE
def check_geo_license! def check_geo_license!
if ::Gitlab::Geo.secondary? && !::Gitlab::Geo.license_allows? if ::Gitlab::Geo.secondary? && !::Gitlab::Geo.license_allows?
raise ::Gitlab::GitAccess::UnauthorizedError, 'Your current license does not have GitLab Geo add-on enabled.' raise ::Gitlab::GitAccess::ForbiddenError, 'Your current license does not have GitLab Geo add-on enabled.'
end end
end end
def check_smartcard_access! def check_smartcard_access!
unless can_access_without_new_smartcard_login? unless can_access_without_new_smartcard_login?
raise ::Gitlab::GitAccess::UnauthorizedError, 'Project requires smartcard login. Please login to GitLab using a smartcard.' raise ::Gitlab::GitAccess::ForbiddenError, 'Project requires smartcard login. Please login to GitLab using a smartcard.'
end end
end end
...@@ -98,14 +98,14 @@ module EE ...@@ -98,14 +98,14 @@ module EE
def check_size_before_push! def check_size_before_push!
if check_size_limit? && project.above_size_limit? if check_size_limit? && project.above_size_limit?
raise ::Gitlab::GitAccess::UnauthorizedError, ::Gitlab::RepositorySizeError.new(project).push_error raise ::Gitlab::GitAccess::ForbiddenError, ::Gitlab::RepositorySizeError.new(project).push_error
end end
end end
def check_if_license_blocks_changes! def check_if_license_blocks_changes!
if ::License.block_changes? if ::License.block_changes?
message = ::LicenseHelper.license_message(signed_in: true, is_admin: (user && user.admin?)) message = ::LicenseHelper.license_message(signed_in: true, is_admin: (user && user.admin?))
raise ::Gitlab::GitAccess::UnauthorizedError, strip_tags(message) raise ::Gitlab::GitAccess::ForbiddenError, strip_tags(message)
end end
end end
...@@ -154,7 +154,7 @@ module EE ...@@ -154,7 +154,7 @@ module EE
def check_size_against_limit(size) def check_size_against_limit(size)
if project.changes_will_exceed_size_limit?(size) if project.changes_will_exceed_size_limit?(size)
raise ::Gitlab::GitAccess::UnauthorizedError, ::Gitlab::RepositorySizeError.new(project).new_changes_error raise ::Gitlab::GitAccess::ForbiddenError, ::Gitlab::RepositorySizeError.new(project).new_changes_error
end end
end end
......
...@@ -15,13 +15,13 @@ module Gitlab ...@@ -15,13 +15,13 @@ module Gitlab
def check_protocol! def check_protocol!
if protocol != 'web' if protocol != 'web'
raise ::Gitlab::GitAccess::UnauthorizedError, "Designs are only accessible using the web interface" raise ::Gitlab::GitAccess::ForbiddenError, "Designs are only accessible using the web interface"
end end
end end
def check_can_create_design! def check_can_create_design!
unless user&.can?(:create_design, project) unless user&.can?(:create_design, project)
raise ::Gitlab::GitAccess::UnauthorizedError, "You are not allowed to manage designs of this project" raise ::Gitlab::GitAccess::ForbiddenError, "You are not allowed to manage designs of this project"
end end
end end
end end
......
...@@ -12,13 +12,13 @@ describe EE::Gitlab::Checks::PushRules::BranchCheck do ...@@ -12,13 +12,13 @@ describe EE::Gitlab::Checks::PushRules::BranchCheck do
it_behaves_like 'check ignored when push rule unlicensed' it_behaves_like 'check ignored when push rule unlicensed'
it 'rejects the branch that is not allowed' do it 'rejects the branch that is not allowed' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Branch name does not follow the pattern '^(w*)$'") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Branch name does not follow the pattern '^(w*)$'")
end end
it 'returns an error if the regex is invalid' do it 'returns an error if the regex is invalid' do
push_rule.branch_name_regex = '+' push_rule.branch_name_regex = '+'
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /\ARegular expression '\+' is invalid/) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, /\ARegular expression '\+' is invalid/)
end end
context 'when the ref is not a branch ref' do context 'when the ref is not a branch ref' do
...@@ -35,7 +35,7 @@ describe EE::Gitlab::Checks::PushRules::BranchCheck do ...@@ -35,7 +35,7 @@ describe EE::Gitlab::Checks::PushRules::BranchCheck do
end end
it 'rejects the branch that is not allowed' do it 'rejects the branch that is not allowed' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Branch name does not follow the pattern '^(w*)$'") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Branch name does not follow the pattern '^(w*)$'")
end end
end end
......
...@@ -12,27 +12,27 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do ...@@ -12,27 +12,27 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do
it_behaves_like 'check ignored when push rule unlicensed' it_behaves_like 'check ignored when push rule unlicensed'
it 'returns an error if the rule fails due to missing required characters' do it 'returns an error if the rule fails due to missing required characters' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Commit message does not follow the pattern '#{push_rule.commit_message_regex}'") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Commit message does not follow the pattern '#{push_rule.commit_message_regex}'")
end end
it 'returns an error if the rule fails due to forbidden characters' do it 'returns an error if the rule fails due to forbidden characters' do
push_rule.commit_message_regex = nil push_rule.commit_message_regex = nil
push_rule.commit_message_negative_regex = '.*' push_rule.commit_message_negative_regex = '.*'
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Commit message contains the forbidden pattern '#{push_rule.commit_message_negative_regex}'") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Commit message contains the forbidden pattern '#{push_rule.commit_message_negative_regex}'")
end end
it 'returns an error if the regex is invalid' do it 'returns an error if the regex is invalid' do
push_rule.commit_message_regex = '+' push_rule.commit_message_regex = '+'
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /\ARegular expression '\+' is invalid/) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, /\ARegular expression '\+' is invalid/)
end end
it 'returns an error if the negative regex is invalid' do it 'returns an error if the negative regex is invalid' do
push_rule.commit_message_regex = nil push_rule.commit_message_regex = nil
push_rule.commit_message_negative_regex = '+' push_rule.commit_message_negative_regex = '+'
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /\ARegular expression '\+' is invalid/) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, /\ARegular expression '\+' is invalid/)
end end
end end
...@@ -49,19 +49,19 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do ...@@ -49,19 +49,19 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do
it 'returns an error if the rule fails for the committer' do it 'returns an error if the rule fails for the committer' do
allow_any_instance_of(Commit).to receive(:committer_email).and_return('ana@invalid.com') allow_any_instance_of(Commit).to receive(:committer_email).and_return('ana@invalid.com')
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Committer's email 'ana@invalid.com' does not follow the pattern '.*@valid.com'") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Committer's email 'ana@invalid.com' does not follow the pattern '.*@valid.com'")
end end
it 'returns an error if the rule fails for the author' do it 'returns an error if the rule fails for the author' do
allow_any_instance_of(Commit).to receive(:author_email).and_return('joan@invalid.com') allow_any_instance_of(Commit).to receive(:author_email).and_return('joan@invalid.com')
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Author's email 'joan@invalid.com' does not follow the pattern '.*@valid.com'") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Author's email 'joan@invalid.com' does not follow the pattern '.*@valid.com'")
end end
it 'returns an error if the regex is invalid' do it 'returns an error if the regex is invalid' do
push_rule.author_email_regex = '+' push_rule.author_email_regex = '+'
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /\ARegular expression '\+' is invalid/) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, /\ARegular expression '\+' is invalid/)
end end
end end
...@@ -74,7 +74,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do ...@@ -74,7 +74,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do
allow_any_instance_of(Commit).to receive(:author_email).and_return(user_email) allow_any_instance_of(Commit).to receive(:author_email).and_return(user_email)
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Author '#{user_email}' is not a member of team") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Author '#{user_email}' is not a member of team")
end end
it 'returns true when private commit email was associated to a user' do it 'returns true when private commit email was associated to a user' do
...@@ -93,7 +93,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do ...@@ -93,7 +93,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do
it_behaves_like 'check ignored when push rule unlicensed' it_behaves_like 'check ignored when push rule unlicensed'
it 'returns an error if the commit author is not a GitLab member' do it 'returns an error if the commit author is not a GitLab member' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Author 'some@mail.com' is not a member of team") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Author 'some@mail.com' is not a member of team")
end end
end end
end end
...@@ -119,7 +119,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do ...@@ -119,7 +119,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do
end end
it 'returns an error' do it 'returns an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Commit must be signed with a GPG key") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Commit must be signed with a GPG key")
end end
end end
end end
...@@ -131,7 +131,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do ...@@ -131,7 +131,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do
end end
it 'returns an error' do it 'returns an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Commit must be signed with a GPG key") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Commit must be signed with a GPG key")
end end
context 'but the change is made in the web application' do context 'but the change is made in the web application' do
...@@ -190,7 +190,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do ...@@ -190,7 +190,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do
allow_any_instance_of(Commit).to receive(:committer_email).and_return(user_email) allow_any_instance_of(Commit).to receive(:committer_email).and_return(user_email)
expect { subject.validate! } expect { subject.validate! }
.to raise_error(Gitlab::GitAccess::UnauthorizedError, .to raise_error(Gitlab::GitAccess::ForbiddenError,
"You cannot push commits for '#{user_email}'. You can only push commits that were committed with one of your own verified emails.") "You cannot push commits for '#{user_email}'. You can only push commits that were committed with one of your own verified emails.")
end end
end end
...@@ -216,7 +216,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do ...@@ -216,7 +216,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do
allow_any_instance_of(Commit).to receive(:committer_email).and_return('secondary_email@user.com') allow_any_instance_of(Commit).to receive(:committer_email).and_return('secondary_email@user.com')
expect { subject.validate! } expect { subject.validate! }
.to raise_error(Gitlab::GitAccess::UnauthorizedError, .to raise_error(Gitlab::GitAccess::ForbiddenError,
"Committer email 'secondary_email@user.com' is not verified.") "Committer email 'secondary_email@user.com' is not verified.")
end end
...@@ -224,7 +224,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do ...@@ -224,7 +224,7 @@ describe EE::Gitlab::Checks::PushRules::CommitCheck do
allow_any_instance_of(Commit).to receive(:committer_email).and_return('some@mail.com') allow_any_instance_of(Commit).to receive(:committer_email).and_return('some@mail.com')
expect { subject.validate! } expect { subject.validate! }
.to raise_error(Gitlab::GitAccess::UnauthorizedError, .to raise_error(Gitlab::GitAccess::ForbiddenError,
"You cannot push commits for 'some@mail.com'. You can only push commits that were committed with one of your own verified emails.") "You cannot push commits for 'some@mail.com'. You can only push commits that were committed with one of your own verified emails.")
end end
end end
......
...@@ -19,7 +19,7 @@ describe EE::Gitlab::Checks::PushRules::FileSizeCheck do ...@@ -19,7 +19,7 @@ describe EE::Gitlab::Checks::PushRules::FileSizeCheck do
it_behaves_like 'check ignored when push rule unlicensed' it_behaves_like 'check ignored when push rule unlicensed'
it 'returns an error if file exceeds the maximum file size' do it 'returns an error if file exceeds the maximum file size' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "File \"file.bin\" is larger than the allowed size of 1 MB") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "File \"file.bin\" is larger than the allowed size of 1 MB")
end end
end end
end end
...@@ -14,7 +14,7 @@ describe EE::Gitlab::Checks::PushRules::TagCheck do ...@@ -14,7 +14,7 @@ describe EE::Gitlab::Checks::PushRules::TagCheck do
it_behaves_like 'check ignored when push rule unlicensed' it_behaves_like 'check ignored when push rule unlicensed'
it 'returns an error if the rule denies tag deletion' do it 'returns an error if the rule denies tag deletion' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You cannot delete a tag') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You cannot delete a tag')
end end
context 'when tag is deleted in web UI' do context 'when tag is deleted in web UI' do
......
...@@ -108,13 +108,13 @@ describe Gitlab::Checks::DiffCheck do ...@@ -108,13 +108,13 @@ describe Gitlab::Checks::DiffCheck do
it_behaves_like 'check ignored when push rule unlicensed' it_behaves_like 'check ignored when push rule unlicensed'
it "returns an error if a new or renamed filed doesn't match the file name regex" do it "returns an error if a new or renamed filed doesn't match the file name regex" do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "File name README was blacklisted by the pattern READ*.") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "File name README was blacklisted by the pattern READ*.")
end end
it 'returns an error if the regex is invalid' do it 'returns an error if the regex is invalid' do
push_rule.file_name_regex = '+' push_rule.file_name_regex = '+'
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /\ARegular expression '\+' is invalid/) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, /\ARegular expression '\+' is invalid/)
end end
end end
...@@ -164,7 +164,7 @@ describe Gitlab::Checks::DiffCheck do ...@@ -164,7 +164,7 @@ describe Gitlab::Checks::DiffCheck do
project.repository.commits_between(old_rev, new_rev) project.repository.commits_between(old_rev, new_rev)
) )
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /File name #{file_path} was blacklisted by the pattern/) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, /File name #{file_path} was blacklisted by the pattern/)
end end
end end
end end
...@@ -177,7 +177,7 @@ describe Gitlab::Checks::DiffCheck do ...@@ -177,7 +177,7 @@ describe Gitlab::Checks::DiffCheck do
it 'returns an error if the changes update a path locked by another user' do it 'returns an error if the changes update a path locked by another user' do
path_lock path_lock
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "The path 'README' is locked by #{path_lock.user.name}") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "The path 'README' is locked by #{path_lock.user.name}")
end end
it 'memoizes the validate_path_locks? call' do it 'memoizes the validate_path_locks? call' do
......
...@@ -28,7 +28,7 @@ describe Gitlab::GitAccessDesign do ...@@ -28,7 +28,7 @@ describe Gitlab::GitAccessDesign do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
it "raises an error " do it "raises an error " do
expect { subject }.to raise_error(::Gitlab::GitAccess::UnauthorizedError) expect { subject }.to raise_error(::Gitlab::GitAccess::ForbiddenError)
end end
end end
...@@ -36,7 +36,7 @@ describe Gitlab::GitAccessDesign do ...@@ -36,7 +36,7 @@ describe Gitlab::GitAccessDesign do
let(:protocol) { 'https' } let(:protocol) { 'https' }
it "raises an error " do it "raises an error " do
expect { subject }.to raise_error(::Gitlab::GitAccess::UnauthorizedError) expect { subject }.to raise_error(::Gitlab::GitAccess::ForbiddenError)
end end
end end
......
...@@ -49,13 +49,13 @@ describe Gitlab::GitAccess do ...@@ -49,13 +49,13 @@ describe Gitlab::GitAccess do
it 'returns false when a commit message is missing required matches (positive regex match)' do it 'returns false when a commit message is missing required matches (positive regex match)' do
project.create_push_rule(commit_message_regex: "@only.com") project.create_push_rule(commit_message_regex: "@only.com")
expect { push_changes(changes) }.to raise_error(described_class::UnauthorizedError) expect { push_changes(changes) }.to raise_error(described_class::ForbiddenError)
end end
it 'returns false when a commit message contains forbidden characters (negative regex match)' do it 'returns false when a commit message contains forbidden characters (negative regex match)' do
project.create_push_rule(commit_message_negative_regex: "@gmail.com") project.create_push_rule(commit_message_negative_regex: "@gmail.com")
expect { push_changes(changes) }.to raise_error(described_class::UnauthorizedError) expect { push_changes(changes) }.to raise_error(described_class::ForbiddenError)
end end
it 'returns true for tags' do it 'returns true for tags' do
...@@ -101,7 +101,7 @@ describe Gitlab::GitAccess do ...@@ -101,7 +101,7 @@ describe Gitlab::GitAccess do
project.create_push_rule(commit_message_regex: "Change some files") project.create_push_rule(commit_message_regex: "Change some files")
# push to new branch, so use a blank old rev and new ref # push to new branch, so use a blank old rev and new ref
expect { push_changes("#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError) expect { push_changes("#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::ForbiddenError)
end end
end end
...@@ -113,7 +113,7 @@ describe Gitlab::GitAccess do ...@@ -113,7 +113,7 @@ describe Gitlab::GitAccess do
end end
it 'returns false for non-member user' do it 'returns false for non-member user' do
expect { push_changes(changes) }.to raise_error(described_class::UnauthorizedError) expect { push_changes(changes) }.to raise_error(described_class::ForbiddenError)
end end
it 'returns true if committer is a gitlab member' do it 'returns true if committer is a gitlab member' do
...@@ -136,7 +136,7 @@ describe Gitlab::GitAccess do ...@@ -136,7 +136,7 @@ describe Gitlab::GitAccess do
it 'returns false when filename is prohibited' do it 'returns false when filename is prohibited' do
project.create_push_rule(file_name_regex: "jpg$") project.create_push_rule(file_name_regex: "jpg$")
expect { push_changes(changes) }.to raise_error(described_class::UnauthorizedError) expect { push_changes(changes) }.to raise_error(described_class::ForbiddenError)
end end
it 'returns true if file name is allowed' do it 'returns true if file name is allowed' do
...@@ -162,7 +162,7 @@ describe Gitlab::GitAccess do ...@@ -162,7 +162,7 @@ describe Gitlab::GitAccess do
project.create_push_rule(max_file_size: 1) project.create_push_rule(max_file_size: 1)
expect(repository.new_blobs(end_sha)).to be_present expect(repository.new_blobs(end_sha)).to be_present
expect { push_changes(changes) }.to raise_error(described_class::UnauthorizedError) expect { push_changes(changes) }.to raise_error(described_class::ForbiddenError)
end end
it "returns true when size is allowed" do it "returns true when size is allowed" do
...@@ -194,7 +194,7 @@ describe Gitlab::GitAccess do ...@@ -194,7 +194,7 @@ describe Gitlab::GitAccess do
it 'rejects the push' do it 'rejects the push' do
expect do expect do
push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_smallest_changes} refs/heads/master") push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_smallest_changes} refs/heads/master")
end.to raise_error(described_class::UnauthorizedError, /Your push has been rejected/) end.to raise_error(described_class::ForbiddenError, /Your push has been rejected/)
end end
context 'when deleting a branch' do context 'when deleting a branch' do
...@@ -244,7 +244,7 @@ describe Gitlab::GitAccess do ...@@ -244,7 +244,7 @@ describe Gitlab::GitAccess do
expect do expect do
push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_2_mb_file} refs/heads/my_branch_2") push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_2_mb_file} refs/heads/my_branch_2")
end.to raise_error(described_class::UnauthorizedError, /Your push to this repository would cause it to exceed the size limit/) end.to raise_error(described_class::ForbiddenError, /Your push to this repository would cause it to exceed the size limit/)
end end
end end
...@@ -294,7 +294,7 @@ describe Gitlab::GitAccess do ...@@ -294,7 +294,7 @@ describe Gitlab::GitAccess do
it 'rejects the push' do it 'rejects the push' do
expect do expect do
push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_2_mb_file} refs/heads/my_branch_2") push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_2_mb_file} refs/heads/my_branch_2")
end.to raise_error(described_class::UnauthorizedError, /Your push to this repository would cause it to exceed the size limit/) end.to raise_error(described_class::ForbiddenError, /Your push to this repository would cause it to exceed the size limit/)
end end
end end
...@@ -350,7 +350,7 @@ describe Gitlab::GitAccess do ...@@ -350,7 +350,7 @@ describe Gitlab::GitAccess do
end end
context 'git push' do context 'git push' do
it { expect { push_changes }.to raise_unauthorized(Gitlab::GitAccess::ERROR_MESSAGES[:upload]) } it { expect { push_changes }.to raise_forbidden(Gitlab::GitAccess::ERROR_MESSAGES[:upload]) }
end end
end end
...@@ -423,7 +423,7 @@ describe Gitlab::GitAccess do ...@@ -423,7 +423,7 @@ describe Gitlab::GitAccess do
expect(&check).not_to raise_error, expect(&check).not_to raise_error,
-> { "expected #{action} to be allowed" } -> { "expected #{action} to be allowed" }
else else
expect(&check).to raise_error(Gitlab::GitAccess::UnauthorizedError), expect(&check).to raise_error(Gitlab::GitAccess::ForbiddenError),
-> { "expected #{action} to be disallowed" } -> { "expected #{action} to be disallowed" }
end end
end end
...@@ -450,7 +450,7 @@ describe Gitlab::GitAccess do ...@@ -450,7 +450,7 @@ describe Gitlab::GitAccess do
expect(&check).not_to raise_error, expect(&check).not_to raise_error,
-> { "expected #{action} to be allowed" } -> { "expected #{action} to be allowed" }
else else
expect(&check).to raise_error(Gitlab::GitAccess::UnauthorizedError), expect(&check).to raise_error(Gitlab::GitAccess::ForbiddenError),
-> { "expected #{action} to be disallowed" } -> { "expected #{action} to be disallowed" }
end end
end end
...@@ -642,11 +642,11 @@ describe Gitlab::GitAccess do ...@@ -642,11 +642,11 @@ describe Gitlab::GitAccess do
context 'user without a smartcard session' do context 'user without a smartcard session' do
it 'does not allow pull changes' do it 'does not allow pull changes' do
expect { pull_changes }.to raise_error(Gitlab::GitAccess::UnauthorizedError) expect { pull_changes }.to raise_error(Gitlab::GitAccess::ForbiddenError)
end end
it 'does not allow push changes' do it 'does not allow push changes' do
expect { push_changes }.to raise_error(Gitlab::GitAccess::UnauthorizedError) expect { push_changes }.to raise_error(Gitlab::GitAccess::ForbiddenError)
end end
end end
...@@ -687,7 +687,7 @@ describe Gitlab::GitAccess do ...@@ -687,7 +687,7 @@ describe Gitlab::GitAccess do
access.check('git-upload-pack', changes) access.check('git-upload-pack', changes)
end end
def raise_unauthorized(message) def raise_forbidden(message)
raise_error(Gitlab::GitAccess::UnauthorizedError, message) raise_error(Gitlab::GitAccess::ForbiddenError, message)
end end
end end
...@@ -43,7 +43,8 @@ describe Gitlab::GitAccessWiki do ...@@ -43,7 +43,8 @@ describe Gitlab::GitAccessWiki do
access.check('git-receive-pack', changes) access.check('git-receive-pack', changes)
end end
def raise_unauthorized(message) # It is needed by the shared examples
raise_error(Gitlab::GitAccess::UnauthorizedError, message) def raise_forbidden(message)
raise_error(Gitlab::GitAccess::ForbiddenError, message)
end end
end end
...@@ -132,7 +132,7 @@ describe API::Internal::Base do ...@@ -132,7 +132,7 @@ describe API::Internal::Base do
protocol: 'ssh' protocol: 'ssh'
}) })
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:forbidden)
end end
end end
...@@ -237,7 +237,7 @@ describe API::Internal::Base do ...@@ -237,7 +237,7 @@ describe API::Internal::Base do
it "does not allow access" do it "does not allow access" do
subject subject
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:forbidden)
expect(json_response['message']).to eql('Project requires smartcard login. Please login to GitLab using a smartcard.') expect(json_response['message']).to eql('Project requires smartcard login. Please login to GitLab using a smartcard.')
end end
end end
......
...@@ -4,7 +4,7 @@ RSpec.shared_examples 'a read-only GitLab instance' do ...@@ -4,7 +4,7 @@ RSpec.shared_examples 'a read-only GitLab instance' do
it 'denies push access' do it 'denies push access' do
project.add_maintainer(user) project.add_maintainer(user)
expect { push_changes }.to raise_unauthorized("You can't push code to a read-only GitLab instance.") expect { push_changes }.to raise_forbidden("You can't push code to a read-only GitLab instance.")
end end
context 'for a Geo setup' do context 'for a Geo setup' do
...@@ -22,7 +22,7 @@ RSpec.shared_examples 'a read-only GitLab instance' do ...@@ -22,7 +22,7 @@ RSpec.shared_examples 'a read-only GitLab instance' do
it 'denies push access with primary present' do it 'denies push access with primary present' do
project.add_maintainer(user) project.add_maintainer(user)
expect { push_changes }.to raise_unauthorized(error_message) expect { push_changes }.to raise_forbidden(error_message)
end end
end end
......
...@@ -49,8 +49,8 @@ module API ...@@ -49,8 +49,8 @@ module API
result = access_checker.check(params[:action], params[:changes]) result = access_checker.check(params[:action], params[:changes])
@project ||= access_checker.project @project ||= access_checker.project
result result
rescue Gitlab::GitAccess::UnauthorizedError => e rescue Gitlab::GitAccess::ForbiddenError => e
return response_with_status(code: 401, success: false, message: e.message) return response_with_status(code: 403, success: false, message: e.message)
rescue Gitlab::GitAccess::TimeoutError => e rescue Gitlab::GitAccess::TimeoutError => e
return response_with_status(code: 503, success: false, message: e.message) return response_with_status(code: 503, success: false, message: e.message)
rescue Gitlab::GitAccess::NotFoundError => e rescue Gitlab::GitAccess::NotFoundError => e
......
...@@ -28,7 +28,7 @@ module Gitlab ...@@ -28,7 +28,7 @@ module Gitlab
logger.log_timed(LOG_MESSAGES[:delete_default_branch_check]) do logger.log_timed(LOG_MESSAGES[:delete_default_branch_check]) do
if deletion? && branch_name == project.default_branch if deletion? && branch_name == project.default_branch
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:delete_default_branch] raise GitAccess::ForbiddenError, ERROR_MESSAGES[:delete_default_branch]
end end
end end
...@@ -42,7 +42,7 @@ module Gitlab ...@@ -42,7 +42,7 @@ module Gitlab
return unless ProtectedBranch.protected?(project, branch_name) # rubocop:disable Cop/AvoidReturnFromBlocks return unless ProtectedBranch.protected?(project, branch_name) # rubocop:disable Cop/AvoidReturnFromBlocks
if forced_push? if forced_push?
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:force_push_protected_branch] raise GitAccess::ForbiddenError, ERROR_MESSAGES[:force_push_protected_branch]
end end
end end
...@@ -62,15 +62,15 @@ module Gitlab ...@@ -62,15 +62,15 @@ module Gitlab
break if user_access.can_push_to_branch?(branch_name) break if user_access.can_push_to_branch?(branch_name)
unless user_access.can_merge_to_branch?(branch_name) unless user_access.can_merge_to_branch?(branch_name)
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:create_protected_branch] raise GitAccess::ForbiddenError, ERROR_MESSAGES[:create_protected_branch]
end end
unless safe_commit_for_new_protected_branch? unless safe_commit_for_new_protected_branch?
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:invalid_commit_create_protected_branch] raise GitAccess::ForbiddenError, ERROR_MESSAGES[:invalid_commit_create_protected_branch]
end end
unless updated_from_web? unless updated_from_web?
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:non_web_create_protected_branch] raise GitAccess::ForbiddenError, ERROR_MESSAGES[:non_web_create_protected_branch]
end end
end end
end end
...@@ -78,11 +78,11 @@ module Gitlab ...@@ -78,11 +78,11 @@ module Gitlab
def protected_branch_deletion_checks def protected_branch_deletion_checks
logger.log_timed(LOG_MESSAGES[:protected_branch_deletion_checks]) do logger.log_timed(LOG_MESSAGES[:protected_branch_deletion_checks]) do
unless user_access.can_delete_branch?(branch_name) unless user_access.can_delete_branch?(branch_name)
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:non_master_delete_protected_branch] raise GitAccess::ForbiddenError, ERROR_MESSAGES[:non_master_delete_protected_branch]
end end
unless updated_from_web? unless updated_from_web?
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:non_web_delete_protected_branch] raise GitAccess::ForbiddenError, ERROR_MESSAGES[:non_web_delete_protected_branch]
end end
end end
end end
...@@ -91,11 +91,11 @@ module Gitlab ...@@ -91,11 +91,11 @@ module Gitlab
logger.log_timed(LOG_MESSAGES[:protected_branch_push_checks]) do logger.log_timed(LOG_MESSAGES[:protected_branch_push_checks]) do
if matching_merge_request? if matching_merge_request?
unless user_access.can_merge_to_branch?(branch_name) || user_access.can_push_to_branch?(branch_name) unless user_access.can_merge_to_branch?(branch_name) || user_access.can_push_to_branch?(branch_name)
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:merge_protected_branch] raise GitAccess::ForbiddenError, ERROR_MESSAGES[:merge_protected_branch]
end end
else else
unless user_access.can_push_to_branch?(branch_name) unless user_access.can_push_to_branch?(branch_name)
raise GitAccess::UnauthorizedError, push_to_protected_branch_rejected_message raise GitAccess::ForbiddenError, push_to_protected_branch_rejected_message
end end
end end
end end
......
...@@ -46,7 +46,7 @@ module Gitlab ...@@ -46,7 +46,7 @@ module Gitlab
def validate_diff(diff) def validate_diff(diff)
validations_for_diff.each do |validation| validations_for_diff.each do |validation|
if error = validation.call(diff) if error = validation.call(diff)
raise ::Gitlab::GitAccess::UnauthorizedError, error raise ::Gitlab::GitAccess::ForbiddenError, error
end end
end end
end end
...@@ -77,7 +77,7 @@ module Gitlab ...@@ -77,7 +77,7 @@ module Gitlab
logger.log_timed(LOG_MESSAGES[__method__]) do logger.log_timed(LOG_MESSAGES[__method__]) do
path_validations.each do |validation| path_validations.each do |validation|
if error = validation.call(file_paths) if error = validation.call(file_paths)
raise ::Gitlab::GitAccess::UnauthorizedError, error raise ::Gitlab::GitAccess::ForbiddenError, error
end end
end end
end end
......
...@@ -15,7 +15,7 @@ module Gitlab ...@@ -15,7 +15,7 @@ module Gitlab
lfs_check = Checks::LfsIntegrity.new(project, newrev, logger.time_left) lfs_check = Checks::LfsIntegrity.new(project, newrev, logger.time_left)
if lfs_check.objects_missing? if lfs_check.objects_missing?
raise GitAccess::UnauthorizedError, ERROR_MESSAGE raise GitAccess::ForbiddenError, ERROR_MESSAGE
end end
end end
end end
......
...@@ -6,7 +6,7 @@ module Gitlab ...@@ -6,7 +6,7 @@ module Gitlab
def validate! def validate!
logger.log_timed("Checking if you are allowed to push...") do logger.log_timed("Checking if you are allowed to push...") do
unless can_push? unless can_push?
raise GitAccess::UnauthorizedError, GitAccess::ERROR_MESSAGES[:push_code] raise GitAccess::ForbiddenError, GitAccess::ERROR_MESSAGES[:push_code]
end end
end end
end end
......
...@@ -20,7 +20,7 @@ module Gitlab ...@@ -20,7 +20,7 @@ module Gitlab
logger.log_timed(LOG_MESSAGES[:tag_checks]) do logger.log_timed(LOG_MESSAGES[:tag_checks]) do
if tag_exists? && user_access.cannot_do_action?(:admin_tag) if tag_exists? && user_access.cannot_do_action?(:admin_tag)
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:change_existing_tags] raise GitAccess::ForbiddenError, ERROR_MESSAGES[:change_existing_tags]
end end
end end
...@@ -33,11 +33,11 @@ module Gitlab ...@@ -33,11 +33,11 @@ module Gitlab
logger.log_timed(LOG_MESSAGES[__method__]) do logger.log_timed(LOG_MESSAGES[__method__]) do
return unless ProtectedTag.protected?(project, tag_name) # rubocop:disable Cop/AvoidReturnFromBlocks return unless ProtectedTag.protected?(project, tag_name) # rubocop:disable Cop/AvoidReturnFromBlocks
raise(GitAccess::UnauthorizedError, ERROR_MESSAGES[:update_protected_tag]) if update? raise(GitAccess::ForbiddenError, ERROR_MESSAGES[:update_protected_tag]) if update?
raise(GitAccess::UnauthorizedError, ERROR_MESSAGES[:delete_protected_tag]) if deletion? raise(GitAccess::ForbiddenError, ERROR_MESSAGES[:delete_protected_tag]) if deletion?
unless user_access.can_create_tag?(tag_name) unless user_access.can_create_tag?(tag_name)
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:create_protected_tag] raise GitAccess::ForbiddenError, ERROR_MESSAGES[:create_protected_tag]
end end
end end
end end
......
...@@ -6,7 +6,7 @@ module Gitlab ...@@ -6,7 +6,7 @@ module Gitlab
class GitAccess class GitAccess
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
UnauthorizedError = Class.new(StandardError) ForbiddenError = Class.new(StandardError)
NotFoundError = Class.new(StandardError) NotFoundError = Class.new(StandardError)
ProjectCreationError = Class.new(StandardError) ProjectCreationError = Class.new(StandardError)
TimeoutError = Class.new(StandardError) TimeoutError = Class.new(StandardError)
...@@ -125,7 +125,7 @@ module Gitlab ...@@ -125,7 +125,7 @@ module Gitlab
return unless actor.is_a?(Key) return unless actor.is_a?(Key)
unless actor.valid? unless actor.valid?
raise UnauthorizedError, "Your SSH key #{actor.errors[:key].first}." raise ForbiddenError, "Your SSH key #{actor.errors[:key].first}."
end end
end end
...@@ -133,7 +133,7 @@ module Gitlab ...@@ -133,7 +133,7 @@ module Gitlab
return if request_from_ci_build? return if request_from_ci_build?
unless protocol_allowed? unless protocol_allowed?
raise UnauthorizedError, "Git access over #{protocol.upcase} is not allowed" raise ForbiddenError, "Git access over #{protocol.upcase} is not allowed"
end end
end end
...@@ -148,7 +148,7 @@ module Gitlab ...@@ -148,7 +148,7 @@ module Gitlab
unless user_access.allowed? unless user_access.allowed?
message = Gitlab::Auth::UserAccessDeniedReason.new(user).rejection_message message = Gitlab::Auth::UserAccessDeniedReason.new(user).rejection_message
raise UnauthorizedError, message raise ForbiddenError, message
end end
end end
...@@ -156,11 +156,11 @@ module Gitlab ...@@ -156,11 +156,11 @@ module Gitlab
case cmd case cmd
when *DOWNLOAD_COMMANDS when *DOWNLOAD_COMMANDS
unless authentication_abilities.include?(:download_code) || authentication_abilities.include?(:build_download_code) unless authentication_abilities.include?(:download_code) || authentication_abilities.include?(:build_download_code)
raise UnauthorizedError, ERROR_MESSAGES[:auth_download] raise ForbiddenError, ERROR_MESSAGES[:auth_download]
end end
when *PUSH_COMMANDS when *PUSH_COMMANDS
unless authentication_abilities.include?(:push_code) unless authentication_abilities.include?(:push_code)
raise UnauthorizedError, ERROR_MESSAGES[:auth_upload] raise ForbiddenError, ERROR_MESSAGES[:auth_upload]
end end
end end
end end
...@@ -189,19 +189,19 @@ module Gitlab ...@@ -189,19 +189,19 @@ module Gitlab
def check_upload_pack_disabled! def check_upload_pack_disabled!
if http? && upload_pack_disabled_over_http? if http? && upload_pack_disabled_over_http?
raise UnauthorizedError, ERROR_MESSAGES[:upload_pack_disabled_over_http] raise ForbiddenError, ERROR_MESSAGES[:upload_pack_disabled_over_http]
end end
end end
def check_receive_pack_disabled! def check_receive_pack_disabled!
if http? && receive_pack_disabled_over_http? if http? && receive_pack_disabled_over_http?
raise UnauthorizedError, ERROR_MESSAGES[:receive_pack_disabled_over_http] raise ForbiddenError, ERROR_MESSAGES[:receive_pack_disabled_over_http]
end end
end end
def check_command_existence!(cmd) def check_command_existence!(cmd)
unless ALL_COMMANDS.include?(cmd) unless ALL_COMMANDS.include?(cmd)
raise UnauthorizedError, ERROR_MESSAGES[:command_not_allowed] raise ForbiddenError, ERROR_MESSAGES[:command_not_allowed]
end end
end end
...@@ -209,7 +209,7 @@ module Gitlab ...@@ -209,7 +209,7 @@ module Gitlab
return unless receive_pack?(cmd) return unless receive_pack?(cmd)
if Gitlab::Database.read_only? if Gitlab::Database.read_only?
raise UnauthorizedError, push_to_read_only_message raise ForbiddenError, push_to_read_only_message
end end
end end
...@@ -253,23 +253,23 @@ module Gitlab ...@@ -253,23 +253,23 @@ module Gitlab
guest_can_download_code? guest_can_download_code?
unless passed unless passed
raise UnauthorizedError, ERROR_MESSAGES[:download] raise ForbiddenError, ERROR_MESSAGES[:download]
end end
end end
def check_push_access! def check_push_access!
if project.repository_read_only? if project.repository_read_only?
raise UnauthorizedError, ERROR_MESSAGES[:read_only] raise ForbiddenError, ERROR_MESSAGES[:read_only]
end end
if deploy_key? if deploy_key?
unless deploy_key.can_push_to?(project) unless deploy_key.can_push_to?(project)
raise UnauthorizedError, ERROR_MESSAGES[:deploy_key_upload] raise ForbiddenError, ERROR_MESSAGES[:deploy_key_upload]
end end
elsif user elsif user
# User access is verified in check_change_access! # User access is verified in check_change_access!
else else
raise UnauthorizedError, ERROR_MESSAGES[:upload] raise ForbiddenError, ERROR_MESSAGES[:upload]
end end
check_change_access! check_change_access!
...@@ -284,7 +284,7 @@ module Gitlab ...@@ -284,7 +284,7 @@ module Gitlab
project.any_branch_allows_collaboration?(user_access.user) project.any_branch_allows_collaboration?(user_access.user)
unless can_push unless can_push
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:push_code] raise ForbiddenError, ERROR_MESSAGES[:push_code]
end end
else else
# If there are worktrees with a HEAD pointing to a non-existent object, # If there are worktrees with a HEAD pointing to a non-existent object,
......
...@@ -19,11 +19,11 @@ module Gitlab ...@@ -19,11 +19,11 @@ module Gitlab
def check_change_access! def check_change_access!
unless user_access.can_do_action?(:create_wiki) unless user_access.can_do_action?(:create_wiki)
raise UnauthorizedError, ERROR_MESSAGES[:write_to_wiki] raise ForbiddenError, ERROR_MESSAGES[:write_to_wiki]
end end
if Gitlab::Database.read_only? if Gitlab::Database.read_only?
raise UnauthorizedError, push_to_read_only_message raise ForbiddenError, push_to_read_only_message
end end
true true
......
...@@ -15,7 +15,7 @@ describe Gitlab::Checks::BranchCheck do ...@@ -15,7 +15,7 @@ describe Gitlab::Checks::BranchCheck do
let(:ref) { 'refs/heads/master' } let(:ref) { 'refs/heads/master' }
it 'raises an error' do it 'raises an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'The default branch of a project cannot be deleted.') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'The default branch of a project cannot be deleted.')
end end
end end
...@@ -28,7 +28,7 @@ describe Gitlab::Checks::BranchCheck do ...@@ -28,7 +28,7 @@ describe Gitlab::Checks::BranchCheck do
it 'raises an error if the user is not allowed to do forced pushes to protected branches' do it 'raises an error if the user is not allowed to do forced pushes to protected branches' do
expect(Gitlab::Checks::ForcePush).to receive(:force_push?).and_return(true) expect(Gitlab::Checks::ForcePush).to receive(:force_push?).and_return(true)
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to force push code to a protected branch on this project.') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You are not allowed to force push code to a protected branch on this project.')
end end
it 'raises an error if the user is not allowed to merge to protected branches' do it 'raises an error if the user is not allowed to merge to protected branches' do
...@@ -38,13 +38,13 @@ describe Gitlab::Checks::BranchCheck do ...@@ -38,13 +38,13 @@ describe Gitlab::Checks::BranchCheck do
expect(user_access).to receive(:can_merge_to_branch?).and_return(false) expect(user_access).to receive(:can_merge_to_branch?).and_return(false)
expect(user_access).to receive(:can_push_to_branch?).and_return(false) expect(user_access).to receive(:can_push_to_branch?).and_return(false)
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to merge code into protected branches on this project.') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You are not allowed to merge code into protected branches on this project.')
end end
it 'raises an error if the user is not allowed to push to protected branches' do it 'raises an error if the user is not allowed to push to protected branches' do
expect(user_access).to receive(:can_push_to_branch?).and_return(false) expect(user_access).to receive(:can_push_to_branch?).and_return(false)
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to protected branches on this project.') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You are not allowed to push code to protected branches on this project.')
end end
context 'when project repository is empty' do context 'when project repository is empty' do
...@@ -58,7 +58,7 @@ describe Gitlab::Checks::BranchCheck do ...@@ -58,7 +58,7 @@ describe Gitlab::Checks::BranchCheck do
end end
it 'raises an error' do it 'raises an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /Ask a project Owner or Maintainer to create a default branch/) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, /Ask a project Owner or Maintainer to create a default branch/)
end end
end end
...@@ -109,7 +109,7 @@ describe Gitlab::Checks::BranchCheck do ...@@ -109,7 +109,7 @@ describe Gitlab::Checks::BranchCheck do
end end
it 'raises an error' do it 'raises an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to create protected branches on this project.') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You are not allowed to create protected branches on this project.')
end end
end end
...@@ -135,7 +135,7 @@ describe Gitlab::Checks::BranchCheck do ...@@ -135,7 +135,7 @@ describe Gitlab::Checks::BranchCheck do
end end
it 'raises an error' do it 'raises an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only use an existing protected branch ref as the basis of a new protected branch.') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You can only use an existing protected branch ref as the basis of a new protected branch.')
end end
end end
...@@ -157,7 +157,7 @@ describe Gitlab::Checks::BranchCheck do ...@@ -157,7 +157,7 @@ describe Gitlab::Checks::BranchCheck do
context 'via SSH' do context 'via SSH' do
it 'raises an error' do it 'raises an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only create protected branches using the web interface and API.') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You can only create protected branches using the web interface and API.')
end end
end end
end end
...@@ -171,7 +171,7 @@ describe Gitlab::Checks::BranchCheck do ...@@ -171,7 +171,7 @@ describe Gitlab::Checks::BranchCheck do
context 'if the user is not allowed to delete protected branches' do context 'if the user is not allowed to delete protected branches' do
it 'raises an error' do it 'raises an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to delete protected branches from this project. Only a project maintainer or owner can delete a protected branch.') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You are not allowed to delete protected branches from this project. Only a project maintainer or owner can delete a protected branch.')
end end
end end
...@@ -190,7 +190,7 @@ describe Gitlab::Checks::BranchCheck do ...@@ -190,7 +190,7 @@ describe Gitlab::Checks::BranchCheck do
context 'over SSH or HTTP' do context 'over SSH or HTTP' do
it 'raises an error' do it 'raises an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only delete protected branches using the web interface.') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You can only delete protected branches using the web interface.')
end end
end end
end end
......
...@@ -34,7 +34,7 @@ describe Gitlab::Checks::DiffCheck do ...@@ -34,7 +34,7 @@ describe Gitlab::Checks::DiffCheck do
context 'when change is sent by a different user' do context 'when change is sent by a different user' do
it 'raises an error if the user is not allowed to update the file' do it 'raises an error if the user is not allowed to update the file' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "The path 'README' is locked in Git LFS by #{lock.user.name}") expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "The path 'README' is locked in Git LFS by #{lock.user.name}")
end end
end end
......
...@@ -50,7 +50,7 @@ describe Gitlab::Checks::LfsCheck do ...@@ -50,7 +50,7 @@ describe Gitlab::Checks::LfsCheck do
end end
it 'fails if any LFS blobs are missing' do it 'fails if any LFS blobs are missing' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /LFS objects are missing/) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, /LFS objects are missing/)
end end
it 'succeeds if LFS objects have already been uploaded' do it 'succeeds if LFS objects have already been uploaded' do
......
...@@ -15,7 +15,7 @@ describe Gitlab::Checks::PushCheck do ...@@ -15,7 +15,7 @@ describe Gitlab::Checks::PushCheck do
expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false) expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false)
expect(project).to receive(:branch_allows_collaboration?).with(user_access.user, 'master').and_return(false) expect(project).to receive(:branch_allows_collaboration?).with(user_access.user, 'master').and_return(false)
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You are not allowed to push code to this project.')
end end
end end
end end
......
...@@ -11,7 +11,7 @@ describe Gitlab::Checks::TagCheck do ...@@ -11,7 +11,7 @@ describe Gitlab::Checks::TagCheck do
it 'raises an error when user does not have access' do it 'raises an error when user does not have access' do
allow(user_access).to receive(:can_do_action?).with(:admin_tag).and_return(false) allow(user_access).to receive(:can_do_action?).with(:admin_tag).and_return(false)
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to change existing tags on this project.') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You are not allowed to change existing tags on this project.')
end end
context 'with protected tag' do context 'with protected tag' do
...@@ -27,7 +27,7 @@ describe Gitlab::Checks::TagCheck do ...@@ -27,7 +27,7 @@ describe Gitlab::Checks::TagCheck do
let(:newrev) { '0000000000000000000000000000000000000000' } let(:newrev) { '0000000000000000000000000000000000000000' }
it 'is prevented' do it 'is prevented' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /cannot be deleted/) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, /cannot be deleted/)
end end
end end
...@@ -36,7 +36,7 @@ describe Gitlab::Checks::TagCheck do ...@@ -36,7 +36,7 @@ describe Gitlab::Checks::TagCheck do
let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' } let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' }
it 'is prevented' do it 'is prevented' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /cannot be updated/) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, /cannot be updated/)
end end
end end
end end
...@@ -47,7 +47,7 @@ describe Gitlab::Checks::TagCheck do ...@@ -47,7 +47,7 @@ describe Gitlab::Checks::TagCheck do
let(:ref) { 'refs/tags/v9.1.0' } let(:ref) { 'refs/tags/v9.1.0' }
it 'prevents creation below access level' do it 'prevents creation below access level' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /allowed to create this tag as it is protected/) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, /allowed to create this tag as it is protected/)
end end
context 'when user has access' do context 'when user has access' do
......
This diff is collapsed.
...@@ -33,7 +33,7 @@ describe Gitlab::GitAccessWiki do ...@@ -33,7 +33,7 @@ describe Gitlab::GitAccessWiki do
end end
it 'does not give access to upload wiki code' do it 'does not give access to upload wiki code' do
expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "You can't push code to a read-only GitLab instance.") expect { subject }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You can't push code to a read-only GitLab instance.")
end end
end end
end end
...@@ -70,7 +70,7 @@ describe Gitlab::GitAccessWiki do ...@@ -70,7 +70,7 @@ describe Gitlab::GitAccessWiki do
it 'does not give access to download wiki code' do it 'does not give access to download wiki code' do
project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED) project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED)
expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to download code from this project.') expect { subject }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You are not allowed to download code from this project.')
end end
end end
end end
......
...@@ -409,7 +409,7 @@ describe API::Internal::Base do ...@@ -409,7 +409,7 @@ describe API::Internal::Base do
it do it do
pull(key, project) pull(key, project)
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:forbidden)
expect(json_response["status"]).to be_falsey expect(json_response["status"]).to be_falsey
expect(user.reload.last_activity_on).to be_nil expect(user.reload.last_activity_on).to be_nil
end end
...@@ -419,7 +419,7 @@ describe API::Internal::Base do ...@@ -419,7 +419,7 @@ describe API::Internal::Base do
it do it do
push(key, project) push(key, project)
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:forbidden)
expect(json_response["status"]).to be_falsey expect(json_response["status"]).to be_falsey
expect(user.reload.last_activity_on).to be_nil expect(user.reload.last_activity_on).to be_nil
end end
...@@ -518,7 +518,7 @@ describe API::Internal::Base do ...@@ -518,7 +518,7 @@ describe API::Internal::Base do
it do it do
pull(key, personal_project) pull(key, personal_project)
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:forbidden)
expect(json_response["status"]).to be_falsey expect(json_response["status"]).to be_falsey
expect(user.reload.last_activity_on).to be_nil expect(user.reload.last_activity_on).to be_nil
end end
...@@ -528,7 +528,7 @@ describe API::Internal::Base do ...@@ -528,7 +528,7 @@ describe API::Internal::Base do
it do it do
push(key, personal_project) push(key, personal_project)
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:forbidden)
expect(json_response["status"]).to be_falsey expect(json_response["status"]).to be_falsey
expect(user.reload.last_activity_on).to be_nil expect(user.reload.last_activity_on).to be_nil
end end
...@@ -572,7 +572,7 @@ describe API::Internal::Base do ...@@ -572,7 +572,7 @@ describe API::Internal::Base do
it do it do
push(key, project) push(key, project)
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:forbidden)
expect(json_response["status"]).to be_falsey expect(json_response["status"]).to be_falsey
end end
end end
...@@ -654,7 +654,7 @@ describe API::Internal::Base do ...@@ -654,7 +654,7 @@ describe API::Internal::Base do
it 'rejects the SSH push' do it 'rejects the SSH push' do
push(key, project) push(key, project)
expect(response.status).to eq(401) expect(response).to have_gitlab_http_status(:forbidden)
expect(json_response['status']).to be_falsey expect(json_response['status']).to be_falsey
expect(json_response['message']).to eq 'Git access over SSH is not allowed' expect(json_response['message']).to eq 'Git access over SSH is not allowed'
end end
...@@ -662,7 +662,7 @@ describe API::Internal::Base do ...@@ -662,7 +662,7 @@ describe API::Internal::Base do
it 'rejects the SSH pull' do it 'rejects the SSH pull' do
pull(key, project) pull(key, project)
expect(response.status).to eq(401) expect(response).to have_gitlab_http_status(:forbidden)
expect(json_response['status']).to be_falsey expect(json_response['status']).to be_falsey
expect(json_response['message']).to eq 'Git access over SSH is not allowed' expect(json_response['message']).to eq 'Git access over SSH is not allowed'
end end
...@@ -676,7 +676,7 @@ describe API::Internal::Base do ...@@ -676,7 +676,7 @@ describe API::Internal::Base do
it 'rejects the HTTP push' do it 'rejects the HTTP push' do
push(key, project, 'http') push(key, project, 'http')
expect(response.status).to eq(401) expect(response).to have_gitlab_http_status(:forbidden)
expect(json_response['status']).to be_falsey expect(json_response['status']).to be_falsey
expect(json_response['message']).to eq 'Git access over HTTP is not allowed' expect(json_response['message']).to eq 'Git access over HTTP is not allowed'
end end
...@@ -684,7 +684,7 @@ describe API::Internal::Base do ...@@ -684,7 +684,7 @@ describe API::Internal::Base do
it 'rejects the HTTP pull' do it 'rejects the HTTP pull' do
pull(key, project, 'http') pull(key, project, 'http')
expect(response.status).to eq(401) expect(response).to have_gitlab_http_status(:forbidden)
expect(json_response['status']).to be_falsey expect(json_response['status']).to be_falsey
expect(json_response['message']).to eq 'Git access over HTTP is not allowed' expect(json_response['message']).to eq 'Git access over HTTP is not allowed'
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