From f390191c9fb768dd428e6ecbee48c5122eb8c1d7 Mon Sep 17 00:00:00 2001 From: Dmytro Biryukov Date: Mon, 20 Mar 2023 15:49:43 +0100 Subject: [PATCH 1/8] Add audit logging for secure CI_JOB_TOKEN setting Audit ci_cd_settings update Add audit event on enable/disable CI_JOB_TOKEN on inbound scope Add audit events on remove/add project Move audit on add/remove project to service level Remove redundant tap/save and fix remove project spec Fix default direction with inbound --- .../ci/job_token_scope/add_project_service.rb | 3 +- .../job_token_scope/remove_project_service.rb | 2 + .../ci/project_ci_cd_settings_update.rb | 26 +++++ .../ci/job_token_scope/add_project_service.rb | 36 +++++++ .../job_token_scope/remove_project_service.rb | 36 +++++++ .../secure_ci_job_token_inbound_disabled.yml | 8 ++ .../secure_ci_job_token_inbound_enabled.yml | 8 ++ .../secure_ci_job_token_project_added.yml | 8 ++ .../secure_ci_job_token_project_removed.yml | 8 ++ .../ci/job_token_scope/add_project_spec.rb | 87 +++++++++++++++++ .../ci/job_token_scope/remove_project_spec.rb | 96 +++++++++++++++++++ .../ci/project_ci_cd_settings_update_spec.rb | 63 +++++++++++- 12 files changed, 378 insertions(+), 3 deletions(-) create mode 100644 ee/app/services/ee/ci/job_token_scope/add_project_service.rb create mode 100644 ee/app/services/ee/ci/job_token_scope/remove_project_service.rb create mode 100644 ee/config/audit_events/types/secure_ci_job_token_inbound_disabled.yml create mode 100644 ee/config/audit_events/types/secure_ci_job_token_inbound_enabled.yml create mode 100644 ee/config/audit_events/types/secure_ci_job_token_project_added.yml create mode 100644 ee/config/audit_events/types/secure_ci_job_token_project_removed.yml create mode 100644 ee/spec/graphql/ee/mutations/ci/job_token_scope/add_project_spec.rb create mode 100644 ee/spec/graphql/ee/mutations/ci/job_token_scope/remove_project_spec.rb diff --git a/app/services/ci/job_token_scope/add_project_service.rb b/app/services/ci/job_token_scope/add_project_service.rb index 704aa28f8c5236..2e087f444832df 100644 --- a/app/services/ci/job_token_scope/add_project_service.rb +++ b/app/services/ci/job_token_scope/add_project_service.rb @@ -12,7 +12,6 @@ def execute(target_project, direction: :inbound) .add!(target_project, user: current_user) ServiceResponse.success(payload: { project_link: link }) - rescue ActiveRecord::RecordNotUnique ServiceResponse.error(message: "Target project is already in the job token scope") rescue ActiveRecord::RecordInvalid => e @@ -29,3 +28,5 @@ def allowlist(direction) end end end + +Ci::JobTokenScope::AddProjectService.prepend_mod_with('Ci::JobTokenScope::AddProjectService') diff --git a/app/services/ci/job_token_scope/remove_project_service.rb b/app/services/ci/job_token_scope/remove_project_service.rb index 864f9318c68067..d6a2defd5b941f 100644 --- a/app/services/ci/job_token_scope/remove_project_service.rb +++ b/app/services/ci/job_token_scope/remove_project_service.rb @@ -31,3 +31,5 @@ def execute(target_project, direction) end end end + +Ci::JobTokenScope::RemoveProjectService.prepend_mod_with('Ci::JobTokenScope::RemoveProjectService') diff --git a/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb b/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb index a1cb07380c5ec7..034e77bed9abbf 100644 --- a/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb +++ b/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb @@ -7,6 +7,14 @@ module ProjectCiCdSettingsUpdate extend ActiveSupport::Concern extend ::Gitlab::Utils::Override + override :resolve + + def resolve(full_path:, **args) + super.tap do |result| + audit(project, result[:ci_cd_settings], current_user, args[:inbound_job_token_scope_enabled]) + end + end + prepended do argument :merge_pipelines_enabled, GraphQL::Types::Boolean, required: false, @@ -16,6 +24,24 @@ module ProjectCiCdSettingsUpdate required: false, description: 'Indicates if merge trains are enabled for the project.' end + + private + + def audit(scope, target, author, inbound_job_token_scope_enabled) + audit_action = inbound_job_token_scope_enabled ? 'enabled' : 'disabled' + audit_message = "Secure ci_job_token was #{audit_action} for inbound" + event_name = "secure_ci_job_token_inbound_#{audit_action}" + + audit_context = { + name: event_name, + author: author, + scope: scope, + target: target, + message: audit_message + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/app/services/ee/ci/job_token_scope/add_project_service.rb b/ee/app/services/ee/ci/job_token_scope/add_project_service.rb new file mode 100644 index 00000000000000..2647a1eb98e44a --- /dev/null +++ b/ee/app/services/ee/ci/job_token_scope/add_project_service.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module EE + module Ci + module JobTokenScope + module AddProjectService + extend ::Gitlab::Utils::Override + + override :execute + def execute(target_project, direction: :inbound) + super.tap do |response| + audit(project, target_project, current_user) if direction == :inbound && response.success? + end + end + + private + + def audit(scope, target, author) + audit_message = + "Project #{target.full_path} was added to inbound list of allowed projects for #{scope.full_path}" + event_name = 'secure_ci_job_token_project_added' + + audit_context = { + name: event_name, + author: author, + scope: scope, + target: target, + message: audit_message + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end + end +end diff --git a/ee/app/services/ee/ci/job_token_scope/remove_project_service.rb b/ee/app/services/ee/ci/job_token_scope/remove_project_service.rb new file mode 100644 index 00000000000000..0446740319a760 --- /dev/null +++ b/ee/app/services/ee/ci/job_token_scope/remove_project_service.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module EE + module Ci + module JobTokenScope + module RemoveProjectService + extend ::Gitlab::Utils::Override + + override :execute + def execute(target_project, direction) + super.tap do |response| + audit(project, target_project, current_user) if direction == :inbound && response.success? + end + end + + private + + def audit(scope, target, author) + audit_message = + "Project #{target.full_path} was removed from inbound list of allowed projects for #{scope.full_path}" + event_name = 'secure_ci_job_token_project_removed' + + audit_context = { + name: event_name, + author: author, + scope: scope, + target: target, + message: audit_message + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end + end +end diff --git a/ee/config/audit_events/types/secure_ci_job_token_inbound_disabled.yml b/ee/config/audit_events/types/secure_ci_job_token_inbound_disabled.yml new file mode 100644 index 00000000000000..dc2f8100dc8dc1 --- /dev/null +++ b/ee/config/audit_events/types/secure_ci_job_token_inbound_disabled.yml @@ -0,0 +1,8 @@ +name: secure_ci_job_token_inbound_disabled +description: Event triggered when CI_JOB_TOKEN permissions disabled for inbound +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/338255 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115350 +feature_category: "verify_security" +milestone: "16.0" +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/secure_ci_job_token_inbound_enabled.yml b/ee/config/audit_events/types/secure_ci_job_token_inbound_enabled.yml new file mode 100644 index 00000000000000..e8b9725b501c51 --- /dev/null +++ b/ee/config/audit_events/types/secure_ci_job_token_inbound_enabled.yml @@ -0,0 +1,8 @@ +name: secure_ci_job_token_inbound_enabled +description: Event triggered when CI_JOB_TOKEN permissions enabled for inbound +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/338255 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115350 +feature_category: "verify_security" +milestone: "16.0" +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/secure_ci_job_token_project_added.yml b/ee/config/audit_events/types/secure_ci_job_token_project_added.yml new file mode 100644 index 00000000000000..f7a6c74f7d3829 --- /dev/null +++ b/ee/config/audit_events/types/secure_ci_job_token_project_added.yml @@ -0,0 +1,8 @@ +name: secure_ci_job_token_project_added +description: Event triggered when project added to inbound CI_JOB_TOKEN scope +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/338255 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115350 +feature_category: "verify_security" +milestone: "16.0" +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/secure_ci_job_token_project_removed.yml b/ee/config/audit_events/types/secure_ci_job_token_project_removed.yml new file mode 100644 index 00000000000000..eef733bbd973bc --- /dev/null +++ b/ee/config/audit_events/types/secure_ci_job_token_project_removed.yml @@ -0,0 +1,8 @@ +name: secure_ci_job_token_project_removed +description: Event triggered when project removed from inbound CI_JOB_TOKEN scope +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/338255 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115350 +feature_category: "verify_security" +milestone: "16.0" +saved_to_database: true +streamed: true diff --git a/ee/spec/graphql/ee/mutations/ci/job_token_scope/add_project_spec.rb b/ee/spec/graphql/ee/mutations/ci/job_token_scope/add_project_spec.rb new file mode 100644 index 00000000000000..607feaefe7cae4 --- /dev/null +++ b/ee/spec/graphql/ee/mutations/ci/job_token_scope/add_project_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::Ci::JobTokenScope::AddProject, feature_category: :continuous_integration do + let(:mutation) do + described_class.new(object: nil, context: { current_user: current_user }, field: nil) + end + + describe '#resolve' do + let_it_be(:project) do + create(:project, ci_outbound_job_token_scope_enabled: true) + end + + let_it_be(:target_project) { create(:project) } + + let(:target_project_path) { target_project.full_path } + let(:project_path) { project.full_path } + let(:mutation_args) { { project_path: project.full_path, target_project_path: target_project_path } } + let(:current_user) { create(:user) } + + let(:expected_audit_context) do + { + name: event_name, + author: current_user, + scope: project, + target: target_project, + message: expected_audit_message + } + end + + subject do + mutation.resolve(**mutation_args) + end + + before do + project.add_maintainer(current_user) + target_project.add_guest(current_user) + end + + context 'when user adds target project to the inbound job token scope' do + let(:expected_audit_message) do + "Project #{target_project_path} was added to inbound list of allowed projects for #{project_path}" + end + + let(:event_name) { 'secure_ci_job_token_project_added' } + let(:mutation_args) do + { project_path: project.full_path, + target_project_path: target_project_path, + direction: :inbound } + end + + it 'logs an audit event' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including(expected_audit_context)) + + subject + end + + context 'and service returns an error' do + let(:service) { instance_double('Ci::JobTokenScope::AddProjectService') } + + it 'does not log an audit event' do + expect(::Ci::JobTokenScope::AddProjectService).to receive(:new).with( + project, + current_user + ).and_return(service) + expect(service) + .to receive(:execute) + .with(target_project, direction: :inbound) + .and_return(ServiceResponse.error(message: 'The error message')) + + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + subject + end + end + end + + context 'when user adds target project to the default outbound job token scope' do + it 'does not log an audit event' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + subject + end + end + end +end diff --git a/ee/spec/graphql/ee/mutations/ci/job_token_scope/remove_project_spec.rb b/ee/spec/graphql/ee/mutations/ci/job_token_scope/remove_project_spec.rb new file mode 100644 index 00000000000000..22eb3f21f7ce86 --- /dev/null +++ b/ee/spec/graphql/ee/mutations/ci/job_token_scope/remove_project_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::Ci::JobTokenScope::RemoveProject, feature_category: :continuous_integration do + let(:mutation) do + described_class.new(object: nil, context: { current_user: current_user }, field: nil) + end + + describe '#resolve' do + let_it_be(:project) do + create(:project, ci_outbound_job_token_scope_enabled: true) + end + + let_it_be(:target_project) { create(:project) } + + let_it_be(:link) do + create(:ci_job_token_project_scope_link, + direction: :inbound, + source_project: project, + target_project: target_project) + end + + let(:links_relation) { Ci::JobToken::ProjectScopeLink.with_source(project).with_target(target_project) } + + let(:target_project_path) { target_project.full_path } + let(:project_path) { project.full_path } + let(:mutation_args) { { project_path: project.full_path, target_project_path: target_project_path } } + let(:current_user) { create(:user) } + + let(:expected_audit_context) do + { + name: event_name, + author: current_user, + scope: project, + target: target_project, + message: expected_audit_message + } + end + + subject do + mutation.resolve(**mutation_args) + end + + before do + project.add_maintainer(current_user) + target_project.add_guest(current_user) + end + + context 'when user removes target project to the inbound job token scope' do + let(:expected_audit_message) do + "Project #{target_project_path} was removed from inbound list of allowed projects for #{project_path}" + end + + let(:event_name) { 'secure_ci_job_token_project_removed' } + let(:mutation_args) do + { project_path: project.full_path, target_project_path: target_project_path, direction: :inbound } + end + + let(:service) do + instance_double('Ci::JobTokenScope::RemoveProjectService') + end + + it 'logs an audit event' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including(expected_audit_context)) + + subject + end + + context 'and service returns an error' do + it 'does not log an audit event' do + expect(::Ci::JobTokenScope::RemoveProjectService).to receive(:new).with( + project, + current_user + ).and_return(service) + expect(service) + .to receive(:execute) + .with(target_project, :inbound) + .and_return(ServiceResponse.error(message: 'The error message')) + + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + subject + end + end + end + + context 'when user removes target project to the default outbound job token scope' do + it 'does not log an audit event' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + subject + end + end + end +end diff --git a/ee/spec/graphql/ee/mutations/ci/project_ci_cd_settings_update_spec.rb b/ee/spec/graphql/ee/mutations/ci/project_ci_cd_settings_update_spec.rb index 85a41c8e66139c..12e118a3015d85 100644 --- a/ee/spec/graphql/ee/mutations/ci/project_ci_cd_settings_update_spec.rb +++ b/ee/spec/graphql/ee/mutations/ci/project_ci_cd_settings_update_spec.rb @@ -17,11 +17,14 @@ stub_feature_flags(disable_merge_trains: false) project.merge_pipelines_enabled = nil project.merge_trains_enabled = false - subject - project.reload end describe '#resolve' do + before do + subject + project.reload + end + context 'when merge trains are set to true and merge pipelines are set to false' do let(:mutation_params) do { @@ -51,4 +54,60 @@ end end end + + describe 'inbound_job_token_scope_enabled' do + let(:mutation_params) do + { + full_path: project.full_path, + inbound_job_token_scope_enabled: inbound_job_token_scope_enabled_target + } + end + + let(:project) do + create(:project, + keep_latest_artifact: true, + ci_inbound_job_token_scope_enabled: inbound_job_token_scope_enabled_origin + ) + end + + let(:expected_target) do + project.ci_cd_settings + end + + let(:expected_audit_context) do + { + name: event_name, + author: user, + scope: project, + target: expected_target, + message: expected_audit_message + } + end + + context 'when changes from enabled to disabled' do + let(:inbound_job_token_scope_enabled_origin) { true } + let(:inbound_job_token_scope_enabled_target) { false } + let(:expected_audit_message) { 'Secure ci_job_token was disabled for inbound' } + let(:event_name) { 'secure_ci_job_token_inbound_disabled' } + + it 'logs an audit event' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including(expected_audit_context)) + + subject + end + end + + context 'when changes from disabled to enabled' do + let(:inbound_job_token_scope_enabled_origin) { false } + let(:inbound_job_token_scope_enabled_target) { true } + let(:expected_audit_message) { 'Secure ci_job_token was enabled for inbound' } + let(:event_name) { 'secure_ci_job_token_inbound_enabled' } + + it 'logs an audit event' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including(expected_audit_context)) + + subject + end + end + end end -- GitLab From 19e4560c7cd27e22935cb7b16a9a91398ae2f9e1 Mon Sep 17 00:00:00 2001 From: Dmytro Biryukov Date: Thu, 27 Apr 2023 21:19:39 +0200 Subject: [PATCH 2/8] Fix project definition --- .../ci/project_ci_cd_settings_update.rb | 14 ++----- .../ci/job_token_scope/add_project_spec.rb | 38 ++++++++++--------- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb b/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb index 034e77bed9abbf..1074b4621dda6a 100644 --- a/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb +++ b/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb @@ -11,20 +11,12 @@ module ProjectCiCdSettingsUpdate def resolve(full_path:, **args) super.tap do |result| - audit(project, result[:ci_cd_settings], current_user, args[:inbound_job_token_scope_enabled]) + ci_cd_settings = result[:ci_cd_settings] + audit_project = ci_cd_settings.project + audit(audit_project, ci_cd_settings, current_user, args[:inbound_job_token_scope_enabled]) end end - prepended do - argument :merge_pipelines_enabled, GraphQL::Types::Boolean, - required: false, - description: 'Indicates if merge pipelines are enabled for the project.' - - argument :merge_trains_enabled, GraphQL::Types::Boolean, - required: false, - description: 'Indicates if merge trains are enabled for the project.' - end - private def audit(scope, target, author, inbound_job_token_scope_enabled) diff --git a/ee/spec/graphql/ee/mutations/ci/job_token_scope/add_project_spec.rb b/ee/spec/graphql/ee/mutations/ci/job_token_scope/add_project_spec.rb index 607feaefe7cae4..eabbeeb8c0df7f 100644 --- a/ee/spec/graphql/ee/mutations/ci/job_token_scope/add_project_spec.rb +++ b/ee/spec/graphql/ee/mutations/ci/job_token_scope/add_project_spec.rb @@ -16,7 +16,14 @@ let(:target_project_path) { target_project.full_path } let(:project_path) { project.full_path } - let(:mutation_args) { { project_path: project.full_path, target_project_path: target_project_path } } + let(:mutation_args) do + { + project_path: project.full_path, + target_project_path: target_project_path, + direction: :inbound + } + end + let(:current_user) { create(:user) } let(:expected_audit_context) do @@ -44,11 +51,6 @@ end let(:event_name) { 'secure_ci_job_token_project_added' } - let(:mutation_args) do - { project_path: project.full_path, - target_project_path: target_project_path, - direction: :inbound } - end it 'logs an audit event' do expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including(expected_audit_context)) @@ -57,17 +59,13 @@ end context 'and service returns an error' do - let(:service) { instance_double('Ci::JobTokenScope::AddProjectService') } - it 'does not log an audit event' do - expect(::Ci::JobTokenScope::AddProjectService).to receive(:new).with( - project, - current_user - ).and_return(service) - expect(service) - .to receive(:execute) - .with(target_project, direction: :inbound) - .and_return(ServiceResponse.error(message: 'The error message')) + expect_next_instance_of(::Ci::JobTokenScope::AddProjectService) do |service| + expect(service) + .to receive(:validate_edit!) + .with(project, target_project, current_user) + .and_raise(ActiveRecord::RecordNotUnique) + end expect(::Gitlab::Audit::Auditor).not_to receive(:audit) @@ -76,7 +74,13 @@ end end - context 'when user adds target project to the default outbound job token scope' do + context 'when user adds target project to the outbound job token scope' do + let(:mutation_args) do + { project_path: project.full_path, + target_project_path: target_project_path, + direction: :outbound } + end + it 'does not log an audit event' do expect(::Gitlab::Audit::Auditor).not_to receive(:audit) -- GitLab From 0ab0489ebee73d86c88f95519b319aa0c8ea56e8 Mon Sep 17 00:00:00 2001 From: Dmytro Biryukov Date: Thu, 27 Apr 2023 21:21:11 +0200 Subject: [PATCH 3/8] Update graphql docs --- doc/api/graphql/reference/index.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 312b02356a5ae2..c831819d373339 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -1440,8 +1440,6 @@ Input type: `CiCdSettingsUpdateInput` | `inboundJobTokenScopeEnabled` | [`Boolean`](#boolean) | Indicates CI/CD job tokens generated in other projects have restricted access to this project. | | `jobTokenScopeEnabled` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated:** Outbound job token scope is being removed. This field can now only be set to false. Deprecated in 16.0. | | `keepLatestArtifact` | [`Boolean`](#boolean) | Indicates if the latest artifact should be kept for the project. | -| `mergePipelinesEnabled` | [`Boolean`](#boolean) | Indicates if merge pipelines are enabled for the project. | -| `mergeTrainsEnabled` | [`Boolean`](#boolean) | Indicates if merge trains are enabled for the project. | | `optInJwt` | [`Boolean`](#boolean) | When disabled, the JSON Web Token is always available in all jobs in the pipeline. | #### Fields @@ -4887,8 +4885,6 @@ Input type: `ProjectCiCdSettingsUpdateInput` | `inboundJobTokenScopeEnabled` | [`Boolean`](#boolean) | Indicates CI/CD job tokens generated in other projects have restricted access to this project. | | `jobTokenScopeEnabled` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated:** Outbound job token scope is being removed. This field can now only be set to false. Deprecated in 16.0. | | `keepLatestArtifact` | [`Boolean`](#boolean) | Indicates if the latest artifact should be kept for the project. | -| `mergePipelinesEnabled` | [`Boolean`](#boolean) | Indicates if merge pipelines are enabled for the project. | -| `mergeTrainsEnabled` | [`Boolean`](#boolean) | Indicates if merge trains are enabled for the project. | | `optInJwt` | [`Boolean`](#boolean) | When disabled, the JSON Web Token is always available in all jobs in the pipeline. | #### Fields -- GitLab From 238f8dbedde38a2f95aa59ab0df8e391f69ce5ec Mon Sep 17 00:00:00 2001 From: Dmytro Biryukov Date: Fri, 28 Apr 2023 10:22:28 +0200 Subject: [PATCH 4/8] Restore graphql arguments, audit only when change happens --- .../ci/project_ci_cd_settings_update.rb | 15 ++++++++++++++- .../ci/project_ci_cd_settings_update_spec.rb | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb b/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb index 1074b4621dda6a..c6d3ee6bb7c582 100644 --- a/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb +++ b/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb @@ -7,13 +7,26 @@ module ProjectCiCdSettingsUpdate extend ActiveSupport::Concern extend ::Gitlab::Utils::Override + prepended do + argument :merge_pipelines_enabled, GraphQL::Types::Boolean, + required: false, + description: 'Indicates if merge pipelines are enabled for the project.' + + argument :merge_trains_enabled, GraphQL::Types::Boolean, + required: false, + description: 'Indicates if merge trains are enabled for the project.' + end + override :resolve def resolve(full_path:, **args) super.tap do |result| ci_cd_settings = result[:ci_cd_settings] audit_project = ci_cd_settings.project - audit(audit_project, ci_cd_settings, current_user, args[:inbound_job_token_scope_enabled]) + if ci_cd_settings.inbound_job_token_scope_enabled_previously_changed? + audit(audit_project, ci_cd_settings, current_user, + ci_cd_settings.inbound_job_token_scope_enabled) + end end end diff --git a/ee/spec/graphql/ee/mutations/ci/project_ci_cd_settings_update_spec.rb b/ee/spec/graphql/ee/mutations/ci/project_ci_cd_settings_update_spec.rb index 12e118a3015d85..af5c93914476e4 100644 --- a/ee/spec/graphql/ee/mutations/ci/project_ci_cd_settings_update_spec.rb +++ b/ee/spec/graphql/ee/mutations/ci/project_ci_cd_settings_update_spec.rb @@ -70,6 +70,8 @@ ) end + let(:ci_cd_settings) { project.ci_cd_settings } + let(:expected_target) do project.ci_cd_settings end @@ -84,6 +86,10 @@ } end + before do + ci_cd_settings.update!(inbound_job_token_scope_enabled: inbound_job_token_scope_enabled_origin) + end + context 'when changes from enabled to disabled' do let(:inbound_job_token_scope_enabled_origin) { true } let(:inbound_job_token_scope_enabled_target) { false } @@ -109,5 +115,17 @@ subject end end + + context 'when there are no changes' do + let(:inbound_job_token_scope_enabled_origin) { true } + let(:inbound_job_token_scope_enabled_target) { true } + + it 'logs an audit event' do + expect(::Gitlab::Audit::Auditor).to_not receive(:audit) + + subject + end + end + end end -- GitLab From 0c84410d3f258e9b2ad451761d9ca35cc59f0eac Mon Sep 17 00:00:00 2001 From: Dmytro Biryukov Date: Fri, 28 Apr 2023 10:30:21 +0200 Subject: [PATCH 5/8] Add a test when the inbound_job_token_scope parameter is not provided --- .../ci/project_ci_cd_settings_update_spec.rb | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/ee/spec/graphql/ee/mutations/ci/project_ci_cd_settings_update_spec.rb b/ee/spec/graphql/ee/mutations/ci/project_ci_cd_settings_update_spec.rb index af5c93914476e4..92b4560b935bb5 100644 --- a/ee/spec/graphql/ee/mutations/ci/project_ci_cd_settings_update_spec.rb +++ b/ee/spec/graphql/ee/mutations/ci/project_ci_cd_settings_update_spec.rb @@ -55,6 +55,22 @@ end end + describe 'when the inbound_job_token_scope parameter is not provided' do + let(:mutation_params) do + { + full_path: project.full_path, + merge_pipelines_enabled: true, + merge_trains_enabled: true + } + end + + it 'does not log an audit event' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + subject + end + end + describe 'inbound_job_token_scope_enabled' do let(:mutation_params) do { @@ -120,12 +136,11 @@ let(:inbound_job_token_scope_enabled_origin) { true } let(:inbound_job_token_scope_enabled_target) { true } - it 'logs an audit event' do - expect(::Gitlab::Audit::Auditor).to_not receive(:audit) + it 'does not log an audit event' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) subject end end - end end -- GitLab From fe003e5aedbf06fc8ef3ffa9f2e1145478bb1726 Mon Sep 17 00:00:00 2001 From: Dmytro Biryukov Date: Fri, 28 Apr 2023 10:31:53 +0200 Subject: [PATCH 6/8] Update graphql docs --- app/services/ci/job_token_scope/add_project_service.rb | 1 + doc/api/graphql/reference/index.md | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/app/services/ci/job_token_scope/add_project_service.rb b/app/services/ci/job_token_scope/add_project_service.rb index 2e087f444832df..8fb543a279607e 100644 --- a/app/services/ci/job_token_scope/add_project_service.rb +++ b/app/services/ci/job_token_scope/add_project_service.rb @@ -12,6 +12,7 @@ def execute(target_project, direction: :inbound) .add!(target_project, user: current_user) ServiceResponse.success(payload: { project_link: link }) + rescue ActiveRecord::RecordNotUnique ServiceResponse.error(message: "Target project is already in the job token scope") rescue ActiveRecord::RecordInvalid => e diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index c831819d373339..312b02356a5ae2 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -1440,6 +1440,8 @@ Input type: `CiCdSettingsUpdateInput` | `inboundJobTokenScopeEnabled` | [`Boolean`](#boolean) | Indicates CI/CD job tokens generated in other projects have restricted access to this project. | | `jobTokenScopeEnabled` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated:** Outbound job token scope is being removed. This field can now only be set to false. Deprecated in 16.0. | | `keepLatestArtifact` | [`Boolean`](#boolean) | Indicates if the latest artifact should be kept for the project. | +| `mergePipelinesEnabled` | [`Boolean`](#boolean) | Indicates if merge pipelines are enabled for the project. | +| `mergeTrainsEnabled` | [`Boolean`](#boolean) | Indicates if merge trains are enabled for the project. | | `optInJwt` | [`Boolean`](#boolean) | When disabled, the JSON Web Token is always available in all jobs in the pipeline. | #### Fields @@ -4885,6 +4887,8 @@ Input type: `ProjectCiCdSettingsUpdateInput` | `inboundJobTokenScopeEnabled` | [`Boolean`](#boolean) | Indicates CI/CD job tokens generated in other projects have restricted access to this project. | | `jobTokenScopeEnabled` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated:** Outbound job token scope is being removed. This field can now only be set to false. Deprecated in 16.0. | | `keepLatestArtifact` | [`Boolean`](#boolean) | Indicates if the latest artifact should be kept for the project. | +| `mergePipelinesEnabled` | [`Boolean`](#boolean) | Indicates if merge pipelines are enabled for the project. | +| `mergeTrainsEnabled` | [`Boolean`](#boolean) | Indicates if merge trains are enabled for the project. | | `optInJwt` | [`Boolean`](#boolean) | When disabled, the JSON Web Token is always available in all jobs in the pipeline. | #### Fields -- GitLab From cb21b94cc7f62c790758ba3c6f67d31ae0039231 Mon Sep 17 00:00:00 2001 From: drew stachon <730684-drew@users.noreply.gitlab.com> Date: Tue, 2 May 2023 10:29:38 +0000 Subject: [PATCH 7/8] Apply 1 suggestion(s) to 1 file(s) --- .../graphql/ee/mutations/ci/project_ci_cd_settings_update.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb b/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb index c6d3ee6bb7c582..cdefcd9c55b887 100644 --- a/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb +++ b/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb @@ -23,9 +23,9 @@ def resolve(full_path:, **args) super.tap do |result| ci_cd_settings = result[:ci_cd_settings] audit_project = ci_cd_settings.project + if ci_cd_settings.inbound_job_token_scope_enabled_previously_changed? - audit(audit_project, ci_cd_settings, current_user, - ci_cd_settings.inbound_job_token_scope_enabled) + audit(audit_project, ci_cd_settings, current_user, ci_cd_settings.inbound_job_token_scope_enabled) end end end -- GitLab From 539027968acf70a8f9ce442407acc2adee02ef44 Mon Sep 17 00:00:00 2001 From: drew stachon <730684-drew@users.noreply.gitlab.com> Date: Tue, 2 May 2023 10:29:44 +0000 Subject: [PATCH 8/8] Apply 1 suggestion(s) to 1 file(s) --- ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb b/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb index cdefcd9c55b887..7a401485be50e9 100644 --- a/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb +++ b/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb @@ -18,7 +18,6 @@ module ProjectCiCdSettingsUpdate end override :resolve - def resolve(full_path:, **args) super.tap do |result| ci_cd_settings = result[:ci_cd_settings] -- GitLab