Reject group-routes as names of child namespaces

1 files changed, 71 insertions, 18 deletions

diff --git a/app/validators/dynamic_path_validator.rb b/app/validators/dynamic_path_validator.rb
index 96da92b5d76..c4ed5ac85eb 100644
--- a/app/validators/dynamic_path_validator.rb
+++ b/app/validators/dynamic_path_validator.rb
@@ -55,6 +55,7 @@ class DynamicPathValidator < ActiveModel::EachValidator
     snippets
     teams
     u
+    unicorn_test
     unsubscribes
     uploads
     users
@@ -92,7 +93,52 @@ class DynamicPathValidator < ActiveModel::EachValidator
     wikis
   ]).freeze
 
-  STRICT_RESERVED = (TOP_LEVEL_ROUTES | WILDCARD_ROUTES).freeze
+  # These are all the paths that follow `/groups/*id/ or `/groups/*group_id`
+  # We need to reject these because we have a `/groups/*id` page that is the same
+  # as the `/*id`.
+  #
+  # If we would allow a subgroup to be created with the name `activity` then
+  # this group would not be accessible through `/groups/parent/activity` since
+  # this would map to the activity-page of it's parent.
+  GROUP_ROUTES = Set.new(%w[
+    activity
+    avatar
+    edit
+    group_members
+    issues
+    labels
+    merge_requests
+    milestones
+    projects
+    subgroups
+  ])
+
+  CHILD_ROUTES = (WILDCARD_ROUTES | GROUP_ROUTES).freeze
+
+  def self.without_reserved_wildcard_paths_regex
+    @full_path_without_wildcard_regex ||= regex_excluding_child_paths(WILDCARD_ROUTES)
+  end
+
+  def self.without_reserved_child_paths_regex
+    @full_path_without_child_routes_regex ||= regex_excluding_child_paths(CHILD_ROUTES)
+  end
+
+  # This is used to validate a full path.
+  # It doesn't match paths
+  #   - Starting with one of the top level words
+  #   - Containing one of the child level words in the middle of a path
+  def self.regex_excluding_child_paths(child_routes)
+    reserved_top_level_words = Regexp.union(TOP_LEVEL_ROUTES.to_a)
+    not_starting_in_reserved_word = %r{^(/?)(?!(#{reserved_top_level_words})(/|$))}
+
+    reserved_child_level_words = Regexp.union(child_routes.to_a)
+    not_containing_reserved_child = %r{(?!(\S+)/(#{reserved_child_level_words})(/|$))}
+
+    @full_path_regex = %r{
+      #{not_starting_in_reserved_word}
+      #{not_containing_reserved_child}
+      #{Gitlab::Regex::FULL_NAMESPACE_REGEX_STR}}x
+  end
 
   def self.valid?(path)
     path_segments = path.split('/')
@@ -102,41 +148,48 @@ class DynamicPathValidator < ActiveModel::EachValidator
 
   def self.reserved?(path)
     path = path.to_s.downcase
-    top_level, wildcard_part = path.split('/', 2)
+    _project_parts, namespace_parts = path.reverse.split('/', 2).map(&:reverse)
 
-    includes_reserved_top_level?(top_level) || includes_reserved_wildcard?(wildcard_part)
+    wildcard_reserved?(path) || any_reserved?(namespace_parts)
   end
 
-  def self.includes_reserved_wildcard?(path)
-    WILDCARD_ROUTES.any? do |reserved_word|
-      contains_path_part?(path, reserved_word)
-    end
-  end
+  def self.any_reserved?(path)
+    return false unless path
 
-  def self.includes_reserved_top_level?(path)
-    TOP_LEVEL_ROUTES.any? do |reserved_route|
-      contains_path_part?(path, reserved_route)
-    end
+    path !~ without_reserved_child_paths_regex
   end
 
-  def self.contains_path_part?(path, part)
-    path =~ %r{(/|\A)#{Regexp.quote(part)}(/|\z)}
+  def self.wildcard_reserved?(path)
+    return false unless path
+
+    path !~ without_reserved_wildcard_paths_regex
   end
 
   def self.follow_format?(value)
     value =~ Gitlab::Regex.namespace_regex
   end
 
-  delegate :reserved?, :follow_format?, to: :class
+  delegate :reserved?,
+           :any_reserved?,
+           :follow_format?, to: :class
+
+  def valid_full_path?(record, value)
+    full_path = record.respond_to?(:full_path) ? record.full_path : value
+
+    case record
+    when Project || User
+      reserved?(full_path)
+    else
+      any_reserved?(full_path)
+    end
+  end
 
   def validate_each(record, attribute, value)
     unless follow_format?(value)
       record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
     end
 
-    full_path = record.respond_to?(:full_path) ? record.full_path : value
-
-    if reserved?(full_path)
+    if valid_full_path?(record, value)
       record.errors.add(attribute, "#{value} is a reserved name")
     end
   end