diff options
author | Bob Van Landuyt <bob@gitlab.com> | 2017-04-05 15:41:00 +0200 |
---|---|---|
committer | Bob Van Landuyt <bob@gitlab.com> | 2017-05-01 11:14:24 +0200 |
commit | 74fcccaab30ac0f9e11ed9a076c008ade13a50d0 (patch) | |
tree | e3341f6aa020659655b2b6941fe8660befe315bf /app/validators | |
parent | 536f2bdfd17ac3bab38851de2973dd1c89dccc3f (diff) | |
download | gitlab-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.rb | 93 | ||||
-rw-r--r-- | app/validators/project_path_validator.rb | 22 |
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 |