Cleanup & tests for UserAccess#can_create_tag?

4 files changed, 76 insertions, 9 deletions

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