kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Fix private feature Elasticsearch leak

Browse source 
Add spec to test different combinations.
This commit is contained in:
Mark Chao 2019-09-12 15:17:49 +08:00 committed by Bob Van Landuyt
parent 7b621c274c
commit 9adcdaab51
6 changed files with 216 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
app/models/project.rb
View file

@ -482,7 +482,7 @@ class Project < ApplicationRecord
# the feature is either public, enabled, or internal with permission for the user.
# Note: this scope doesn't enforce that the user has access to the projects, it just checks
# that the user has access to the feature. It's important to use this scope with others
# that checks project authorizations first.
# that checks project authorizations first (e.g. `filter_by_feature_visibility`).
#
# This method uses an optimised version of `with_feature_access_level` for
# logged in users to more efficiently get private projects with the given
@ -510,6 +510,11 @@ class Project < ApplicationRecord
end
end
# This scope returns projects where user has access to both the project and the feature.
def self.filter_by_feature_visibility(feature, user)
with_feature_available_for_user(feature, user).public_or_visible_to_user(user)
end
scope :active, -> { joins(:issues, :notes, :merge_requests).order('issues.created_at, notes.created_at, merge_requests.created_at DESC') }
scope :abandoned, -> { where('projects.last_activity_at < ?', 6.months.ago) }

5
changelogs/unreleased/security-29491.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Fix private feature Elasticsearch leak
merge_request:
author:
type: security

30
spec/models/project_spec.rb
View file

@ -3469,6 +3469,36 @@ describe Project do
end
end
describe '.filter_by_feature_visibility' do
include_context 'ProjectPolicyTable context'
include ProjectHelpers
using RSpec::Parameterized::TableSyntax
set(:group) { create(:group) }
let!(:project) { create(:project, project_level, namespace: group ) }
let(:user) { create_user_from_membership(project, membership) }
context 'reporter level access' do
let(:feature) { MergeRequest }
where(:project_level, :feature_access_level, :membership, :expected_count) do
permission_table_for_reporter_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
end
describe '#pages_available?' do
let(:project) { create(:project, group: group) }

25
spec/support/helpers/project_helpers.rb Normal file
View file

@ -0,0 +1,25 @@
# frozen_string_literal: true
module ProjectHelpers
# @params target [Project] membership target
# @params membership [Symbol] accepts the membership levels :guest, :reporter...
# and phony levels :non_member and :anonymous
def create_user_from_membership(target, membership)
case membership
when :anonymous
nil
when :non_member
create(:user, name: membership)
else
create(:user, name: membership).tap { |u| target.add_user(u, membership) }
end
end
def update_feature_access_level(project, access_level)
project.update!(
repository_access_level: access_level,
merge_requests_access_level: access_level,
builds_access_level: access_level
)
end
end

32
spec/support/helpers/search_results_helpers.rb Normal file
View file

@ -0,0 +1,32 @@
# frozen_string_literal: true
module SearchResultHelpers
# @param target [Symbol] search target, e.g. "merge_requests", "blobs"
def expect_search_results(users, target, expected_count: nil, expected_objects: nil)
# TODO: https://gitlab.com/gitlab-org/gitlab/issues/32645
return if expected_count && expected_count > 0
users = Array(users)
target = target.to_s
users.each do |user|
user_name = user&.name || 'anonymous user'
results = yield(user)
objects = results.objects(target)
if expected_count
actual_count = results.public_send("#{target}_count")
expect(actual_count).to eq(expected_count), "expected count to be #{expected_count} for #{user_name}, got #{actual_count}"
end
if expected_objects
if expected_objects.empty?
expect(objects.empty?).to eq(true)
else
expect(objects).to contain_exactly(*expected_objects)
end
end
end
end
end

118
spec/support/shared_contexts/policies/project_policy_table_shared_context.rb Normal file
View file

@ -0,0 +1,118 @@
# frozen_string_literal: true
RSpec.shared_context 'ProjectPolicyTable context' do
using RSpec::Parameterized::TableSyntax
# rubocop:disable Metrics/AbcSize
def permission_table_for_reporter_feature_access
:public | :enabled | :reporter | 1
:public | :enabled | :guest | 1
:public | :enabled | :non_member | 1
:public | :enabled | :anonymous | 1
:public | :private | :reporter | 1
:public | :private | :guest | 0
: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 | :reporter | 1
:internal | :enabled | :guest | 1
:internal | :enabled | :non_member | 1
:internal | :enabled | :anonymous | 0
:internal | :private | :reporter | 1
:internal | :private | :guest | 0
: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 | :enabled | :reporter | 1
:private | :enabled | :guest | 1
:private | :enabled | :non_member | 0
:private | :enabled | :anonymous | 0
: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
def permission_table_for_guest_feature_access
:public | :enabled | :reporter | 1
:public | :enabled | :guest | 1
:public | :enabled | :non_member | 1
:public | :enabled | :anonymous | 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 | :reporter | 1
:internal | :enabled | :guest | 1
:internal | :enabled | :non_member | 1
:internal | :enabled | :anonymous | 0
: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 | :enabled | :reporter | 1
:private | :enabled | :guest | 1
:private | :enabled | :non_member | 0
:private | :enabled | :anonymous | 0
:private | :private | :reporter | 1
:private | :private | :guest | 1
: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
def permission_table_for_project_access
:public | :reporter | 1
:public | :guest | 1
:public | :non_member | 1
:public | :anonymous | 1
:internal | :reporter | 1
:internal | :guest | 1
:internal | :non_member | 1
:internal | :anonymous | 0
:private | :reporter | 1
:private | :guest | 1
:private | :non_member | 0
:private | :anonymous | 0
end
# rubocop:enable Metrics/AbcSize
end