diff --git a/app/services/authz/redaction_service.rb b/app/services/authz/redaction_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..562c612d866bfe4921b691cce6fcb8441b8e9416 --- /dev/null +++ b/app/services/authz/redaction_service.rb @@ -0,0 +1,148 @@ +# frozen_string_literal: true + +module Authz + # Service to perform batch authorization checks for resources. + # + # This service is designed to check whether a user has read access to a batch + # of resources using GitLab's standard Ability system. It's used by the + # Knowledge Graph service for final redaction but is generic enough + # to be used by any service requiring batch authorization checks. + # + # IMPORTANT: This service assumes that the user has already been authenticated + # and authorized to make API requests. It does NOT perform user-level validation + # (e.g., checking if user is blocked or deactivated). The caller is responsible + # for ensuring the user is valid before invoking this service. + # + # @example + # service = Authz::RedactionService.new( + # user: current_user, + # resources_by_type: { + # 'issues' => [123, 456], + # 'merge_requests' => [789] + # }, + # source: 'knowledge_graph' + # ) + # result = service.execute + # # => { + # # 'issues' => { 123 => true, 456 => false }, + # # 'merge_requests' => { 789 => true } + # # } + class RedactionService + include Gitlab::Allowable + + RESOURCE_CLASSES = { + 'issues' => ::Issue, + 'merge_requests' => ::MergeRequest, + 'projects' => ::Project, + 'milestones' => ::Milestone, + 'snippets' => ::Snippet + }.freeze + + PRELOAD_ASSOCIATIONS = { + 'issues' => [{ project: [:namespace, :project_feature, :group] }, :author, :work_item_type], + 'merge_requests' => [{ target_project: [:namespace, :project_feature, :group] }, :author], + 'projects' => [:namespace, :project_feature, :group], + 'milestones' => [{ project: [:namespace, :project_feature] }, :group], + 'snippets' => [{ project: [:namespace, :project_feature] }, :author] + }.freeze + + def self.supported_types + RESOURCE_CLASSES.keys + end + + def initialize(user:, resources_by_type:, source:, logger: nil) + raise ArgumentError, 'user is required' if user.nil? + + @user = user + @resources_by_type = resources_by_type + @source = source + @logger = logger + end + + def execute + return {} if resources_by_type.empty? + + loaded_resources_by_type = load_all_resources + + results = DeclarativePolicy.user_scope do + resources_by_type.each_with_object({}) do |(type, ids), authorization_results| + authorization_results[type] = authorize_resources_of_type(type, ids, loaded_resources_by_type[type] || {}) + end + end + + log_redacted_results(results) + + results + end + + private + + attr_reader :user, :resources_by_type, :source, :logger + + def load_all_resources + resources_by_type.each_with_object({}) do |(type, ids), loaded| + loaded[type] = load_resources_for_type(type, ids) + end + end + + # rubocop:disable CodeReuse/ActiveRecord -- Batch loading with preloads for authorization checks + def load_resources_for_type(type, ids) + return {} if ids.blank? + + klass = RESOURCE_CLASSES[type] + return {} unless klass + + preloads = PRELOAD_ASSOCIATIONS[type] + relation = klass.where(id: ids) + relation = relation.includes(*preloads) if preloads + relation.index_by(&:id) + end + # rubocop:enable CodeReuse/ActiveRecord + + def authorize_resources_of_type(type, ids, loaded_resources) + return {} if ids.blank? + + klass = RESOURCE_CLASSES[type] + return ids.index_with { false } unless klass + + ids.index_with do |id| + resource = loaded_resources[id] + + next false if resource.nil? + + visible_result?(resource) + end + end + + def visible_result?(resource) + return true unless resource.respond_to?(:to_ability_name) && DeclarativePolicy.has_policy?(resource) + + Ability.allowed?(user, :"read_#{resource.to_ability_name}", resource) + end + + def log_redacted_results(results) + return unless logger + + redacted_by_type = results.transform_values do |id_results| + id_results.count { |_id, authorized| !authorized } + end + + total_redacted = redacted_by_type.values.sum + return if total_redacted == 0 + + log_info = { + class: self.class.name, + message: 'redacted_authorization_results', + source: source, + user_id: user.id, + total_requested: results.values.sum(&:size), + total_redacted: total_redacted, + redacted_by_type: redacted_by_type + } + + logger.error(log_info) + end + end +end + +Authz::RedactionService.prepend_mod_with('Authz::RedactionService') diff --git a/ee/app/services/ee/authz/redaction_service.rb b/ee/app/services/ee/authz/redaction_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..6b247b37a1188942ddedaf1322d6da690ad66e13 --- /dev/null +++ b/ee/app/services/ee/authz/redaction_service.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module EE + module Authz + module RedactionService + extend ::Gitlab::Utils::Override + + EE_RESOURCE_CLASSES = { + 'epics' => ::Epic, + 'vulnerabilities' => ::Vulnerability + }.freeze + + EE_PRELOAD_ASSOCIATIONS = { + 'epics' => [:group], + 'vulnerabilities' => [{ project: [:namespace, :project_feature, :group] }] + }.freeze + + module ClassMethods + extend ::Gitlab::Utils::Override + + override :supported_types + def supported_types + super + EE::Authz::RedactionService::EE_RESOURCE_CLASSES.keys + end + end + + def self.prepended(base) + base.singleton_class.prepend ClassMethods + end + + private + + override :load_resources_for_type + # rubocop:disable CodeReuse/ActiveRecord -- Batch loading with preloads for authorization checks + def load_resources_for_type(type, ids) + return super unless EE_RESOURCE_CLASSES.key?(type) + return {} if ids.blank? + + klass = EE_RESOURCE_CLASSES[type] + preloads = EE_PRELOAD_ASSOCIATIONS[type] + relation = klass.where(id: ids) + relation = relation.includes(*preloads) if preloads + relation.index_by(&:id) + end + # rubocop:enable CodeReuse/ActiveRecord + + override :authorize_resources_of_type + def authorize_resources_of_type(type, ids, loaded_resources) + return super unless EE_RESOURCE_CLASSES.key?(type) + return {} if ids.blank? + + ids.index_with do |id| + resource = loaded_resources[id] + + next false if resource.nil? + + visible_result?(resource) + end + end + end + end +end diff --git a/ee/spec/services/ee/authz/redaction_service_spec.rb b/ee/spec/services/ee/authz/redaction_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..4b8d9ab665b452c0e7ac928a8ec434f7b1d888f1 --- /dev/null +++ b/ee/spec/services/ee/authz/redaction_service_spec.rb @@ -0,0 +1,203 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Authz::RedactionService, feature_category: :permissions do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:private_group) { create(:group, :private) } + let_it_be(:private_group_with_access) { create(:group, :private) } + let_it_be(:project) { create(:project, :public, group: group) } + let_it_be(:private_project) { create(:project, :private, group: private_group) } + let_it_be(:private_project_with_access) { create(:project, :private, group: private_group_with_access) } + + before_all do + private_group_with_access.add_developer(user) + end + + before do + stub_licensed_features(epics: true, security_dashboard: true) + end + + describe '.supported_types' do + it 'includes EE resource types' do + expect(described_class.supported_types).to include('epics', 'vulnerabilities') + end + + it 'includes CE resource types' do + expect(described_class.supported_types).to include( + 'issues', 'merge_requests', 'projects', 'milestones', 'snippets' + ) + end + end + + describe '#execute' do + subject(:result) { service.execute } + + let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') } + + context 'with epics' do + let_it_be(:public_epic) { create(:epic, group: group) } + let_it_be(:private_epic) { create(:epic, group: private_group) } + let_it_be(:accessible_epic) { create(:epic, group: private_group_with_access) } + let_it_be(:confidential_epic) { create(:epic, :confidential, group: private_group_with_access) } + + context 'when user can access public epic' do + let(:resources_by_type) { { 'epics' => [public_epic.id] } } + + it 'allows access' do + expect(result).to eq({ 'epics' => { public_epic.id => true } }) + end + end + + context 'when user cannot access private epic' do + let(:resources_by_type) { { 'epics' => [private_epic.id] } } + + it 'denies access' do + expect(result).to eq({ 'epics' => { private_epic.id => false } }) + end + end + + context 'when user has group access' do + let(:resources_by_type) { { 'epics' => [accessible_epic.id] } } + + it 'allows access' do + expect(result).to eq({ 'epics' => { accessible_epic.id => true } }) + end + end + + context 'when user has group access to confidential epic' do + let(:resources_by_type) { { 'epics' => [confidential_epic.id] } } + + it 'allows access for group member' do + expect(result).to eq({ 'epics' => { confidential_epic.id => true } }) + end + end + + context 'when checking multiple epics at once' do + let(:resources_by_type) do + { 'epics' => [public_epic.id, private_epic.id, accessible_epic.id] } + end + + it 'returns correct authorization for each epic' do + expect(result).to eq({ + 'epics' => { + public_epic.id => true, + private_epic.id => false, + accessible_epic.id => true + } + }) + end + end + + context 'with non-existent epic' do + let(:resources_by_type) { { 'epics' => [non_existing_record_id] } } + + it 'denies access' do + expect(result).to eq({ 'epics' => { non_existing_record_id => false } }) + end + end + end + + context 'with vulnerabilities' do + let_it_be(:accessible_vulnerability) { create(:vulnerability, project: private_project_with_access) } + let_it_be(:inaccessible_vulnerability) { create(:vulnerability, project: private_project) } + + context 'when user has project access' do + let(:resources_by_type) { { 'vulnerabilities' => [accessible_vulnerability.id] } } + + it 'allows access' do + expect(result).to eq({ 'vulnerabilities' => { accessible_vulnerability.id => true } }) + end + end + + context 'when user does not have project access' do + let(:resources_by_type) { { 'vulnerabilities' => [inaccessible_vulnerability.id] } } + + it 'denies access' do + expect(result).to eq({ 'vulnerabilities' => { inaccessible_vulnerability.id => false } }) + end + end + + context 'when checking multiple vulnerabilities at once' do + let(:resources_by_type) do + { 'vulnerabilities' => [accessible_vulnerability.id, inaccessible_vulnerability.id] } + end + + it 'returns correct authorization for each vulnerability' do + expect(result).to eq({ + 'vulnerabilities' => { + accessible_vulnerability.id => true, + inaccessible_vulnerability.id => false + } + }) + end + end + + context 'with non-existent vulnerability' do + let(:resources_by_type) { { 'vulnerabilities' => [non_existing_record_id] } } + + it 'denies access' do + expect(result).to eq({ 'vulnerabilities' => { non_existing_record_id => false } }) + end + end + end + + context 'with mixed CE and EE resource types' do + let_it_be(:public_issue) { create(:issue, project: project) } + let_it_be(:public_epic) { create(:epic, group: group) } + let_it_be(:accessible_vulnerability) { create(:vulnerability, project: private_project_with_access) } + let_it_be(:private_mr) { create(:merge_request, source_project: private_project) } + + let(:resources_by_type) do + { + 'issues' => [public_issue.id], + 'epics' => [public_epic.id], + 'vulnerabilities' => [accessible_vulnerability.id], + 'merge_requests' => [private_mr.id] + } + end + + it 'handles both CE and EE resource types correctly' do + expect(result).to eq({ + 'issues' => { public_issue.id => true }, + 'epics' => { public_epic.id => true }, + 'vulnerabilities' => { accessible_vulnerability.id => true }, + 'merge_requests' => { private_mr.id => false } + }) + end + end + + context 'with empty arrays for EE types' do + let(:resources_by_type) { { 'epics' => [], 'vulnerabilities' => [] } } + + it 'returns empty hashes for those types' do + expect(result).to eq({ 'epics' => {}, 'vulnerabilities' => {} }) + end + end + end + + describe 'load_resources_for_type behavior' do + context 'when EE resource type has no preload associations defined' do + let_it_be(:public_epic) { create(:epic, group: group) } + let(:resources_by_type) { { 'epics' => [public_epic.id] } } + let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') } + + before do + stub_const( + "EE::Authz::RedactionService::EE_PRELOAD_ASSOCIATIONS", + EE::Authz::RedactionService::EE_PRELOAD_ASSOCIATIONS.except('epics') + ) + end + + it 'does not raise an error when preloads are not defined' do + expect { service.execute }.not_to raise_error + end + + it 'still performs authorization correctly' do + result = service.execute + expect(result).to eq({ 'epics' => { public_epic.id => true } }) + end + end + end +end diff --git a/spec/services/authz/redaction_service_spec.rb b/spec/services/authz/redaction_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..09ede324ef40914a0a0eb83a67245e2b5e6c959b --- /dev/null +++ b/spec/services/authz/redaction_service_spec.rb @@ -0,0 +1,400 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Authz::RedactionService, feature_category: :permissions do + let_it_be(:user) { create(:user) } + let_it_be(:public_project) { create(:project, :public) } + let_it_be(:private_project) { create(:project, :private) } + let_it_be(:private_project_with_access) { create(:project, :private) } + + before_all do + private_project_with_access.add_reporter(user) + end + + describe '.supported_types' do + it 'returns the list of supported resource types' do + expect(described_class.supported_types).to include( + 'issues', 'merge_requests', 'projects', 'milestones', 'snippets' + ) + end + end + + describe '#initialize' do + context 'when user is nil' do + it 'raises ArgumentError' do + expect do + described_class.new(user: nil, resources_by_type: {}, source: 'test') + end.to raise_error(ArgumentError, 'user is required') + end + end + + context 'when user is provided' do + it 'does not raise an error' do + expect do + described_class.new(user: user, resources_by_type: {}, source: 'test') + end.not_to raise_error + end + end + end + + describe '#execute' do + subject(:result) { service.execute } + + let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') } + + context 'with empty resources' do + let(:resources_by_type) { {} } + + it 'returns an empty hash' do + expect(result).to eq({}) + end + end + + context 'with issues' do + let_it_be(:public_issue) { create(:issue, project: public_project) } + let_it_be(:private_issue) { create(:issue, project: private_project) } + let_it_be(:accessible_issue) { create(:issue, project: private_project_with_access) } + let_it_be(:confidential_issue) { create(:issue, :confidential, project: private_project_with_access) } + + context 'when user has access to public issue' do + let(:resources_by_type) { { 'issues' => [public_issue.id] } } + + it 'allows access' do + expect(result).to eq({ 'issues' => { public_issue.id => true } }) + end + end + + context 'when user does not have access to private issue' do + let(:resources_by_type) { { 'issues' => [private_issue.id] } } + + it 'denies access' do + expect(result).to eq({ 'issues' => { private_issue.id => false } }) + end + end + + context 'when user has project access' do + let(:resources_by_type) { { 'issues' => [accessible_issue.id] } } + + it 'allows access' do + expect(result).to eq({ 'issues' => { accessible_issue.id => true } }) + end + end + + context 'when user has project access to confidential issue' do + let(:resources_by_type) { { 'issues' => [confidential_issue.id] } } + + it 'allows access for project member' do + expect(result).to eq({ 'issues' => { confidential_issue.id => true } }) + end + end + + context 'when checking multiple issues at once' do + let(:resources_by_type) do + { 'issues' => [public_issue.id, private_issue.id, accessible_issue.id] } + end + + it 'returns correct authorization for each issue' do + expect(result).to eq({ + 'issues' => { + public_issue.id => true, + private_issue.id => false, + accessible_issue.id => true + } + }) + end + end + end + + context 'with merge_requests' do + let_it_be(:public_mr) { create(:merge_request, source_project: public_project) } + let_it_be(:private_mr) { create(:merge_request, source_project: private_project) } + let_it_be(:accessible_mr) { create(:merge_request, source_project: private_project_with_access) } + + context 'when user has access to public MR' do + let(:resources_by_type) { { 'merge_requests' => [public_mr.id] } } + + it 'allows access' do + expect(result).to eq({ 'merge_requests' => { public_mr.id => true } }) + end + end + + context 'when user does not have access to private MR' do + let(:resources_by_type) { { 'merge_requests' => [private_mr.id] } } + + it 'denies access' do + expect(result).to eq({ 'merge_requests' => { private_mr.id => false } }) + end + end + + context 'when user has project access' do + let(:resources_by_type) { { 'merge_requests' => [accessible_mr.id] } } + + it 'allows access' do + expect(result).to eq({ 'merge_requests' => { accessible_mr.id => true } }) + end + end + end + + context 'with projects' do + context 'when user can access public project' do + let(:resources_by_type) { { 'projects' => [public_project.id] } } + + it 'allows access' do + expect(result).to eq({ 'projects' => { public_project.id => true } }) + end + end + + context 'when user cannot access private project' do + let(:resources_by_type) { { 'projects' => [private_project.id] } } + + it 'denies access' do + expect(result).to eq({ 'projects' => { private_project.id => false } }) + end + end + + context 'when user has project access' do + let(:resources_by_type) { { 'projects' => [private_project_with_access.id] } } + + it 'allows access' do + expect(result).to eq({ 'projects' => { private_project_with_access.id => true } }) + end + end + end + + context 'with non-existent resources' do + let(:resources_by_type) { { 'issues' => [non_existing_record_id] } } + + it 'denies access to non-existent resources' do + expect(result).to eq({ 'issues' => { non_existing_record_id => false } }) + end + end + + context 'with mixed resource types' do + let_it_be(:public_issue) { create(:issue, project: public_project) } + let_it_be(:private_mr) { create(:merge_request, source_project: private_project) } + + let(:resources_by_type) do + { + 'issues' => [public_issue.id], + 'merge_requests' => [private_mr.id], + 'projects' => [public_project.id, private_project.id] + } + end + + it 'handles multiple resource types correctly' do + expect(result).to eq({ + 'issues' => { public_issue.id => true }, + 'merge_requests' => { private_mr.id => false }, + 'projects' => { + public_project.id => true, + private_project.id => false + } + }) + end + end + + context 'with empty arrays for a type' do + let(:resources_by_type) { { 'issues' => [] } } + + it 'returns empty hash for that type' do + expect(result).to eq({ 'issues' => {} }) + end + end + + context 'with unsupported resource type' do + let(:resources_by_type) { { 'unknown_type' => [1, 2, 3] } } + + it 'denies access for all IDs of unsupported type' do + expect(result).to eq({ 'unknown_type' => { 1 => false, 2 => false, 3 => false } }) + end + end + + context 'with milestones' do + let_it_be(:public_milestone) { create(:milestone, project: public_project) } + let_it_be(:private_milestone) { create(:milestone, project: private_project) } + + context 'when user can access public milestone' do + let(:resources_by_type) { { 'milestones' => [public_milestone.id] } } + + it 'allows access' do + expect(result).to eq({ 'milestones' => { public_milestone.id => true } }) + end + end + + context 'when user cannot access private milestone' do + let(:resources_by_type) { { 'milestones' => [private_milestone.id] } } + + it 'denies access' do + expect(result).to eq({ 'milestones' => { private_milestone.id => false } }) + end + end + end + + context 'with snippets' do + let_it_be(:public_snippet) { create(:project_snippet, :public, project: public_project) } + let_it_be(:private_snippet) { create(:project_snippet, :private, project: private_project) } + let_it_be(:accessible_snippet) { create(:project_snippet, :private, project: private_project_with_access) } + + context 'when user can access public snippet' do + let(:resources_by_type) { { 'snippets' => [public_snippet.id] } } + + it 'allows access' do + expect(result).to eq({ 'snippets' => { public_snippet.id => true } }) + end + end + + context 'when user cannot access private snippet' do + let(:resources_by_type) { { 'snippets' => [private_snippet.id] } } + + it 'denies access' do + expect(result).to eq({ 'snippets' => { private_snippet.id => false } }) + end + end + + context 'when user has project access' do + let(:resources_by_type) { { 'snippets' => [accessible_snippet.id] } } + + it 'allows access' do + expect(result).to eq({ 'snippets' => { accessible_snippet.id => true } }) + end + end + end + + context 'with logger parameter' do + let(:logger) { instance_double(Logger) } + let(:service) do + described_class.new(user: user, resources_by_type: resources_by_type, source: 'knowledge_graph', logger: logger) + end + + let(:resources_by_type) { { 'issues' => [] } } + + it 'accepts a logger parameter' do + expect { service }.not_to raise_error + expect(result).to eq({ 'issues' => {} }) + end + + context 'when resources are redacted' do + let_it_be(:private_issue) { create(:issue, project: private_project) } + let(:resources_by_type) { { 'issues' => [private_issue.id] } } + + it 'logs redacted results' do + expect(logger).to receive(:error).with( + hash_including( + class: 'Authz::RedactionService', + message: 'redacted_authorization_results', + source: 'knowledge_graph', + user_id: user.id, + total_requested: 1, + total_redacted: 1, + redacted_by_type: { 'issues' => 1 } + ) + ) + + result + end + end + + context 'when no resources are redacted' do + let_it_be(:public_issue) { create(:issue, project: public_project) } + let(:resources_by_type) { { 'issues' => [public_issue.id] } } + + it 'does not log' do + expect(logger).not_to receive(:error) + + result + end + end + end + end + + describe 'visible_result? behavior' do + let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') } + + context 'when resource does not respond to to_ability_name' do + let(:plain_object) { Struct.new(:id).new(999) } + let(:resources_by_type) { { 'issues' => [999] } } + + before do + allow(service).to receive(:load_all_resources).and_return({ + 'issues' => { 999 => plain_object } + }) + end + + it 'allows access for resources without to_ability_name' do + result = service.execute + expect(result).to eq({ 'issues' => { 999 => true } }) + end + end + + context 'when resource has no policy' do + let(:object_without_policy) do + Struct.new(:id, :to_ability_name).new(888, 'unknown_object') + end + + let(:resources_by_type) { { 'issues' => [888] } } + + before do + allow(service).to receive(:load_all_resources).and_return({ + 'issues' => { 888 => object_without_policy } + }) + allow(DeclarativePolicy).to receive(:has_policy?).with(object_without_policy).and_return(false) + end + + it 'allows access for resources without policy' do + result = service.execute + expect(result).to eq({ 'issues' => { 888 => true } }) + end + end + end + + describe 'load_resources_for_type behavior' do + context 'when resource type has no preload associations defined' do + let_it_be(:public_issue) { create(:issue, project: public_project) } + let(:resources_by_type) { { 'issues' => [public_issue.id] } } + let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') } + + before do + stub_const("#{described_class}::PRELOAD_ASSOCIATIONS", described_class::PRELOAD_ASSOCIATIONS.except('issues')) + end + + it 'does not raise an error when preloads are not defined' do + expect { service.execute }.not_to raise_error + end + + it 'still performs authorization correctly' do + result = service.execute + expect(result).to eq({ 'issues' => { public_issue.id => true } }) + end + end + end + + describe 'performance optimization' do + let_it_be(:issues) { create_list(:issue, 3, project: public_project) } + let(:resources_by_type) { { 'issues' => issues.map(&:id) } } + let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') } + + it 'uses DeclarativePolicy.user_scope for optimization' do + expect(DeclarativePolicy).to receive(:user_scope).and_call_original + service.execute + end + + it 'batch loads resources to prevent N+1 queries' do + service.execute + + new_service = described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') + + expect do + new_service.execute + end.not_to exceed_query_limit(10) # Reasonable limit for batch operation + end + + it 'preloads nested associations to avoid N+1 in policies' do + service.execute + + expect(described_class::PRELOAD_ASSOCIATIONS['issues']).to include( + a_hash_including(project: array_including(:namespace, :project_feature)) + ) + end + end +end