diff --git a/app/graphql/mutations/clusters/agents/create.rb b/app/graphql/mutations/clusters/agents/create.rb index 0f1dbcff5550893360e1e13ce36727b44c3a0709..9fecf6d55f22422267e9d562041f854d78857bee 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 c8df31e02b4ffab5ae2908a22aadbc2f32ef460a..fed96bc86fcbafab6f1ef0d15ee07bff45e81fff 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 568f168d63b94fe5e0519fa04f0361188e6d9995..052f4caae5ff98020840bd91baa16ed195abbdc4 100644 --- a/app/services/clusters/agents/create_service.rb +++ b/app/services/clusters/agents/create_service.rb @@ -3,15 +3,15 @@ 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) + 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,8 +22,12 @@ 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 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 2132dffa6068cf0cbbb8a7c03e2ca8a48fcb3f92..bf08545558773521140bc2be608ea556c2be22c4 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 11593e806a634e70102829ed06c1a4c27c53888a..2668478660fcd2ba69cd7910680c0755ee3f0bdf 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -234,6 +234,10 @@ 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 new file mode 100644 index 0000000000000000000000000000000000000000..a0c516e1d8e4b31ccd47632740bfe80f93a670e6 --- /dev/null +++ b/ee/app/services/ee/clusters/agents/create_service.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module EE + module Clusters + module Agents + module CreateService + def execute + super.tap do |response| + if response.success? + send_success_audit_event(response) + else + send_failure_audit_event(response) + end + end + end + + private + + def send_success_audit_event(response) + cluster_agent = response[:cluster_agent] + + audit_context = { + name: 'cluster_agent_created', + author: current_user, + scope: project, + 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) + 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 0000000000000000000000000000000000000000..616811a8ee55b5eb79fc2bc20280e17d7ec1b65e --- /dev/null +++ b/ee/app/services/ee/clusters/agents/delete_service.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module EE + module Clusters + module Agents + module DeleteService + def execute + super.tap do |response| + if response.success? + send_success_audit_event + else + send_failure_audit_event(response) + end + end + end + + private + + def send_success_audit_event + cluster_agent = params[:cluster_agent] + + audit_context = { + name: 'cluster_agent_deleted', + author: current_user, + scope: project, + target: cluster_agent, + 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) + end + end + end + end +end 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 0000000000000000000000000000000000000000..364d2aa91b6abbce5c374b66805f57360960aefd --- /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_created.yml b/ee/config/audit_events/types/cluster_agent_created.yml new file mode 100644 index 0000000000000000000000000000000000000000..b2002194038f823ac305e13a38eff65a0383730d --- /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_delete_failed.yml b/ee/config/audit_events/types/cluster_agent_delete_failed.yml new file mode 100644 index 0000000000000000000000000000000000000000..165ef590ed29799d8c5285bb37b161f4bb109f1c --- /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/config/audit_events/types/cluster_agent_deleted.yml b/ee/config/audit_events/types/cluster_agent_deleted.yml new file mode 100644 index 0000000000000000000000000000000000000000..7d78cd46da3971c74f5c27eacf8bfe9247733c51 --- /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 0000000000000000000000000000000000000000..5f6ae28d641c8bdd71adf1609a8223f535c55d4c --- /dev/null +++ b/ee/spec/services/clusters/agents/create_service_audit_log_spec.rb @@ -0,0 +1,68 @@ +# 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( + 'cluster_agent_created', + 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( + 'cluster_agent_create_failed', + 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(event_type, current_user, scope, target, message) + audit_context = { + name: event_type, + 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 0000000000000000000000000000000000000000..9021ac261c3406b58b7a2627da50d3b2436988e1 --- /dev/null +++ b/ee/spec/services/clusters/agents/delete_service_audit_log_spec.rb @@ -0,0 +1,70 @@ +# 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( + 'cluster_agent_deleted', + 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( + 'cluster_agent_delete_failed', + 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(event_type, current_user, scope, target, message) + audit_context = { + name: event_type, + 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 02469fbad211d77a41ed570b8d5bb1cbfdfdf7f6..aed4ad87fe681f1a9b940877c8118c69026b0322 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 @@ -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/create_service_spec.rb b/spec/services/clusters/agents/create_service_spec.rb index 85607fcdf3a28d27ee68eef6c40999e8c86cccc5..f07f7ef42fb93937d4fc043ed8d8ca5e4d79e45c 100644 --- a/spec/services/clusters/agents/create_service_spec.rb +++ b/spec/services/clusters/agents/create_service_spec.rb @@ -3,18 +3,19 @@ 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({ - 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 @@ -24,28 +25,34 @@ 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') + 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 - 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 + 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 end diff --git a/spec/services/clusters/agents/delete_service_spec.rb b/spec/services/clusters/agents/delete_service_spec.rb index febbb7ba5c86078461b60cc91fe862197be84394..6ed9fe67aef02436c92c5b793325f0322736232e 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_agent: 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