diff --git a/app/models/concerns/protected_ref.rb b/app/models/concerns/protected_ref.rb index a04dea0bc55..ab28eb19b64 100644 --- a/app/models/concerns/protected_ref.rb +++ b/app/models/concerns/protected_ref.rb @@ -8,7 +8,7 @@ module ProtectedRef delegate :matching, :matches?, :wildcard?, to: :ref_matcher - def self.matching_refs_accesible_to(ref, user, action: :push) + def self.protected_ref_accessible_to?(ref, user, action: :push) access_levels_for_ref(ref, action: action).any? do |access_level| access_level.check_access(user) end diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb index d0bbd713710..0d96c4d41d7 100644 --- a/lib/gitlab/checks/change_access.rb +++ b/lib/gitlab/checks/change_access.rb @@ -79,7 +79,7 @@ module Gitlab return "Protected tags cannot be deleted." end - unless user_access.can_push_tag?(@tag_name) + unless user_access.can_create_tag?(@tag_name) return "You are not allowed to create this tag as it is protected." end end diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index 5541a45e948..6af5de4dc08 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -28,14 +28,11 @@ module Gitlab true end - #TODO: Test this - #TODO move most to ProtectedTag::AccessChecker. Or maybe UserAccess::Protections::Tag - #TODO: then consider removing method, if it turns out can_access_git? and can?(:push_code are checked in change_access - def can_push_tag?(ref) + def can_create_tag?(ref) return false unless can_access_git? if ProtectedTag.protected?(project, ref) - project.protected_tags.matching_refs_accesible_to(ref, user) + project.protected_tags.protected_ref_accessible_to?(ref, user) else user.can?(:push_code, project) end @@ -47,7 +44,7 @@ module Gitlab if ProtectedBranch.protected?(project, ref) return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user) - has_access = project.protected_branches.matching_refs_accesible_to(ref, user, action: :push) + has_access = project.protected_branches.protected_ref_accessible_to?(ref, user, action: :push) has_access || !project.repository.branch_exists?(ref) && can_merge_to_branch?(ref) else @@ -59,7 +56,7 @@ module Gitlab return false unless can_access_git? if ProtectedBranch.protected?(project, ref) - project.protected_branches.matching_refs_accesible_to(ref, user, action: :merge) + project.protected_branches.protected_ref_accessible_to?(ref, user, action: :merge) else user.can?(:push_code, project) end diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index 369e55f61f1..c425aef359a 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -142,4 +142,74 @@ describe Gitlab::UserAccess, lib: true do end end end + + describe 'can_create_tag?' do + describe 'push to none protected tag' do + it 'returns true if user is a master' do + project.add_user(user, :master) + + expect(access.can_create_tag?('random_tag')).to be_truthy + end + + it 'returns true if user is a developer' do + project.add_user(user, :developer) + + expect(access.can_create_tag?('random_tag')).to be_truthy + end + + it 'returns false if user is a reporter' do + project.add_user(user, :reporter) + + expect(access.can_create_tag?('random_tag')).to be_falsey + end + end + + + describe 'push to protected tag' do + let(:tag) { create(:protected_tag, project: project, name: "test") } + let(:not_existing_tag) { create :protected_tag, project: project } + + it 'returns true if user is a master' do + project.add_user(user, :master) + + expect(access.can_create_tag?(tag.name)).to be_truthy + end + + it 'returns false if user is a developer' do + project.add_user(user, :developer) + + expect(access.can_create_tag?(tag.name)).to be_falsey + end + + it 'returns false if user is a reporter' do + project.add_user(user, :reporter) + + expect(access.can_create_tag?(tag.name)).to be_falsey + end + end + + describe 'push to protected tag if allowed for developers' do + before do + @tag = create(:protected_tag, :developers_can_push, project: project) + end + + it 'returns true if user is a master' do + project.add_user(user, :master) + + expect(access.can_create_tag?(@tag.name)).to be_truthy + end + + it 'returns true if user is a developer' do + project.add_user(user, :developer) + + expect(access.can_create_tag?(@tag.name)).to be_truthy + end + + it 'returns false if user is a reporter' do + project.add_user(user, :reporter) + + expect(access.can_create_tag?(@tag.name)).to be_falsey + end + end + end end