Commit ef418be3 authored by Patrick Steinhardt's avatar Patrick Steinhardt

access: Move access checks into standalone function

We're about to split up access checks into those which are specific to
references and those which are specific to all changed objects. This
requires some refactoring on our side where we not only iterate over all
changes separately, but also where we do another batched check for all
objects introduced via all changes at once.

To ease the transition, refactor the access checks into their own
function. This allows us to more easily handle timeouts across both
kinds of checks.
parent e96d2971
......@@ -334,26 +334,26 @@ module Gitlab
# clear stale lock files.
project.repository.clean_stale_repository_files if project.present?
# Iterate over all changes to find if user allowed all of them to be applied
changes_list.each.with_index do |change, index|
first_change = index == 0
# 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, skip_lfs_integrity_check: !first_change)
end
check_access!
end
end
def check_single_change_access(change, skip_lfs_integrity_check: false)
Checks::ChangeAccess.new(
change,
user_access: user_access,
project: project,
skip_lfs_integrity_check: skip_lfs_integrity_check,
protocol: protocol,
logger: logger
).validate!
def check_access!
# Iterate over all changes to find if user allowed all of them to be applied
changes_list.each.with_index do |change, index|
skip_lfs_integrity_check = index != 0
# If user does not have access to make at least one change, cancel all
# push by allowing the exception to bubble up
Checks::ChangeAccess.new(
change,
user_access: user_access,
project: project,
skip_lfs_integrity_check: skip_lfs_integrity_check,
protocol: protocol,
logger: logger
).validate!
end
rescue Checks::TimedLogger::TimeoutError
raise TimeoutError, logger.full_message
end
......
......@@ -109,20 +109,18 @@ module Gitlab
end
check_size_before_push!
check_access!
check_push_size!
end
override :check_access!
def check_access!
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)
Checks::SnippetCheck.new(change, default_branch: snippet.default_branch, root_ref: snippet.repository.root_ref, logger: logger).validate!
Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet.max_file_limit, logger: logger).validate!
end
check_push_size!
end
override :check_single_change_access
def check_single_change_access(change, _skip_lfs_integrity_check: false)
Checks::SnippetCheck.new(change, default_branch: snippet.default_branch, root_ref: snippet.repository.root_ref, logger: logger).validate!
Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet.max_file_limit, logger: logger).validate!
rescue Checks::TimedLogger::TimeoutError
raise TimeoutError, logger.full_message
end
......
......@@ -1006,7 +1006,7 @@ RSpec.describe Gitlab::GitAccess do
expect { access.check('git-receive-pack', changes) }.not_to exceed_query_limit(control_count).with_threshold(2)
end
it 'raises TimeoutError when #check_single_change_access raises a timeout error' do
it 'raises TimeoutError when #check_access! raises a timeout error' do
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|
......
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