Commit 93b25cdd authored by Markus Koller's avatar Markus Koller

Merge branch 'ajk-group-wiki-git-access' into 'master'

[RUN AS-IF-FOSS] Group-wiki git-access checks

Closes #207871

See merge request gitlab-org/gitlab!34673
parents e9a44ddb 461dd303
...@@ -17,7 +17,7 @@ module ChecksCollaboration ...@@ -17,7 +17,7 @@ module ChecksCollaboration
# used across multiple calls in the view # used across multiple calls in the view
def user_access(project) def user_access(project)
@user_access ||= {} @user_access ||= {}
@user_access[project] ||= Gitlab::UserAccess.new(current_user, project: project) @user_access[project] ||= Gitlab::UserAccess.new(current_user, container: project)
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables # rubocop:enable Gitlab/ModuleWithInstanceVariables
end end
...@@ -403,7 +403,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -403,7 +403,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
return access_denied! unless @merge_request.source_branch_exists? return access_denied! unless @merge_request.source_branch_exists?
access_check = ::Gitlab::UserAccess access_check = ::Gitlab::UserAccess
.new(current_user, project: @merge_request.source_project) .new(current_user, container: @merge_request.source_project)
.can_push_to_branch?(@merge_request.source_branch) .can_push_to_branch?(@merge_request.source_branch)
access_denied! unless access_check access_denied! unless access_check
......
...@@ -105,7 +105,7 @@ module Repositories ...@@ -105,7 +105,7 @@ module Repositories
access.check(git_command, Gitlab::GitAccess::ANY) access.check(git_command, Gitlab::GitAccess::ANY)
if repo_type.project? && !container if repo_type.project? && !container
@project = @container = access.project @project = @container = access.container
end end
end end
......
...@@ -1180,12 +1180,12 @@ class MergeRequest < ApplicationRecord ...@@ -1180,12 +1180,12 @@ class MergeRequest < ApplicationRecord
end end
def can_be_merged_by?(user) def can_be_merged_by?(user)
access = ::Gitlab::UserAccess.new(user, project: project) access = ::Gitlab::UserAccess.new(user, container: project)
access.can_update_branch?(target_branch) access.can_update_branch?(target_branch)
end end
def can_be_merged_via_command_line_by?(user) def can_be_merged_via_command_line_by?(user)
access = ::Gitlab::UserAccess.new(user, project: project) access = ::Gitlab::UserAccess.new(user, container: project)
access.can_push_to_branch?(target_branch) access.can_push_to_branch?(target_branch)
end end
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Ci module Ci
class BuildPolicy < CommitStatusPolicy class BuildPolicy < CommitStatusPolicy
condition(:protected_ref) do condition(:protected_ref) do
access = ::Gitlab::UserAccess.new(@user, project: @subject.project) access = ::Gitlab::UserAccess.new(@user, container: @subject.project)
if @subject.tag? if @subject.tag?
!access.can_create_tag?(@subject.ref) !access.can_create_tag?(@subject.ref)
......
...@@ -42,7 +42,7 @@ module Ci ...@@ -42,7 +42,7 @@ module Ci
end end
def ref_protected?(user, project, tag, ref) def ref_protected?(user, project, tag, ref)
access = ::Gitlab::UserAccess.new(user, project: project) access = ::Gitlab::UserAccess.new(user, container: project)
if tag if tag
!access.can_create_tag?(ref) !access.can_create_tag?(ref)
......
...@@ -4,7 +4,7 @@ class SuggestionPolicy < BasePolicy ...@@ -4,7 +4,7 @@ class SuggestionPolicy < BasePolicy
delegate { @subject.project } delegate { @subject.project }
condition(:can_push_to_branch) do condition(:can_push_to_branch) do
Gitlab::UserAccess.new(@user, project: @subject.project).can_push_to_branch?(@subject.branch) Gitlab::UserAccess.new(@user, container: @subject.project).can_push_to_branch?(@subject.branch)
end end
rule { can_push_to_branch }.enable :apply_suggestion rule { can_push_to_branch }.enable :apply_suggestion
......
...@@ -179,7 +179,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated ...@@ -179,7 +179,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
return false unless source_branch_exists? return false unless source_branch_exists?
!!::Gitlab::UserAccess !!::Gitlab::UserAccess
.new(current_user, project: source_project) .new(current_user, container: source_project)
.can_push_to_branch?(source_branch) .can_push_to_branch?(source_branch)
end end
......
...@@ -98,7 +98,7 @@ module Ci ...@@ -98,7 +98,7 @@ module Ci
end end
def can_update_branch?(target_ref) def can_update_branch?(target_ref)
::Gitlab::UserAccess.new(current_user, project: downstream_project).can_update_branch?(target_ref) ::Gitlab::UserAccess.new(current_user, container: downstream_project).can_update_branch?(target_ref)
end end
def downstream_project def downstream_project
......
...@@ -69,7 +69,7 @@ module Commits ...@@ -69,7 +69,7 @@ module Commits
end end
def validate_permissions! def validate_permissions!
allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@branch_name) allowed = ::Gitlab::UserAccess.new(current_user, container: project).can_push_to_branch?(@branch_name)
unless allowed unless allowed
raise_error("You are not allowed to push into this branch") raise_error("You are not allowed to push into this branch")
......
...@@ -8,7 +8,7 @@ module MergeRequests ...@@ -8,7 +8,7 @@ module MergeRequests
def can_be_resolved_by?(user) def can_be_resolved_by?(user)
return false unless merge_request.source_project return false unless merge_request.source_project
access = ::Gitlab::UserAccess.new(user, project: merge_request.source_project) access = ::Gitlab::UserAccess.new(user, container: merge_request.source_project)
access.can_push_to_branch?(merge_request.source_branch) access.can_push_to_branch?(merge_request.source_branch)
end end
......
...@@ -112,7 +112,7 @@ module Metrics ...@@ -112,7 +112,7 @@ module Metrics
end end
def push_authorized? def push_authorized?
Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch) Gitlab::UserAccess.new(current_user, container: project).can_push_to_branch?(branch)
end end
def dashboard_template def dashboard_template
......
...@@ -68,7 +68,7 @@ module Metrics ...@@ -68,7 +68,7 @@ module Metrics
end end
def push_authorized? def push_authorized?
Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch) Gitlab::UserAccess.new(current_user, container: project).can_push_to_branch?(branch)
end end
def valid_branch_name? def valid_branch_name?
......
...@@ -92,7 +92,7 @@ module MergeRequests ...@@ -92,7 +92,7 @@ module MergeRequests
def can_create_merge_request?(source_branch) def can_create_merge_request?(source_branch)
can?(@current_user, :create_merge_request_in, @project) && can?(@current_user, :create_merge_request_in, @project) &&
can?(@current_user, :create_merge_request_from, @project) && can?(@current_user, :create_merge_request_from, @project) &&
::Gitlab::UserAccess.new(@current_user, project: @project).can_push_to_branch?(source_branch) ::Gitlab::UserAccess.new(@current_user, container: @project).can_push_to_branch?(source_branch)
end end
end end
end end
...@@ -18,8 +18,8 @@ module EE ...@@ -18,8 +18,8 @@ module EE
private private
def custom_action_for(cmd) def geo_custom_action
return unless custom_action_for?(cmd) return unless geo_custom_action?
payload = { payload = {
'action' => 'geo_proxy_to_primary', 'action' => 'geo_proxy_to_primary',
...@@ -32,15 +32,17 @@ module EE ...@@ -32,15 +32,17 @@ module EE
::Gitlab::GitAccessResult::CustomAction.new(payload, messages) ::Gitlab::GitAccessResult::CustomAction.new(payload, messages)
end end
def custom_action_for?(cmd) def geo_custom_action?
return unless ::Gitlab::Database.read_only? return unless ::Gitlab::Database.read_only?
return unless ::Gitlab::Geo.secondary_with_primary? return unless ::Gitlab::Geo.secondary_with_primary?
receive_pack?(cmd) || upload_pack_and_not_replicated?(cmd) receive_pack? || upload_pack_and_not_replicated?
end end
def upload_pack_and_not_replicated?(cmd) def upload_pack_and_not_replicated?
upload_pack?(cmd) && !::Geo::ProjectRegistry.repository_replicated_for?(project.id) return false unless project
upload_pack? && !::Geo::ProjectRegistry.repository_replicated_for?(project.id)
end end
def messages def messages
...@@ -90,7 +92,7 @@ module EE ...@@ -90,7 +92,7 @@ module EE
end end
def custom_action_api_endpoints_for(cmd) def custom_action_api_endpoints_for(cmd)
receive_pack?(cmd) ? custom_action_push_api_endpoints : custom_action_pull_api_endpoints receive_pack? ? custom_action_push_api_endpoints : custom_action_pull_api_endpoints
end end
def custom_action_pull_api_endpoints def custom_action_pull_api_endpoints
......
...@@ -23,6 +23,14 @@ module EE ...@@ -23,6 +23,14 @@ module EE
super super
end end
def group?
container.is_a?(Group)
end
def group
container if group?
end
protected protected
override :user override :user
...@@ -35,11 +43,8 @@ module EE ...@@ -35,11 +43,8 @@ module EE
private private
override :check_custom_action override :check_custom_action
def check_custom_action(cmd) def check_custom_action
custom_action = custom_action_for(cmd) geo_custom_action || super
return custom_action if custom_action
super
end end
override :check_for_console_messages override :check_for_console_messages
......
...@@ -3,12 +3,64 @@ ...@@ -3,12 +3,64 @@
module EE module EE
module Gitlab module Gitlab
module GitAccessWiki module GitAccessWiki
extend ::Gitlab::Utils::Override
include GeoGitAccess include GeoGitAccess
ERROR_MESSAGES = {
write_to_group_wiki: "You are not allowed to write to this group's wiki.",
group_not_found: 'The group you were looking for could not be found.',
no_group_repo: 'A repository for this group wiki does not exist yet.'
}.freeze
override :project?
def project?
!group?
end
override :check_container!
def check_container!
return check_group! if group?
super
end
override :check_push_access!
def check_push_access!
return check_change_access! if group?
super
end
override :write_to_wiki_message
def write_to_wiki_message
return ERROR_MESSAGES[:write_to_group_wiki] if group?
super
end
override :no_repo_message
def no_repo_message
return ERROR_MESSAGES[:no_group_repo] if group?
super
end
private private
def check_group!
raise ::Gitlab::GitAccess::NotFoundError, ERROR_MESSAGES[:group_not_found] unless can_read_group?
end
def can_read_group?
if user
user.can?(:read_group, container)
else
Guest.can?(:read_group, container)
end
end
def project_or_wiki def project_or_wiki
project.wiki container.wiki
end end
end end
end end
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Checks::ChangeAccess do RSpec.describe Gitlab::Checks::ChangeAccess do
describe '#exec' do describe '#validate!' do
include_context 'push rules checks context' include_context 'push rules checks context'
let(:push_rule) { create(:push_rule, deny_delete_tag: true) } let(:push_rule) { create(:push_rule, deny_delete_tag: true) }
...@@ -17,7 +17,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do ...@@ -17,7 +17,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do
expect(instance).to receive(:validate!) expect(instance).to receive(:validate!)
end end
subject.exec subject.validate!
end end
end end
end end
...@@ -17,6 +17,18 @@ RSpec.describe Gitlab::GitAccess do ...@@ -17,6 +17,18 @@ RSpec.describe Gitlab::GitAccess do
let(:authentication_abilities) { %i[read_project download_code push_code] } let(:authentication_abilities) { %i[read_project download_code push_code] }
let(:redirected_path) { nil } let(:redirected_path) { nil }
let(:access_class) do
Class.new(described_class) do
def push_ability
:push_code
end
def download_ability
:download_code
end
end
end
context "when in a read-only GitLab instance" do context "when in a read-only GitLab instance" do
before do before do
create(:protected_branch, name: 'feature', project: project) create(:protected_branch, name: 'feature', project: project)
...@@ -720,7 +732,7 @@ RSpec.describe Gitlab::GitAccess do ...@@ -720,7 +732,7 @@ RSpec.describe Gitlab::GitAccess do
private private
def access def access
described_class.new( access_class.new(
actor, actor,
project, project,
protocol, protocol,
......
...@@ -11,6 +11,106 @@ RSpec.describe Gitlab::GitAccessWiki do ...@@ -11,6 +11,106 @@ RSpec.describe Gitlab::GitAccessWiki do
let(:access) { described_class.new(user, project, 'web', authentication_abilities: authentication_abilities, redirected_path: redirected_path) } let(:access) { described_class.new(user, project, 'web', authentication_abilities: authentication_abilities, redirected_path: redirected_path) }
describe 'group wiki access' do
let_it_be(:group, reload: true) { create(:group, :private, :wiki_repo) }
let(:access) do
described_class.new(user, group, 'web',
authentication_abilities: authentication_abilities,
redirected_path: redirected_path)
end
describe '#push_access_check' do
subject { access.check('git-receive-pack', changes) }
context 'when user can :create_wiki' do
before do
group.add_developer(user)
end
it { expect { subject }.not_to raise_error }
context 'when in a read-only GitLab instance' do
before do
allow(Gitlab::Database).to receive(:read_only?) { true }
end
it 'does not give access to upload wiki code' do
expect { subject }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You can't push code to a read-only GitLab instance.")
end
end
end
context 'when user cannot :create_wiki' do
before do
group.add_reporter(user)
end
specify do
expect { subject }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You are not allowed to write to this group's wiki.")
end
end
end
describe '#check_download_access!' do
subject { access.check('git-upload-pack', Gitlab::GitAccess::ANY) }
context 'the user has at least reporter access' do
before do
group.add_reporter(user)
end
context 'when wiki feature is enabled' do
it 'gives access to download wiki code' do
expect { subject }.not_to raise_error
end
context 'when the wiki repository does not exist' do
let(:group) { create(:group) }
it_behaves_like 'not-found git access' do
let(:message) { 'A repository for this group wiki does not exist yet.' }
end
end
end
context 'when wiki feature is disabled' do
before do
stub_feature_flags(group_wiki: false)
end
it_behaves_like 'forbidden git access' do
let(:message) { 'You are not allowed to download files from this wiki.' }
end
end
end
context 'the user does not have access' do
it_behaves_like 'not-found git access' do
let(:message) { 'The group you were looking for could not be found.' }
end
end
context 'the group is public' do
let(:group) { create(:group, :public, :wiki_repo) }
it 'gives access to download wiki code' do
expect { subject }.not_to raise_error
end
context 'when wiki feature is disabled' do
before do
stub_feature_flags(group_wiki: false)
end
it_behaves_like 'forbidden git access' do
let(:message) { 'You are not allowed to download files from this wiki.' }
end
end
end
end
end
context "when in a read-only GitLab instance" do context "when in a read-only GitLab instance" do
subject { access.check('git-receive-pack', changes) } subject { access.check('git-receive-pack', changes) }
......
...@@ -6,7 +6,7 @@ RSpec.describe Gitlab::UserAccess do ...@@ -6,7 +6,7 @@ RSpec.describe Gitlab::UserAccess do
include ExternalAuthorizationServiceHelpers include ExternalAuthorizationServiceHelpers
let(:user) { create(:user) } let(:user) { create(:user) }
let(:access) { described_class.new(user, project: project) } let(:access) { described_class.new(user, container: project) }
describe '#can_push_to_branch?' do describe '#can_push_to_branch?' do
describe 'push to empty project' do describe 'push to empty project' do
......
...@@ -13,7 +13,7 @@ module API ...@@ -13,7 +13,7 @@ module API
helpers do helpers do
def user_access def user_access
@user_access ||= Gitlab::UserAccess.new(current_user, project: user_project) @user_access ||= Gitlab::UserAccess.new(current_user, container: user_project)
end end
def authorize_push_to_branch!(branch) def authorize_push_to_branch!(branch)
......
...@@ -32,7 +32,7 @@ module API ...@@ -32,7 +32,7 @@ module API
end end
expose :can_push do |repo_branch, options| expose :can_push do |repo_branch, options|
Gitlab::UserAccess.new(options[:current_user], project: options[:project]).can_push_to_branch?(repo_branch.name) Gitlab::UserAccess.new(options[:current_user], container: options[:project]).can_push_to_branch?(repo_branch.name)
end end
expose :default do |repo_branch, options| expose :default do |repo_branch, options|
......
...@@ -92,7 +92,7 @@ module API ...@@ -92,7 +92,7 @@ module API
# If we have created a project directly from a git push # If we have created a project directly from a git push
# we have to assign its value to both @project and @container # we have to assign its value to both @project and @container
@project = @container = access_checker.project @project = @container = access_checker.container
end end
end end
end end
......
...@@ -97,7 +97,7 @@ module API ...@@ -97,7 +97,7 @@ module API
user_access = Gitlab::UserAccess.new( user_access = Gitlab::UserAccess.new(
current_user, current_user,
project: merge_request.source_project container: merge_request.source_project
) )
forbidden!('Cannot push to source branch') unless forbidden!('Cannot push to source branch') unless
......
...@@ -2,8 +2,6 @@ ...@@ -2,8 +2,6 @@
module Gitlab module Gitlab
class BuildAccess < UserAccess class BuildAccess < UserAccess
attr_accessor :user, :project
# This bypasses the `can?(:access_git)`-check we normally do in `UserAccess` # This bypasses the `can?(:access_git)`-check we normally do in `UserAccess`
# for CI. That way if a user was able to trigger a pipeline, then the # for CI. That way if a user was able to trigger a pipeline, then the
# build is allowed to clone the project. # build is allowed to clone the project.
......
...@@ -25,7 +25,7 @@ module Gitlab ...@@ -25,7 +25,7 @@ module Gitlab
@logger.append_message("Running checks for ref: #{@branch_name || @tag_name}") @logger.append_message("Running checks for ref: #{@branch_name || @tag_name}")
end end
def exec def validate!
ref_level_checks ref_level_checks
# Check of commits should happen as the last step # Check of commits should happen as the last step
# given they're expensive in terms of performance # given they're expensive in terms of performance
......
...@@ -34,7 +34,7 @@ module Gitlab ...@@ -34,7 +34,7 @@ module Gitlab
end end
def allowed_to_write_ref? def allowed_to_write_ref?
access = Gitlab::UserAccess.new(current_user, project: project) access = Gitlab::UserAccess.new(current_user, container: project)
if @command.branch_exists? if @command.branch_exists?
access.can_update_branch?(@command.ref) access.can_update_branch?(@command.ref)
......
This diff is collapsed.
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
module Gitlab module Gitlab
class GitAccessDesign < GitAccess class GitAccessDesign < GitAccess
extend ::Gitlab::Utils::Override
def check(_cmd, _changes) def check(_cmd, _changes)
check_protocol! check_protocol!
check_can_create_design! check_can_create_design!
...@@ -9,6 +11,11 @@ module Gitlab ...@@ -9,6 +11,11 @@ module Gitlab
success_result success_result
end end
override :push_ability
def push_ability
:create_design
end
private private
def check_protocol! def check_protocol!
...@@ -18,7 +25,7 @@ module Gitlab ...@@ -18,7 +25,7 @@ module Gitlab
end end
def check_can_create_design! def check_can_create_design!
unless user&.can?(:create_design, project) unless user_can_push?
raise ::Gitlab::GitAccess::ForbiddenError, "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
......
...@@ -6,21 +6,41 @@ module Gitlab ...@@ -6,21 +6,41 @@ module Gitlab
CreationError = Class.new(StandardError) CreationError = Class.new(StandardError)
ERROR_MESSAGES = {
namespace_not_found: 'The namespace you were looking for could not be found.'
}.freeze
override :download_ability
def download_ability
:download_code
end
override :push_ability
def push_ability
:push_code
end
private private
override :check_project! override :check_container!
def check_project!(cmd) def check_container!
ensure_project_on_push!(cmd) check_namespace!
ensure_project_on_push!
super super
end end
def ensure_project_on_push!(cmd) def check_namespace!
return if project || deploy_key? raise NotFoundError, ERROR_MESSAGES[:namespace_not_found] unless namespace_path.present?
return unless receive_pack?(cmd) && changes == ANY && authentication_abilities.include?(:push_code) end
namespace = Namespace.find_by_full_path(namespace_path) def namespace
@namespace ||= Namespace.find_by_full_path(namespace_path)
end
def ensure_project_on_push!
return if project || deploy_key?
return unless receive_pack? && changes == ANY && authentication_abilities.include?(:push_code)
return unless user&.can?(:create_projects, namespace) return unless user&.can?(:create_projects, namespace)
project_params = { project_params = {
...@@ -35,8 +55,8 @@ module Gitlab ...@@ -35,8 +55,8 @@ module Gitlab
raise CreationError, "Could not create project: #{project.errors.full_messages.join(', ')}" raise CreationError, "Could not create project: #{project.errors.full_messages.join(', ')}"
end end
@project = project self.container = project
user_access.project = @project user_access.container = project
Checks::ProjectCreated.new(repository, user, protocol).add_message Checks::ProjectCreated.new(repository, user, protocol).add_message
end end
......
...@@ -9,50 +9,68 @@ module Gitlab ...@@ -9,50 +9,68 @@ module Gitlab
read_snippet: 'You are not allowed to read this snippet.', read_snippet: 'You are not allowed to read this snippet.',
update_snippet: 'You are not allowed to update this snippet.', update_snippet: 'You are not allowed to update this snippet.',
snippet_not_found: 'The snippet you were looking for could not be found.', snippet_not_found: 'The snippet you were looking for could not be found.',
repository_not_found: 'The snippet repository you were looking for could not be found.' no_repo: 'The snippet repository you were looking for could not be found.'
}.freeze }.freeze
attr_reader :snippet alias_method :snippet, :container
alias_method :container, :snippet
def initialize(actor, snippet, protocol, **kwargs) def initialize(actor, snippet, protocol, **kwargs)
@snippet = snippet super(actor, snippet, protocol, **kwargs)
super(actor, snippet&.project, protocol, **kwargs)
@auth_result_type = nil @auth_result_type = nil
@authentication_abilities &= [:download_code, :push_code] @authentication_abilities &= [:download_code, :push_code]
end end
override :check
def check(cmd, changes) def check(cmd, changes)
# TODO: Investigate if expanding actor/authentication types are needed.
# https://gitlab.com/gitlab-org/gitlab/issues/202190
if actor && !actor.is_a?(User) && !actor.instance_of?(Key)
raise ForbiddenError, ERROR_MESSAGES[:authentication_mechanism]
end
check_snippet_accessibility! check_snippet_accessibility!
super super
end end
override :download_ability
def download_ability
:read_snippet
end
override :push_ability
def push_ability
:update_snippet
end
private private
override :check_namespace! # TODO: Implement EE/Geo https://gitlab.com/gitlab-org/gitlab/issues/205629
def check_namespace! override :check_custom_action
return unless snippet.is_a?(ProjectSnippet) def check_custom_action
# snippets never return custom actions, such as geo replication.
end
super override :project?
def project?
project_snippet?
end
override :project
def project
snippet&.project
end end
override :check_project! override :check_valid_actor!
def check_project!(cmd) def check_valid_actor!
return unless snippet.is_a?(ProjectSnippet) # TODO: Investigate if expanding actor/authentication types are needed.
# https://gitlab.com/gitlab-org/gitlab/issues/202190
if actor && !actor.is_a?(User) && !actor.instance_of?(Key)
raise ForbiddenError, ERROR_MESSAGES[:authentication_mechanism]
end
super super
end end
def project_snippet?
snippet.is_a?(ProjectSnippet)
end
override :check_push_access! override :check_push_access!
def check_push_access! def check_push_access!
raise ForbiddenError, ERROR_MESSAGES[:update_snippet] unless user raise ForbiddenError, ERROR_MESSAGES[:update_snippet] unless user
...@@ -82,19 +100,9 @@ module Gitlab ...@@ -82,19 +100,9 @@ module Gitlab
end end
end end
override :guest_can_download_code?
def guest_can_download_code?
Guest.can?(:read_snippet, snippet)
end
override :user_can_download_code?
def user_can_download_code?
authentication_abilities.include?(:download_code) && user_access.can_do_action?(:read_snippet)
end
override :check_change_access! override :check_change_access!
def check_change_access! def check_change_access!
unless user_access.can_do_action?(:update_snippet) unless user_can_push?
raise ForbiddenError, ERROR_MESSAGES[:update_snippet] raise ForbiddenError, ERROR_MESSAGES[:update_snippet]
end end
...@@ -109,31 +117,19 @@ module Gitlab ...@@ -109,31 +117,19 @@ module Gitlab
check_push_size! check_push_size!
end end
def check_single_change_access(change) override :check_single_change_access
def check_single_change_access(change, _skip_lfs_integrity_check: false)
Checks::SnippetCheck.new(change, logger: logger).validate! Checks::SnippetCheck.new(change, logger: logger).validate!
Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet.max_file_limit(user), logger: logger).validate! Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet.max_file_limit(user), logger: logger).validate!
rescue Checks::TimedLogger::TimeoutError rescue Checks::TimedLogger::TimeoutError
raise TimeoutError, logger.full_message raise TimeoutError, logger.full_message
end end
override :check_repository_existence!
def check_repository_existence!
unless repository.exists?
raise NotFoundError, ERROR_MESSAGES[:repository_not_found]
end
end
override :user_access override :user_access
def user_access def user_access
@user_access ||= UserAccessSnippet.new(user, snippet: snippet) @user_access ||= UserAccessSnippet.new(user, snippet: snippet)
end end
# TODO: Implement EE/Geo https://gitlab.com/gitlab-org/gitlab/issues/205629
override :check_custom_action
def check_custom_action(cmd)
nil
end
override :check_size_limit? override :check_size_limit?
def check_size_limit? def check_size_limit?
return false if user&.migration_bot? return false if user&.migration_bot?
......
...@@ -2,41 +2,50 @@ ...@@ -2,41 +2,50 @@
module Gitlab module Gitlab
class GitAccessWiki < GitAccess class GitAccessWiki < GitAccess
prepend_if_ee('EE::Gitlab::GitAccessWiki') # rubocop: disable Cop/InjectEnterpriseEditionModule extend ::Gitlab::Utils::Override
ERROR_MESSAGES = { ERROR_MESSAGES = {
read_only: "You can't push code to a read-only GitLab instance.", download: 'You are not allowed to download files from this wiki.',
not_found: 'The wiki you were looking for could not be found.',
no_repo: 'A repository for this wiki does not exist yet.',
read_only: "You can't push code to a read-only GitLab instance.",
write_to_wiki: "You are not allowed to write to this project's wiki." write_to_wiki: "You are not allowed to write to this project's wiki."
}.freeze }.freeze
def guest_can_download_code? override :download_ability
Guest.can?(:download_wiki_code, project) def download_ability
:download_wiki_code
end end
def user_can_download_code? override :push_ability
authentication_abilities.include?(:download_code) && user_access.can_do_action?(:download_wiki_code) def push_ability
:create_wiki
end end
override :check_change_access!
def check_change_access! def check_change_access!
unless user_access.can_do_action?(:create_wiki) raise ForbiddenError, write_to_wiki_message unless user_can_push?
raise ForbiddenError, ERROR_MESSAGES[:write_to_wiki]
end
if Gitlab::Database.read_only?
raise ForbiddenError, push_to_read_only_message
end
true true
end end
def push_to_read_only_message def push_to_read_only_message
ERROR_MESSAGES[:read_only] error_message(:read_only)
end end
private def write_to_wiki_message
error_message(:write_to_wiki)
end
def not_found_message
error_message(:not_found)
end
override :repository
def repository def repository
project.wiki.repository container.wiki.repository
end end
end end
end end
Gitlab::GitAccessWiki.prepend_if_ee('EE::Gitlab::GitAccessWiki')
...@@ -37,19 +37,19 @@ module Gitlab ...@@ -37,19 +37,19 @@ module Gitlab
end end
def wiki? def wiki?
self == WIKI name == :wiki
end end
def project? def project?
self == PROJECT name == :project
end end
def snippet? def snippet?
self == SNIPPET name == :snippet
end end
def design? def design?
self == DESIGN name == :design
end end
def path_suffix def path_suffix
......
...@@ -5,15 +5,16 @@ module Gitlab ...@@ -5,15 +5,16 @@ module Gitlab
extend Gitlab::Cache::RequestCache extend Gitlab::Cache::RequestCache
request_cache_key do request_cache_key do
[user&.id, project&.id] [user&.id, container&.to_global_id]
end end
attr_reader :user attr_reader :user, :push_ability
attr_accessor :project attr_accessor :container
def initialize(user, project: nil) def initialize(user, container: nil, push_ability: :push_code)
@user = user @user = user
@project = project @container = container
@push_ability = push_ability
end end
def can_do_action?(action) def can_do_action?(action)
...@@ -21,7 +22,7 @@ module Gitlab ...@@ -21,7 +22,7 @@ module Gitlab
permission_cache[action] = permission_cache[action] =
permission_cache.fetch(action) do permission_cache.fetch(action) do
user.can?(action, project) user.can?(action, container)
end end
end end
...@@ -42,20 +43,20 @@ module Gitlab ...@@ -42,20 +43,20 @@ module Gitlab
request_cache def can_create_tag?(ref) request_cache def can_create_tag?(ref)
return false unless can_access_git? return false unless can_access_git?
if protected?(ProtectedTag, project, ref) if protected?(ProtectedTag, ref)
protected_tag_accessible_to?(ref, action: :create) protected_tag_accessible_to?(ref, action: :create)
else else
user.can?(:admin_tag, project) user.can?(:admin_tag, container)
end end
end end
request_cache def can_delete_branch?(ref) request_cache def can_delete_branch?(ref)
return false unless can_access_git? return false unless can_access_git?
if protected?(ProtectedBranch, project, ref) if protected?(ProtectedBranch, ref)
user.can?(:push_to_delete_protected_branch, project) user.can?(:push_to_delete_protected_branch, container)
else else
user.can?(:push_code, project) can_push?
end end
end end
...@@ -64,36 +65,36 @@ module Gitlab ...@@ -64,36 +65,36 @@ module Gitlab
end end
request_cache def can_push_to_branch?(ref) request_cache def can_push_to_branch?(ref)
return false unless can_access_git? return false unless can_access_git? && container && can_collaborate?(ref)
return false unless project return true unless protected?(ProtectedBranch, ref)
# Checking for an internal project to prevent an infinite loop:
# https://gitlab.com/gitlab-org/gitlab/issues/36805
if project.internal?
return false unless user.can?(:push_code, project)
else
return false if !user.can?(:push_code, project) && !project.branch_allows_collaboration?(user, ref)
end
if protected?(ProtectedBranch, project, ref) protected_branch_accessible_to?(ref, action: :push)
protected_branch_accessible_to?(ref, action: :push)
else
true
end
end end
request_cache def can_merge_to_branch?(ref) request_cache def can_merge_to_branch?(ref)
return false unless can_access_git? return false unless can_access_git?
if protected?(ProtectedBranch, project, ref) if protected?(ProtectedBranch, ref)
protected_branch_accessible_to?(ref, action: :merge) protected_branch_accessible_to?(ref, action: :merge)
else else
user.can?(:push_code, project) can_push?
end end
end end
private private
def can_push?
user.can?(push_ability, container)
end
def can_collaborate?(ref)
assert_project!
# Checking for an internal project or group to prevent an infinite loop:
# https://gitlab.com/gitlab-org/gitlab/issues/36805
can_push? || (!project.internal? && project.branch_allows_collaboration?(user, ref))
end
def permission_cache def permission_cache
@permission_cache ||= {} @permission_cache ||= {}
end end
...@@ -103,6 +104,8 @@ module Gitlab ...@@ -103,6 +104,8 @@ module Gitlab
end end
def protected_branch_accessible_to?(ref, action:) def protected_branch_accessible_to?(ref, action:)
assert_project!
ProtectedBranch.protected_ref_accessible_to?( ProtectedBranch.protected_ref_accessible_to?(
ref, user, ref, user,
project: project, project: project,
...@@ -111,6 +114,8 @@ module Gitlab ...@@ -111,6 +114,8 @@ module Gitlab
end end
def protected_tag_accessible_to?(ref, action:) def protected_tag_accessible_to?(ref, action:)
assert_project!
ProtectedTag.protected_ref_accessible_to?( ProtectedTag.protected_ref_accessible_to?(
ref, user, ref, user,
project: project, project: project,
...@@ -118,8 +123,22 @@ module Gitlab ...@@ -118,8 +123,22 @@ module Gitlab
protected_refs: project.protected_tags) protected_refs: project.protected_tags)
end end
request_cache def protected?(kind, project, refs) request_cache def protected?(kind, refs)
assert_project!
kind.protected?(project, refs) kind.protected?(project, refs)
end end
def project
container
end
# Any method that assumes that it is operating on a project should make this
# explicit by calling `#assert_project!`.
# TODO: remove when we make this class polymorphic enough not to care about projects
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/227635
def assert_project!
raise "No project! #{project.inspect} is not a Project" unless project.is_a?(::Project)
end
end end
end end
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
module Gitlab module Gitlab
class UserAccessSnippet < UserAccess class UserAccessSnippet < UserAccess
extend ::Gitlab::Utils::Override
extend ::Gitlab::Cache::RequestCache extend ::Gitlab::Cache::RequestCache
# TODO: apply override check https://gitlab.com/gitlab-org/gitlab/issues/205677 # TODO: apply override check https://gitlab.com/gitlab-org/gitlab/issues/205677
...@@ -9,11 +10,10 @@ module Gitlab ...@@ -9,11 +10,10 @@ module Gitlab
[user&.id, snippet&.id] [user&.id, snippet&.id]
end end
attr_reader :snippet alias_method :snippet, :container
def initialize(user, snippet: nil) def initialize(user, snippet: nil)
@user = user super(user, container: snippet)
@snippet = snippet
@project = snippet&.project @project = snippet&.project
end end
...@@ -43,13 +43,9 @@ module Gitlab ...@@ -43,13 +43,9 @@ module Gitlab
def can_push_to_branch?(ref) def can_push_to_branch?(ref)
return true if snippet_migration? return true if snippet_migration?
super
return false unless snippet return false unless snippet
return false unless can_do_action?(:update_snippet)
true can_do_action?(:update_snippet)
end end
def can_merge_to_branch?(ref) def can_merge_to_branch?(ref)
...@@ -59,5 +55,8 @@ module Gitlab ...@@ -59,5 +55,8 @@ module Gitlab
def snippet_migration? def snippet_migration?
user&.migration_bot? && snippet user&.migration_bot? && snippet
end end
override :project
attr_reader :project
end end
end end
...@@ -30,6 +30,10 @@ module Gitlab ...@@ -30,6 +30,10 @@ module Gitlab
Gitlab::UrlBuilder.instance Gitlab::UrlBuilder.instance
end end
def is_a?(type)
super || subject.is_a?(type)
end
class_methods do class_methods do
def presenter? def presenter?
true true
......
...@@ -33,7 +33,7 @@ RSpec.describe ChecksCollaboration do ...@@ -33,7 +33,7 @@ RSpec.describe ChecksCollaboration do
it 'is true when the user can push to a branch of the project' do it 'is true when the user can push to a branch of the project' do
fake_access = double('Gitlab::UserAccess') fake_access = double('Gitlab::UserAccess')
expect(fake_access).to receive(:can_push_to_branch?).with('a-branch').and_return(true) expect(fake_access).to receive(:can_push_to_branch?).with('a-branch').and_return(true)
expect(Gitlab::UserAccess).to receive(:new).with(user, project: project).and_return(fake_access) expect(Gitlab::UserAccess).to receive(:new).with(user, container: project).and_return(fake_access)
expect(helper.can_collaborate_with_project?(project, ref: 'a-branch')).to be_truthy expect(helper.can_collaborate_with_project?(project, ref: 'a-branch')).to be_truthy
end end
......
...@@ -7,7 +7,7 @@ RSpec.describe Gitlab::BuildAccess do ...@@ -7,7 +7,7 @@ RSpec.describe Gitlab::BuildAccess do
let(:project) { create(:project) } let(:project) { create(:project) }
describe '#can_do_action' do describe '#can_do_action' do
subject { described_class.new(user, project: project).can_do_action?(:download_code) } subject { described_class.new(user, container: project).can_do_action?(:download_code) }
context 'when the user can do an action on the project but cannot access git' do context 'when the user can do an action on the project but cannot access git' do
before do before do
......
...@@ -3,14 +3,14 @@ ...@@ -3,14 +3,14 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Checks::ChangeAccess do RSpec.describe Gitlab::Checks::ChangeAccess do
describe '#exec' do describe '#validate!' do
include_context 'change access checks context' include_context 'change access checks context'
subject { change_access } subject { change_access }
context 'without failed checks' do context 'without failed checks' do
it "doesn't raise an error" do it "doesn't raise an error" do
expect { subject.exec }.not_to raise_error expect { subject.validate! }.not_to raise_error
end end
it 'calls pushes checks' do it 'calls pushes checks' do
...@@ -18,7 +18,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do ...@@ -18,7 +18,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do
expect(instance).to receive(:validate!) expect(instance).to receive(:validate!)
end end
subject.exec subject.validate!
end end
it 'calls branches checks' do it 'calls branches checks' do
...@@ -26,7 +26,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do ...@@ -26,7 +26,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do
expect(instance).to receive(:validate!) expect(instance).to receive(:validate!)
end end
subject.exec subject.validate!
end end
it 'calls tags checks' do it 'calls tags checks' do
...@@ -34,7 +34,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do ...@@ -34,7 +34,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do
expect(instance).to receive(:validate!) expect(instance).to receive(:validate!)
end end
subject.exec subject.validate!
end end
it 'calls lfs checks' do it 'calls lfs checks' do
...@@ -42,7 +42,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do ...@@ -42,7 +42,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do
expect(instance).to receive(:validate!) expect(instance).to receive(:validate!)
end end
subject.exec subject.validate!
end end
it 'calls diff checks' do it 'calls diff checks' do
...@@ -50,7 +50,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do ...@@ -50,7 +50,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do
expect(instance).to receive(:validate!) expect(instance).to receive(:validate!)
end end
subject.exec subject.validate!
end end
end end
...@@ -63,7 +63,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do ...@@ -63,7 +63,7 @@ RSpec.describe Gitlab::Checks::ChangeAccess do
protocol: protocol, protocol: protocol,
logger: logger) logger: logger)
expect { access.exec }.to raise_error(Gitlab::Checks::TimedLogger::TimeoutError) expect { access.validate! }.to raise_error(Gitlab::Checks::TimedLogger::TimeoutError)
end end
end end
end end
......
...@@ -5,6 +5,7 @@ require 'spec_helper' ...@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe Gitlab::GitAccessProject do RSpec.describe Gitlab::GitAccessProject do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :repository) } let_it_be(:project) { create(:project, :repository) }
let(:container) { project }
let(:actor) { user } let(:actor) { user }
let(:project_path) { project.path } let(:project_path) { project.path }
let(:namespace_path) { project&.namespace&.path } let(:namespace_path) { project&.namespace&.path }
...@@ -13,19 +14,32 @@ RSpec.describe Gitlab::GitAccessProject do ...@@ -13,19 +14,32 @@ RSpec.describe Gitlab::GitAccessProject do
let(:changes) { Gitlab::GitAccess::ANY } let(:changes) { Gitlab::GitAccess::ANY }
let(:push_access_check) { access.check('git-receive-pack', changes) } let(:push_access_check) { access.check('git-receive-pack', changes) }
let(:pull_access_check) { access.check('git-upload-pack', changes) } let(:pull_access_check) { access.check('git-upload-pack', changes) }
let(:access) do
described_class.new(actor, container, protocol,
authentication_abilities: authentication_abilities,
repository_path: project_path, namespace_path: namespace_path)
end
describe '#check_namespace!' do
context 'when namespace is nil' do
let(:namespace_path) { nil }
it 'does not allow push and pull access' do
aggregate_failures do
expect { push_access_check }.to raise_namespace_not_found
expect { pull_access_check }.to raise_namespace_not_found
end
end
end
end
describe '#check_project_accessibility!' do describe '#check_project_accessibility!' do
context 'when the project is nil' do context 'when the project is nil' do
let(:project) { nil } let(:container) { nil }
let(:project_path) { "new-project" } let(:project_path) { "new-project" }
context 'when user is allowed to create project in namespace' do context 'when user is allowed to create project in namespace' do
let(:namespace_path) { user.namespace.path } let(:namespace_path) { user.namespace.path }
let(:access) do
described_class.new(actor, nil,
protocol, authentication_abilities: authentication_abilities,
repository_path: project_path, namespace_path: namespace_path)
end
it 'blocks pull access with "not found"' do it 'blocks pull access with "not found"' do
expect { pull_access_check }.to raise_not_found expect { pull_access_check }.to raise_not_found
...@@ -39,11 +53,6 @@ RSpec.describe Gitlab::GitAccessProject do ...@@ -39,11 +53,6 @@ RSpec.describe Gitlab::GitAccessProject do
context 'when user is not allowed to create project in namespace' do context 'when user is not allowed to create project in namespace' do
let(:user2) { create(:user) } let(:user2) { create(:user) }
let(:namespace_path) { user2.namespace.path } let(:namespace_path) { user2.namespace.path }
let(:access) do
described_class.new(actor, nil,
protocol, authentication_abilities: authentication_abilities,
repository_path: project_path, namespace_path: namespace_path)
end
it 'blocks push and pull with "not found"' do it 'blocks push and pull with "not found"' do
aggregate_failures do aggregate_failures do
...@@ -56,22 +65,27 @@ RSpec.describe Gitlab::GitAccessProject do ...@@ -56,22 +65,27 @@ RSpec.describe Gitlab::GitAccessProject do
end end
describe '#ensure_project_on_push!' do describe '#ensure_project_on_push!' do
let(:access) do
described_class.new(actor, project,
protocol, authentication_abilities: authentication_abilities,
repository_path: project_path, namespace_path: namespace_path)
end
before do before do
allow(access).to receive(:changes).and_return(changes) allow(access).to receive(:changes).and_return(changes)
end end
shared_examples 'no project is created' do
let(:raise_specific_error) { raise_not_found }
let(:action) { push_access_check }
it 'does not create a new project' do
expect { action }
.to raise_specific_error
.and change { Project.count }.by(0)
end
end
context 'when push' do context 'when push' do
let(:cmd) { 'git-receive-pack' } let(:cmd) { 'git-receive-pack' }
context 'when project does not exist' do context 'when project does not exist' do
let(:project_path) { "nonexistent" } let(:project_path) { "nonexistent" }
let(:project) { nil } let(:container) { nil }
context 'when changes is _any' do context 'when changes is _any' do
let(:changes) { Gitlab::GitAccess::ANY } let(:changes) { Gitlab::GitAccess::ANY }
...@@ -82,8 +96,8 @@ RSpec.describe Gitlab::GitAccessProject do ...@@ -82,8 +96,8 @@ RSpec.describe Gitlab::GitAccessProject do
context 'when user can create project in namespace' do context 'when user can create project in namespace' do
let(:namespace_path) { user.namespace.path } let(:namespace_path) { user.namespace.path }
it 'creates a new project' do it 'creates a new project in the correct namespace' do
expect { access.send(:ensure_project_on_push!, cmd) } expect { push_access_check }
.to change { Project.count }.by(1) .to change { Project.count }.by(1)
.and change { Project.where(namespace: user.namespace, name: project_path).count }.by(1) .and change { Project.where(namespace: user.namespace, name: project_path).count }.by(1)
end end
...@@ -93,9 +107,7 @@ RSpec.describe Gitlab::GitAccessProject do ...@@ -93,9 +107,7 @@ RSpec.describe Gitlab::GitAccessProject do
let(:user2) { create(:user) } let(:user2) { create(:user) }
let(:namespace_path) { user2.namespace.path } let(:namespace_path) { user2.namespace.path }
it 'does not create a new project' do it_behaves_like 'no project is created'
expect { access.send(:ensure_project_on_push!, cmd) }.not_to change { Project.count }
end
end end
end end
...@@ -105,8 +117,8 @@ RSpec.describe Gitlab::GitAccessProject do ...@@ -105,8 +117,8 @@ RSpec.describe Gitlab::GitAccessProject do
context 'when user can create project in namespace' do context 'when user can create project in namespace' do
let(:namespace_path) { user.namespace.path } let(:namespace_path) { user.namespace.path }
it 'does not create a new project' do it_behaves_like 'no project is created' do
expect { access.send(:ensure_project_on_push!, cmd) }.not_to change { Project.count } let(:raise_specific_error) { raise_forbidden }
end end
end end
end end
...@@ -115,32 +127,26 @@ RSpec.describe Gitlab::GitAccessProject do ...@@ -115,32 +127,26 @@ RSpec.describe Gitlab::GitAccessProject do
context 'when check contains actual changes' do context 'when check contains actual changes' do
let(:changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch" } let(:changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch" }
it 'does not create a new project' do it_behaves_like 'no project is created'
expect { access.send(:ensure_project_on_push!, cmd) }.not_to change { Project.count }
end
end end
end end
context 'when project exists' do context 'when project exists' do
let(:changes) { Gitlab::GitAccess::ANY } let(:changes) { Gitlab::GitAccess::ANY }
let!(:project) { create(:project) } let!(:container) { project }
it 'does not create a new project' do it_behaves_like 'no project is created'
expect { access.send(:ensure_project_on_push!, cmd) }.not_to change { Project.count }
end
end end
context 'when deploy key is used' do context 'when deploy key is used' do
let(:key) { create(:deploy_key, user: user) } let(:key) { create(:deploy_key, user: user) }
let(:actor) { key } let(:actor) { key }
let(:project_path) { "nonexistent" } let(:project_path) { "nonexistent" }
let(:project) { nil } let(:container) { nil }
let(:namespace_path) { user.namespace.path } let(:namespace_path) { user.namespace.path }
let(:changes) { Gitlab::GitAccess::ANY } let(:changes) { Gitlab::GitAccess::ANY }
it 'does not create a new project' do it_behaves_like 'no project is created'
expect { access.send(:ensure_project_on_push!, cmd) }.not_to change { Project.count }
end
end end
end end
...@@ -151,10 +157,10 @@ RSpec.describe Gitlab::GitAccessProject do ...@@ -151,10 +157,10 @@ RSpec.describe Gitlab::GitAccessProject do
context 'when project does not exist' do context 'when project does not exist' do
let(:project_path) { "new-project" } let(:project_path) { "new-project" }
let(:namespace_path) { user.namespace.path } let(:namespace_path) { user.namespace.path }
let(:project) { nil } let(:container) { nil }
it 'does not create a new project' do it_behaves_like 'no project is created' do
expect { access.send(:ensure_project_on_push!, cmd) }.not_to change { Project.count } let(:action) { pull_access_check }
end end
end end
end end
...@@ -163,4 +169,12 @@ RSpec.describe Gitlab::GitAccessProject do ...@@ -163,4 +169,12 @@ RSpec.describe Gitlab::GitAccessProject do
def raise_not_found def raise_not_found
raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found]) raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found])
end end
def raise_forbidden
raise_error(Gitlab::GitAccess::ForbiddenError)
end
def raise_namespace_not_found
raise_error(Gitlab::GitAccess::NotFoundError, described_class::ERROR_MESSAGES[:namespace_not_found])
end
end end
...@@ -20,6 +20,18 @@ RSpec.describe Gitlab::GitAccess do ...@@ -20,6 +20,18 @@ RSpec.describe Gitlab::GitAccess do
let(:push_access_check) { access.check('git-receive-pack', changes) } let(:push_access_check) { access.check('git-receive-pack', changes) }
let(:pull_access_check) { access.check('git-upload-pack', changes) } let(:pull_access_check) { access.check('git-upload-pack', changes) }
let(:access_class) do
Class.new(described_class) do
def push_ability
:push_code
end
def download_ability
:download_code
end
end
end
describe '#check with single protocols allowed' do describe '#check with single protocols allowed' do
def disable_protocol(protocol) def disable_protocol(protocol)
allow(Gitlab::ProtocolAccess).to receive(:allowed?).with(protocol).and_return(false) allow(Gitlab::ProtocolAccess).to receive(:allowed?).with(protocol).and_return(false)
...@@ -58,7 +70,7 @@ RSpec.describe Gitlab::GitAccess do ...@@ -58,7 +70,7 @@ RSpec.describe Gitlab::GitAccess do
it "doesn't block http pull" do it "doesn't block http pull" do
aggregate_failures do aggregate_failures do
expect { pull_access_check }.not_to raise_forbidden('Git access over HTTP is not allowed') expect { pull_access_check }.not_to raise_error
end end
end end
...@@ -67,7 +79,7 @@ RSpec.describe Gitlab::GitAccess do ...@@ -67,7 +79,7 @@ RSpec.describe Gitlab::GitAccess do
it "doesn't block http pull" do it "doesn't block http pull" do
aggregate_failures do aggregate_failures do
expect { pull_access_check }.not_to raise_forbidden('Git access over HTTP is not allowed') expect { pull_access_check }.not_to raise_error
end end
end end
end end
...@@ -75,33 +87,6 @@ RSpec.describe Gitlab::GitAccess do ...@@ -75,33 +87,6 @@ RSpec.describe Gitlab::GitAccess do
end end
end end
describe '#check_namespace!' do
context 'when namespace exists' do
before do
project.add_maintainer(user)
end
it 'allows push and pull access' do
aggregate_failures do
expect { push_access_check }.not_to raise_error
expect { pull_access_check }.not_to raise_error
end
end
end
context 'when namespace and project are nil' do
let(:project) { nil }
let(:namespace_path) { nil }
it 'does not allow push and pull access' do
aggregate_failures do
expect { push_access_check }.to raise_namespace_not_found
expect { pull_access_check }.to raise_namespace_not_found
end
end
end
end
describe '#check_project_accessibility!' do describe '#check_project_accessibility!' do
context 'when the project exists' do context 'when the project exists' do
context 'when actor exists' do context 'when actor exists' do
...@@ -464,7 +449,7 @@ RSpec.describe Gitlab::GitAccess do ...@@ -464,7 +449,7 @@ RSpec.describe Gitlab::GitAccess do
let(:public_project) { create(:project, :public, :repository) } let(:public_project) { create(:project, :public, :repository) }
let(:project_path) { public_project.path } let(:project_path) { public_project.path }
let(:namespace_path) { public_project.namespace.path } let(:namespace_path) { public_project.namespace.path }
let(:access) { described_class.new(nil, public_project, 'web', authentication_abilities: [:download_code], repository_path: project_path, namespace_path: namespace_path) } let(:access) { access_class.new(nil, public_project, 'web', authentication_abilities: [:download_code], repository_path: project_path, namespace_path: namespace_path) }
context 'when repository is enabled' do context 'when repository is enabled' do
it 'give access to download code' do it 'give access to download code' do
...@@ -859,7 +844,7 @@ RSpec.describe Gitlab::GitAccess do ...@@ -859,7 +844,7 @@ RSpec.describe Gitlab::GitAccess do
message = "Push operation timed out\n\nTiming information for debugging purposes:\nRunning checks for ref: wow" message = "Push operation timed out\n\nTiming information for debugging purposes:\nRunning checks for ref: wow"
expect_next_instance_of(Gitlab::Checks::ChangeAccess) do |check| expect_next_instance_of(Gitlab::Checks::ChangeAccess) do |check|
expect(check).to receive(:exec).and_raise(Gitlab::Checks::TimedLogger::TimeoutError) expect(check).to receive(:validate!).and_raise(Gitlab::Checks::TimedLogger::TimeoutError)
end end
expect { access.check('git-receive-pack', changes) }.to raise_error(described_class::TimeoutError, message) expect { access.check('git-receive-pack', changes) }.to raise_error(described_class::TimeoutError, message)
...@@ -1067,7 +1052,7 @@ RSpec.describe Gitlab::GitAccess do ...@@ -1067,7 +1052,7 @@ RSpec.describe Gitlab::GitAccess do
private private
def access def access
described_class.new(actor, project, protocol, access_class.new(actor, project, protocol,
authentication_abilities: authentication_abilities, authentication_abilities: authentication_abilities,
namespace_path: namespace_path, repository_path: project_path, namespace_path: namespace_path, repository_path: project_path,
redirected_path: redirected_path, auth_result_type: auth_result_type) redirected_path: redirected_path, auth_result_type: auth_result_type)
...@@ -1078,15 +1063,11 @@ RSpec.describe Gitlab::GitAccess do ...@@ -1078,15 +1063,11 @@ RSpec.describe Gitlab::GitAccess do
end end
def raise_forbidden(message) def raise_forbidden(message)
raise_error(Gitlab::GitAccess::ForbiddenError, message) raise_error(described_class::ForbiddenError, message)
end end
def raise_not_found def raise_not_found
raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found]) raise_error(described_class::NotFoundError, described_class::ERROR_MESSAGES[:project_not_found])
end
def raise_namespace_not_found
raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:namespace_not_found])
end end
def build_authentication_abilities def build_authentication_abilities
......
...@@ -4,8 +4,8 @@ require 'spec_helper' ...@@ -4,8 +4,8 @@ require 'spec_helper'
RSpec.describe Gitlab::GitAccessWiki do RSpec.describe Gitlab::GitAccessWiki do
let(:access) { described_class.new(user, project, 'web', authentication_abilities: authentication_abilities, redirected_path: redirected_path) } let(:access) { described_class.new(user, project, 'web', authentication_abilities: authentication_abilities, redirected_path: redirected_path) }
let(:project) { create(:project, :wiki_repo) } let_it_be(:project) { create(:project, :wiki_repo) }
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:changes) { ['6f6d7e7ed 570e7b2ab refs/heads/master'] } let(:changes) { ['6f6d7e7ed 570e7b2ab refs/heads/master'] }
let(:redirected_path) { nil } let(:redirected_path) { nil }
let(:authentication_abilities) do let(:authentication_abilities) do
...@@ -17,56 +17,65 @@ RSpec.describe Gitlab::GitAccessWiki do ...@@ -17,56 +17,65 @@ RSpec.describe Gitlab::GitAccessWiki do
end end
describe '#push_access_check' do describe '#push_access_check' do
subject { access.check('git-receive-pack', changes) }
context 'when user can :create_wiki' do context 'when user can :create_wiki' do
before do before do
create(:protected_branch, name: 'master', project: project)
project.add_developer(user) project.add_developer(user)
end end
subject { access.check('git-receive-pack', changes) }
it { expect { subject }.not_to raise_error } it { expect { subject }.not_to raise_error }
context 'when in a read-only GitLab instance' do context 'when in a read-only GitLab instance' do
let(:message) { "You can't push code to a read-only GitLab instance." }
before do before do
allow(Gitlab::Database).to receive(:read_only?) { true } allow(Gitlab::Database).to receive(:read_only?) { true }
end end
it 'does not give access to upload wiki code' do it_behaves_like 'forbidden git access'
expect { subject }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You can't push code to a read-only GitLab instance.") end
end end
context 'the user cannot :create_wiki' do
it_behaves_like 'not-found git access' do
let(:message) { 'The wiki you were looking for could not be found.' }
end end
end end
end end
describe '#access_check_download!' do describe '#check_download_access!' do
subject { access.check('git-upload-pack', Gitlab::GitAccess::ANY) } subject { access.check('git-upload-pack', Gitlab::GitAccess::ANY) }
before do context 'the user can :download_wiki_code' do
project.add_developer(user) before do
end project.add_developer(user)
context 'when wiki feature is enabled' do
it 'give access to download wiki code' do
expect { subject }.not_to raise_error
end end
context 'when the wiki repository does not exist' do context 'when wiki feature is disabled' do
let(:project) { create(:project) } before do
project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED)
end
it 'returns not found' do it_behaves_like 'forbidden git access' do
expect(project.wiki_repository_exists?).to eq(false) let(:message) { include('wiki') }
end
end
expect { subject }.to raise_error(Gitlab::GitAccess::NotFoundError, 'A repository for this project does not exist yet.') context 'when the repository does not exist' do
before do
allow(project.wiki).to receive(:repository).and_return(double('Repository', exists?: false))
end
it_behaves_like 'not-found git access' do
let(:message) { include('for this wiki') }
end end
end end
end end
context 'when wiki feature is disabled' do context 'the user cannot :download_wiki_code' do
it 'does not give access to download wiki code' do it_behaves_like 'not-found git access' do
project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED) let(:message) { include('wiki') }
expect { subject }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You are not allowed to download code from this project.')
end end
end end
end end
......
...@@ -5,7 +5,7 @@ require 'spec_helper' ...@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe Gitlab::UserAccess do RSpec.describe Gitlab::UserAccess do
include ProjectForksHelper include ProjectForksHelper
let(:access) { described_class.new(user, project: project) } let(:access) { described_class.new(user, container: project) }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:user) { create(:user) } let(:user) { create(:user) }
...@@ -43,7 +43,7 @@ RSpec.describe Gitlab::UserAccess do ...@@ -43,7 +43,7 @@ RSpec.describe Gitlab::UserAccess do
describe 'push to empty project' do describe 'push to empty project' do
let(:empty_project) { create(:project_empty_repo) } let(:empty_project) { create(:project_empty_repo) }
let(:project_access) { described_class.new(user, project: empty_project) } let(:project_access) { described_class.new(user, container: empty_project) }
it 'returns true for admins' do it 'returns true for admins' do
user.update!(admin: true) user.update!(admin: true)
......
...@@ -525,7 +525,7 @@ RSpec.describe ProjectPresenter do ...@@ -525,7 +525,7 @@ RSpec.describe ProjectPresenter do
end end
describe '#statistics_buttons' do describe '#statistics_buttons' do
let(:project) { build(:project) } let(:project) { build_stubbed(:project) }
it 'orders the items correctly' do it 'orders the items correctly' do
allow(project.repository).to receive(:readme).and_return(double(name: 'readme')) allow(project.repository).to receive(:readme).and_return(double(name: 'readme'))
......
...@@ -301,14 +301,14 @@ RSpec.describe 'Git HTTP requests' do ...@@ -301,14 +301,14 @@ RSpec.describe 'Git HTTP requests' do
it 'rejects clones with 404 Not Found' do it 'rejects clones with 404 Not Found' do
download(path, user: user.username, password: user.password) do |response| download(path, user: user.username, password: user.password) do |response|
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
expect(response.body).to eq(git_access_error(:project_not_found)) expect(response.body).to eq(git_access_wiki_error(:not_found))
end end
end end
it 'rejects pushes with 404 Not Found' do it 'rejects pushes with 404 Not Found' do
upload(path, user: user.username, password: user.password) do |response| upload(path, user: user.username, password: user.password) do |response|
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
expect(response.body).to eq(git_access_error(:project_not_found)) expect(response.body).to eq(git_access_wiki_error(:not_found))
end end
end end
end end
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
RSpec.shared_context 'change access checks context' do RSpec.shared_context 'change access checks context' do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:user_access) { Gitlab::UserAccess.new(user, project: project) } let(:user_access) { Gitlab::UserAccess.new(user, container: project) }
let(:oldrev) { 'be93687618e4b132087f430a4d8fc3a609c9b77c' } let(:oldrev) { 'be93687618e4b132087f430a4d8fc3a609c9b77c' }
let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' } let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' }
let(:ref) { 'refs/heads/master' } let(:ref) { 'refs/heads/master' }
......
# frozen_string_literal: true
RSpec.shared_examples 'forbidden git access' do
let(:message) { /You can't/ }
it 'prevents access' do
expect { subject }.to raise_error(Gitlab::GitAccess::ForbiddenError, message)
end
end
RSpec.shared_examples 'not-found git access' do
let(:message) { /not found/ }
it 'prevents access' do
expect { subject }.to raise_error(Gitlab::GitAccess::NotFoundError, message)
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