Streamline the path validation in groups & projects

`Project` uses `ProjectPathValidator` which is now a
`NamespaceValidator` that skips the format validation.

That way we're sure we are using the same collection of reserved
paths.

I updated the path constraints to reflect the changes: We now allow
some values that are only used on a top level namespace as a name for
a nested group/project.

2 files changed, 73 insertions, 42 deletions

diff --git a/app/validators/namespace_validator.rb b/app/validators/namespace_validator.rb
index 2aef4204e31..5a8b482d3db 100644
--- a/app/validators/namespace_validator.rb
+++ b/app/validators/namespace_validator.rb
@@ -5,7 +5,16 @@
 # Values are checked for formatting and exclusion from a list of reserved path
 # names.
 class NamespaceValidator < ActiveModel::EachValidator
-  RESERVED = %w[
+  # All routes that appear on the top level must be listed here.
+  # This will make sure that groups cannot be created with these names
+  # as these routes would be masked by the paths already in place.
+  #
+  # Example:
+  #   /api/api-project
+  #
+  #  the path `api` shouldn't be allowed because it would be masked by `api/*`
+  #
+  TOP_LEVEL_ROUTES = Set.new(%w[
     .well-known
     admin
     all
@@ -49,35 +58,56 @@ class NamespaceValidator < ActiveModel::EachValidator
     jwt
     oauth
     sent_notifications
-  ].freeze
+  ]).freeze
 
-  WILDCARD_ROUTES = %w[tree commits wikis new edit create update logs_tree
-                       preview blob blame raw files create_dir find_file
-                       artifacts graphs refs badges info git-upload-pack
-                       git-receive-pack gitlab-lfs autocomplete_sources
-                       templates avatar commit pages compare network snippets
-                       services mattermost deploy_keys forks import merge_requests
-                       branches merged_branches tags protected_branches variables
-                       triggers pipelines environments cycle_analytics builds
-                       hooks container_registry milestones labels issues
-                       project_members group_links notes noteable boards todos
-                       uploads runners runner_projects settings repository
-                       transfer remove_fork archive unarchive housekeeping
-                       toggle_star preview_markdown export remove_export
-                       generate_new_export download_export activity
-                       new_issue_address registry].freeze
+  # All project routes with wildcard argument must be listed here.
+  # Otherwise it can lead to routing issues when route considered as project name.
+  #
+  # Example:
+  #  /group/project/tree/deploy_keys
+  #
+  #  without tree as reserved name routing can match 'group/project' as group name,
+  #  'tree' as project name and 'deploy_keys' as route.
+  #
+  WILDCARD_ROUTES = Set.new(%w[tree commits wikis new edit create update logs_tree
+                               preview blob blame raw files create_dir find_file
+                               artifacts graphs refs badges info git-upload-pack
+                               git-receive-pack gitlab-lfs autocomplete_sources
+                               templates avatar commit pages compare network snippets
+                               services mattermost deploy_keys forks import merge_requests
+                               branches merged_branches tags protected_branches variables
+                               triggers pipelines environments cycle_analytics builds
+                               hooks container_registry milestones labels issues
+                               project_members group_links notes noteable boards todos
+                               uploads runners runner_projects settings repository
+                               transfer remove_fork archive unarchive housekeeping
+                               toggle_star preview_markdown export remove_export
+                               generate_new_export download_export activity
+                               new_issue_address registry])
 
-  STRICT_RESERVED = (RESERVED + WILDCARD_ROUTES).freeze
+  STRICT_RESERVED = (TOP_LEVEL_ROUTES | WILDCARD_ROUTES)
 
-  def self.valid?(value)
-    !reserved?(value) && follow_format?(value)
+  def self.valid_full_path?(full_path)
+    pieces = full_path.split('/')
+    first_part = pieces.first
+    pieces.all? do |namespace|
+      type = first_part == namespace ? :top_level : :wildcard
+      valid?(namespace, type: type)
+    end
   end
 
-  def self.reserved?(value, strict: false)
-    if strict
-      STRICT_RESERVED.include?(value)
+  def self.valid?(value, type: :strict)
+    !reserved?(value, type: type) && follow_format?(value)
+  end
+
+  def self.reserved?(value, type: :strict)
+    case type
+    when :wildcard
+      WILDCARD_ROUTES.include?(value)
+    when :top_level
+      TOP_LEVEL_ROUTES.include?(value)
     else
-      RESERVED.include?(value)
+      STRICT_RESERVED.include?(value)
     end
   end
 
@@ -92,10 +122,19 @@ class NamespaceValidator < ActiveModel::EachValidator
       record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
     end
 
-    strict = record.is_a?(Group) && record.parent_id
-
-    if reserved?(value, strict: strict)
+    if reserved?(value, type: validation_type(record))
       record.errors.add(attribute, "#{value} is a reserved name")
     end
   end
+
+  def validation_type(record)
+    case record
+    when Group
+      record.parent_id ? :wildcard : :top_level
+    when Project
+      :wildcard
+    else
+      :strict
+    end
+  end
 end
diff --git a/app/validators/project_path_validator.rb b/app/validators/project_path_validator.rb
index ee2ae65be7b..d41bdaeab84 100644
--- a/app/validators/project_path_validator.rb
+++ b/app/validators/project_path_validator.rb
@@ -4,25 +4,17 @@
 #
 # Values are checked for formatting and exclusion from a list of reserved path
 # names.
-class ProjectPathValidator < ActiveModel::EachValidator
-  # All project routes with wildcard argument must be listed here.
-  # Otherwise it can lead to routing issues when route considered as project name.
-  #
-  # Example:
-  #  /group/project/tree/deploy_keys
-  #
-  #  without tree as reserved name routing can match 'group/project' as group name,
-  #  'tree' as project name and 'deploy_keys' as route.
-  #
-  RESERVED = (NamespaceValidator::STRICT_RESERVED -
-              %w[dashboard help ci admin search notes services assets profile public]).freeze
-
+#
+# This is basically the same as the `NamespaceValidator` but it skips the validation
+# of the format with `Gitlab::Regex.namespace_regex`. The format of projects
+# is validated in the class itself.
+class ProjectPathValidator < NamespaceValidator
   def self.valid?(value)
     !reserved?(value)
   end
 
-  def self.reserved?(value)
-    RESERVED.include?(value)
+  def self.reserved?(value, type: :wildcard)
+    super(value, type: :wildcard)
   end
 
   delegate :reserved?, to: :class