Commit d97c7c31 authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre

Merge branch '36439-git-redact' into 'master'

Redact search to include code and wiki

Closes #36439

See merge request gitlab-org/gitlab!21611
parents 8c3165ef eb5bd9fd
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
class Blob < SimpleDelegator class Blob < SimpleDelegator
include Presentable include Presentable
include BlobLanguageFromGitAttributes include BlobLanguageFromGitAttributes
include BlobActiveModel
CACHE_TIME = 60 # Cache raw blobs referred to by a (mutable) ref for 1 minute CACHE_TIME = 60 # Cache raw blobs referred to by a (mutable) ref for 1 minute
CACHE_TIME_IMMUTABLE = 3600 # Cache blobs referred to by an immutable reference for 1 hour CACHE_TIME_IMMUTABLE = 3600 # Cache blobs referred to by an immutable reference for 1 hour
......
# frozen_string_literal: true
# To be included in blob classes which are to be
# treated as ActiveModel.
#
# The blob class must respond_to `project`
module BlobActiveModel
extend ActiveSupport::Concern
class_methods do
def declarative_policy_class
'BlobPolicy'
end
end
def to_ability_name
'blob'
end
end
# frozen_string_literal: true # frozen_string_literal: true
class ReadmeBlob < SimpleDelegator class ReadmeBlob < SimpleDelegator
include BlobActiveModel
attr_reader :repository attr_reader :repository
def initialize(blob, repository) def initialize(blob, repository)
......
...@@ -274,6 +274,10 @@ class WikiPage ...@@ -274,6 +274,10 @@ class WikiPage
@attributes.merge!(attrs) @attributes.merge!(attrs)
end end
def to_ability_name
'wiki_page'
end
private private
# Process and format the title based on the user input. # Process and format the title based on the user input.
......
# frozen_string_literal: true
class BlobPolicy < BasePolicy
delegate { @subject.project }
rule { can?(:download_code) }.enable :read_blob
end
# frozen_string_literal: true
class WikiPagePolicy < BasePolicy
delegate { @subject.wiki.project }
rule { can?(:read_wiki) }.enable :read_wiki_page
end
...@@ -2,10 +2,14 @@ ...@@ -2,10 +2,14 @@
module EE module EE
module SearchService module SearchService
# Both of these classes conform to the necessary pagination interface and # All of these classes conform to the necessary pagination interface and
# both of these are returned in various places from search results. There # all of these are returned in various places from search results. There
# doesn't seem to be a common ancestor to check. # doesn't seem to be a common ancestor to check.
REDACTABLE_RESULTS = [Kaminari::PaginatableArray, Elasticsearch::Model::Response::Records].freeze REDACTABLE_RESULTS = [
Kaminari::PaginatableArray,
Elasticsearch::Model::Response::Records,
Elasticsearch::Model::Response::Response
].freeze
# This is a proper method instead of a `delegate` in order to # This is a proper method instead of a `delegate` in order to
# avoid adding unnecessary methods to Search::SnippetService # avoid adding unnecessary methods to Search::SnippetService
......
...@@ -26,7 +26,7 @@ module EE ...@@ -26,7 +26,7 @@ module EE
def process_results(results) def process_results(results)
return [] if results.empty? return [] if results.empty?
if results.is_a?(::Elasticsearch::Model::Response::Response) if results.any? { |result| result.is_a?(::Elasticsearch::Model::Response::Result) && result.respond_to?(:blob) }
return paginate(results).map { |blob| ::Gitlab::Elastic::SearchResults.parse_search_result(blob) } return paginate(results).map { |blob| ::Gitlab::Elastic::SearchResults.parse_search_result(blob) }
end end
......
...@@ -15,12 +15,14 @@ describe SearchService do ...@@ -15,12 +15,14 @@ describe SearchService do
let(:milestone_in_project) { create(:milestone, project: project) } let(:milestone_in_project) { create(:milestone, project: project) }
# Resources the user does not have access to # Resources the user does not have access to
let(:unauthorized_project) { create(:project) } let(:unauthorized_project) { create(:project, :repository, :wiki_repo) }
let(:issue1_in_unauthorized_project) { create(:issue, project: unauthorized_project) } let(:issue1_in_unauthorized_project) { create(:issue, project: unauthorized_project) }
let(:issue2_in_unauthorized_project) { create(:issue, project: unauthorized_project) } let(:issue2_in_unauthorized_project) { create(:issue, project: unauthorized_project) }
let(:note_on_unauthorized_issue) { create(:note, project: unauthorized_project, noteable: issue1_in_unauthorized_project) } let(:note_on_unauthorized_issue) { create(:note, project: unauthorized_project, noteable: issue1_in_unauthorized_project) }
let(:merge_request_in_unauthorized_project) { create(:merge_request_with_diffs, target_project: unauthorized_project, source_project: unauthorized_project) } let(:merge_request_in_unauthorized_project) { create(:merge_request_with_diffs, target_project: unauthorized_project, source_project: unauthorized_project) }
let(:milestone_in_unauthorized_project) { create(:milestone, project: unauthorized_project) } let(:milestone_in_unauthorized_project) { create(:milestone, project: unauthorized_project) }
let(:wiki_page) { WikiPages::CreateService.new(unauthorized_project, user, { title: "foo", content: "wiki_blobs" }).execute }
let(:commit) { unauthorized_project.repository.commit(SeedRepo::FirstCommit::ID) }
let(:search_service) { described_class.new(user, search: 'some-search-string', page: 1) } let(:search_service) { described_class.new(user, search: 'some-search-string', page: 1) }
let(:mock_global_service) { instance_double(Search::GlobalService, scope: 'some-scope') } let(:mock_global_service) { instance_double(Search::GlobalService, scope: 'some-scope') }
...@@ -145,6 +147,55 @@ describe SearchService do ...@@ -145,6 +147,55 @@ describe SearchService do
expect(subject).to be_kind_of(Kaminari::PaginatableArray) expect(subject).to be_kind_of(Kaminari::PaginatableArray)
expect(subject).to contain_exactly(note_on_issue_in_project) expect(subject).to contain_exactly(note_on_issue_in_project)
end end
it 'redacts commits the user does not have access to' do
allow(mock_results).to receive(:objects)
.and_return(
Kaminari.paginate_array(
[
commit
],
total_count: 1,
limit: 1,
offset: 0
)
)
expect(subject).to be_kind_of(Kaminari::PaginatableArray)
expect(subject).to be_empty
end
it 'redacts blobs the user does not have access to' do
blob = unauthorized_project.repository.blob_at(SeedRepo::FirstCommit::ID, 'README.md')
response = Elasticsearch::Model::Response::Response.new Blob, double(:search)
allow(response).to receive_messages(
results: [blob],
total_count: 1,
limit_value: 10,
offset_value: 0
)
allow(mock_results).to receive(:objects).and_return(response)
expect(subject).to be_kind_of(Kaminari::PaginatableArray)
expect(subject).to be_empty
end
it 'redacts wikis the user does not have access to' do
wiki_page = create(:wiki_page, wiki: unauthorized_project.wiki)
response = Elasticsearch::Model::Response::Response.new WikiPage, double(:search)
allow(response).to receive_messages(
results: [wiki_page],
total_count: 1,
limit_value: 10,
offset_value: 0
)
allow(mock_results).to receive(:objects).and_return(response)
expect(subject).to be_kind_of(Kaminari::PaginatableArray)
expect(subject).to be_empty
end
end end
end end
end end
...@@ -421,4 +421,21 @@ describe Blob do ...@@ -421,4 +421,21 @@ describe Blob do
end end
end end
end end
describe 'policy' do
let(:project) { build(:project) }
subject { described_class.new(fake_blob(path: 'foo'), project) }
it 'works with policy' do
expect(Ability.allowed?(project.creator, :read_blob, subject)).to be_truthy
end
context 'when project is nil' do
subject { described_class.new(fake_blob(path: 'foo')) }
it 'does not err' do
expect(Ability.allowed?(project.creator, :read_blob, subject)).to be_falsey
end
end
end
end end
# frozen_string_literal: true
require 'spec_helper'
describe ReadmeBlob do
include FakeBlobHelpers
describe 'policy' do
let(:project) { build(:project, :repository) }
subject { described_class.new(fake_blob(path: 'README.md'), project.repository) }
it 'works with policy' do
expect(Ability.allowed?(project.creator, :read_blob, subject)).to be_truthy
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe BlobPolicy do
include_context 'ProjectPolicyTable context'
include ProjectHelpers
using RSpec::Parameterized::TableSyntax
let(:project) { create(:project, :repository, project_level) }
let(:user) { create_user_from_membership(project, membership) }
let(:blob) { project.repository.blob_at(SeedRepo::FirstCommit::ID, 'README.md') }
subject(:policy) { described_class.new(user, blob) }
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 "grants permission" do
update_feature_access_level(project, feature_access_level)
if expected_count == 1
expect(policy).to be_allowed(:read_blob)
else
expect(policy).to be_disallowed(:read_blob)
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe WikiPagePolicy do
include_context 'ProjectPolicyTable context'
include ProjectHelpers
using RSpec::Parameterized::TableSyntax
let(:project) { create(:project, :wiki_repo, project_level) }
let(:user) { create_user_from_membership(project, membership) }
let(:wiki_page) { create(:wiki_page, wiki: project.wiki) }
subject(:policy) { described_class.new(user, wiki_page) }
where(:project_level, :feature_access_level, :membership, :expected_count) do
permission_table_for_guest_feature_access
end
with_them do
it "grants permission" do
update_feature_access_level(project, feature_access_level)
if expected_count == 1
expect(policy).to be_allowed(:read_wiki_page)
else
expect(policy).to be_disallowed(:read_wiki_page)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment