From 80a9c140b9425738bc94c98a7bf4a68fe4ff3817 Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Tue, 12 Jul 2022 15:01:08 +0530 Subject: [PATCH 1/3] Event type information in deploy key audit event EE: true Changelog: added --- app/models/deploy_key.rb | 4 + .../services/ee/deploy_keys/create_service.rb | 16 +++- .../ee/projects/disable_deploy_key_service.rb | 16 +++- .../ee/projects/enable_deploy_key_service.rb | 16 +++- .../projects/deploy_keys_controller_spec.rb | 78 ++++++++++++++++--- spec/models/deploy_key_spec.rb | 6 ++ 6 files changed, 113 insertions(+), 23 deletions(-) diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb index 4ed38f578ee3d7..f9acd398374a99 100644 --- a/app/models/deploy_key.rb +++ b/app/models/deploy_key.rb @@ -40,6 +40,10 @@ def user super || User.ghost end + def audit_details + title + end + def has_access_to?(project) deploy_keys_project_for(project).present? end diff --git a/ee/app/services/ee/deploy_keys/create_service.rb b/ee/app/services/ee/deploy_keys/create_service.rb index 353791322d5034..8354218bf69caf 100644 --- a/ee/app/services/ee/deploy_keys/create_service.rb +++ b/ee/app/services/ee/deploy_keys/create_service.rb @@ -9,16 +9,24 @@ module CreateService def execute(project: nil) super.tap do |key| if project && key.persisted? - log_audit_event(key.title, project, action: :create) + log_audit_event(key, project) end end end private - def log_audit_event(key_title, project, options = {}) - ::AuditEventService.new(user, project, options) - .for_deploy_key(key_title).security_event + def log_audit_event(key, project) + audit_context = { + name: 'deploy_key_added', + author: user, + scope: project, + target: key, + message: "Added deploy key", + additional_details: { add: "deploy_key" } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) end end end diff --git a/ee/app/services/ee/projects/disable_deploy_key_service.rb b/ee/app/services/ee/projects/disable_deploy_key_service.rb index deab70c26e7a6e..6f284d97b72e7c 100644 --- a/ee/app/services/ee/projects/disable_deploy_key_service.rb +++ b/ee/app/services/ee/projects/disable_deploy_key_service.rb @@ -9,14 +9,22 @@ def execute super.tap do |deploy_key_project| break unless deploy_key_project - log_audit_event(deploy_key_project.deploy_key.title, action: :destroy) + log_audit_event(deploy_key_project.deploy_key) end end private - def log_audit_event(key_title, options = {}) - AuditEventService.new(current_user, project, options) - .for_deploy_key(key_title).security_event + def log_audit_event(key) + audit_context = { + name: 'deploy_key_removed', + author: current_user, + scope: project, + target: key, + message: "Removed deploy key", + additional_details: { remove: "deploy_key" } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) end end diff --git a/ee/app/services/ee/projects/enable_deploy_key_service.rb b/ee/app/services/ee/projects/enable_deploy_key_service.rb index 2e966b121304fb..284c8e67b76fa1 100644 --- a/ee/app/services/ee/projects/enable_deploy_key_service.rb +++ b/ee/app/services/ee/projects/enable_deploy_key_service.rb @@ -9,14 +9,22 @@ def execute super.tap do |key| break unless key - log_audit_event(key.title, action: :create) + log_audit_event(key) end end private - def log_audit_event(key_title, options = {}) - AuditEventService.new(current_user, project, options) - .for_deploy_key(key_title).security_event + def log_audit_event(key) + audit_context = { + name: 'deploy_key_added', + author: current_user, + scope: project, + target: key, + message: "Added deploy key", + additional_details: { add: "deploy_key" } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) end end diff --git a/ee/spec/controllers/projects/deploy_keys_controller_spec.rb b/ee/spec/controllers/projects/deploy_keys_controller_spec.rb index f2bc03e08bf2dc..6b8bb6d41ae8de 100644 --- a/ee/spec/controllers/projects/deploy_keys_controller_spec.rb +++ b/ee/spec/controllers/projects/deploy_keys_controller_spec.rb @@ -3,13 +3,17 @@ require 'spec_helper' RSpec.describe Projects::DeployKeysController do - let(:project) { create(:project, :repository) } + let(:group) { create(:group) } + let!(:destination) { create(:external_audit_event_destination, group: group) } + let(:project) { create(:project, :repository, group: group) } let(:user) { create(:user) } before do project.add_maintainer(user) sign_in(user) + stub_licensed_features(audit_events: true) + stub_licensed_features(external_audit_events: true) end describe 'POST create' do @@ -27,16 +31,36 @@ } end - it 'records an audit event' do - expect { post :create, params: params }.to change { AuditEvent.count }.by(1) + let(:subject) { post :create, params: params } + + it 'records an audit event', :aggregate_failures do + expect { subject }.to change { AuditEvent.count }.by(1) + audit_event = AuditEvent.last + + expect(audit_event.author_id).to eq(user.id) + expect(audit_event.entity_id).to eq(project.id) + expect(audit_event.entity_type).to eq(project.class.name) + expect(audit_event.details).to include({ + add: "deploy_key", + author_name: user.name, + custom_message: "Added deploy key", + target_details: title, + target_type: "DeployKey" + }) expect(response).to redirect_to(project_settings_repository_path(project, anchor: 'js-deploy-keys-settings')) end + + it_behaves_like 'sends correct event type in audit event stream' do + let_it_be(:event_type) { "deploy_key_added" } + end end describe '/enable/:id' do let(:deploy_key) { create(:deploy_key) } - let(:project2) { create(:project) } + let(:project2) { create(:project, group: group) } + let(:subject) {put :enable, params: { id: deploy_key.id, namespace_id: project.namespace, project_id: project }} + let!(:deploy_keys_project_internal) do create(:deploy_keys_project, project: project2, deploy_key: deploy_key) end @@ -47,9 +71,24 @@ end it 'records an audit event' do - expect do - put :enable, params: { id: deploy_key.id, namespace_id: project.namespace, project_id: project } - end.to change { AuditEvent.count }.by(1) + expect { subject }.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.last + + expect(audit_event.author_id).to eq(user.id) + expect(audit_event.entity_id).to eq(project.id) + expect(audit_event.entity_type).to eq(project.class.name) + expect(audit_event.details).to include({ + add: "deploy_key", + author_name: user.name, + custom_message: "Added deploy key", + target_details: deploy_key.title, + target_type: "DeployKey" + }) + end + + it_behaves_like 'sends correct event type in audit event stream' do + let_it_be(:event_type) { "deploy_key_added" } end it 'returns 404' do @@ -61,18 +100,35 @@ end describe '/disable/:id' do + let(:admin) {create(:admin)} let(:deploy_key) { create(:deploy_key) } let!(:deploy_key_project) { create(:deploy_keys_project, project: project, deploy_key: deploy_key) } + let(:subject) { put :disable, params: { id: deploy_key.id, namespace_id: project.namespace, project_id: project }} context 'with admin', :enable_admin_mode do before do - sign_in(create(:admin)) + sign_in(admin) end it 'records an audit event' do - expect do - put :disable, params: { id: deploy_key.id, namespace_id: project.namespace, project_id: project } - end.to change { AuditEvent.count }.by(1) + expect { subject }.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.last + + expect(audit_event.author_id).to eq(admin.id) + expect(audit_event.entity_id).to eq(project.id) + expect(audit_event.entity_type).to eq(project.class.name) + expect(audit_event.details).to include({ + remove: "deploy_key", + author_name: admin.name, + custom_message: "Removed deploy key", + target_details: deploy_key.title, + target_type: "DeployKey" + }) + end + + it_behaves_like 'sends correct event type in audit event stream' do + let_it_be(:event_type) { "deploy_key_removed" } end end end diff --git a/spec/models/deploy_key_spec.rb b/spec/models/deploy_key_spec.rb index c22bad0e062441..8c3b02427ae992 100644 --- a/spec/models/deploy_key_spec.rb +++ b/spec/models/deploy_key_spec.rb @@ -146,4 +146,10 @@ end end end + + describe '#audit_details' do + it "equals to the key's title" do + expect(subject.audit_details).to eq(subject.title) + end + end end -- GitLab From 955808e820fc47b43175ed400d0f513328dd5488 Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Mon, 18 Jul 2022 14:50:42 +0530 Subject: [PATCH 2/3] Specs for auditing services --- .../projects/deploy_keys_controller_spec.rb | 78 +++---------------- .../deploy_keys/create_service_spec.rb | 42 ++++++++++ .../disable_deploy_key_service_spec.rb | 43 ++++++++++ .../enable_deploy_key_service_spec.rb | 42 ++++++++++ 4 files changed, 138 insertions(+), 67 deletions(-) create mode 100644 ee/spec/services/deploy_keys/create_service_spec.rb create mode 100644 ee/spec/services/projects/disable_deploy_key_service_spec.rb create mode 100644 ee/spec/services/projects/enable_deploy_key_service_spec.rb diff --git a/ee/spec/controllers/projects/deploy_keys_controller_spec.rb b/ee/spec/controllers/projects/deploy_keys_controller_spec.rb index 6b8bb6d41ae8de..f2bc03e08bf2dc 100644 --- a/ee/spec/controllers/projects/deploy_keys_controller_spec.rb +++ b/ee/spec/controllers/projects/deploy_keys_controller_spec.rb @@ -3,17 +3,13 @@ require 'spec_helper' RSpec.describe Projects::DeployKeysController do - let(:group) { create(:group) } - let!(:destination) { create(:external_audit_event_destination, group: group) } - let(:project) { create(:project, :repository, group: group) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } before do project.add_maintainer(user) sign_in(user) - stub_licensed_features(audit_events: true) - stub_licensed_features(external_audit_events: true) end describe 'POST create' do @@ -31,36 +27,16 @@ } end - let(:subject) { post :create, params: params } - - it 'records an audit event', :aggregate_failures do - expect { subject }.to change { AuditEvent.count }.by(1) - audit_event = AuditEvent.last - - expect(audit_event.author_id).to eq(user.id) - expect(audit_event.entity_id).to eq(project.id) - expect(audit_event.entity_type).to eq(project.class.name) - expect(audit_event.details).to include({ - add: "deploy_key", - author_name: user.name, - custom_message: "Added deploy key", - target_details: title, - target_type: "DeployKey" - }) + it 'records an audit event' do + expect { post :create, params: params }.to change { AuditEvent.count }.by(1) expect(response).to redirect_to(project_settings_repository_path(project, anchor: 'js-deploy-keys-settings')) end - - it_behaves_like 'sends correct event type in audit event stream' do - let_it_be(:event_type) { "deploy_key_added" } - end end describe '/enable/:id' do let(:deploy_key) { create(:deploy_key) } - let(:project2) { create(:project, group: group) } - let(:subject) {put :enable, params: { id: deploy_key.id, namespace_id: project.namespace, project_id: project }} - + let(:project2) { create(:project) } let!(:deploy_keys_project_internal) do create(:deploy_keys_project, project: project2, deploy_key: deploy_key) end @@ -71,24 +47,9 @@ end it 'records an audit event' do - expect { subject }.to change { AuditEvent.count }.by(1) - - audit_event = AuditEvent.last - - expect(audit_event.author_id).to eq(user.id) - expect(audit_event.entity_id).to eq(project.id) - expect(audit_event.entity_type).to eq(project.class.name) - expect(audit_event.details).to include({ - add: "deploy_key", - author_name: user.name, - custom_message: "Added deploy key", - target_details: deploy_key.title, - target_type: "DeployKey" - }) - end - - it_behaves_like 'sends correct event type in audit event stream' do - let_it_be(:event_type) { "deploy_key_added" } + expect do + put :enable, params: { id: deploy_key.id, namespace_id: project.namespace, project_id: project } + end.to change { AuditEvent.count }.by(1) end it 'returns 404' do @@ -100,35 +61,18 @@ end describe '/disable/:id' do - let(:admin) {create(:admin)} let(:deploy_key) { create(:deploy_key) } let!(:deploy_key_project) { create(:deploy_keys_project, project: project, deploy_key: deploy_key) } - let(:subject) { put :disable, params: { id: deploy_key.id, namespace_id: project.namespace, project_id: project }} context 'with admin', :enable_admin_mode do before do - sign_in(admin) + sign_in(create(:admin)) end it 'records an audit event' do - expect { subject }.to change { AuditEvent.count }.by(1) - - audit_event = AuditEvent.last - - expect(audit_event.author_id).to eq(admin.id) - expect(audit_event.entity_id).to eq(project.id) - expect(audit_event.entity_type).to eq(project.class.name) - expect(audit_event.details).to include({ - remove: "deploy_key", - author_name: admin.name, - custom_message: "Removed deploy key", - target_details: deploy_key.title, - target_type: "DeployKey" - }) - end - - it_behaves_like 'sends correct event type in audit event stream' do - let_it_be(:event_type) { "deploy_key_removed" } + expect do + put :disable, params: { id: deploy_key.id, namespace_id: project.namespace, project_id: project } + end.to change { AuditEvent.count }.by(1) end end end diff --git a/ee/spec/services/deploy_keys/create_service_spec.rb b/ee/spec/services/deploy_keys/create_service_spec.rb new file mode 100644 index 00000000000000..ada93491874f9c --- /dev/null +++ b/ee/spec/services/deploy_keys/create_service_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe DeployKeys::CreateService do + let_it_be(:group) { create(:group) } + let_it_be(:destination) { create(:external_audit_event_destination, group: group) } + let_it_be(:project) { create(:project, :repository, group: group) } + let_it_be(:user) { create(:user) } + let_it_be(:params) { attributes_for(:deploy_key) } + + subject { described_class.new(user, params).execute(project: project) } + + before do + stub_licensed_features(audit_events: true) + stub_licensed_features(external_audit_events: true) + end + + it "creates a deploy key" do + expect { subject }.to change { DeployKey.where(params.merge(user: user)).count }.by(1) + end + + it 'records an audit event', :aggregate_failures do + expect { subject }.to change { AuditEvent.count }.by(1) + audit_event = AuditEvent.last + + expect(audit_event.author_id).to eq(user.id) + expect(audit_event.entity_id).to eq(project.id) + expect(audit_event.entity_type).to eq(project.class.name) + expect(audit_event.details).to include({ + add: "deploy_key", + author_name: user.name, + custom_message: "Added deploy key", + target_details: params[:title], + target_type: "DeployKey" + }) + end + + it_behaves_like 'sends correct event type in audit event stream' do + let_it_be(:event_type) { "deploy_key_added" } + end +end diff --git a/ee/spec/services/projects/disable_deploy_key_service_spec.rb b/ee/spec/services/projects/disable_deploy_key_service_spec.rb new file mode 100644 index 00000000000000..d2df9e0eedaf78 --- /dev/null +++ b/ee/spec/services/projects/disable_deploy_key_service_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::DisableDeployKeyService do + let_it_be(:group) { create(:group) } + let_it_be(:destination) { create(:external_audit_event_destination, group: group) } + let_it_be(:deploy_key) { create(:deploy_key) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:deploy_key_project) { create(:deploy_keys_project, project: project, deploy_key: deploy_key) } + let_it_be(:user) { project.creator } + let_it_be(:params) { { id: deploy_key.id } } + + let_it_be(:service) { described_class.new(project, user, params) } + + before do + stub_licensed_features(audit_events: true) + stub_licensed_features(external_audit_events: true) + end + + it 'records an audit event' do + expect { service.execute }.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.last + + expect(audit_event.author_id).to eq(user.id) + expect(audit_event.entity_id).to eq(project.id) + expect(audit_event.entity_type).to eq(project.class.name) + expect(audit_event.details).to include({ + remove: "deploy_key", + author_name: user.name, + custom_message: "Removed deploy key", + target_details: deploy_key.title, + target_type: "DeployKey" + }) + end + + it_behaves_like 'sends correct event type in audit event stream' do + let(:subject) { service.execute } + + let_it_be(:event_type) { "deploy_key_removed" } + end +end diff --git a/ee/spec/services/projects/enable_deploy_key_service_spec.rb b/ee/spec/services/projects/enable_deploy_key_service_spec.rb new file mode 100644 index 00000000000000..fb8edfc3ad4e7f --- /dev/null +++ b/ee/spec/services/projects/enable_deploy_key_service_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::EnableDeployKeyService do + let_it_be(:group) { create(:group) } + let_it_be(:destination) { create(:external_audit_event_destination, group: group) } + let_it_be(:deploy_key) { create(:deploy_key, public: true) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:user) { project.creator } + let_it_be(:params) { { key_id: deploy_key.id } } + + let_it_be(:service) { described_class.new(project, user, params) } + + before do + stub_licensed_features(audit_events: true) + stub_licensed_features(external_audit_events: true) + end + + it 'records an audit event' do + expect { service.execute }.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.last + + expect(audit_event.author_id).to eq(user.id) + expect(audit_event.entity_id).to eq(project.id) + expect(audit_event.entity_type).to eq(project.class.name) + expect(audit_event.details).to include({ + add: "deploy_key", + author_name: user.name, + custom_message: "Added deploy key", + target_details: deploy_key.title, + target_type: "DeployKey" + }) + end + + it_behaves_like 'sends correct event type in audit event stream' do + let(:subject) { service.execute } + + let_it_be(:event_type) { "deploy_key_added" } + end +end -- GitLab From ee5da1a1b7fedf7f4b4ed20b47cabf1834bcf8c6 Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Tue, 19 Jul 2022 23:05:36 +0530 Subject: [PATCH 3/3] Make stubs single line --- ee/spec/services/deploy_keys/create_service_spec.rb | 3 +-- ee/spec/services/projects/disable_deploy_key_service_spec.rb | 3 +-- ee/spec/services/projects/enable_deploy_key_service_spec.rb | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/ee/spec/services/deploy_keys/create_service_spec.rb b/ee/spec/services/deploy_keys/create_service_spec.rb index ada93491874f9c..48c2d365909592 100644 --- a/ee/spec/services/deploy_keys/create_service_spec.rb +++ b/ee/spec/services/deploy_keys/create_service_spec.rb @@ -12,8 +12,7 @@ subject { described_class.new(user, params).execute(project: project) } before do - stub_licensed_features(audit_events: true) - stub_licensed_features(external_audit_events: true) + stub_licensed_features(audit_events: true, external_audit_events: true) end it "creates a deploy key" do diff --git a/ee/spec/services/projects/disable_deploy_key_service_spec.rb b/ee/spec/services/projects/disable_deploy_key_service_spec.rb index d2df9e0eedaf78..cb322f352f4675 100644 --- a/ee/spec/services/projects/disable_deploy_key_service_spec.rb +++ b/ee/spec/services/projects/disable_deploy_key_service_spec.rb @@ -14,8 +14,7 @@ let_it_be(:service) { described_class.new(project, user, params) } before do - stub_licensed_features(audit_events: true) - stub_licensed_features(external_audit_events: true) + stub_licensed_features(audit_events: true, external_audit_events: true) end it 'records an audit event' do diff --git a/ee/spec/services/projects/enable_deploy_key_service_spec.rb b/ee/spec/services/projects/enable_deploy_key_service_spec.rb index fb8edfc3ad4e7f..a2f42a23321c3e 100644 --- a/ee/spec/services/projects/enable_deploy_key_service_spec.rb +++ b/ee/spec/services/projects/enable_deploy_key_service_spec.rb @@ -13,8 +13,7 @@ let_it_be(:service) { described_class.new(project, user, params) } before do - stub_licensed_features(audit_events: true) - stub_licensed_features(external_audit_events: true) + stub_licensed_features(audit_events: true, external_audit_events: true) end it 'records an audit event' do -- GitLab