提交 402c7500 编写于 作者: G GitLab Release Tools Bot

Merge branch 'security-33712-ce' into 'master'

Fix private comment Elasticsearch leak

See merge request gitlab/gitlabhq!3521
...@@ -517,7 +517,11 @@ class Project < ApplicationRecord ...@@ -517,7 +517,11 @@ class Project < ApplicationRecord
# This scope returns projects where user has access to both the project and the feature. # This scope returns projects where user has access to both the project and the feature.
def self.filter_by_feature_visibility(feature, user) def self.filter_by_feature_visibility(feature, user)
with_feature_available_for_user(feature, user).public_or_visible_to_user(user) with_feature_available_for_user(feature, user)
.public_or_visible_to_user(
user,
ProjectFeature.required_minimum_access_level_for_private_project(feature)
)
end end
scope :active, -> { joins(:issues, :notes, :merge_requests).order('issues.created_at, notes.created_at, merge_requests.created_at DESC') } scope :active, -> { joins(:issues, :notes, :merge_requests).order('issues.created_at, notes.created_at, merge_requests.created_at DESC') }
......
...@@ -24,6 +24,7 @@ class ProjectFeature < ApplicationRecord ...@@ -24,6 +24,7 @@ class ProjectFeature < ApplicationRecord
FEATURES = %i(issues merge_requests wiki snippets builds repository pages).freeze FEATURES = %i(issues merge_requests wiki snippets builds repository pages).freeze
PRIVATE_FEATURES_MIN_ACCESS_LEVEL = { merge_requests: Gitlab::Access::REPORTER }.freeze PRIVATE_FEATURES_MIN_ACCESS_LEVEL = { merge_requests: Gitlab::Access::REPORTER }.freeze
PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT = { repository: Gitlab::Access::REPORTER }.freeze
STRING_OPTIONS = HashWithIndifferentAccess.new({ STRING_OPTIONS = HashWithIndifferentAccess.new({
'disabled' => DISABLED, 'disabled' => DISABLED,
'private' => PRIVATE, 'private' => PRIVATE,
...@@ -51,6 +52,15 @@ class ProjectFeature < ApplicationRecord ...@@ -51,6 +52,15 @@ class ProjectFeature < ApplicationRecord
PRIVATE_FEATURES_MIN_ACCESS_LEVEL.fetch(feature, Gitlab::Access::GUEST) PRIVATE_FEATURES_MIN_ACCESS_LEVEL.fetch(feature, Gitlab::Access::GUEST)
end end
# 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_FOR_PRIVATE_PROJECT.fetch(feature) do
required_minimum_access_level(feature)
end
end
def access_level_from_str(level) def access_level_from_str(level)
STRING_OPTIONS.fetch(level) STRING_OPTIONS.fetch(level)
end end
......
...@@ -6,6 +6,16 @@ describe ProjectFeature do ...@@ -6,6 +6,16 @@ describe ProjectFeature do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } 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 describe '.quoted_access_level_column' do
it 'returns the table name and quoted column name for a feature' do it 'returns the table name and quoted column name for a feature' do
expected = '"project_features"."issues_access_level"' expected = '"project_features"."issues_access_level"'
...@@ -246,10 +256,24 @@ describe ProjectFeature do ...@@ -246,10 +256,24 @@ describe ProjectFeature do
expect(described_class.required_minimum_access_level('merge_requests')).to eq(Gitlab::Access::REPORTER) expect(described_class.required_minimum_access_level('merge_requests')).to eq(Gitlab::Access::REPORTER)
end 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 it 'raises error if feature is invalid' do
expect do expect do
described_class.required_minimum_access_level(:foos) described_class.required_minimum_access_level(:foos)
end.to raise_error end.to raise_error
end end
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 end
...@@ -2924,7 +2924,7 @@ describe Project do ...@@ -2924,7 +2924,7 @@ describe Project do
end end
describe '#any_lfs_file_locks?', :request_store do describe '#any_lfs_file_locks?', :request_store do
set(:project) { create(:project) } let_it_be(:project) { create(:project) }
it 'returns false when there are no LFS file locks' do it 'returns false when there are no LFS file locks' do
expect(project.any_lfs_file_locks?).to be_falsey expect(project.any_lfs_file_locks?).to be_falsey
...@@ -3411,6 +3411,20 @@ describe Project do ...@@ -3411,6 +3411,20 @@ describe Project do
expect(projects).to eq([public_project]) expect(projects).to eq([public_project])
end end
end end
context 'min_access_level' do
let!(:private_project) { create(:project, :private) }
before do
private_project.add_guest(user)
end
it 'excludes projects when user does not have required minimum access level' do
projects = described_class.all.public_or_visible_to_user(user, Gitlab::Access::REPORTER)
expect(projects).to contain_exactly(public_project)
end
end
end end
describe '.ids_with_issuables_available_for' do describe '.ids_with_issuables_available_for' do
...@@ -3539,7 +3553,7 @@ describe Project do ...@@ -3539,7 +3553,7 @@ describe Project do
include ProjectHelpers include ProjectHelpers
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
set(:group) { create(:group) } let_it_be(:group) { create(:group) }
let!(:project) { create(:project, project_level, namespace: group ) } let!(:project) { create(:project, project_level, namespace: group ) }
let(:user) { create_user_from_membership(project, membership) } let(:user) { create_user_from_membership(project, membership) }
...@@ -3562,6 +3576,66 @@ describe Project do ...@@ -3562,6 +3576,66 @@ describe Project do
end end
end end
end end
context 'issues' do
let(:feature) { Issue }
where(:project_level, :feature_access_level, :membership, :expected_count) do
permission_table_for_guest_feature_access
end
with_them do
it "respects visibility" do
update_feature_access_level(project, feature_access_level)
expected_objects = expected_count == 1 ? [project] : []
expect(
described_class.filter_by_feature_visibility(feature, user)
).to eq(expected_objects)
end
end
end
context 'wiki' do
let(:feature) { :wiki }
where(:project_level, :feature_access_level, :membership, :expected_count) do
permission_table_for_guest_feature_access
end
with_them do
it "respects visibility" do
update_feature_access_level(project, feature_access_level)
expected_objects = expected_count == 1 ? [project] : []
expect(
described_class.filter_by_feature_visibility(feature, user)
).to eq(expected_objects)
end
end
end
context 'code' do
let(:feature) { :repository }
where(:project_level, :feature_access_level, :membership, :expected_count) do
permission_table_for_guest_feature_access_and_non_private_project_only
end
with_them do
it "respects visibility" do
update_feature_access_level(project, feature_access_level)
expected_objects = expected_count == 1 ? [project] : []
expect(
described_class.filter_by_feature_visibility(feature, user)
).to eq(expected_objects)
end
end
end
end end
describe '#pages_available?' do describe '#pages_available?' do
......
...@@ -10,16 +10,18 @@ module ProjectHelpers ...@@ -10,16 +10,18 @@ module ProjectHelpers
nil nil
when :non_member when :non_member
create(:user, name: membership) create(:user, name: membership)
when :admin
create(:user, :admin, name: 'admin')
else else
create(:user, name: membership).tap { |u| target.add_user(u, membership) } create(:user, name: membership).tap { |u| target.add_user(u, membership) }
end end
end end
def update_feature_access_level(project, access_level) def update_feature_access_level(project, access_level)
project.update!( features = ProjectFeature::FEATURES.dup
repository_access_level: access_level, features.delete(:pages)
merge_requests_access_level: access_level, params = features.each_with_object({}) { |feature, h| h["#{feature}_access_level"] = access_level }
builds_access_level: access_level
) project.update!(params)
end end
end end
...@@ -20,6 +20,12 @@ module SearchHelpers ...@@ -20,6 +20,12 @@ module SearchHelpers
end end
end end
def has_search_scope?(scope)
page.within '.search-filter' do
has_link?(scope)
end
end
def max_limited_count def max_limited_count
Gitlab::SearchResults::COUNT_LIMIT_MESSAGE Gitlab::SearchResults::COUNT_LIMIT_MESSAGE
end end
......
...@@ -3,13 +3,28 @@ ...@@ -3,13 +3,28 @@
RSpec.shared_context 'ProjectPolicyTable context' do RSpec.shared_context 'ProjectPolicyTable context' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
let(:pendings) { {} }
let(:pending?) do
pendings.include?(
{
project_level: project_level,
feature_access_level: feature_access_level,
membership: membership,
expected_count: expected_count
}
)
end
# rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/AbcSize
# project_level, :feature_access_level, :membership, :expected_count
def permission_table_for_reporter_feature_access def permission_table_for_reporter_feature_access
:public | :enabled | :admin | 1
:public | :enabled | :reporter | 1 :public | :enabled | :reporter | 1
:public | :enabled | :guest | 1 :public | :enabled | :guest | 1
:public | :enabled | :non_member | 1 :public | :enabled | :non_member | 1
:public | :enabled | :anonymous | 1 :public | :enabled | :anonymous | 1
:public | :private | :admin | 1
:public | :private | :reporter | 1 :public | :private | :reporter | 1
:public | :private | :guest | 0 :public | :private | :guest | 0
:public | :private | :non_member | 0 :public | :private | :non_member | 0
...@@ -20,11 +35,13 @@ RSpec.shared_context 'ProjectPolicyTable context' do ...@@ -20,11 +35,13 @@ RSpec.shared_context 'ProjectPolicyTable context' do
:public | :disabled | :non_member | 0 :public | :disabled | :non_member | 0
:public | :disabled | :anonymous | 0 :public | :disabled | :anonymous | 0
:internal | :enabled | :admin | 1
:internal | :enabled | :reporter | 1 :internal | :enabled | :reporter | 1
:internal | :enabled | :guest | 1 :internal | :enabled | :guest | 1
:internal | :enabled | :non_member | 1 :internal | :enabled | :non_member | 1
:internal | :enabled | :anonymous | 0 :internal | :enabled | :anonymous | 0
:internal | :private | :admin | 1
:internal | :private | :reporter | 1 :internal | :private | :reporter | 1
:internal | :private | :guest | 0 :internal | :private | :guest | 0
:internal | :private | :non_member | 0 :internal | :private | :non_member | 0
...@@ -35,11 +52,7 @@ RSpec.shared_context 'ProjectPolicyTable context' do ...@@ -35,11 +52,7 @@ RSpec.shared_context 'ProjectPolicyTable context' do
:internal | :disabled | :non_member | 0 :internal | :disabled | :non_member | 0
:internal | :disabled | :anonymous | 0 :internal | :disabled | :anonymous | 0
:private | :enabled | :reporter | 1 :private | :private | :admin | 1
:private | :enabled | :guest | 1
:private | :enabled | :non_member | 0
:private | :enabled | :anonymous | 0
:private | :private | :reporter | 1 :private | :private | :reporter | 1
:private | :private | :guest | 0 :private | :private | :guest | 0
:private | :private | :non_member | 0 :private | :private | :non_member | 0
...@@ -51,12 +64,15 @@ RSpec.shared_context 'ProjectPolicyTable context' do ...@@ -51,12 +64,15 @@ RSpec.shared_context 'ProjectPolicyTable context' do
:private | :disabled | :anonymous | 0 :private | :disabled | :anonymous | 0
end end
# project_level, :feature_access_level, :membership, :expected_count
def permission_table_for_guest_feature_access def permission_table_for_guest_feature_access
:public | :enabled | :admin | 1
:public | :enabled | :reporter | 1 :public | :enabled | :reporter | 1
:public | :enabled | :guest | 1 :public | :enabled | :guest | 1
:public | :enabled | :non_member | 1 :public | :enabled | :non_member | 1
:public | :enabled | :anonymous | 1 :public | :enabled | :anonymous | 1
:public | :private | :admin | 1
:public | :private | :reporter | 1 :public | :private | :reporter | 1
:public | :private | :guest | 1 :public | :private | :guest | 1
:public | :private | :non_member | 0 :public | :private | :non_member | 0
...@@ -67,11 +83,13 @@ RSpec.shared_context 'ProjectPolicyTable context' do ...@@ -67,11 +83,13 @@ RSpec.shared_context 'ProjectPolicyTable context' do
:public | :disabled | :non_member | 0 :public | :disabled | :non_member | 0
:public | :disabled | :anonymous | 0 :public | :disabled | :anonymous | 0
:internal | :enabled | :admin | 1
:internal | :enabled | :reporter | 1 :internal | :enabled | :reporter | 1
:internal | :enabled | :guest | 1 :internal | :enabled | :guest | 1
:internal | :enabled | :non_member | 1 :internal | :enabled | :non_member | 1
:internal | :enabled | :anonymous | 0 :internal | :enabled | :anonymous | 0
:internal | :private | :admin | 1
:internal | :private | :reporter | 1 :internal | :private | :reporter | 1
:internal | :private | :guest | 1 :internal | :private | :guest | 1
:internal | :private | :non_member | 0 :internal | :private | :non_member | 0
...@@ -82,11 +100,7 @@ RSpec.shared_context 'ProjectPolicyTable context' do ...@@ -82,11 +100,7 @@ RSpec.shared_context 'ProjectPolicyTable context' do
:internal | :disabled | :non_member | 0 :internal | :disabled | :non_member | 0
:internal | :disabled | :anonymous | 0 :internal | :disabled | :anonymous | 0
:private | :enabled | :reporter | 1 :private | :private | :admin | 1
:private | :enabled | :guest | 1
:private | :enabled | :non_member | 0
:private | :enabled | :anonymous | 0
:private | :private | :reporter | 1 :private | :private | :reporter | 1
:private | :private | :guest | 1 :private | :private | :guest | 1
:private | :private | :non_member | 0 :private | :private | :non_member | 0
...@@ -98,6 +112,196 @@ RSpec.shared_context 'ProjectPolicyTable context' do ...@@ -98,6 +112,196 @@ RSpec.shared_context 'ProjectPolicyTable context' do
:private | :disabled | :anonymous | 0 :private | :disabled | :anonymous | 0
end end
# This table is based on permission_table_for_guest_feature_access,
# but with a slight twist.
# Some features can be hidden away to GUEST, when project is private.
# (see ProjectFeature::PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT)
# This is the table for such features.
#
# e.g. `repository` feature has minimum requirement of GUEST,
# but a GUEST are prohibited from reading code if project is private.
#
# project_level, :feature_access_level, :membership, :expected_count
def permission_table_for_guest_feature_access_and_non_private_project_only
:public | :enabled | :admin | 1
:public | :enabled | :reporter | 1
:public | :enabled | :guest | 1
:public | :enabled | :non_member | 1
:public | :enabled | :anonymous | 1
:public | :private | :admin | 1
:public | :private | :reporter | 1
:public | :private | :guest | 1
:public | :private | :non_member | 0
:public | :private | :anonymous | 0
:public | :disabled | :reporter | 0
:public | :disabled | :guest | 0
:public | :disabled | :non_member | 0
:public | :disabled | :anonymous | 0
:internal | :enabled | :admin | 1
:internal | :enabled | :reporter | 1
:internal | :enabled | :guest | 1
:internal | :enabled | :non_member | 1
:internal | :enabled | :anonymous | 0
:internal | :private | :admin | 1
:internal | :private | :reporter | 1
:internal | :private | :guest | 1
:internal | :private | :non_member | 0
:internal | :private | :anonymous | 0
:internal | :disabled | :reporter | 0
:internal | :disabled | :guest | 0
:internal | :disabled | :non_member | 0
:internal | :disabled | :anonymous | 0
:private | :private | :admin | 1
:private | :private | :reporter | 1
:private | :private | :guest | 0
:private | :private | :non_member | 0
:private | :private | :anonymous | 0
:private | :disabled | :reporter | 0
:private | :disabled | :guest | 0
:private | :disabled | :non_member | 0
:private | :disabled | :anonymous | 0
end
# :project_level, :issues_access_level, :merge_requests_access_level, :membership, :expected_count
def permission_table_for_milestone_access
:public | :enabled | :enabled | :admin | 1
:public | :enabled | :enabled | :reporter | 1
:public | :enabled | :enabled | :guest | 1
:public | :enabled | :enabled | :non_member | 1
:public | :enabled | :enabled | :anonymous | 1
:public | :enabled | :private | :admin | 1
:public | :enabled | :private | :reporter | 1
:public | :enabled | :private | :guest | 1
:public | :enabled | :private | :non_member | 1
:public | :enabled | :private | :anonymous | 1
:public | :enabled | :disabled | :admin | 1
:public | :enabled | :disabled | :reporter | 1
:public | :enabled | :disabled | :guest | 1
:public | :enabled | :disabled | :non_member | 1
:public | :enabled | :disabled | :anonymous | 1
:public | :private | :enabled | :admin | 1
:public | :private | :enabled | :reporter | 1
:public | :private | :enabled | :guest | 1
:public | :private | :enabled | :non_member | 1
:public | :private | :enabled | :anonymous | 1
:public | :private | :private | :admin | 1
:public | :private | :private | :reporter | 1
:public | :private | :private | :guest | 1
:public | :private | :private | :non_member | 0
:public | :private | :private | :anonymous | 0
:public | :private | :disabled | :admin | 1
:public | :private | :disabled | :reporter | 1
:public | :private | :disabled | :guest | 1
:public | :private | :disabled | :non_member | 0
:public | :private | :disabled | :anonymous | 0
:public | :disabled | :enabled | :admin | 1
:public | :disabled | :enabled | :reporter | 1
:public | :disabled | :enabled | :guest | 1
:public | :disabled | :enabled | :non_member | 1
:public | :disabled | :enabled | :anonymous | 1
:public | :disabled | :private | :admin | 1
:public | :disabled | :private | :reporter | 1
:public | :disabled | :private | :guest | 0
:public | :disabled | :private | :non_member | 0
:public | :disabled | :private | :anonymous | 0
:public | :disabled | :disabled | :reporter | 0
:public | :disabled | :disabled | :guest | 0
:public | :disabled | :disabled | :non_member | 0
:public | :disabled | :disabled | :anonymous | 0
:internal | :enabled | :enabled | :admin | 1
:internal | :enabled | :enabled | :reporter | 1
:internal | :enabled | :enabled | :guest | 1
:internal | :enabled | :enabled | :non_member | 1
:internal | :enabled | :enabled | :anonymous | 0
:internal | :enabled | :private | :admin | 1
:internal | :enabled | :private | :reporter | 1
:internal | :enabled | :private | :guest | 1
:internal | :enabled | :private | :non_member | 1
:internal | :enabled | :private | :anonymous | 0
:internal | :enabled | :disabled | :admin | 1
:internal | :enabled | :disabled | :reporter | 1
:internal | :enabled | :disabled | :guest | 1
:internal | :enabled | :disabled | :non_member | 1
:internal | :enabled | :disabled | :anonymous | 0
:internal | :private | :enabled | :admin | 1
:internal | :private | :enabled | :reporter | 1
:internal | :private | :enabled | :guest | 1
:internal | :private | :enabled | :non_member | 1
:internal | :private | :enabled | :anonymous | 0
:internal | :private | :private | :admin | 1
:internal | :private | :private | :reporter | 1
:internal | :private | :private | :guest | 1
:internal | :private | :private | :non_member | 0
:internal | :private | :private | :anonymous | 0
:internal | :private | :disabled | :admin | 1
:internal | :private | :disabled | :reporter | 1
:internal | :private | :disabled | :guest | 1
:internal | :private | :disabled | :non_member | 0
:internal | :private | :disabled | :anonymous | 0
:internal | :disabled | :enabled | :admin | 1
:internal | :disabled | :enabled | :reporter | 1
:internal | :disabled | :enabled | :guest | 1
:internal | :disabled | :enabled | :non_member | 1
:internal | :disabled | :enabled | :anonymous | 0
:internal | :disabled | :private | :admin | 1
:internal | :disabled | :private | :reporter | 1
:internal | :disabled | :private | :guest | 0
:internal | :disabled | :private | :non_member | 0
:internal | :disabled | :private | :anonymous | 0
:internal | :disabled | :disabled | :reporter | 0
:internal | :disabled | :disabled | :guest | 0
:internal | :disabled | :disabled | :non_member | 0
:internal | :disabled | :disabled | :anonymous | 0
:private | :private | :private | :admin | 1
:private | :private | :private | :reporter | 1
:private | :private | :private | :guest | 1
:private | :private | :private | :non_member | 0
:private | :private | :private | :anonymous | 0
:private | :private | :disabled | :admin | 1
:private | :private | :disabled | :reporter | 1
:private | :private | :disabled | :guest | 1
:private | :private | :disabled | :non_member | 0
:private | :private | :disabled | :anonymous | 0
:private | :disabled | :private | :admin | 1
:private | :disabled | :private | :reporter | 1
:private | :disabled | :private | :guest | 0
:private | :disabled | :private | :non_member | 0
:private | :disabled | :private | :anonymous | 0
:private | :disabled | :disabled | :reporter | 0
:private | :disabled | :disabled | :guest | 0
:private | :disabled | :disabled | :non_member | 0
:private | :disabled | :disabled | :anonymous | 0
end
# :project_level, :membership, :expected_count
def permission_table_for_project_access def permission_table_for_project_access
:public | :reporter | 1 :public | :reporter | 1
:public | :guest | 1 :public | :guest | 1
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册