Add latest changes from gitlab-org/gitlab@master

1 files changed, 62 insertions, 46 deletions

diff --git a/lib/gitlab/checks/tag_check.rb b/lib/gitlab/checks/tag_check.rb
index d5addab74b8..5684b897256 100644
--- a/lib/gitlab/checks/tag_check.rb
+++ b/lib/gitlab/checks/tag_check.rb
@@ -6,8 +6,8 @@ module Gitlab
       ERROR_MESSAGES = {
         change_existing_tags: 'You are not allowed to change existing tags on this project.',
         update_protected_tag: 'Protected tags cannot be updated.',
-        delete_protected_tag: 'You are not allowed to delete protected tags from this project. '\
-          'Only a project maintainer or owner can delete a protected tag.',
+        delete_protected_tag: 'You are not allowed to delete protected tags from this project. ' \
+                              'Only a project maintainer or owner can delete a protected tag.',
         delete_protected_tag_non_web: 'You can only delete protected tags using the web interface.',
         create_protected_tag: 'You are not allowed to create this tag as it is protected.',
         default_branch_collision: 'You cannot use default branch name to create a tag',
@@ -24,69 +24,85 @@ module Gitlab
       def validate!
         return unless tag_name
 
-        logger.log_timed(LOG_MESSAGES[:tag_checks]) do
-          if tag_exists? && user_access.cannot_do_action?(:admin_tag)
-            raise GitAccess::ForbiddenError, ERROR_MESSAGES[:change_existing_tags]
-          end
-        end
-
-        default_branch_collision_check
+        logger.log_timed(LOG_MESSAGES[:tag_checks]) { tag_checks }
+        logger.log_timed(LOG_MESSAGES[:default_branch_collision_check]) { default_branch_collision_check }
         prohibited_tag_checks
-        protected_tag_checks
+        logger.log_timed(LOG_MESSAGES[:protected_tag_checks]) { protected_tag_checks }
       end
 
       private
 
+      def tag_checks
+        return unless tag_exists? && user_access.cannot_do_action?(:admin_tag)
+
+        raise GitAccess::ForbiddenError, ERROR_MESSAGES[:change_existing_tags]
+      end
+
+      def default_branch_collision_check
+        return unless creation? && tag_name == project.default_branch
+
+        raise GitAccess::ForbiddenError, ERROR_MESSAGES[:default_branch_collision]
+      end
+
       def prohibited_tag_checks
         return if deletion?
 
-        unless Gitlab::GitRefValidator.validate(tag_name)
-          raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_tag_name]
-        end
+        # Incorrectly encoded tags names may raise during other checks so we
+        # need to validate the encoding first
+        validate_encoding!
+        validate_valid_tag_name!
+        validate_tag_name_not_fully_qualified!
+      end
 
-        if tag_name.start_with?("refs/tags/") # rubocop: disable Style/GuardClause
-          raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_tag_name]
-        end
+      def protected_tag_checks
+        return unless ProtectedTag.protected?(project, tag_name)
 
-        # rubocop: disable Style/GuardClause
-        # rubocop: disable Style/SoleNestedConditional
-        if Feature.enabled?(:prohibited_tag_name_encoding_check, project)
-          unless Gitlab::EncodingHelper.force_encode_utf8(tag_name).valid_encoding?
-            raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_tag_name_encoding]
-          end
-        end
-        # rubocop: enable Style/SoleNestedConditional
-        # rubocop: enable Style/GuardClause
+        validate_protected_tag_update!
+        validate_protected_tag_deletion!
+        validate_protected_tag_creation!
       end
 
-      def protected_tag_checks
-        logger.log_timed(LOG_MESSAGES[__method__]) do
-          return unless ProtectedTag.protected?(project, tag_name) # rubocop:disable Cop/AvoidReturnFromBlocks
+      def validate_encoding!
+        return unless Feature.enabled?(:prohibited_tag_name_encoding_check, project)
+        return if Gitlab::EncodingHelper.force_encode_utf8(tag_name).valid_encoding?
 
-          raise(GitAccess::ForbiddenError, ERROR_MESSAGES[:update_protected_tag]) if update?
+        raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_tag_name_encoding]
+      end
+
+      def validate_valid_tag_name!
+        return if Gitlab::GitRefValidator.validate(tag_name)
 
-          if deletion?
-            unless user_access.user.can?(:maintainer_access, project)
-              raise(GitAccess::ForbiddenError, ERROR_MESSAGES[:delete_protected_tag])
-            end
+        raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_tag_name]
+      end
 
-            unless updated_from_web?
-              raise GitAccess::ForbiddenError, ERROR_MESSAGES[:delete_protected_tag_non_web]
-            end
-          end
+      def validate_tag_name_not_fully_qualified!
+        return unless tag_name.start_with?("refs/tags/")
 
-          unless user_access.can_create_tag?(tag_name)
-            raise GitAccess::ForbiddenError, ERROR_MESSAGES[:create_protected_tag]
-          end
-        end
+        raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_tag_name]
       end
 
-      def default_branch_collision_check
-        logger.log_timed(LOG_MESSAGES[:default_branch_collision_check]) do
-          if creation? && tag_name == project.default_branch
-            raise GitAccess::ForbiddenError, ERROR_MESSAGES[:default_branch_collision]
-          end
+      def validate_protected_tag_update!
+        return unless update?
+
+        raise(GitAccess::ForbiddenError, ERROR_MESSAGES[:update_protected_tag])
+      end
+
+      def validate_protected_tag_deletion!
+        return unless deletion?
+
+        unless user_access.user.can?(:maintainer_access, project)
+          raise(GitAccess::ForbiddenError, ERROR_MESSAGES[:delete_protected_tag])
         end
+
+        return if updated_from_web?
+
+        raise GitAccess::ForbiddenError, ERROR_MESSAGES[:delete_protected_tag_non_web]
+      end
+
+      def validate_protected_tag_creation!
+        return if user_access.can_create_tag?(tag_name)
+
+        raise GitAccess::ForbiddenError, ERROR_MESSAGES[:create_protected_tag]
       end
     end
   end