summaryrefslogtreecommitdiff
path: root/app/validators
diff options
context:
space:
mode:
authorBob Van Landuyt <bob@gitlab.com>2017-04-05 15:41:00 +0200
committerBob Van Landuyt <bob@gitlab.com>2017-05-01 11:14:24 +0200
commit74fcccaab30ac0f9e11ed9a076c008ade13a50d0 (patch)
treee3341f6aa020659655b2b6941fe8660befe315bf /app/validators
parent536f2bdfd17ac3bab38851de2973dd1c89dccc3f (diff)
downloadgitlab-ce-74fcccaab30ac0f9e11ed9a076c008ade13a50d0.tar.gz
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.
Diffstat (limited to 'app/validators')
-rw-r--r--app/validators/namespace_validator.rb93
-rw-r--r--app/validators/project_path_validator.rb22
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