Commit e16e1d57 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'dz-nested-groups-restrictions' into 'master'

Nested groups path restrictions pt. 1

See merge request !9738
parents 9533fc35 e6cc7a0a
...@@ -35,12 +35,21 @@ class NamespaceValidator < ActiveModel::EachValidator ...@@ -35,12 +35,21 @@ class NamespaceValidator < ActiveModel::EachValidator
users users
].freeze ].freeze
WILDCARD_ROUTES = %w[tree commits wikis new edit create update logs_tree
preview blob blame raw files create_dir find_file].freeze
STRICT_RESERVED = (RESERVED + WILDCARD_ROUTES).freeze
def self.valid?(value) def self.valid?(value)
!reserved?(value) && follow_format?(value) !reserved?(value) && follow_format?(value)
end end
def self.reserved?(value) def self.reserved?(value, strict: false)
RESERVED.include?(value) if strict
STRICT_RESERVED.include?(value)
else
RESERVED.include?(value)
end
end end
def self.follow_format?(value) def self.follow_format?(value)
...@@ -54,7 +63,9 @@ class NamespaceValidator < ActiveModel::EachValidator ...@@ -54,7 +63,9 @@ class NamespaceValidator < ActiveModel::EachValidator
record.errors.add(attribute, Gitlab::Regex.namespace_regex_message) record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
end end
if reserved?(value) strict = record.is_a?(Group) && record.parent_id
if reserved?(value, strict: strict)
record.errors.add(attribute, "#{value} is a reserved name") record.errors.add(attribute, "#{value} is a reserved name")
end end
end end
......
...@@ -14,10 +14,8 @@ class ProjectPathValidator < ActiveModel::EachValidator ...@@ -14,10 +14,8 @@ class ProjectPathValidator < ActiveModel::EachValidator
# without tree as reserved name routing can match 'group/project' as group name, # without tree as reserved name routing can match 'group/project' as group name,
# 'tree' as project name and 'deploy_keys' as route. # 'tree' as project name and 'deploy_keys' as route.
# #
RESERVED = (NamespaceValidator::RESERVED - RESERVED = (NamespaceValidator::STRICT_RESERVED -
%w[dashboard help ci admin search notes services assets profile public] + %w[dashboard help ci admin search notes services assets profile public]).freeze
%w[tree commits wikis new edit create update logs_tree
preview blob blame raw files create_dir find_file]).freeze
def self.valid?(value) def self.valid?(value)
!reserved?(value) !reserved?(value)
......
---
title: Restrict nested group names to prevent ambiguous routes
merge_request: 9738
author:
...@@ -28,6 +28,20 @@ describe Namespace, models: true do ...@@ -28,6 +28,20 @@ describe Namespace, models: true do
expect(nested).not_to be_valid expect(nested).not_to be_valid
expect(nested.errors[:parent_id].first).to eq('has too deep level of nesting') expect(nested.errors[:parent_id].first).to eq('has too deep level of nesting')
end end
describe 'reserved path validation' do
context 'nested group' do
let(:group) { build(:group, :nested, path: 'tree') }
it { expect(group).not_to be_valid }
end
context 'top-level group' do
let(:group) { build(:group, path: 'tree') }
it { expect(group).to be_valid }
end
end
end end
describe "Respond to" do describe "Respond to" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment