summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2017-02-06 17:16:50 +0200
committerDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2017-02-08 11:59:01 +0200
commitf642a4608d99b1344d862134900c2e078eda9cfc (patch)
tree96e10f2af7a6279afa3498136eb5f728d9f5ab2b
parent99fceff4f9e98cc7cb725882abec6e5b7c9b0170 (diff)
downloadgitlab-ce-f642a4608d99b1344d862134900c2e078eda9cfc.tar.gz
Limit level of nesting for groupsdz-limit-nested-groups
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
-rw-r--r--app/models/namespace.rb15
-rw-r--r--spec/models/namespace_spec.rb33
2 files changed, 36 insertions, 12 deletions
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 2fb2eb44aaa..0d2600fe3bc 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -7,6 +7,11 @@ class Namespace < ActiveRecord::Base
include Gitlab::CurrentSettings
include Routable
+ # Prevent users from creating unreasonably deep level of nesting.
+ # The number 20 was taken based on maximum nesting level of
+ # Android repo (15) + some extra backup.
+ NUMBER_OF_ANCESTORS_ALLOWED = 20
+
cache_markdown_field :description, pipeline: :description
has_many :projects, dependent: :destroy
@@ -29,6 +34,8 @@ class Namespace < ActiveRecord::Base
length: { maximum: 255 },
namespace: true
+ validate :nesting_level_allowed
+
delegate :name, to: :owner, allow_nil: true, prefix: true
after_update :move_dir, if: :path_changed?
@@ -194,7 +201,7 @@ class Namespace < ActiveRecord::Base
# Scopes the model on ancestors of the record
def ancestors
if parent_id
- path = route.path
+ path = route ? route.path : full_path
paths = []
until path.blank?
@@ -270,4 +277,10 @@ class Namespace < ActiveRecord::Base
path_was
end
end
+
+ def nesting_level_allowed
+ if ancestors.count > Group::NUMBER_OF_ANCESTORS_ALLOWED
+ errors.add(:parent_id, "has too deep level of nesting")
+ end
+ end
end
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 4e96f19eb6f..9f07dd30c6c 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -3,21 +3,32 @@ require 'spec_helper'
describe Namespace, models: true do
let!(:namespace) { create(:namespace) }
- it { is_expected.to have_many :projects }
- it { is_expected.to have_many :project_statistics }
- it { is_expected.to belong_to :parent }
- it { is_expected.to have_many :children }
+ describe 'associations' do
+ it { is_expected.to have_many :projects }
+ it { is_expected.to have_many :project_statistics }
+ it { is_expected.to belong_to :parent }
+ it { is_expected.to have_many :children }
+ end
- it { is_expected.to validate_presence_of(:name) }
- it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) }
- it { is_expected.to validate_length_of(:name).is_at_most(255) }
+ describe 'validations' do
+ it { is_expected.to validate_presence_of(:name) }
+ it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) }
+ it { is_expected.to validate_length_of(:name).is_at_most(255) }
+ it { is_expected.to validate_length_of(:description).is_at_most(255) }
+ it { is_expected.to validate_presence_of(:path) }
+ it { is_expected.to validate_length_of(:path).is_at_most(255) }
+ it { is_expected.to validate_presence_of(:owner) }
- it { is_expected.to validate_length_of(:description).is_at_most(255) }
+ it 'does not allow too deep nesting' do
+ ancestors = (1..21).to_a
+ nested = build(:namespace, parent: namespace)
- it { is_expected.to validate_presence_of(:path) }
- it { is_expected.to validate_length_of(:path).is_at_most(255) }
+ allow(nested).to receive(:ancestors).and_return(ancestors)
- it { is_expected.to validate_presence_of(:owner) }
+ expect(nested).not_to be_valid
+ expect(nested.errors[:parent_id].first).to eq('has too deep level of nesting')
+ end
+ end
describe "Respond to" do
it { is_expected.to respond_to(:human_name) }