diff options
author | Mark Chao <mchao@gitlab.com> | 2019-11-14 10:50:19 +0800 |
---|---|---|
committer | Mark Chao <mchao@gitlab.com> | 2019-11-22 18:14:04 +0800 |
commit | 443db2868d899b9a41e85766d78d1c4203c4cb1f (patch) | |
tree | 2f981b48fe96fb9372da2f006ca3221d48f05a28 | |
parent | 0de1bfeac34a2f26f481e871210fe74d17f75375 (diff) | |
download | gitlab-ce-443db2868d899b9a41e85766d78d1c4203c4cb1f.tar.gz |
Internalize private project minimum access level
Some feature allows GUEST to access only if project is not private.
This method returns access level when targeting private projects.
-rw-r--r-- | app/models/project.rb | 12 | ||||
-rw-r--r-- | app/models/project_feature.rb | 13 | ||||
-rw-r--r-- | spec/models/project_feature_spec.rb | 24 |
3 files changed, 37 insertions, 12 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index f319c5b1d9f..7ae4e2a4cd7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -517,13 +517,11 @@ class Project < ApplicationRecord # This scope returns projects where user has access to both the project and the feature. def self.filter_by_feature_visibility(feature, user) - scope = with_feature_available_for_user(feature, user) - - if ProjectFeature.guest_allowed_on_private_project?(feature) - scope.public_or_visible_to_user(user) - else - scope.public_or_visible_to_user(user, Gitlab::Access::REPORTER) - end + with_feature_available_for_user(feature, user) + .public_or_visible_to_user( + user, + ProjectFeature.required_minimum_access_level_for_private_project(feature) + ) end scope :active, -> { joins(:issues, :notes, :merge_requests).order('issues.created_at, notes.created_at, merge_requests.created_at DESC') } diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index 564e531c320..caa65d32c86 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -24,7 +24,7 @@ class ProjectFeature < ApplicationRecord FEATURES = %i(issues merge_requests wiki snippets builds repository pages).freeze PRIVATE_FEATURES_MIN_ACCESS_LEVEL = { merge_requests: Gitlab::Access::REPORTER }.freeze - FEATURES_ALLOWED_BY_GUEST_ON_PRIVATE_PROJECT = %i(issues wiki).freeze + PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT = { repository: Gitlab::Access::REPORTER }.freeze STRING_OPTIONS = HashWithIndifferentAccess.new({ 'disabled' => DISABLED, 'private' => PRIVATE, @@ -46,16 +46,19 @@ class ProjectFeature < ApplicationRecord "#{table}.#{attribute}" end - def guest_allowed_on_private_project?(feature) + def required_minimum_access_level(feature) feature = ensure_feature!(feature) - FEATURES_ALLOWED_BY_GUEST_ON_PRIVATE_PROJECT.include?(feature) + PRIVATE_FEATURES_MIN_ACCESS_LEVEL.fetch(feature, Gitlab::Access::GUEST) end - def required_minimum_access_level(feature) + # Guest users can perform certain features on public and internal projects, but not private projects. + def required_minimum_access_level_for_private_project(feature) feature = ensure_feature!(feature) - PRIVATE_FEATURES_MIN_ACCESS_LEVEL.fetch(feature, Gitlab::Access::GUEST) + PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT.fetch(feature) do + required_minimum_access_level(feature) + end end def access_level_from_str(level) diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index 5b448c343a0..9ce1b8fd895 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -6,6 +6,16 @@ describe ProjectFeature do let(:project) { create(:project) } let(:user) { create(:user) } + describe 'PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT' do + it 'has higher level than that of PRIVATE_FEATURES_MIN_ACCESS_LEVEL' do + described_class::PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT.each do |feature, level| + if generic_level = described_class::PRIVATE_FEATURES_MIN_ACCESS_LEVEL[feature] + expect(level).to be >= generic_level + end + end + end + end + describe '.quoted_access_level_column' do it 'returns the table name and quoted column name for a feature' do expected = '"project_features"."issues_access_level"' @@ -246,10 +256,24 @@ describe ProjectFeature do expect(described_class.required_minimum_access_level('merge_requests')).to eq(Gitlab::Access::REPORTER) end + it 'handles repository' do + expect(described_class.required_minimum_access_level(:repository)).to eq(Gitlab::Access::GUEST) + end + it 'raises error if feature is invalid' do expect do described_class.required_minimum_access_level(:foos) end.to raise_error end end + + describe '.required_minimum_access_level_for_private_project' do + it 'returns higher permission for repository' do + expect(described_class.required_minimum_access_level_for_private_project(:repository)).to eq(Gitlab::Access::REPORTER) + end + + it 'returns normal permission for issues' do + expect(described_class.required_minimum_access_level_for_private_project(:issues)).to eq(Gitlab::Access::GUEST) + end + end end |