diff options
author | Sean McGivern <sean@gitlab.com> | 2019-01-04 08:42:20 +0000 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2019-01-04 08:42:20 +0000 |
commit | 626f3d03676cb7d714d9c5e9d78c76bdfb2ddda0 (patch) | |
tree | b765d63b06e18561e463ef77d3ceb04146a26c00 /lib | |
parent | d56124b5e16e15afd830a1bdc8c34a4a57d898d8 (diff) | |
parent | e893f560e01c0039a4fc4c6c93be8e82b5888b3b (diff) | |
download | gitlab-ce-626f3d03676cb7d714d9c5e9d78c76bdfb2ddda0.tar.gz |
Merge branch 'dm-git-access-any-ce' into 'master'
[CE] Don't run checks for changed refs when specific changes are unknown
See merge request gitlab-org/gitlab-ce!23990
Diffstat (limited to 'lib')
-rw-r--r-- | lib/gitlab/checks/base_checker.rb | 6 | ||||
-rw-r--r-- | lib/gitlab/checks/change_access.rb | 5 | ||||
-rw-r--r-- | lib/gitlab/checks/diff_check.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/checks/push_check.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/git_access.rb | 50 | ||||
-rw-r--r-- | lib/gitlab/git_access_wiki.rb | 2 |
6 files changed, 42 insertions, 27 deletions
diff --git a/lib/gitlab/checks/base_checker.rb b/lib/gitlab/checks/base_checker.rb index 7fbcf6a4ff4..09b17b5b76b 100644 --- a/lib/gitlab/checks/base_checker.rb +++ b/lib/gitlab/checks/base_checker.rb @@ -18,12 +18,16 @@ module Gitlab private + def creation? + Gitlab::Git.blank_ref?(oldrev) + end + def deletion? Gitlab::Git.blank_ref?(newrev) end def update? - !Gitlab::Git.blank_ref?(oldrev) && !deletion? + !creation? && !deletion? end def updated_from_web? diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb index 7778d3068cc..8a57a3a6d9a 100644 --- a/lib/gitlab/checks/change_access.rb +++ b/lib/gitlab/checks/change_access.rb @@ -10,7 +10,7 @@ module Gitlab attr_reader(*ATTRIBUTES) def initialize( - change, user_access:, project:, skip_authorization: false, + change, user_access:, project:, skip_lfs_integrity_check: false, protocol:, logger: ) @oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref) @@ -18,7 +18,6 @@ module Gitlab @tag_name = Gitlab::Git.tag_name(@ref) @user_access = user_access @project = project - @skip_authorization = skip_authorization @skip_lfs_integrity_check = skip_lfs_integrity_check @protocol = protocol @@ -27,8 +26,6 @@ module Gitlab end def exec - return true if skip_authorization - ref_level_checks # Check of commits should happen as the last step # given they're expensive in terms of performance diff --git a/lib/gitlab/checks/diff_check.rb b/lib/gitlab/checks/diff_check.rb index 63da9a3d6b5..ea0d8c85a66 100644 --- a/lib/gitlab/checks/diff_check.rb +++ b/lib/gitlab/checks/diff_check.rb @@ -11,7 +11,7 @@ module Gitlab }.freeze def validate! - return if deletion? || newrev.nil? + return if deletion? return unless should_run_diff_validations? return if commits.empty? diff --git a/lib/gitlab/checks/push_check.rb b/lib/gitlab/checks/push_check.rb index f3a52f09868..91f8d0bdbc8 100644 --- a/lib/gitlab/checks/push_check.rb +++ b/lib/gitlab/checks/push_check.rb @@ -6,7 +6,7 @@ module Gitlab def validate! logger.log_timed("Checking if you are allowed to push...") do unless can_push? - raise GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.' + raise GitAccess::UnauthorizedError, GitAccess::ERROR_MESSAGES[:push_code] end end end @@ -15,7 +15,7 @@ module Gitlab def can_push? user_access.can_do_action?(:push_code) || - user_access.can_push_to_branch?(branch_name) + project.branch_allows_collaboration?(user_access.user, branch_name) end end end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 802fa65dd63..010bd0e520c 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -12,6 +12,10 @@ module Gitlab TimeoutError = Class.new(StandardError) ProjectMovedError = Class.new(NotFoundError) + # Use the magic string '_any' to indicate we do not know what the + # changes are. This is also what gitlab-shell does. + ANY = '_any' + ERROR_MESSAGES = { upload: 'You are not allowed to upload code for this project.', download: 'You are not allowed to download code from this project.', @@ -24,7 +28,8 @@ module Gitlab upload_pack_disabled_over_http: 'Pulling over HTTP is not allowed.', receive_pack_disabled_over_http: 'Pushing over HTTP is not allowed.', read_only: 'The repository is temporarily read-only. Please try again later.', - cannot_push_to_read_only: "You can't push code to a read-only GitLab instance." + cannot_push_to_read_only: "You can't push code to a read-only GitLab instance.", + push_code: 'You are not allowed to push code to this project.' }.freeze INTERNAL_TIMEOUT = 50.seconds.freeze @@ -199,7 +204,7 @@ module Gitlab def ensure_project_on_push!(cmd, changes) return if project || deploy_key? - return unless receive_pack?(cmd) && changes == '_any' && authentication_abilities.include?(:push_code) + return unless receive_pack?(cmd) && changes == ANY && authentication_abilities.include?(:push_code) namespace = Namespace.find_by_full_path(namespace_path) @@ -256,24 +261,34 @@ module Gitlab raise UnauthorizedError, ERROR_MESSAGES[:upload] end - return if changes.blank? # Allow access this is needed for EE. - check_change_access! end def check_change_access! - # If there are worktrees with a HEAD pointing to a non-existent object, - # calls to `git rev-list --all` will fail in git 2.15+. This should also - # clear stale lock files. - project.repository.clean_stale_repository_files - - # 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) + # Deploy keys with write access can push anything + return if deploy_key? + + if changes == ANY + can_push = user_access.can_do_action?(:push_code) || + project.any_branch_allows_collaboration?(user_access.user) + + unless can_push + raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:push_code] + end + else + # If there are worktrees with a HEAD pointing to a non-existent object, + # calls to `git rev-list --all` will fail in git 2.15+. This should also + # clear stale lock files. + project.repository.clean_stale_repository_files + + # 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 end end @@ -282,7 +297,6 @@ module Gitlab change, user_access: user_access, project: project, - skip_authorization: deploy_key?, skip_lfs_integrity_check: skip_lfs_integrity_check, protocol: protocol, logger: logger @@ -348,7 +362,7 @@ module Gitlab protected def changes_list - @changes_list ||= Gitlab::ChangesList.new(changes) + @changes_list ||= Gitlab::ChangesList.new(changes == ANY ? [] : changes) end def user diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb index 3f24001e4ee..0af91957fa8 100644 --- a/lib/gitlab/git_access_wiki.rb +++ b/lib/gitlab/git_access_wiki.rb @@ -15,7 +15,7 @@ module Gitlab authentication_abilities.include?(:download_code) && user_access.can_do_action?(:download_wiki_code) end - def check_single_change_access(change, _options = {}) + def check_change_access! unless user_access.can_do_action?(:create_wiki) raise UnauthorizedError, ERROR_MESSAGES[:write_to_wiki] end |