diff --git a/app/models/ci/job_token/allowlist.rb b/app/models/ci/job_token/allowlist.rb index 80a640b791964753eddde4960840586886873420..2f73adce424b67d9db1cce377f4ac24009c5fbe1 100644 --- a/app/models/ci/job_token/allowlist.rb +++ b/app/models/ci/job_token/allowlist.rb @@ -59,6 +59,16 @@ def add_group!(target_group, user:, default_permissions: true, policies: []) ) end + def project_link_traversal_ids + project_links.includes(target_project: :project_namespace).map do |p| + p.target_project.project_namespace.traversal_ids + end + end + + def group_link_traversal_ids + group_links.includes(:target_group).map { |g| g.target_group.traversal_ids } + end + def project_links Ci::JobToken::ProjectScopeLink .with_source(@source_project) diff --git a/app/models/ci/job_token/authorizations_compactor.rb b/app/models/ci/job_token/authorizations_compactor.rb index 6485c2371bad8f6a730d7dab2f82f9ad11c2fa0a..49e7a8ade2fe6c74a013a2a165c4081402c4de73 100644 --- a/app/models/ci/job_token/authorizations_compactor.rb +++ b/app/models/ci/job_token/authorizations_compactor.rb @@ -3,6 +3,8 @@ module Ci module JobToken class AuthorizationsCompactor + include Gitlab::Utils::StrongMemoize + attr_reader :allowlist_groups, :allowlist_projects UnexpectedCompactionEntry = Class.new(StandardError) @@ -28,14 +30,17 @@ def origin_project_traversal_ids origin_project_id_batches.each do |batch| projects = Project.where(id: batch).includes(:project_namespace) - origin_project_traversal_ids += projects.map { |p| p.project_namespace.traversal_ids } + origin_project_traversal_ids += projects.filter_map do |p| + p.project_namespace.traversal_ids unless path_already_allowlisted?(p.project_namespace.traversal_ids) + end end origin_project_traversal_ids end end - def compact(limit) + def compact(requested_limit) + limit = requested_limit - existing_links_traversal_ids.count compacted_traversal_ids = Gitlab::Utils::TraversalIdCompactor.compact(origin_project_traversal_ids, limit) Gitlab::Utils::TraversalIdCompactor.validate!(origin_project_traversal_ids, compacted_traversal_ids) @@ -51,6 +56,19 @@ def compact(limit) end end end + + def path_already_allowlisted?(namespace_path) + existing_links_traversal_ids.any? do |existing_links_traversal_id| + namespace_path.size >= existing_links_traversal_id.size && + namespace_path.first(existing_links_traversal_id.size) == existing_links_traversal_id + end + end + + def existing_links_traversal_ids + allowlist = Ci::JobToken::Allowlist.new(Project.find(@project_id)) + allowlist.group_link_traversal_ids + allowlist.project_link_traversal_ids + end + strong_memoize_attr :existing_links_traversal_ids end end end diff --git a/spec/models/ci/job_token/authorizations_compactor_spec.rb b/spec/models/ci/job_token/authorizations_compactor_spec.rb index 0611570e051409657bf885baedac1f9e040af881..70ad6454886be24b61737344240d447519916821 100644 --- a/spec/models/ci/job_token/authorizations_compactor_spec.rb +++ b/spec/models/ci/job_token/authorizations_compactor_spec.rb @@ -4,6 +4,7 @@ RSpec.describe Ci::JobToken::AuthorizationsCompactor, feature_category: :secrets_management do let_it_be(:accessed_project) { create(:project) } + let(:excluded_namespace_paths) { [] } let(:compactor) { described_class.new(accessed_project.id) } # [1, 21], ns1, p1 @@ -111,6 +112,82 @@ end.not_to raise_error end end + + context 'with exiting group scope links' do + describe 'when a single group exists' do + before do + create(:ci_job_token_group_scope_link, source_project: accessed_project, target_group: ns6) + end + + it 'removes it from the compaction process' do + compactor.compact(4) + + expect(compactor.allowlist_groups).to match_array([ns2, ns4]) + expect(compactor.allowlist_projects).to match_array([pns1.project]) + end + end + + describe 'when a multiple groups exist' do + before do + create(:ci_job_token_group_scope_link, source_project: accessed_project, target_group: ns6) + create(:ci_job_token_group_scope_link, source_project: accessed_project, target_group: ns2) + end + + it 'removes it from the compaction process' do + compactor.compact(4) + + expect(compactor.allowlist_groups).to match_array([ns4]) + expect(compactor.allowlist_projects).to match_array([pns1.project]) + end + end + end + end + + context 'with exiting project scope links' do + describe 'when a single project exists' do + before do + create(:ci_job_token_project_scope_link, source_project: accessed_project, direction: :inbound, + target_project: pns8.project) + end + + it 'removes it from the compaction process' do + compactor.compact(4) + + expect(compactor.allowlist_groups).to match_array([ns2, ns4]) + expect(compactor.allowlist_projects).to match_array([pns1.project]) + end + end + + describe 'when a multiple projects exist' do + before do + create(:ci_job_token_project_scope_link, source_project: accessed_project, direction: :inbound, + target_project: pns8.project) + create(:ci_job_token_project_scope_link, source_project: accessed_project, direction: :inbound, + target_project: pns2.project) + end + + it 'removes it from the compaction process' do + compactor.compact(6) + + expect(compactor.allowlist_groups).to match_array([ns2, ns4]) + expect(compactor.allowlist_projects).to match_array([pns1.project]) + end + end + end + + context 'with exiting group and project scope links' do + before do + create(:ci_job_token_group_scope_link, source_project: accessed_project, target_group: ns2) + create(:ci_job_token_project_scope_link, source_project: accessed_project, direction: :inbound, + target_project: pns8.project) + end + + it 'removes it from the compaction process' do + compactor.compact(4) + + expect(compactor.allowlist_groups).to match_array([ns4]) + expect(compactor.allowlist_projects).to match_array([pns1.project]) + end end describe '#origin_traversal_ids' do