Skip to content
Snippets Groups Projects
Commit 01c22328 authored by GitLab Release Tools Bot's avatar GitLab Release Tools Bot
Browse files

Merge branch 'security-zoekt-omit-private-repos-from-public-projects-17-1' into '17-1-stable-ee'

Remove search results from public projects with unauthorized repos

See merge request https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4210



Merged-by: default avatarGitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>
Approved-by: default avatarDmitry Gruzd <dgruzd@gitlab.com>
Co-authored-by: default avatarJohn Mason <jmason@gitlab.com>
parents 017a9ba1 74bebdd2
No related branches found
No related tags found
No related merge requests found
......@@ -924,13 +924,30 @@ def self.with_feature_available_for_user(feature, user)
end
def self.projects_user_can(projects, user, action)
projects = where(id: projects)
DeclarativePolicy.user_scope do
projects.select { |project| Ability.allowed?(user, action, project) }
end
end
def self.filter_out_public_projects_with_unauthorized_private_repos(projects, user)
public_projects_with_private_repos = projects.with_project_feature.where(
visibility_level: Gitlab::VisibilityLevel::PUBLIC,
project_features: { repository_access_level: ProjectFeature::PRIVATE }
).pluck(:id)
return projects unless public_projects_with_private_repos.present?
authorized_public_projects_with_private_repos = projects.filter_by_feature_visibility(
:repository, user
)
rejected_projects_with_private_repos = (
public_projects_with_private_repos - authorized_public_projects_with_private_repos.pluck(:id)
)
projects.where.not(id: rejected_projects_with_private_repos)
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)
......
......@@ -268,13 +268,14 @@ def filtered_projects(projects)
return Project.all if projects == :any
filtered_projects = projects.without_order
filtered_projects = filtered_projects.non_archived unless filters[:include_archived]
if Feature.enabled?(:search_add_fork_filter_to_zoekt, current_user) && !filters[:include_forked]
filtered_projects = filtered_projects.not_a_fork
end
filtered_projects
Project.filter_out_public_projects_with_unauthorized_private_repos(filtered_projects, current_user)
end
def zoekt_targets
......
......@@ -12,7 +12,7 @@
end
describe '#results' do
let_it_be(:project) { create(:project, :repository) }
let_it_be(:project) { create(:project, :public, :repository) }
let(:node_id) { ::Search::Zoekt::Node.last.id }
let(:results) { described_class.new.results }
......
......@@ -284,6 +284,27 @@
end
end
context 'without N+1 queries' do
it 'does not have N+1 queries for projects' do
projects = [project_1, project_2]
collection = ::Project.id_in(projects.map(&:id))
control = ActiveRecord::QueryRecorder.new(skip_cached: false) do
described_class.new(user, query, collection, node_id: node_id, filters: filters).objects('blobs')
end
projects << create(:project, group: create(:group))
projects << create(:project, :mirror)
collection = ::Project.id_in(projects.map(&:id))
expect do
described_class.new(user, query, collection, node_id: node_id, filters: filters).objects('blobs')
end.not_to issue_same_number_of_queries_as(control)
end
end
context 'when no filters are passed' do
it 'calls search on Gitlab::Search::Zoekt::Client with non archived project ids' do
expect(Gitlab::Search::Zoekt::Client).to receive(:search).with(
......@@ -298,6 +319,41 @@
end
end
context 'when there is a public project with a private repository' do
let(:limit_projects) { ::Project.id_in(public_project_with_private_repo.id) }
let(:query) { ".*" }
let(:public_project_with_private_repo) do
create(:project, :public, :repository, :repository_private, :custom_repo,
files: { 'foo/a.txt' => 'foo', 'b.txt' => 'bar' })
end
before do
zoekt_ensure_project_indexed!(public_project_with_private_repo)
end
it 'does not include results from private repository' do
expect(Gitlab::Search::Zoekt::Client).not_to receive(:search)
expect(search).to be_empty
end
context 'when there are also permitted repositories in project list' do
let(:limit_projects) { ::Project.id_in([public_project_with_private_repo.id, project_1.id]) }
it 'still returns results from permitted repositories' do
expect(Gitlab::Search::Zoekt::Client).to receive(:search).with(
query,
num: described_class::ZOEKT_COUNT_LIMIT,
project_ids: [project_1.id],
node_id: node_id,
search_mode: :exact
).and_call_original
expect(search).not_to be_empty
end
end
end
describe 'archive filters' do
context 'when include_archived filter is set to true' do
let(:filters) { { include_archived: true } }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment