From a37d8d9b841006adc448ceaced9a7b5a9ff4ff8b Mon Sep 17 00:00:00 2001 From: Timo Furrer Date: Tue, 16 Jul 2024 16:32:18 +0200 Subject: [PATCH 1/4] Emit audit event for agent creation and deletion This change set emits two new audit events: one when a new cluster agent is created and one when a cluster agent is deleted or attempts of the two. Closes https://gitlab.com/gitlab-org/gitlab/-/issues/462749 Changelog: added Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/462749 EE: true MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159593 --- .../mutations/clusters/agents/create.rb | 2 +- .../mutations/clusters/agents/delete.rb | 4 +- .../clusters/agents/create_service.rb | 6 +- .../clusters/agents/delete_service.rb | 6 +- doc/user/compliance/audit_event_types.md | 2 + .../ee/clusters/agents/create_service.rb | 38 +++++++++++ .../ee/clusters/agents/delete_service.rb | 38 +++++++++++ .../types/cluster_agent_created.yml | 10 +++ .../types/cluster_agent_deleted.yml | 10 +++ .../agents/create_service_audit_log_spec.rb | 66 ++++++++++++++++++ .../agents/delete_service_audit_log_spec.rb | 68 +++++++++++++++++++ lib/api/clusters/agents.rb | 2 +- .../clusters/agents/create_service_spec.rb | 27 +++++--- .../clusters/agents/delete_service_spec.rb | 8 ++- 14 files changed, 266 insertions(+), 21 deletions(-) create mode 100644 ee/app/services/ee/clusters/agents/create_service.rb create mode 100644 ee/app/services/ee/clusters/agents/delete_service.rb create mode 100644 ee/config/audit_events/types/cluster_agent_created.yml create mode 100644 ee/config/audit_events/types/cluster_agent_deleted.yml create mode 100644 ee/spec/services/clusters/agents/create_service_audit_log_spec.rb create mode 100644 ee/spec/services/clusters/agents/delete_service_audit_log_spec.rb diff --git a/app/graphql/mutations/clusters/agents/create.rb b/app/graphql/mutations/clusters/agents/create.rb index 0f1dbcff555089..9fecf6d55f2242 100644 --- a/app/graphql/mutations/clusters/agents/create.rb +++ b/app/graphql/mutations/clusters/agents/create.rb @@ -25,7 +25,7 @@ class Create < BaseMutation def resolve(project_path:, name:) project = authorized_find!(project_path) - result = ::Clusters::Agents::CreateService.new(project, current_user).execute(name: name) + result = ::Clusters::Agents::CreateService.new(project, current_user, { name: name }).execute { cluster_agent: result[:cluster_agent], diff --git a/app/graphql/mutations/clusters/agents/delete.rb b/app/graphql/mutations/clusters/agents/delete.rb index c8df31e02b4ffa..fed96bc86fcbaf 100644 --- a/app/graphql/mutations/clusters/agents/delete.rb +++ b/app/graphql/mutations/clusters/agents/delete.rb @@ -17,8 +17,8 @@ class Delete < BaseMutation def resolve(id:) cluster_agent = authorized_find!(id: id) result = ::Clusters::Agents::DeleteService - .new(container: cluster_agent.project, current_user: current_user) - .execute(cluster_agent) + .new(container: cluster_agent.project, current_user: current_user, params: { cluster_agent: cluster_agent }) + .execute { errors: Array.wrap(result.message) diff --git a/app/services/clusters/agents/create_service.rb b/app/services/clusters/agents/create_service.rb index 568f168d63b94f..894df1eebba568 100644 --- a/app/services/clusters/agents/create_service.rb +++ b/app/services/clusters/agents/create_service.rb @@ -3,10 +3,10 @@ module Clusters module Agents class CreateService < BaseService - def execute(name:) + def execute return error_no_permissions unless cluster_agent_permissions? - agent = ::Clusters::Agent.new(name: name, project: project, created_by_user: current_user) + agent = ::Clusters::Agent.new(name: params[:name], project: project, created_by_user: current_user) if agent.save success.merge(cluster_agent: agent) @@ -27,3 +27,5 @@ def error_no_permissions end end end + +Clusters::Agents::CreateService.prepend_mod diff --git a/app/services/clusters/agents/delete_service.rb b/app/services/clusters/agents/delete_service.rb index 2132dffa6068cf..bf085455587735 100644 --- a/app/services/clusters/agents/delete_service.rb +++ b/app/services/clusters/agents/delete_service.rb @@ -3,7 +3,9 @@ module Clusters module Agents class DeleteService < ::BaseContainerService - def execute(cluster_agent) + def execute + cluster_agent = params[:cluster_agent] + return error_no_permissions unless current_user.can?(:admin_cluster, cluster_agent) if cluster_agent.destroy @@ -21,3 +23,5 @@ def error_no_permissions end end end + +Clusters::Agents::DeleteService.prepend_mod diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 11593e806a634e..81d8f918b0a7dd 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -234,6 +234,8 @@ Audit event types belong to the following product categories. | Name | Description | Saved to database | Streamed | Introduced in | Scope | |:------------|:------------|:------------------|:---------|:--------------|:--------------| +| [`cluster_agent_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159593) | Event triggered when a user creates a cluster agent| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.3](https://gitlab.com/gitlab-org/gitlab/-/issues/462749) | Project | +| [`cluster_agent_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159593) | Event triggered when a user deletes a cluster agent| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.3](https://gitlab.com/gitlab-org/gitlab/-/issues/462749) | Project | | [`cluster_agent_token_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036) | Event triggered when a user creates a cluster agent token| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.10](https://gitlab.com/gitlab-org/gitlab/-/issues/382133) | Project | | [`cluster_agent_token_revoked`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036) | Event triggered when a user revokes a cluster agent token| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.10](https://gitlab.com/gitlab-org/gitlab/-/issues/382133) | Project | diff --git a/ee/app/services/ee/clusters/agents/create_service.rb b/ee/app/services/ee/clusters/agents/create_service.rb new file mode 100644 index 00000000000000..2aca0c929b99f1 --- /dev/null +++ b/ee/app/services/ee/clusters/agents/create_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module EE + module Clusters + module Agents + module CreateService + def execute + super.tap do |response| + send_audit_event(response) + end + end + + private + + def send_audit_event(response) + if response[:status] == :success + message = "Created cluster agent '#{response[:cluster_agent].name}' with id #{response[:cluster_agent].id}" + target = response[:cluster_agent] + else + message = "Attempted to create cluster agent '#{params[:name]}' " \ + "but failed with message: #{response[:message]}" + target = project + end + + audit_context = { + name: 'cluster_agent_created', + author: current_user, + scope: project, + target: target, + message: message + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end + end +end diff --git a/ee/app/services/ee/clusters/agents/delete_service.rb b/ee/app/services/ee/clusters/agents/delete_service.rb new file mode 100644 index 00000000000000..400ec312476352 --- /dev/null +++ b/ee/app/services/ee/clusters/agents/delete_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module EE + module Clusters + module Agents + module DeleteService + def execute + super.tap do |response| + send_audit_event(response) + end + end + + private + + def send_audit_event(response) + cluster_agent = params[:cluster_agent] + + message = if response.success? + "Deleted cluster agent '#{cluster_agent.name}' with id #{cluster_agent.id}" + else + "Attempted to delete cluster agent '#{cluster_agent.name}' but " \ + "failed with message: #{response.message}" + end + + audit_context = { + name: 'cluster_agent_deleted', + author: current_user, + scope: project, + target: cluster_agent, + message: message + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end + end +end diff --git a/ee/config/audit_events/types/cluster_agent_created.yml b/ee/config/audit_events/types/cluster_agent_created.yml new file mode 100644 index 00000000000000..b2002194038f82 --- /dev/null +++ b/ee/config/audit_events/types/cluster_agent_created.yml @@ -0,0 +1,10 @@ +--- +name: cluster_agent_created +description: Event triggered when a user creates a cluster agent +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/462749 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159593 +feature_category: deployment_management +milestone: '17.3' +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/config/audit_events/types/cluster_agent_deleted.yml b/ee/config/audit_events/types/cluster_agent_deleted.yml new file mode 100644 index 00000000000000..7d78cd46da3971 --- /dev/null +++ b/ee/config/audit_events/types/cluster_agent_deleted.yml @@ -0,0 +1,10 @@ +--- +name: cluster_agent_deleted +description: Event triggered when a user deletes a cluster agent +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/462749 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159593 +feature_category: deployment_management +milestone: '17.3' +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/spec/services/clusters/agents/create_service_audit_log_spec.rb b/ee/spec/services/clusters/agents/create_service_audit_log_spec.rb new file mode 100644 index 00000000000000..799bde72be73d9 --- /dev/null +++ b/ee/spec/services/clusters/agents/create_service_audit_log_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Clusters::Agents::CreateService, feature_category: :deployment_management do + describe '#execute' do + let_it_be(:name) { 'some-agent' } + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + + context 'when user is authorized' do + before_all do + project.add_maintainer(user) + end + + context 'when user creates agent' do + it 'creates AuditEvent with success message' do + expect_to_audit( + user, + project, + an_object_having_attributes(class: Clusters::Agent, name: 'some-agent'), + /Created cluster agent 'some-agent' with id \d+/ + ) + + described_class.new(project, user, { name: name }).execute + end + end + end + + context 'when user is not authorized' do + let_it_be(:unauthorized_user) { create(:user) } + + before_all do + project.add_guest(unauthorized_user) + end + + context 'when user attempts to create agent' do + it 'creates audit logs with failure message' do + expect_to_audit( + unauthorized_user, + project, + project, + "Attempted to create cluster agent 'some-agent' but failed with message: " \ + 'You have insufficient permissions to create a cluster agent for this project' + ) + + described_class.new(project, unauthorized_user, { name: name }).execute + end + end + end + end + + def expect_to_audit(current_user, scope, target, message) + audit_context = { + name: 'cluster_agent_created', + author: current_user, + scope: scope, + target: target, + message: message + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) + .and_call_original + end +end diff --git a/ee/spec/services/clusters/agents/delete_service_audit_log_spec.rb b/ee/spec/services/clusters/agents/delete_service_audit_log_spec.rb new file mode 100644 index 00000000000000..f65947623d6348 --- /dev/null +++ b/ee/spec/services/clusters/agents/delete_service_audit_log_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Clusters::Agents::DeleteService, feature_category: :deployment_management do + describe '#execute' do + let_it_be(:agent) { create(:cluster_agent, name: 'some-agent') } + + let(:user) { agent.created_by_user } + let(:project) { agent.project } + + context 'when user is authorized' do + before do + project.add_maintainer(user) + end + + context 'when user deletes agent' do + it 'creates AuditEvent with success message' do + expect_to_audit( + user, + project, + agent, + /Deleted cluster agent 'some-agent' with id \d+/ + ) + + described_class.new(container: project, current_user: user, params: { cluster_agent: agent }).execute + end + end + end + + context 'when user is not authorized' do + let(:unauthorized_user) { create(:user) } + + before do + project.add_guest(unauthorized_user) + end + + context 'when user attempts to delete agent' do + it 'creates audit logs with failure message' do + expect_to_audit( + unauthorized_user, + project, + agent, + "Attempted to delete cluster agent 'some-agent' but failed with message: " \ + 'You have insufficient permissions to delete this cluster agent' + ) + + described_class + .new(container: project, current_user: unauthorized_user, params: { cluster_agent: agent }) + .execute + end + end + end + end + + def expect_to_audit(current_user, scope, target, message) + audit_context = { + name: 'cluster_agent_deleted', + author: current_user, + scope: scope, + target: target, + message: message + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) + .and_call_original + end +end diff --git a/lib/api/clusters/agents.rb b/lib/api/clusters/agents.rb index 02469fbad211d7..166bcf1802e70a 100644 --- a/lib/api/clusters/agents.rb +++ b/lib/api/clusters/agents.rb @@ -57,7 +57,7 @@ class Agents < ::API::Base params = declared_params(include_missing: false) - result = ::Clusters::Agents::CreateService.new(user_project, current_user).execute(name: params[:name]) + result = ::Clusters::Agents::CreateService.new(user_project, current_user, { name: params[:name] }).execute bad_request!(result[:message]) if result[:status] == :error diff --git a/spec/services/clusters/agents/create_service_spec.rb b/spec/services/clusters/agents/create_service_spec.rb index 85607fcdf3a28d..217fe3ce330fd5 100644 --- a/spec/services/clusters/agents/create_service_spec.rb +++ b/spec/services/clusters/agents/create_service_spec.rb @@ -3,15 +3,16 @@ require 'spec_helper' RSpec.describe Clusters::Agents::CreateService, feature_category: :deployment_management do - subject(:service) { described_class.new(project, user) } + subject(:service) { described_class.new(project, user, { name: name }) } + let(:name) { 'some-agent' } let(:project) { create(:project, :public, :repository) } let(:user) { create(:user) } describe '#execute' do context 'without user permissions' do it 'returns errors when user does not have permissions' do - expect(service.execute(name: 'missing-permissions')).to eq({ + expect(service.execute).to eq({ status: :error, message: 'You have insufficient permissions to create a cluster agent for this project' }) @@ -24,28 +25,32 @@ end it 'creates a new clusters_agent' do - expect { service.execute(name: 'with-user') }.to change { ::Clusters::Agent.count }.by(1) + expect { service.execute }.to change { ::Clusters::Agent.count }.by(1) end it 'returns success status', :aggregate_failures do - result = service.execute(name: 'success') + result = service.execute expect(result[:status]).to eq(:success) expect(result[:message]).to be_nil end it 'returns agent values', :aggregate_failures do - new_agent = service.execute(name: 'new-agent')[:cluster_agent] + new_agent = service.execute[:cluster_agent] - expect(new_agent.name).to eq('new-agent') + expect(new_agent.name).to eq(name) expect(new_agent.created_by_user).to eq(user) end - it 'generates an error message when name is invalid' do - expect(service.execute(name: '@bad_agent_name!')).to eq({ - status: :error, - message: ["Name can contain only lowercase letters, digits, and '-', but cannot start or end with '-'"] - }) + context 'with invalid name' do + let(:name) { '@bad_agent_name!' } + + it 'generates an error message' do + expect(service.execute).to eq({ + status: :error, + message: ["Name can contain only lowercase letters, digits, and '-', but cannot start or end with '-'"] + }) + end end end end diff --git a/spec/services/clusters/agents/delete_service_spec.rb b/spec/services/clusters/agents/delete_service_spec.rb index febbb7ba5c8607..36bc325064d06c 100644 --- a/spec/services/clusters/agents/delete_service_spec.rb +++ b/spec/services/clusters/agents/delete_service_spec.rb @@ -3,7 +3,9 @@ require 'spec_helper' RSpec.describe Clusters::Agents::DeleteService, feature_category: :deployment_management do - subject(:service) { described_class.new(container: project, current_user: user) } + subject(:service) do + described_class.new(container: project, current_user: user, params: { cluster_agnet: cluster_agent }) + end let(:cluster_agent) { create(:cluster_agent) } let(:project) { cluster_agent.project } @@ -12,7 +14,7 @@ describe '#execute' do context 'without user permissions' do it 'fails to delete when the user has no permissions', :aggregate_failures do - response = service.execute(cluster_agent) + response = service.execute expect(response.status).to eq(:error) expect(response.message).to eq('You have insufficient permissions to delete this cluster agent') @@ -27,7 +29,7 @@ end it 'deletes a cluster agent', :aggregate_failures do - expect { service.execute(cluster_agent) }.to change { ::Clusters::Agent.count }.by(-1) + expect { service.execute }.to change { ::Clusters::Agent.count }.by(-1) expect { cluster_agent.reload }.to raise_error(ActiveRecord::RecordNotFound) end end -- GitLab From 4c68ab8d366c025b619008d5aec72acecc898f24 Mon Sep 17 00:00:00 2001 From: Timo Furrer Date: Wed, 17 Jul 2024 20:03:59 +0200 Subject: [PATCH 2/4] Use cluster agents delete service in REST API --- lib/api/clusters/agents.rb | 6 +++++- spec/services/clusters/agents/delete_service_spec.rb | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/api/clusters/agents.rb b/lib/api/clusters/agents.rb index 166bcf1802e70a..aed4ad87fe681f 100644 --- a/lib/api/clusters/agents.rb +++ b/lib/api/clusters/agents.rb @@ -76,7 +76,11 @@ class Agents < ::API::Base agent = ::Clusters::AgentsFinder.new(user_project, current_user).find(params[:agent_id]) - destroy_conditionally!(agent) + destroy_conditionally!(agent) do |agent| + ::Clusters::Agents::DeleteService + .new(container: agent.project, current_user: current_user, params: { cluster_agent: agent }) + .execute + end end end end diff --git a/spec/services/clusters/agents/delete_service_spec.rb b/spec/services/clusters/agents/delete_service_spec.rb index 36bc325064d06c..6ed9fe67aef024 100644 --- a/spec/services/clusters/agents/delete_service_spec.rb +++ b/spec/services/clusters/agents/delete_service_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Clusters::Agents::DeleteService, feature_category: :deployment_management do subject(:service) do - described_class.new(container: project, current_user: user, params: { cluster_agnet: cluster_agent }) + described_class.new(container: project, current_user: user, params: { cluster_agent: cluster_agent }) end let(:cluster_agent) { create(:cluster_agent) } -- GitLab From 047593a3132f3eec43a5e56cee93986847067133 Mon Sep 17 00:00:00 2001 From: Timo Furrer Date: Thu, 18 Jul 2024 08:17:17 +0200 Subject: [PATCH 3/4] Refactor agents create service to use ServiceResponse --- .../clusters/agents/create_service.rb | 8 ++++--- .../ee/clusters/agents/create_service.rb | 2 +- .../clusters/agents/create_service_spec.rb | 24 ++++++++++--------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/app/services/clusters/agents/create_service.rb b/app/services/clusters/agents/create_service.rb index 894df1eebba568..052f4caae5ff98 100644 --- a/app/services/clusters/agents/create_service.rb +++ b/app/services/clusters/agents/create_service.rb @@ -9,9 +9,9 @@ def execute agent = ::Clusters::Agent.new(name: params[:name], project: project, created_by_user: current_user) if agent.save - success.merge(cluster_agent: agent) + ServiceResponse.new(status: :success, payload: { cluster_agent: agent }, reason: :created) else - error(agent.errors.full_messages) + ServiceResponse.error(message: agent.errors.full_messages) end end @@ -22,7 +22,9 @@ def cluster_agent_permissions? end def error_no_permissions - error(s_('ClusterAgent|You have insufficient permissions to create a cluster agent for this project')) + ServiceResponse.error( + message: s_('ClusterAgent|You have insufficient permissions to create a cluster agent for this project') + ) end end end diff --git a/ee/app/services/ee/clusters/agents/create_service.rb b/ee/app/services/ee/clusters/agents/create_service.rb index 2aca0c929b99f1..37a3a41f31e35e 100644 --- a/ee/app/services/ee/clusters/agents/create_service.rb +++ b/ee/app/services/ee/clusters/agents/create_service.rb @@ -13,7 +13,7 @@ def execute private def send_audit_event(response) - if response[:status] == :success + if response.success? message = "Created cluster agent '#{response[:cluster_agent].name}' with id #{response[:cluster_agent].id}" target = response[:cluster_agent] else diff --git a/spec/services/clusters/agents/create_service_spec.rb b/spec/services/clusters/agents/create_service_spec.rb index 217fe3ce330fd5..f07f7ef42fb939 100644 --- a/spec/services/clusters/agents/create_service_spec.rb +++ b/spec/services/clusters/agents/create_service_spec.rb @@ -12,10 +12,10 @@ describe '#execute' do context 'without user permissions' do it 'returns errors when user does not have permissions' do - expect(service.execute).to eq({ - status: :error, - message: 'You have insufficient permissions to create a cluster agent for this project' - }) + response = service.execute + + expect(response.status).to eq(:error) + expect(response.message).to eq('You have insufficient permissions to create a cluster agent for this project') end end @@ -29,10 +29,10 @@ end it 'returns success status', :aggregate_failures do - result = service.execute + response = service.execute - expect(result[:status]).to eq(:success) - expect(result[:message]).to be_nil + expect(response.status).to eq(:success) + expect(response.message).to be_nil end it 'returns agent values', :aggregate_failures do @@ -46,10 +46,12 @@ let(:name) { '@bad_agent_name!' } it 'generates an error message' do - expect(service.execute).to eq({ - status: :error, - message: ["Name can contain only lowercase letters, digits, and '-', but cannot start or end with '-'"] - }) + response = service.execute + + expect(response.status).to eq(:error) + expect(response.message).to eq( + ["Name can contain only lowercase letters, digits, and '-', but cannot start or end with '-'"] + ) end end end -- GitLab From 8015092685aad1e9d70a06c66e5177344d10e411 Mon Sep 17 00:00:00 2001 From: Timo Furrer Date: Thu, 18 Jul 2024 09:50:21 +0200 Subject: [PATCH 4/4] Refactor agent create and delete audit event types --- doc/user/compliance/audit_event_types.md | 2 ++ .../ee/clusters/agents/create_service.rb | 34 ++++++++++++------- .../ee/clusters/agents/delete_service.rb | 32 +++++++++++------ .../types/cluster_agent_create_failed.yml | 10 ++++++ .../types/cluster_agent_delete_failed.yml | 10 ++++++ .../agents/create_service_audit_log_spec.rb | 6 ++-- .../agents/delete_service_audit_log_spec.rb | 6 ++-- 7 files changed, 74 insertions(+), 26 deletions(-) create mode 100644 ee/config/audit_events/types/cluster_agent_create_failed.yml create mode 100644 ee/config/audit_events/types/cluster_agent_delete_failed.yml diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 81d8f918b0a7dd..2668478660fcd2 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -234,7 +234,9 @@ Audit event types belong to the following product categories. | Name | Description | Saved to database | Streamed | Introduced in | Scope | |:------------|:------------|:------------------|:---------|:--------------|:--------------| +| [`cluster_agent_create_failed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159593) | Event triggered when a user attempts to create a cluster agent but it failed| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.3](https://gitlab.com/gitlab-org/gitlab/-/issues/462749) | Project | | [`cluster_agent_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159593) | Event triggered when a user creates a cluster agent| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.3](https://gitlab.com/gitlab-org/gitlab/-/issues/462749) | Project | +| [`cluster_agent_delete_failed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159593) | Event triggered when a user attempts to delete a cluster agent but it failed| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.3](https://gitlab.com/gitlab-org/gitlab/-/issues/462749) | Project | | [`cluster_agent_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159593) | Event triggered when a user deletes a cluster agent| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.3](https://gitlab.com/gitlab-org/gitlab/-/issues/462749) | Project | | [`cluster_agent_token_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036) | Event triggered when a user creates a cluster agent token| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.10](https://gitlab.com/gitlab-org/gitlab/-/issues/382133) | Project | | [`cluster_agent_token_revoked`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036) | Event triggered when a user revokes a cluster agent token| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.10](https://gitlab.com/gitlab-org/gitlab/-/issues/382133) | Project | diff --git a/ee/app/services/ee/clusters/agents/create_service.rb b/ee/app/services/ee/clusters/agents/create_service.rb index 37a3a41f31e35e..a0c516e1d8e4b3 100644 --- a/ee/app/services/ee/clusters/agents/create_service.rb +++ b/ee/app/services/ee/clusters/agents/create_service.rb @@ -6,28 +6,38 @@ module Agents module CreateService def execute super.tap do |response| - send_audit_event(response) + if response.success? + send_success_audit_event(response) + else + send_failure_audit_event(response) + end end end private - def send_audit_event(response) - if response.success? - message = "Created cluster agent '#{response[:cluster_agent].name}' with id #{response[:cluster_agent].id}" - target = response[:cluster_agent] - else - message = "Attempted to create cluster agent '#{params[:name]}' " \ - "but failed with message: #{response[:message]}" - target = project - end + def send_success_audit_event(response) + cluster_agent = response[:cluster_agent] audit_context = { name: 'cluster_agent_created', author: current_user, scope: project, - target: target, - message: message + target: cluster_agent, + message: "Created cluster agent '#{cluster_agent.name}' with id #{cluster_agent.id}" + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + + def send_failure_audit_event(response) + audit_context = { + name: 'cluster_agent_create_failed', + author: current_user, + scope: project, + target: project, + message: "Attempted to create cluster agent '#{params[:name]}' " \ + "but failed with message: #{response[:message]}" } ::Gitlab::Audit::Auditor.audit(audit_context) diff --git a/ee/app/services/ee/clusters/agents/delete_service.rb b/ee/app/services/ee/clusters/agents/delete_service.rb index 400ec312476352..616811a8ee55b5 100644 --- a/ee/app/services/ee/clusters/agents/delete_service.rb +++ b/ee/app/services/ee/clusters/agents/delete_service.rb @@ -6,28 +6,40 @@ module Agents module DeleteService def execute super.tap do |response| - send_audit_event(response) + if response.success? + send_success_audit_event + else + send_failure_audit_event(response) + end end end private - def send_audit_event(response) + def send_success_audit_event cluster_agent = params[:cluster_agent] - message = if response.success? - "Deleted cluster agent '#{cluster_agent.name}' with id #{cluster_agent.id}" - else - "Attempted to delete cluster agent '#{cluster_agent.name}' but " \ - "failed with message: #{response.message}" - end - audit_context = { name: 'cluster_agent_deleted', author: current_user, scope: project, target: cluster_agent, - message: message + message: "Deleted cluster agent '#{cluster_agent.name}' with id #{cluster_agent.id}" + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + + def send_failure_audit_event(response) + cluster_agent = params[:cluster_agent] + + audit_context = { + name: 'cluster_agent_delete_failed', + author: current_user, + scope: project, + target: cluster_agent, + message: "Attempted to delete cluster agent '#{cluster_agent.name}' but " \ + "failed with message: #{response.message}" } ::Gitlab::Audit::Auditor.audit(audit_context) diff --git a/ee/config/audit_events/types/cluster_agent_create_failed.yml b/ee/config/audit_events/types/cluster_agent_create_failed.yml new file mode 100644 index 00000000000000..364d2aa91b6abb --- /dev/null +++ b/ee/config/audit_events/types/cluster_agent_create_failed.yml @@ -0,0 +1,10 @@ +--- +name: cluster_agent_create_failed +description: Event triggered when a user attempts to create a cluster agent but it failed +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/462749 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159593 +feature_category: deployment_management +milestone: '17.3' +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/config/audit_events/types/cluster_agent_delete_failed.yml b/ee/config/audit_events/types/cluster_agent_delete_failed.yml new file mode 100644 index 00000000000000..165ef590ed2979 --- /dev/null +++ b/ee/config/audit_events/types/cluster_agent_delete_failed.yml @@ -0,0 +1,10 @@ +--- +name: cluster_agent_delete_failed +description: Event triggered when a user attempts to delete a cluster agent but it failed +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/462749 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159593 +feature_category: deployment_management +milestone: '17.3' +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/spec/services/clusters/agents/create_service_audit_log_spec.rb b/ee/spec/services/clusters/agents/create_service_audit_log_spec.rb index 799bde72be73d9..5f6ae28d641c8b 100644 --- a/ee/spec/services/clusters/agents/create_service_audit_log_spec.rb +++ b/ee/spec/services/clusters/agents/create_service_audit_log_spec.rb @@ -17,6 +17,7 @@ context 'when user creates agent' do it 'creates AuditEvent with success message' do expect_to_audit( + 'cluster_agent_created', user, project, an_object_having_attributes(class: Clusters::Agent, name: 'some-agent'), @@ -38,6 +39,7 @@ context 'when user attempts to create agent' do it 'creates audit logs with failure message' do expect_to_audit( + 'cluster_agent_create_failed', unauthorized_user, project, project, @@ -51,9 +53,9 @@ end end - def expect_to_audit(current_user, scope, target, message) + def expect_to_audit(event_type, current_user, scope, target, message) audit_context = { - name: 'cluster_agent_created', + name: event_type, author: current_user, scope: scope, target: target, diff --git a/ee/spec/services/clusters/agents/delete_service_audit_log_spec.rb b/ee/spec/services/clusters/agents/delete_service_audit_log_spec.rb index f65947623d6348..9021ac261c3406 100644 --- a/ee/spec/services/clusters/agents/delete_service_audit_log_spec.rb +++ b/ee/spec/services/clusters/agents/delete_service_audit_log_spec.rb @@ -17,6 +17,7 @@ context 'when user deletes agent' do it 'creates AuditEvent with success message' do expect_to_audit( + 'cluster_agent_deleted', user, project, agent, @@ -38,6 +39,7 @@ context 'when user attempts to delete agent' do it 'creates audit logs with failure message' do expect_to_audit( + 'cluster_agent_delete_failed', unauthorized_user, project, agent, @@ -53,9 +55,9 @@ end end - def expect_to_audit(current_user, scope, target, message) + def expect_to_audit(event_type, current_user, scope, target, message) audit_context = { - name: 'cluster_agent_deleted', + name: event_type, author: current_user, scope: scope, target: target, -- GitLab