diff --git a/app/models/ci/job_token/allowlist.rb b/app/models/ci/job_token/allowlist.rb
index 618dc2da05cb3420520f6ff5041166e66257f2d8..c4071ac32717efb2bcd9da5ea1bc3258cd2ea153 100644
--- a/app/models/ci/job_token/allowlist.rb
+++ b/app/models/ci/job_token/allowlist.rb
@@ -2,17 +2,24 @@
module Ci
module JobToken
class Allowlist
- def initialize(source_project, direction:)
+ def initialize(source_project, direction: :inbound)
@source_project = source_project
@direction = direction
end
- def includes?(target_project)
+ def includes_project?(target_project)
source_links
.with_target(target_project)
.exists?
end
+ def includes_group?(target_project)
+ allowlist_group_ids = group_links.pluck(:target_group_id)
+ target_project_group_path_ids = target_project.parent_groups.map(&:id)
+
+ allowlist_group_ids.intersect?(target_project_group_path_ids)
+ end
+
def projects
Project.from_union(target_projects, remove_duplicates: false)
end
@@ -34,6 +41,11 @@ def source_links
.where(direction: @direction)
end
+ def group_links
+ Ci::JobToken::GroupScopeLink
+ .with_source(@source_project)
+ end
+
def target_project_ids
source_links
# pluck needed to avoid ci and main db join
diff --git a/app/models/ci/job_token/scope.rb b/app/models/ci/job_token/scope.rb
index a29d4fee29944d37b2523e34267d657ebce0b70f..a512c9417ff296f348b35e49589f7c359a7025f5 100644
--- a/app/models/ci/job_token/scope.rb
+++ b/app/models/ci/job_token/scope.rb
@@ -39,15 +39,6 @@ def inbound_projects
inbound_allowlist.projects
end
- def add!(added_project, user:, direction:)
- case direction
- when :inbound
- inbound_allowlist.add!(added_project, user: user)
- when :outbound
- outbound_allowlist.add!(added_project, user: user)
- end
- end
-
private
def outbound_accessible?(accessed_project)
@@ -56,21 +47,27 @@ def outbound_accessible?(accessed_project)
return true unless accessed_project.private?
- outbound_allowlist.includes?(accessed_project)
+ outbound_allowlist.includes_project?(accessed_project)
end
def inbound_accessible?(accessed_project)
# if the setting is disabled any project is considered to be in scope.
return true unless accessed_project.ci_inbound_job_token_scope_enabled?
- inbound_linked_as_accessible?(accessed_project)
+ inbound_linked_as_accessible?(accessed_project) ||
+ (::Feature.enabled?(:ci_job_token_groups_allowlist, accessed_project) &&
+ group_linked_as_accessible?(accessed_project))
end
# We don't check the inbound allowlist here. That is because
# the access check starts from the current project but the inbound
# allowlist contains projects that can access the current project.
def inbound_linked_as_accessible?(accessed_project)
- inbound_accessible_projects(accessed_project).includes?(current_project)
+ inbound_accessible_projects(accessed_project).includes_project?(current_project)
+ end
+
+ def group_linked_as_accessible?(accessed_project)
+ Ci::JobToken::Allowlist.new(accessed_project).includes_group?(current_project)
end
def inbound_accessible_projects(accessed_project)
diff --git a/app/models/project.rb b/app/models/project.rb
index 7012cb7b25bcc84be66ce3ee9e7e2ba4631b4bf7..8af1250da427dd450508d1856ee1faab29d155d9 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -3092,6 +3092,10 @@ def visible_group_links(for_user:)
end
end
+ def parent_groups
+ Gitlab::ObjectHierarchy.new(Group.where(id: group)).base_and_ancestors
+ end
+
def enforced_runner_token_expiration_interval
all_parent_groups = Gitlab::ObjectHierarchy.new(Group.where(id: group)).base_and_ancestors
all_group_settings = NamespaceSetting.where(namespace_id: all_parent_groups)
diff --git a/config/feature_flags/development/ci_job_token_groups_allowlist.yml b/config/feature_flags/development/ci_job_token_groups_allowlist.yml
new file mode 100644
index 0000000000000000000000000000000000000000..85d44d39cb685a4921f962e2f70ab66412a14fa2
--- /dev/null
+++ b/config/feature_flags/development/ci_job_token_groups_allowlist.yml
@@ -0,0 +1,8 @@
+---
+name: ci_job_token_groups_allowlist
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142441
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/439611
+milestone: '16.9'
+type: development
+group: group::pipeline security
+default_enabled: false
\ No newline at end of file
diff --git a/spec/models/ci/job_token/allowlist_spec.rb b/spec/models/ci/job_token/allowlist_spec.rb
index 3d29a637d68fa92782910456f19afc0ea7c0a13a..feea8758490d8a3ca3fff1e2e4fa20f71b05b731 100644
--- a/spec/models/ci/job_token/allowlist_spec.rb
+++ b/spec/models/ci/job_token/allowlist_spec.rb
@@ -64,8 +64,8 @@
end
end
- describe '#includes?' do
- subject { allowlist.includes?(includes_project) }
+ describe '#includes_project?' do
+ subject { allowlist.includes_project?(includes_project) }
context 'without scoped projects' do
let(:unscoped_project) { build(:project) }
@@ -102,5 +102,42 @@
it { is_expected.to be result }
end
end
+
+ describe '#includes_group' do
+ subject { allowlist.includes_group?(target_project) }
+
+ let_it_be(:target_group) { create(:group) }
+ let_it_be(:target_project) do
+ create(:project,
+ ci_inbound_job_token_scope_enabled: true,
+ group: target_group
+ )
+ end
+
+ context 'without scoped groups' do
+ let_it_be(:unscoped_project) { build(:project) }
+
+ where(:source_project, :result) do
+ ref(:unscoped_project) | false
+ end
+
+ with_them do
+ it { is_expected.to be result }
+ end
+ end
+
+ context 'with a group in each allowlist' do
+ include_context 'with accessible and inaccessible project groups'
+
+ where(:source_project, :result) do
+ ref(:project_with_target_project_group_in_allowlist) | true
+ ref(:project_wo_target_project_group_in_allowlist) | false
+ end
+
+ with_them do
+ it { is_expected.to be result }
+ end
+ end
+ end
end
end
diff --git a/spec/models/ci/job_token/scope_spec.rb b/spec/models/ci/job_token/scope_spec.rb
index adb9f461f63a76fc5e184787cde5994e2fadf497..f2ad674b3f530d12bd328c31a48eaf6875b8b663 100644
--- a/spec/models/ci/job_token/scope_spec.rb
+++ b/spec/models/ci/job_token/scope_spec.rb
@@ -57,39 +57,46 @@
end
end
- describe 'add!' do
- let_it_be(:new_project) { create(:project) }
+ describe 'accessible?' do
+ subject { scope.accessible?(accessed_project) }
- subject { scope.add!(new_project, direction: direction, user: user) }
+ context 'with groups in allowlist' do
+ let_it_be(:target_group) { create(:group) }
+ let_it_be(:target_project) do
+ create(:project,
+ ci_inbound_job_token_scope_enabled: true,
+ group: target_group
+ )
+ end
- [:inbound, :outbound].each do |d|
- context "with #{d}" do
- let(:direction) { d }
+ let(:scope) { described_class.new(target_project) }
- it 'adds the project' do
- subject
+ include_context 'with accessible and inaccessible project groups'
- expect(scope.send("#{direction}_projects")).to contain_exactly(current_project, new_project)
- end
+ where(:accessed_project, :result) do
+ ref(:project_with_target_project_group_in_allowlist) | true
+ ref(:project_wo_target_project_group_in_allowlist) | false
end
- end
- # Context and before block can go away leaving just the example in 16.0
- context 'with inbound only enabled' do
- before do
- project.ci_cd_settings.update!(job_token_scope_enabled: false)
+ with_them do
+ it { is_expected.to eq(result) }
end
- it 'provides access' do
- expect do
- scope.add!(new_project, direction: :inbound, user: user)
- end.to change { described_class.new(new_project).accessible?(current_project) }.from(false).to(true)
+ context 'when ci_job_token_groups_allowlist feature flag is disabled' do
+ before do
+ stub_feature_flags(ci_job_token_groups_allowlist: false)
+ end
+
+ where(:accessed_project, :result) do
+ ref(:project_with_target_project_group_in_allowlist) | false
+ ref(:project_wo_target_project_group_in_allowlist) | false
+ end
+
+ with_them do
+ it { is_expected.to eq(result) }
+ end
end
end
- end
-
- describe 'accessible?' do
- subject { scope.accessible?(accessed_project) }
context 'with inbound and outbound scopes enabled' do
context 'when inbound and outbound access setup' do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index d7037df66782be8ba8fd23d6f2ec27042e629436..580ecce1f7d0ed787be80fb3074e711c12c3fd9e 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -9152,4 +9152,26 @@ def create_build(new_pipeline = pipeline, name = 'test')
let_it_be(:model) { create(:project, creator: parent) }
end
end
+
+ describe '#parent_groups' do
+ context 'when project has parent groups' do
+ let_it_be(:nested_group) { create(:group, parent: group) }
+ let_it_be_with_reload(:group) { create(:group, name: 'foo', parent: nested_group) }
+ let_it_be(:project) { create(:project, group: group) }
+
+ it 'builds an groups path' do
+ groups_path = project.parent_groups
+ expect(groups_path).to match_array([group, nested_group])
+ end
+ end
+
+ context 'when project does not have a parent group' do
+ let_it_be(:project) { create(:project) }
+
+ it 'builds an empty path' do
+ groups_path = project.parent_groups
+ expect(groups_path).to eq([])
+ end
+ end
+ end
end
diff --git a/spec/support/helpers/ci/job_token_scope_helpers.rb b/spec/support/helpers/ci/job_token_scope_helpers.rb
index 1d71356917e385b5e04f15e46ebbf21bb2b0d48c..90e415583d000b0fddc1b55fd0b5e00694951214 100644
--- a/spec/support/helpers/ci/job_token_scope_helpers.rb
+++ b/spec/support/helpers/ci/job_token_scope_helpers.rb
@@ -17,6 +17,25 @@ def create_project_in_allowlist(root_project, direction:, target_project: nil)
included_project
end
+ def create_project_with_group_allowlist(target_project, accessible_project: nil)
+ included_project = accessible_project || create(:project,
+ ci_inbound_job_token_scope_enabled: true
+ )
+ create(
+ :ci_job_token_group_scope_link,
+ source_project: included_project,
+ target_group: target_project.group
+ )
+
+ included_project
+ end
+
+ def create_project_without_group_allowlist(accessible_project: nil)
+ accessible_project || create(:project,
+ ci_inbound_job_token_scope_enabled: true
+ )
+ end
+
def create_project_in_both_allowlists(root_project)
create_project_in_allowlist(root_project, direction: :outbound).tap do |new_project|
create_project_in_allowlist(root_project, target_project: new_project, direction: :inbound)
diff --git a/spec/support/shared_contexts/models/ci/job_token_scope.rb b/spec/support/shared_contexts/models/ci/job_token_scope.rb
index a33a3e09c71df8cc9ed3a0a6a88ddaa03f29f509..c363ff443baeef7a5100d0394fc86d92a49e9f58 100644
--- a/spec/support/shared_contexts/models/ci/job_token_scope.rb
+++ b/spec/support/shared_contexts/models/ci/job_token_scope.rb
@@ -14,6 +14,11 @@
include_context 'with inaccessible projects'
end
+RSpec.shared_context 'with accessible and inaccessible project groups' do
+ let_it_be(:project_with_target_project_group_in_allowlist) { create_project_with_group_allowlist(target_project) }
+ let_it_be(:project_wo_target_project_group_in_allowlist) { create_project_without_group_allowlist }
+end
+
RSpec.shared_context 'with inaccessible projects' do
let_it_be(:inbound_allowlist_project) { create_project_in_allowlist(source_project, direction: :inbound) }