Commit c590ebfe authored by Mark Chao's avatar Mark Chao

Snippet git access check

Allow limited actor types to reduce scope.

Block branch creation and deletion because
multi branches aren't supported by the UI yet.
parent 72c71d34
# frozen_string_literal: true
module Gitlab
module Checks
class SnippetCheck < BaseChecker
ERROR_MESSAGES = {
create_delete_branch: 'You can not create or delete branches.'
}.freeze
ATTRIBUTES = %i[oldrev newrev ref branch_name tag_name logger].freeze
attr_reader(*ATTRIBUTES)
def initialize(change, logger:)
@oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref)
@branch_name = Gitlab::Git.branch_name(@ref)
@tag_name = Gitlab::Git.tag_name(@ref)
@logger = logger
@logger.append_message("Running checks for ref: #{@branch_name || @tag_name}")
end
def exec
if creation? || deletion?
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:create_delete_branch]
end
# TODO: https://gitlab.com/gitlab-org/gitlab/issues/205628
# Check operation will not result in more than one file in the repository
true
end
end
end
end
......@@ -60,7 +60,6 @@ module Gitlab
@logger = Checks::TimedLogger.new(timeout: INTERNAL_TIMEOUT, header: LOG_HEADER)
@changes = changes
check_namespace!
check_protocol!
check_valid_actor!
check_active_user!
......@@ -72,11 +71,7 @@ module Gitlab
return custom_action if custom_action
check_db_accessibility!(cmd)
ensure_project_on_push!(cmd, changes)
check_project_accessibility!
add_project_moved_message!
check_project!(changes, cmd)
check_repository_existence!
case cmd
......@@ -113,6 +108,13 @@ module Gitlab
private
def check_project!(changes, cmd)
check_namespace!
ensure_project_on_push!(cmd, changes)
check_project_accessibility!
add_project_moved_message!
end
def check_custom_action(cmd)
nil
end
......
......@@ -2,7 +2,13 @@
module Gitlab
class GitAccessSnippet < GitAccess
extend ::Gitlab::Utils::Override
ERROR_MESSAGES = {
authentication_mechanism: 'The authentication mechanism is not supported.',
read_snippet: 'You are not allowed to read this snippet.',
update_snippet: 'You are not allowed to update this snippet.',
project_not_found: 'The project 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.'
}.freeze
......@@ -12,25 +18,47 @@ module Gitlab
def initialize(actor, snippet, protocol, **kwargs)
@snippet = snippet
super(actor, project, protocol, **kwargs)
super(actor, snippet&.project, protocol, **kwargs)
@auth_result_type = nil
@authentication_abilities &= [:download_code, :push_code]
end
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 UnauthorizedError, ERROR_MESSAGES[:authentication_mechanism]
end
unless Feature.enabled?(:version_snippets, user)
raise NotFoundError, ERROR_MESSAGES[:snippet_not_found]
raise NotFoundError, ERROR_MESSAGES[:project_not_found]
end
check_snippet_accessibility!
success_result(cmd)
super
end
def project
snippet&.project
private
override :check_project!
def check_project!(cmd, changes)
if snippet.is_a?(ProjectSnippet)
check_namespace!
check_project_accessibility!
# TODO add add_project_moved_message! to handle non-project repo https://gitlab.com/gitlab-org/gitlab/issues/205646
end
end
private
override :check_push_access!
def check_push_access!
raise UnauthorizedError, ERROR_MESSAGES[:update_snippet] unless user
check_change_access!
end
override :repository
def repository
snippet&.repository
end
......@@ -39,10 +67,64 @@ module Gitlab
if snippet.blank?
raise NotFoundError, ERROR_MESSAGES[:snippet_not_found]
end
end
override :check_download_access!
def check_download_access!
passed = guest_can_download_code? || user_can_download_code?
unless passed
raise UnauthorizedError, ERROR_MESSAGES[:read_snippet]
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!
def check_change_access!
unless user_access.can_do_action?(:update_snippet)
raise UnauthorizedError, ERROR_MESSAGES[:update_snippet]
end
changes_list.each do |change|
# If user does not have access to make at least one change, cancel all
# push by allowing the exception to bubble up
check_single_change_access(change)
end
end
def check_single_change_access(change)
change_access = Checks::SnippetCheck.new(change, logger: logger)
change_access.exec
rescue Checks::TimedLogger::TimeoutError
raise TimeoutError, logger.full_message
end
unless repository&.exists?
override :check_repository_existence!
def check_repository_existence!
unless repository.exists?
raise NotFoundError, ERROR_MESSAGES[:repository_not_found]
end
end
override :user_access
def user_access
@user_access ||= UserAccessSnippet.new(user, snippet: snippet)
end
# TODO: Implement EE/Geo https://gitlab.com/gitlab-org/gitlab/issues/205629
override :check_custom_action
def check_custom_action(cmd)
nil
end
end
end
# frozen_string_literal: true
module Gitlab
class UserAccessSnippet < UserAccess
extend ::Gitlab::Cache::RequestCache
# TODO: apply override check https://gitlab.com/gitlab-org/gitlab/issues/205677
request_cache_key do
[user&.id, snippet&.id]
end
attr_reader :snippet
def initialize(user, snippet: nil)
@user = user
@snippet = snippet
@project = snippet&.project
end
def can_do_action?(action)
return false unless can_access_git?
permission_cache[action] =
permission_cache.fetch(action) do
Ability.allowed?(user, action, snippet)
end
end
def can_create_tag?(ref)
false
end
def can_delete_branch?(ref)
false
end
def can_push_to_branch?(ref)
super
return false unless snippet
return false unless can_do_action?(:update_snippet)
true
end
def can_merge_to_branch?(ref)
false
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Checks::SnippetCheck do
include_context 'change access checks context'
let(:snippet) { create(:personal_snippet, :repository) }
let(:user_access) { Gitlab::UserAccessSnippet.new(user, snippet: snippet) }
subject { Gitlab::Checks::SnippetCheck.new(changes, logger: logger) }
describe '#exec' do
it 'does not raise any error' do
expect { subject.exec }.not_to raise_error
end
context 'trying to delete the branch' do
let(:newrev) { '0000000000000000000000000000000000000000' }
it 'raises an error' do
expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can not create or delete branches.')
end
end
context 'trying to create the branch' do
let(:oldrev) { '0000000000000000000000000000000000000000' }
it 'raises an error' do
expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can not create or delete branches.')
end
end
end
end
......@@ -3,24 +3,30 @@
require 'spec_helper'
describe Gitlab::GitAccessSnippet do
include GitHelpers
include ProjectHelpers
include TermsHelper
include_context 'ProjectPolicyTable context'
using RSpec::Parameterized::TableSyntax
let_it_be(:user) { create(:user) }
let_it_be(:personal_snippet) { create(:personal_snippet, :private, :repository) }
let_it_be(:project) { create(:project, :public) }
let_it_be(:snippet) { create(:project_snippet, :public, :repository, project: project) }
let(:actor) { user }
let(:protocol) { 'ssh' }
let(:changes) { Gitlab::GitAccess::ANY }
let(:authentication_abilities) { [:download_code, :push_code] }
let(:push_access_check) { access.check('git-receive-pack', changes) }
let(:pull_access_check) { access.check('git-upload-pack', changes) }
let(:snippet) { personal_snippet }
let(:actor) { personal_snippet.author }
describe 'when feature flag :version_snippets is enabled' do
it 'allows push and pull access' do
aggregate_failures do
expect { pull_access_check }.not_to raise_error
expect { push_access_check }.not_to raise_error
end
subject(:access) { Gitlab::GitAccessSnippet.new(actor, snippet, protocol, authentication_abilities: authentication_abilities) }
describe 'when actor is a DeployKey' do
let(:actor) { build(:deploy_key) }
it 'does not allow push and pull access' do
expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:authentication_mechanism])
end
end
......@@ -30,56 +36,186 @@ describe Gitlab::GitAccessSnippet do
end
it 'does not allow push and pull access' do
aggregate_failures do
expect { push_access_check }.to raise_snippet_not_found
expect { pull_access_check }.to raise_snippet_not_found
end
expect { pull_access_check }.to raise_project_not_found
end
end
describe '#check_snippet_accessibility!' do
context 'when the snippet exists' do
it 'allows push and pull access' do
aggregate_failures do
expect { pull_access_check }.not_to raise_error
expect { push_access_check }.not_to raise_error
end
it 'allows access' do
project.add_developer(actor)
expect { pull_access_check }.not_to raise_error
end
end
context 'when the snippet is nil' do
let(:snippet) { nil }
it 'blocks push and pull with "not found"' do
aggregate_failures do
expect { pull_access_check }.to raise_snippet_not_found
expect { push_access_check }.to raise_snippet_not_found
end
it 'blocks access with "not found"' do
expect { pull_access_check }.to raise_snippet_not_found
end
end
context 'when the snippet does not have a repository' do
let(:snippet) { build_stubbed(:personal_snippet) }
it 'blocks push and pull with "not found"' do
aggregate_failures do
expect { pull_access_check }.to raise_snippet_not_found
expect { push_access_check }.to raise_snippet_not_found
it 'blocks access with "not found"' do
expect { pull_access_check }.to raise_snippet_not_found
end
end
end
context 'terms are enforced', :aggregate_failures do
before do
enforce_terms
end
let(:user) { snippet.author }
it 'blocks access when the user did not accept terms' do
message = /must accept the Terms of Service in order to perform this action/
expect { push_access_check }.to raise_unauthorized(message)
expect { pull_access_check }.to raise_unauthorized(message)
end
it 'allows access when the user accepted the terms' do
accept_terms(user)
expect { push_access_check }.not_to raise_error
expect { pull_access_check }.not_to raise_error
end
end
context 'project snippet accessibility', :aggregate_failures do
let(:snippet) { create(:project_snippet, :private, :repository, project: project) }
let(:user) { membership == :author ? snippet.author : create_user_from_membership(project, membership) }
shared_examples_for 'checks accessibility' do
[:anonymous, :non_member, :guest, :reporter, :maintainer, :admin, :author].each do |membership|
context membership.to_s do
let(:membership) { membership }
it 'respects accessibility' do
if Ability.allowed?(user, :update_snippet, snippet)
expect { push_access_check }.not_to raise_error
else
expect { push_access_check }.to raise_error(described_class::UnauthorizedError)
end
if Ability.allowed?(user, :read_snippet, snippet)
expect { pull_access_check }.not_to raise_error
else
expect { pull_access_check }.to raise_error(described_class::UnauthorizedError)
end
end
end
end
end
context 'when project is public' do
it_behaves_like 'checks accessibility'
end
context 'when project is public but snippet feature is private' do
let(:project) { create(:project, :public) }
before do
update_feature_access_level(project, :private)
end
it_behaves_like 'checks accessibility'
end
context 'when project is not accessible' do
let(:project) { create(:project, :private) }
[:anonymous, :non_member].each do |membership|
context membership.to_s do
let(:membership) { membership }
it 'respects accessibility' do
expect { push_access_check }.to raise_error(described_class::NotFoundError)
expect { pull_access_check }.to raise_error(described_class::NotFoundError)
end
end
end
end
end
private
context 'personal snippet accessibility', :aggregate_failures do
let(:snippet) { create(:personal_snippet, snippet_level, :repository) }
let(:user) { membership == :author ? snippet.author : create_user_from_membership(nil, membership) }
where(:snippet_level, :membership, :_expected_count) do
permission_table_for_personal_snippet_access
end
def access
described_class.new(actor, snippet, protocol,
authentication_abilities: [],
namespace_path: nil, project_path: nil,
redirected_path: nil, auth_result_type: nil)
with_them do
it "respects accessibility" do
error_class = described_class::UnauthorizedError
if Ability.allowed?(user, :update_snippet, snippet)
expect { push_access_check }.not_to raise_error
else
expect { push_access_check }.to raise_error(error_class)
end
if Ability.allowed?(user, :read_snippet, snippet)
expect { pull_access_check }.not_to raise_error
else
expect { pull_access_check }.to raise_error(error_class)
end
end
end
end
context 'when geo is enabled', if: Gitlab.ee? do
let(:user) { snippet.author }
let!(:primary_node) { FactoryBot.create(:geo_node, :primary) }
# Without override, push access would return Gitlab::GitAccessResult::CustomAction
it 'skips geo for snippet' do
allow(::Gitlab::Database).to receive(:read_only?).and_return(true)
allow(::Gitlab::Geo).to receive(:secondary_with_primary?).and_return(true)
expect { push_access_check }.to raise_unauthorized(/You can't push code to a read-only GitLab instance/)
end
end
context 'when changes are specific' do
let(:changes) { 'oldrev newrev ref' }
let(:user) { snippet.author }
it 'does not raise error if SnippetCheck does not raise error' do
expect_next_instance_of(Gitlab::Checks::SnippetCheck) do |check|
expect(check).to receive(:exec).and_call_original
end
expect { push_access_check }.not_to raise_error
end
it 'raises error if SnippetCheck raises error' do
expect_next_instance_of(Gitlab::Checks::SnippetCheck) do |check|
allow(check).to receive(:exec).and_raise(Gitlab::GitAccess::UnauthorizedError, 'foo')
end
expect { push_access_check }.to raise_unauthorized('foo')
end
end
private
def raise_snippet_not_found
raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:snippet_not_found])
end
def raise_project_not_found
raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found])
end
def raise_unauthorized(message)
raise_error(Gitlab::GitAccess::UnauthorizedError, message)
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::UserAccessSnippet do
subject(:access) { described_class.new(user, snippet: snippet) }
let_it_be(:project) { create(:project, :private) }
let_it_be(:snippet) { create(:project_snippet, :private, project: project) }
let(:user) { create(:user) }
describe '#can_do_action?' do
before do
allow(Ability).to receive(:allowed?).and_call_original
allow(Ability).to receive(:allowed?).with(user, :ability, snippet).and_return(:foo)
end
context 'when can access_git' do
it 'calls Ability#allowed? and returns its result' do
expect(access.can_do_action?(:ability)).to eq(:foo)
end
end
context 'when can not access_git' do
it 'disallows access' do
expect(Ability).to receive(:allowed?).with(user, :access_git, :global).and_return(false)
expect(access.can_do_action?(:ability)).to eq(false)
end
end
context 'when user is nil' do
let(:user) { nil }
it 'disallows access' do
expect(access.can_do_action?(:ability)).to eq(false)
end
end
end
describe '#can_push_to_branch?' do
include ProjectHelpers
[:anonymous, :non_member, :guest, :reporter, :maintainer, :admin, :author].each do |membership|
context membership.to_s do
let(:user) do
membership == :author ? snippet.author : create_user_from_membership(project, membership)
end
context 'when can access_git' do
it 'respects accessibility' do
expected_result = Ability.allowed?(user, :update_snippet, snippet)
expect(access.can_push_to_branch?('random_branch')).to eq(expected_result)
end
end
context 'when can not access_git' do
it 'disallows access' do
expect(Ability).to receive(:allowed?).with(user, :access_git, :global).and_return(false) if user
expect(access.can_push_to_branch?('random_branch')).to eq(false)
end
end
end
end
context 'when snippet is nil' do
let(:user) { create_user_from_membership(project, :admin) }
let(:snippet) { nil }
it 'disallows access' do
expect(access.can_push_to_branch?('random_branch')).to eq(false)
end
end
end
describe '#can_create_tag?' do
it 'returns false' do
expect(access.can_create_tag?('random_tag')).to be_falsey
end
end
describe '#can_delete_branch?' do
it 'returns false' do
expect(access.can_delete_branch?('random_branch')).to be_falsey
end
end
describe '#can_merge_to_branch?' do
it 'returns false' do
expect(access.can_merge_to_branch?('random_branch')).to be_falsey
end
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