diff --git a/app/models/ci/job_token/allowlist.rb b/app/models/ci/job_token/allowlist.rb new file mode 100644 index 0000000000000000000000000000000000000000..9e9a0a68ebd438671464c6945bc09b7af8a76845 --- /dev/null +++ b/app/models/ci/job_token/allowlist.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true +module Ci + module JobToken + class Allowlist + def initialize(source_project, direction:) + @source_project = source_project + @direction = direction + end + + def includes?(target_project) + source_links + .with_target(target_project) + .exists? + end + + def projects + Project.from_union(target_projects, remove_duplicates: false) + end + + private + + def source_links + Ci::JobToken::ProjectScopeLink + .with_source(@source_project) + .where(direction: @direction) + end + + def target_project_ids + source_links + # pluck needed to avoid ci and main db join + .pluck(:target_project_id) + end + + def target_projects + [ + Project.id_in(@source_project), + Project.id_in(target_project_ids) + ] + end + end + end +end diff --git a/app/models/ci/job_token/project_scope_link.rb b/app/models/ci/job_token/project_scope_link.rb index 3fdf07123e6cfb4c6696773567ef48b32c8a3f86..b784f93651a2b16ef8a21e686a99355920f6a6d2 100644 --- a/app/models/ci/job_token/project_scope_link.rb +++ b/app/models/ci/job_token/project_scope_link.rb @@ -12,8 +12,8 @@ class ProjectScopeLink < Ci::ApplicationRecord belongs_to :target_project, class_name: 'Project' belongs_to :added_by, class_name: 'User' - scope :from_project, ->(project) { where(source_project: project) } - scope :to_project, ->(project) { where(target_project: project) } + scope :with_source, ->(project) { where(source_project: project) } + scope :with_target, ->(project) { where(target_project: project) } validates :source_project, presence: true validates :target_project, presence: true diff --git a/app/models/ci/job_token/scope.rb b/app/models/ci/job_token/scope.rb index 1aa49b95201aa65b06762b07e18f266ad50ef7ed..e320c0f92d14eb8f46f8dc649efdfc8d64340ec6 100644 --- a/app/models/ci/job_token/scope.rb +++ b/app/models/ci/job_token/scope.rb @@ -1,49 +1,58 @@ # frozen_string_literal: true -# This model represents the surface where a CI_JOB_TOKEN can be used. -# A Scope is initialized with the project that the job token belongs to, -# and indicates what are all the other projects that the token could access. +# This model represents the scope of access for a CI_JOB_TOKEN. # -# By default a job token can only access its own project, which is the same -# project that defines the scope. -# By adding ScopeLinks to the scope we can allow other projects to be accessed -# by the job token. This works as an allowlist of projects for a job token. +# A scope is initialized with a project. +# +# Projects can be added to the scope by adding ScopeLinks to +# create an allowlist of projects in either access direction (inbound, outbound). +# +# Currently, projects in the outbound allowlist can be accessed via the token +# in the source project. +# +# TODO(Issue #346298) Projects in the inbound allowlist can use their token to access +# the source project. +# +# CI_JOB_TOKEN should be considered untrusted without these features enabled. # -# If a project is not included in the scope we should not allow the job user -# to access it since operations using CI_JOB_TOKEN should be considered untrusted. module Ci module JobToken class Scope - attr_reader :source_project + attr_reader :current_project - def initialize(project) - @source_project = project + def initialize(current_project) + @current_project = current_project end - def includes?(target_project) - # if the setting is disabled any project is considered to be in scope. - return true unless source_project.ci_outbound_job_token_scope_enabled? + def allows?(accessed_project) + self_referential?(accessed_project) || outbound_allows?(accessed_project) + end - target_project.id == source_project.id || - Ci::JobToken::ProjectScopeLink.from_project(source_project).to_project(target_project).exists? + def outbound_projects + outbound_allowlist.projects end + # Deprecated: use outbound_projects, TODO(Issue #346298) remove references to all_project def all_projects - Project.from_union(target_projects, remove_duplicates: false) + outbound_projects end private - def target_project_ids - Ci::JobToken::ProjectScopeLink.from_project(source_project).pluck(:target_project_id) + def outbound_allows?(accessed_project) + # if the setting is disabled any project is considered to be in scope. + return true unless @current_project.ci_outbound_job_token_scope_enabled? + + outbound_allowlist.includes?(accessed_project) + end + + def outbound_allowlist + Ci::JobToken::Allowlist.new(@current_project, direction: :outbound) end - def target_projects - [ - Project.id_in(source_project), - Project.id_in(target_project_ids) - ] + def self_referential?(accessed_project) + @current_project.id == accessed_project.id end end end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index bfeb1a602ab71b77bef2644c2d2f5e74212cc5ad..2155f84713e34b1a1752a0da304a1a20457b747c 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -121,7 +121,7 @@ class ProjectPolicy < BasePolicy desc "If user is authenticated via CI job token then the target project should be in scope" condition(:project_allowed_for_job_token) do - !@user&.from_ci_job_token? || @user.ci_job_token_scope.includes?(project) + !@user&.from_ci_job_token? || @user.ci_job_token_scope.allows?(project) end with_scope :subject diff --git a/ee/spec/requests/api/ci/triggers_spec.rb b/ee/spec/requests/api/ci/triggers_spec.rb index ef1198718542880313d26634cd5799d557a674e3..5e5b9df0fb0a188a9e7b2e9e1a53ced24c66e1a7 100644 --- a/ee/spec/requests/api/ci/triggers_spec.rb +++ b/ee/spec/requests/api/ci/triggers_spec.rb @@ -19,7 +19,7 @@ end before do - allow_next(Ci::JobToken::Scope).to receive(:includes?).with(project).and_return(true) + allow_next(Ci::JobToken::Scope).to receive(:allows?).with(project).and_return(true) end context 'without user' do @@ -80,7 +80,7 @@ context 'when project is not in the job token scope' do before do allow_next(Ci::JobToken::Scope) - .to receive(:includes?) + .to receive(:allows?) .with(project) .and_return(false) end diff --git a/spec/finders/ci/auth_job_finder_spec.rb b/spec/finders/ci/auth_job_finder_spec.rb index 0a326699875a0c6d388a597ffc0b9824f51532e8..73a65d0c5af4c32477498043076f699283612783 100644 --- a/spec/finders/ci/auth_job_finder_spec.rb +++ b/spec/finders/ci/auth_job_finder_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Ci::AuthJobFinder do +RSpec.describe Ci::AuthJobFinder, feature_category: :continuous_integration do let_it_be(:user, reload: true) { create(:user) } let_it_be(:job, reload: true) { create(:ci_build, status: :running, user: user) } @@ -68,7 +68,7 @@ it 'sets ci_job_token_scope on the job user', :aggregate_failures do expect(subject).to eq(job) expect(subject.user).to be_from_ci_job_token - expect(subject.user.ci_job_token_scope.source_project).to eq(job.project) + expect(subject.user.ci_job_token_scope.current_project).to eq(job.project) end end end diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb index 9283c31a20791898b095c3c635c27e03c3e5ef93..484b4702497e5a469dd1558b2a29fe15caf54b88 100644 --- a/spec/lib/gitlab/auth/auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/auth_finders_spec.rb @@ -69,7 +69,7 @@ def set_bearer_token(token) expect(subject).to eq(user) expect(@current_authenticated_job).to eq job expect(subject).to be_from_ci_job_token - expect(subject.ci_job_token_scope.source_project).to eq(job.project) + expect(subject.ci_job_token_scope.current_project).to eq(job.project) end end @@ -100,7 +100,7 @@ def set_bearer_token(token) expect(subject).to eq(user) expect(@current_authenticated_job).to eq job expect(subject).to be_from_ci_job_token - expect(subject.ci_job_token_scope.source_project).to eq(job.project) + expect(subject.ci_job_token_scope.current_project).to eq(job.project) end else it 'returns nil' do diff --git a/spec/models/ci/job_token/allowlist_spec.rb b/spec/models/ci/job_token/allowlist_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..45083d64393a7741d2ec8fc6952b12f602952c88 --- /dev/null +++ b/spec/models/ci/job_token/allowlist_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::JobToken::Allowlist, feature_category: :continuous_integration do + using RSpec::Parameterized::TableSyntax + + let_it_be(:source_project) { create(:project) } + + let(:allowlist) { described_class.new(source_project, direction: direction) } + let(:direction) { :outbound } + + describe '#projects' do + subject(:projects) { allowlist.projects } + + context 'when no projects are added to the scope' do + [:inbound, :outbound].each do |d| + let(:direction) { d } + + it 'returns the project defining the scope' do + expect(projects).to contain_exactly(source_project) + end + end + end + + context 'when projects are added to the scope' do + include_context 'with scoped projects' + + where(:direction, :additional_project) do + :outbound | ref(:outbound_scoped_project) + :inbound | ref(:inbound_scoped_project) + end + + with_them do + it 'returns all projects that can be accessed from a given scope' do + expect(projects).to contain_exactly(source_project, additional_project) + end + end + end + end + + describe '#includes?' do + subject { allowlist.includes?(includes_project) } + + context 'without scoped projects' do + let(:unscoped_project) { build(:project) } + + where(:includes_project, :direction, :result) do + ref(:source_project) | :outbound | false + ref(:source_project) | :inbound | false + ref(:unscoped_project) | :outbound | false + ref(:unscoped_project) | :inbound | false + end + + with_them do + it { is_expected.to be result } + end + end + + context 'with scoped projects' do + include_context 'with scoped projects' + + where(:includes_project, :direction, :result) do + ref(:source_project) | :outbound | false + ref(:source_project) | :inbound | false + ref(:inbound_scoped_project) | :outbound | false + ref(:inbound_scoped_project) | :inbound | true + ref(:outbound_scoped_project) | :outbound | true + ref(:outbound_scoped_project) | :inbound | false + ref(:unscoped_project1) | :outbound | false + ref(:unscoped_project1) | :inbound | false + ref(:unscoped_project2) | :outbound | false + ref(:unscoped_project2) | :inbound | false + end + + with_them do + it { is_expected.to be result } + end + end + end +end diff --git a/spec/models/ci/job_token/project_scope_link_spec.rb b/spec/models/ci/job_token/project_scope_link_spec.rb index 6f428d1aa8b648fbe808776a82e20895a5439d80..91491733c444bad97048a9a29b30046900d591f9 100644 --- a/spec/models/ci/job_token/project_scope_link_spec.rb +++ b/spec/models/ci/job_token/project_scope_link_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::JobToken::ProjectScopeLink do +RSpec.describe Ci::JobToken::ProjectScopeLink, feature_category: :continuous_integration do let_it_be(:project) { create(:project) } let_it_be(:group) { create(:group) } @@ -50,8 +50,8 @@ end end - describe '.from_project' do - subject { described_class.from_project(project) } + describe '.with_source' do + subject { described_class.with_source(project) } let!(:source_link) { create(:ci_job_token_project_scope_link, source_project: project) } let!(:target_link) { create(:ci_job_token_project_scope_link, target_project: project) } @@ -61,8 +61,8 @@ end end - describe '.to_project' do - subject { described_class.to_project(project) } + describe '.with_target' do + subject { described_class.with_target(project) } let!(:source_link) { create(:ci_job_token_project_scope_link, source_project: project) } let!(:target_link) { create(:ci_job_token_project_scope_link, target_project: project) } diff --git a/spec/models/ci/job_token/scope_spec.rb b/spec/models/ci/job_token/scope_spec.rb index 1e3f6d044d2690bdebc910f47356c20471ceea6d..37c5697350658bbeccab77bb81f37fcaff768550 100644 --- a/spec/models/ci/job_token/scope_spec.rb +++ b/spec/models/ci/job_token/scope_spec.rb @@ -2,58 +2,72 @@ require 'spec_helper' -RSpec.describe Ci::JobToken::Scope do - let_it_be(:project) { create(:project, ci_outbound_job_token_scope_enabled: true).tap(&:save!) } +RSpec.describe Ci::JobToken::Scope, feature_category: :continuous_integration do + let_it_be(:source_project) { create(:project, ci_outbound_job_token_scope_enabled: true) } - let(:scope) { described_class.new(project) } + let(:scope) { described_class.new(source_project) } describe '#all_projects' do subject(:all_projects) { scope.all_projects } context 'when no projects are added to the scope' do it 'returns the project defining the scope' do - expect(all_projects).to contain_exactly(project) + expect(all_projects).to contain_exactly(source_project) end end - context 'when other projects are added to the scope' do - let_it_be(:scoped_project) { create(:project) } - let_it_be(:unscoped_project) { create(:project) } - - let!(:link_in_scope) { create(:ci_job_token_project_scope_link, source_project: project, target_project: scoped_project) } - let!(:link_out_of_scope) { create(:ci_job_token_project_scope_link, target_project: unscoped_project) } + context 'when projects are added to the scope' do + include_context 'with scoped projects' it 'returns all projects that can be accessed from a given scope' do - expect(subject).to contain_exactly(project, scoped_project) + expect(subject).to contain_exactly(source_project, outbound_scoped_project) end end end - describe '#includes?' do - subject { scope.includes?(target_project) } + describe '#allows?' do + subject { scope.allows?(includes_project) } - context 'when param is the project defining the scope' do - let(:target_project) { project } + context 'without scoped projects' do + context 'when self referential' do + let(:includes_project) { source_project } - it { is_expected.to be_truthy } + it { is_expected.to be_truthy } + end end - context 'when param is a project in scope' do - let(:target_link) { create(:ci_job_token_project_scope_link, source_project: project) } - let(:target_project) { target_link.target_project } + context 'with scoped projects' do + include_context 'with scoped projects' - it { is_expected.to be_truthy } - end + context 'when project is in outbound scope' do + let(:includes_project) { outbound_scoped_project } - context 'when param is a project in another scope' do - let(:scope_link) { create(:ci_job_token_project_scope_link) } - let(:target_project) { scope_link.target_project } + it { is_expected.to be_truthy } + end + + context 'when project is in inbound scope' do + let(:includes_project) { inbound_scoped_project } + + it { is_expected.to be_falsey } + end - it { is_expected.to be_falsey } + context 'when project is linked to a different project' do + let(:includes_project) { unscoped_project1 } + + it { is_expected.to be_falsey } + end + + context 'when project is unlinked to a project' do + let(:includes_project) { unscoped_project2 } + + it { is_expected.to be_falsey } + end context 'when project scope setting is disabled' do + let(:includes_project) { unscoped_project1 } + before do - project.ci_outbound_job_token_scope_enabled = false + source_project.ci_outbound_job_token_scope_enabled = false end it 'considers any project to be part of the scope' do diff --git a/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb b/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb index b2f84ab28691108556f9645c2c8917c9359ab1c9..50d6e9b9e9590affc2da7e55d1312f32374fca04 100644 --- a/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb @@ -60,7 +60,7 @@ post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) expect(mutation_response.dig('ciJobTokenScope', 'projects', 'nodes')).not_to be_empty - end.to change { Ci::JobToken::Scope.new(project).includes?(target_project) }.from(false).to(true) + end.to change { Ci::JobToken::Scope.new(project).allows?(target_project) }.from(false).to(true) end context 'when invalid target project is provided' do diff --git a/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb b/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb index 2b0adf89f404ba659e74278b8e63f8ed28241f80..eb1a87f1cda8c2600f8179aa7392015869924792 100644 --- a/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb @@ -66,7 +66,7 @@ post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) expect(mutation_response.dig('ciJobTokenScope', 'projects', 'nodes')).not_to be_empty - end.to change { Ci::JobToken::Scope.new(project).includes?(target_project) }.from(true).to(false) + end.to change { Ci::JobToken::Scope.new(project).allows?(target_project) }.from(true).to(false) end context 'when invalid target project is provided' do diff --git a/spec/support/shared_contexts/models/ci/job_token_scope.rb b/spec/support/shared_contexts/models/ci/job_token_scope.rb new file mode 100644 index 0000000000000000000000000000000000000000..51f671b139d69e78fe78444a8f73b51b537ccfd0 --- /dev/null +++ b/spec/support/shared_contexts/models/ci/job_token_scope.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +RSpec.shared_context 'with scoped projects' do + let_it_be(:inbound_scoped_project) { create_scoped_project(source_project, direction: :inbound) } + let_it_be(:outbound_scoped_project) { create_scoped_project(source_project, direction: :outbound) } + let_it_be(:unscoped_project1) { create(:project) } + let_it_be(:unscoped_project2) { create(:project) } + + let_it_be(:link_out_of_scope) { create(:ci_job_token_project_scope_link, target_project: unscoped_project1) } + + def create_scoped_project(source_project, direction:) + create(:project).tap do |scoped_project| + create( + :ci_job_token_project_scope_link, + source_project: source_project, + target_project: scoped_project, + direction: direction + ) + end + end +end