diff --git a/app/models/organizations/organization.rb b/app/models/organizations/organization.rb
index 5eaef1419c105b77857e2e128fffae7194a53a7a..764f752b55793a52fd0b8ccf3713c7537b42b9ae 100644
--- a/app/models/organizations/organization.rb
+++ b/app/models/organizations/organization.rb
@@ -17,6 +17,7 @@ class Organization < ApplicationRecord
validates :path,
presence: true,
+ 'organizations/path': true,
length: { minimum: 2, maximum: 255 }
def default?
diff --git a/app/validators/abstract_path_validator.rb b/app/validators/abstract_path_validator.rb
index 45ac695c5ec5fc0f578f9f536d6bd9f411790105..ff390a624c5cf529aad9db60c7c06b0db5be03dc 100644
--- a/app/validators/abstract_path_validator.rb
+++ b/app/validators/abstract_path_validator.rb
@@ -26,11 +26,22 @@ def validate_each(record, attribute, value)
return
end
- full_path = record.build_full_path
- return unless full_path
+ if build_full_path_to_validate_against_reserved_names?
+ path_to_validate_against_reserved_names = record.build_full_path
+ return unless path_to_validate_against_reserved_names
+ else
+ path_to_validate_against_reserved_names = value
+ end
- unless self.class.valid_path?(full_path)
+ unless self.class.valid_path?(path_to_validate_against_reserved_names)
record.errors.add(attribute, "#{value} is a reserved name")
end
end
+
+ def build_full_path_to_validate_against_reserved_names?
+ # By default, entities using the `Routable` concern can build full paths.
+ # But entities like `Organization` do not have a parent, and hence cannot build full paths,
+ # and this method can be overridden to return `false` in such cases.
+ true
+ end
end
diff --git a/app/validators/organizations/path_validator.rb b/app/validators/organizations/path_validator.rb
new file mode 100644
index 0000000000000000000000000000000000000000..a1c22654a32a5cdd8fc1b648a849b49c090d1db1
--- /dev/null
+++ b/app/validators/organizations/path_validator.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module Organizations
+ class PathValidator < ::NamespacePathValidator
+ def self.path_regex
+ Gitlab::PathRegex.organization_path_regex
+ end
+
+ def build_full_path_to_validate_against_reserved_names?
+ # full paths cannot be built for organizations because organizations do not have a parent
+ # and it does not include the `Routable` concern.
+ false
+ end
+ end
+end
diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb
index b0804c2ff661197bf70ca70edd7c82ce4509e88f..e112423f1678279d861d93bba594f1e08e6727ed 100644
--- a/lib/gitlab/path_regex.rb
+++ b/lib/gitlab/path_regex.rb
@@ -122,6 +122,7 @@ module PathRegex
ILLEGAL_PROJECT_PATH_WORDS = PROJECT_WILDCARD_ROUTES
ILLEGAL_GROUP_PATH_WORDS = (PROJECT_WILDCARD_ROUTES | GROUP_ROUTES).freeze
+ ILLEGAL_ORGANIZATION_PATH_WORDS = (TOP_LEVEL_ROUTES | PROJECT_WILDCARD_ROUTES | GROUP_ROUTES).freeze
# The namespace regex is used in JavaScript to validate usernames in the "Register" form. However, Javascript
# does not support the negative lookbehind assertion (?<!) that disallows usernames ending in `.git` and `.atom`.
@@ -138,6 +139,17 @@ module PathRegex
PROJECT_PATH_FORMAT_REGEX = /(?:#{PATH_REGEX_STR})#{NO_SUFFIX_REGEX}/.freeze
FULL_NAMESPACE_FORMAT_REGEX = %r{(#{NAMESPACE_FORMAT_REGEX}/){,#{Namespace::NUMBER_OF_ANCESTORS_ALLOWED}}#{NAMESPACE_FORMAT_REGEX}}.freeze
+ def organization_route_regex
+ @organization_route_regex ||= begin
+ illegal_words = Regexp.new(Regexp.union(ILLEGAL_ORGANIZATION_PATH_WORDS).source, Regexp::IGNORECASE)
+
+ single_line_regexp %r{
+ (?!(#{illegal_words})/)
+ #{NAMESPACE_FORMAT_REGEX}
+ }x
+ end
+ end
+
def root_namespace_route_regex
@root_namespace_route_regex ||= begin
illegal_words = Regexp.new(Regexp.union(TOP_LEVEL_ROUTES).source, Regexp::IGNORECASE)
@@ -195,6 +207,10 @@ def full_namespace_path_regex
@full_namespace_path_regex ||= %r{\A#{full_namespace_route_regex}/\z}
end
+ def organization_path_regex
+ @organization_path_regex ||= %r{\A#{organization_route_regex}/\z}
+ end
+
def full_project_path_regex
@full_project_path_regex ||= %r{\A#{full_namespace_route_regex}/#{project_route_regex}/\z}
end
diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb
index 718b20c59edb7d3ef4ebc437bc348329bcd204fb..53dc145dcc4c12bacbc325dff35414c5f522c719 100644
--- a/spec/lib/gitlab/path_regex_spec.rb
+++ b/spec/lib/gitlab/path_regex_spec.rb
@@ -258,6 +258,23 @@ def failure_message(constant_name, migration_helper, missing_words: [], addition
end
end
+ describe '.organization_path_regex' do
+ subject { described_class.organization_path_regex }
+
+ it 'rejects reserved words' do
+ expect(subject).not_to match('admin/')
+ expect(subject).not_to match('api/')
+ expect(subject).not_to match('create/')
+ expect(subject).not_to match('new/')
+ end
+
+ it 'accepts other words' do
+ expect(subject).to match('simple/')
+ expect(subject).to match('org/')
+ expect(subject).to match('my_org/')
+ end
+ end
+
describe '.full_namespace_path_regex' do
subject { described_class.full_namespace_path_regex }
diff --git a/spec/models/organizations/organization_spec.rb b/spec/models/organizations/organization_spec.rb
index bb3d0c2307d66a7dd7b892e2b002e6b69060e726..3b202b85b48ecb070d40282874c920c3a0bade53 100644
--- a/spec/models/organizations/organization_spec.rb
+++ b/spec/models/organizations/organization_spec.rb
@@ -18,6 +18,37 @@
it { is_expected.to validate_length_of(:name).is_at_most(255) }
it { is_expected.to validate_presence_of(:path) }
it { is_expected.to validate_length_of(:path).is_at_least(2).is_at_most(255) }
+
+ describe 'path validator' do
+ using RSpec::Parameterized::TableSyntax
+
+ let(:default_path_error) do
+ "can contain only letters, digits, '_', '-' and '.'. Cannot start with '-' or end in '.', '.git' or '.atom'."
+ end
+
+ let(:reserved_path_error) do
+ "is a reserved name"
+ end
+
+ where(:path, :valid, :error_message) do
+ 'path.' | false | ref(:default_path_error)
+ 'path.git' | false | ref(:default_path_error)
+ 'new' | false | ref(:reserved_path_error)
+ '.path' | true | nil
+ 'org__path' | true | nil
+ 'some-name' | true | nil
+ 'simple' | true | nil
+ end
+
+ with_them do
+ it 'validates organization path' do
+ organization = build(:organization, name: 'Default', path: path)
+
+ expect(organization.valid?).to be(valid)
+ expect(organization.errors.full_messages.to_sentence).to include(error_message) if error_message.present?
+ end
+ end
+ end
end
context 'when using scopes' do
diff --git a/spec/validators/organizations/path_validator_spec.rb b/spec/validators/organizations/path_validator_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..415c10d98dfe5cbb808185cb6993eb6c1eac457d
--- /dev/null
+++ b/spec/validators/organizations/path_validator_spec.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Organizations::PathValidator, feature_category: :cell do
+ let(:validator) { described_class.new(attributes: [:path]) }
+
+ describe '.valid_path?' do
+ it 'handles invalid utf8' do
+ expect(described_class.valid_path?(+"a\0weird\255path")).to be_falsey
+ end
+ end
+
+ describe '#validates_each' do
+ it 'adds a message when the path is not in the correct format' do
+ organization = build(:organization)
+
+ validator.validate_each(organization, :path, "Path with spaces, and comma's!")
+
+ expect(organization.errors[:path]).to include(Gitlab::PathRegex.namespace_format_message)
+ end
+
+ it 'adds a message when the path is reserved when creating' do
+ organization = build(:organization, path: 'help')
+
+ validator.validate_each(organization, :path, 'help')
+
+ expect(organization.errors[:path]).to include('help is a reserved name')
+ end
+
+ it 'adds a message when the path is reserved when updating' do
+ organization = create(:organization)
+ organization.path = 'help'
+
+ validator.validate_each(organization, :path, 'help')
+
+ expect(organization.errors[:path]).to include('help is a reserved name')
+ end
+ end
+end