From 9cb0c2807be0b5c20206eba35d0c1d9da0f1963f Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Tue, 7 Jun 2022 14:02:08 +0530 Subject: [PATCH 1/3] Event type information in streaming audit event for deploy token EE: true Changelog: added --- .../ee/projects/deploy_tokens_controller.rb | 18 +++++------- .../projects/deploy_tokens/create_service.rb | 29 ++++++++++--------- .../projects/deploy_tokens/destroy_service.rb | 18 +++++------- .../projects/deploy_tokens_controller_spec.rb | 12 +++++++- .../deploy_tokens/create_service_spec.rb | 16 +++++++++- .../deploy_tokens/destroy_service_spec.rb | 12 +++++++- ...audit_event_type_stream_shared_examples.rb | 11 +++++++ 7 files changed, 79 insertions(+), 37 deletions(-) create mode 100644 ee/spec/support/shared_examples/audit/audit_event_type_stream_shared_examples.rb diff --git a/ee/app/controllers/ee/projects/deploy_tokens_controller.rb b/ee/app/controllers/ee/projects/deploy_tokens_controller.rb index 8d1c80d793b172..fd90148f8c1938 100644 --- a/ee/app/controllers/ee/projects/deploy_tokens_controller.rb +++ b/ee/app/controllers/ee/projects/deploy_tokens_controller.rb @@ -17,16 +17,14 @@ def revoke def log_audit_event # rubocop:disable Gitlab/ModuleWithInstanceVariables message = "Revoked project deploy token with name: #{@token.name} with token_id: #{@token.id} with scopes: #{@token.scopes}." - - ::AuditEventService.new( - current_user, - @project, - target_id: @token.id, - target_type: @token.class.name, - target_details: @token.name, - action: :custom, - custom_message: message - ).security_event + audit_context = { + name: 'revoked_deploy_token', + author: current_user, + scope: @project, + target: @token, + message: message + } + ::Gitlab::Audit::Auditor.audit(audit_context) # rubocop:enable Gitlab/ModuleWithInstanceVariables end end diff --git a/ee/app/services/ee/projects/deploy_tokens/create_service.rb b/ee/app/services/ee/projects/deploy_tokens/create_service.rb index 1dd762e9f13b0d..ce9dba9847e648 100644 --- a/ee/app/services/ee/projects/deploy_tokens/create_service.rb +++ b/ee/app/services/ee/projects/deploy_tokens/create_service.rb @@ -16,21 +16,22 @@ def execute private def audit_event_service(deploy_token, result) - message = if result[:status] == :success - "Created project deploy token with name: #{deploy_token.name} with token_id: #{deploy_token.id} with scopes: #{deploy_token.scopes}." - else - "Attempted to create project deploy token but failed with message: #{result[:message]}" - end + if result[:status] == :success + message = "Created project deploy token with name: #{deploy_token.name} with token_id: #{deploy_token.id} with scopes: #{deploy_token.scopes}." + name = "created_deploy_token" + else + message = "Attempted to create project deploy token but failed with message: #{result[:message]}" + name = "attempted_to_create_deploy_token" + end - ::AuditEventService.new( - current_user, - project, - target_id: deploy_token.id, - target_type: deploy_token.class.name, - target_details: deploy_token.name, - action: :custom, - custom_message: message - ).security_event + audit_context = { + name: name, + author: current_user, + scope: project, + target: deploy_token, + message: message + } + ::Gitlab::Audit::Auditor.audit(audit_context) end end end diff --git a/ee/app/services/ee/projects/deploy_tokens/destroy_service.rb b/ee/app/services/ee/projects/deploy_tokens/destroy_service.rb index a7d57980d1a06d..b74b0fc146a871 100644 --- a/ee/app/services/ee/projects/deploy_tokens/destroy_service.rb +++ b/ee/app/services/ee/projects/deploy_tokens/destroy_service.rb @@ -17,16 +17,14 @@ def execute def audit_event_service(deploy_token) message = "Destroyed project deploy token with name: #{deploy_token.name} with token_id: #{deploy_token.id} with scopes: #{deploy_token.scopes}." - - ::AuditEventService.new( - current_user, - project, - target_id: deploy_token.id, - target_type: deploy_token.class.name, - target_details: deploy_token.name, - action: :custom, - custom_message: message - ).security_event + audit_context = { + name: 'destroyed_deploy_token', + author: current_user, + scope: project, + target: deploy_token, + message: message + } + ::Gitlab::Audit::Auditor.audit(audit_context) end end end diff --git a/ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb b/ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb index 10431dd843188d..501ed2c30889c0 100644 --- a/ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb +++ b/ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe Projects::DeployTokensController do - let_it_be(:project) { create(:project) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } let_it_be(:user) { create(:user) } let_it_be(:deploy_token) { create(:deploy_token, projects: [project]) } let_it_be(:params) do @@ -33,5 +34,14 @@ expect(AuditEvent.last.details[:custom_message]).to eq(expected_message) end + + before do + stub_licensed_features(external_audit_events: true) + group.external_audit_event_destinations.create!(destination_url: 'http://example.com') + end + + it_behaves_like 'sends correct event type in audit event stream' do + let_it_be(:event_type) { "revoked_deploy_token" } + end end end diff --git a/ee/spec/services/ee/projects/deploy_tokens/create_service_spec.rb b/ee/spec/services/ee/projects/deploy_tokens/create_service_spec.rb index 8b0392947d4335..7c1116cf0e642c 100644 --- a/ee/spec/services/ee/projects/deploy_tokens/create_service_spec.rb +++ b/ee/spec/services/ee/projects/deploy_tokens/create_service_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe Projects::DeployTokens::CreateService do - let_it_be(:entity) { create(:project) } + let_it_be(:group) { create :group } + let_it_be(:entity) { create(:project, group: group) } let_it_be(:user) { create(:user) } let(:deploy_token_params) { attributes_for(:deploy_token) } @@ -11,6 +12,11 @@ describe '#execute' do subject { described_class.new(entity, user, deploy_token_params).execute } + before do + stub_licensed_features(external_audit_events: true) + group.external_audit_event_destinations.create!(destination_url: 'http://example.com') + end + context 'when the deploy token is valid' do it 'creates an audit event' do expect { subject }.to change { AuditEvent.count }.by(1) @@ -22,6 +28,10 @@ expect(AuditEvent.last.details[:custom_message]).to eq(expected_message) end + + it_behaves_like 'sends correct event type in audit event stream' do + let_it_be(:event_type) { "created_deploy_token" } + end end context 'when the deploy token is invalid' do @@ -34,6 +44,10 @@ expect(AuditEvent.last.details[:custom_message]).to eq(expected_message) end + + it_behaves_like 'sends correct event type in audit event stream' do + let_it_be(:event_type) { "attempted_to_create_deploy_token" } + end end end end diff --git a/ee/spec/services/ee/projects/deploy_tokens/destroy_service_spec.rb b/ee/spec/services/ee/projects/deploy_tokens/destroy_service_spec.rb index b81e69bb2acaab..7d66ea6643a7c3 100644 --- a/ee/spec/services/ee/projects/deploy_tokens/destroy_service_spec.rb +++ b/ee/spec/services/ee/projects/deploy_tokens/destroy_service_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe Projects::DeployTokens::DestroyService do - let_it_be(:entity) { create(:project) } + let_it_be(:group) { create :group } + let_it_be(:entity) { create(:project, group: group) } let_it_be(:deploy_token) { create(:deploy_token, projects: [entity]) } let_it_be(:user) { create(:user) } let_it_be(:deploy_token_params) { { token_id: deploy_token.id } } @@ -26,5 +27,14 @@ expect(AuditEvent.last.details[:custom_message]).to eq(expected_message) end + + before do + stub_licensed_features(external_audit_events: true) + group.external_audit_event_destinations.create!(destination_url: 'http://example.com') + end + + it_behaves_like 'sends correct event type in audit event stream' do + let_it_be(:event_type) { "destroyed_deploy_token" } + end end end diff --git a/ee/spec/support/shared_examples/audit/audit_event_type_stream_shared_examples.rb b/ee/spec/support/shared_examples/audit/audit_event_type_stream_shared_examples.rb new file mode 100644 index 00000000000000..02dfbb25de3aac --- /dev/null +++ b/ee/spec/support/shared_examples/audit/audit_event_type_stream_shared_examples.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'sends correct event type in audit event stream' do + it 'sends correct event type in audit event stream' do + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with(event_type, + anything, + anything) + + subject + end +end -- GitLab From 2523de6d125e42c8ce2e7b3b68d5dc3711257154 Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Wed, 8 Jun 2022 21:12:35 +0530 Subject: [PATCH 2/3] Rename event type names --- ee/app/controllers/ee/projects/deploy_tokens_controller.rb | 2 +- ee/app/services/ee/projects/deploy_tokens/create_service.rb | 4 ++-- ee/app/services/ee/projects/deploy_tokens/destroy_service.rb | 2 +- ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb | 2 +- .../services/ee/projects/deploy_tokens/create_service_spec.rb | 4 ++-- .../ee/projects/deploy_tokens/destroy_service_spec.rb | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ee/app/controllers/ee/projects/deploy_tokens_controller.rb b/ee/app/controllers/ee/projects/deploy_tokens_controller.rb index fd90148f8c1938..b2b7c8f8cf5b2e 100644 --- a/ee/app/controllers/ee/projects/deploy_tokens_controller.rb +++ b/ee/app/controllers/ee/projects/deploy_tokens_controller.rb @@ -18,7 +18,7 @@ def log_audit_event # rubocop:disable Gitlab/ModuleWithInstanceVariables message = "Revoked project deploy token with name: #{@token.name} with token_id: #{@token.id} with scopes: #{@token.scopes}." audit_context = { - name: 'revoked_deploy_token', + name: 'deploy_token_revoked', author: current_user, scope: @project, target: @token, diff --git a/ee/app/services/ee/projects/deploy_tokens/create_service.rb b/ee/app/services/ee/projects/deploy_tokens/create_service.rb index ce9dba9847e648..aa44759e04f2fe 100644 --- a/ee/app/services/ee/projects/deploy_tokens/create_service.rb +++ b/ee/app/services/ee/projects/deploy_tokens/create_service.rb @@ -18,10 +18,10 @@ def execute def audit_event_service(deploy_token, result) if result[:status] == :success message = "Created project deploy token with name: #{deploy_token.name} with token_id: #{deploy_token.id} with scopes: #{deploy_token.scopes}." - name = "created_deploy_token" + name = "deploy_token_created" else message = "Attempted to create project deploy token but failed with message: #{result[:message]}" - name = "attempted_to_create_deploy_token" + name = "deploy_token_creation_failed" end audit_context = { diff --git a/ee/app/services/ee/projects/deploy_tokens/destroy_service.rb b/ee/app/services/ee/projects/deploy_tokens/destroy_service.rb index b74b0fc146a871..85658cf59cd64c 100644 --- a/ee/app/services/ee/projects/deploy_tokens/destroy_service.rb +++ b/ee/app/services/ee/projects/deploy_tokens/destroy_service.rb @@ -18,7 +18,7 @@ def execute def audit_event_service(deploy_token) message = "Destroyed project deploy token with name: #{deploy_token.name} with token_id: #{deploy_token.id} with scopes: #{deploy_token.scopes}." audit_context = { - name: 'destroyed_deploy_token', + name: 'deploy_token_destroyed', author: current_user, scope: project, target: deploy_token, diff --git a/ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb b/ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb index 501ed2c30889c0..74232a379209fa 100644 --- a/ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb +++ b/ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb @@ -41,7 +41,7 @@ end it_behaves_like 'sends correct event type in audit event stream' do - let_it_be(:event_type) { "revoked_deploy_token" } + let_it_be(:event_type) { "deploy_token_revoked" } end end end diff --git a/ee/spec/services/ee/projects/deploy_tokens/create_service_spec.rb b/ee/spec/services/ee/projects/deploy_tokens/create_service_spec.rb index 7c1116cf0e642c..7e6528f13a66f1 100644 --- a/ee/spec/services/ee/projects/deploy_tokens/create_service_spec.rb +++ b/ee/spec/services/ee/projects/deploy_tokens/create_service_spec.rb @@ -30,7 +30,7 @@ end it_behaves_like 'sends correct event type in audit event stream' do - let_it_be(:event_type) { "created_deploy_token" } + let_it_be(:event_type) { "deploy_token_created" } end end @@ -46,7 +46,7 @@ end it_behaves_like 'sends correct event type in audit event stream' do - let_it_be(:event_type) { "attempted_to_create_deploy_token" } + let_it_be(:event_type) { "deploy_token_creation_failed" } end end end diff --git a/ee/spec/services/ee/projects/deploy_tokens/destroy_service_spec.rb b/ee/spec/services/ee/projects/deploy_tokens/destroy_service_spec.rb index 7d66ea6643a7c3..f848930fdca66b 100644 --- a/ee/spec/services/ee/projects/deploy_tokens/destroy_service_spec.rb +++ b/ee/spec/services/ee/projects/deploy_tokens/destroy_service_spec.rb @@ -34,7 +34,7 @@ end it_behaves_like 'sends correct event type in audit event stream' do - let_it_be(:event_type) { "destroyed_deploy_token" } + let_it_be(:event_type) { "deploy_token_destroyed" } end end end -- GitLab From 912faa70169b028d976b5e86f2498f6fb01e011b Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Thu, 9 Jun 2022 19:44:32 +0530 Subject: [PATCH 3/3] Additional details merge in audit event details --- .../ee/projects/deploy_tokens_controller.rb | 5 ++- ee/app/services/audit_events/build_service.rb | 5 +-- .../projects/deploy_tokens/create_service.rb | 5 ++- .../projects/deploy_tokens/destroy_service.rb | 5 ++- ee/lib/gitlab/audit/auditor.rb | 5 ++- ee/spec/lib/gitlab/audit/auditor_spec.rb | 34 +++++++++++++++++++ .../projects/deploy_tokens_controller_spec.rb | 4 +++ .../audit_events/build_service_spec.rb | 10 ++++-- .../deploy_tokens/create_service_spec.rb | 10 ++++-- .../deploy_tokens/destroy_service_spec.rb | 5 ++- 10 files changed, 76 insertions(+), 12 deletions(-) diff --git a/ee/app/controllers/ee/projects/deploy_tokens_controller.rb b/ee/app/controllers/ee/projects/deploy_tokens_controller.rb index b2b7c8f8cf5b2e..71679814c4954e 100644 --- a/ee/app/controllers/ee/projects/deploy_tokens_controller.rb +++ b/ee/app/controllers/ee/projects/deploy_tokens_controller.rb @@ -22,7 +22,10 @@ def log_audit_event author: current_user, scope: @project, target: @token, - message: message + message: message, + additional_details: { + action: :custom + } } ::Gitlab::Audit::Auditor.audit(audit_context) # rubocop:enable Gitlab/ModuleWithInstanceVariables diff --git a/ee/app/services/audit_events/build_service.rb b/ee/app/services/audit_events/build_service.rb index 96699ae9d4a84d..c9ef0165d5bb74 100644 --- a/ee/app/services/audit_events/build_service.rb +++ b/ee/app/services/audit_events/build_service.rb @@ -8,7 +8,7 @@ class BuildService # @raise [MissingAttributeError] when required attributes are blank # # @return [BuildService] - def initialize(author:, scope:, target:, message:, created_at: DateTime.current) + def initialize(author:, scope:, target:, message:, created_at: DateTime.current, additional_details: {}) raise MissingAttributeError if missing_attribute?(author, scope, target, message) @author = build_author(author) @@ -17,6 +17,7 @@ def initialize(author:, scope:, target:, message:, created_at: DateTime.current) @ip_address = build_ip_address @message = build_message(message) @created_at = created_at + @additional_details = additional_details end # Create an instance of AuditEvent @@ -64,7 +65,7 @@ def base_details_payload target_type: @target.type, target_details: @target.details, custom_message: @message - } + }.merge(@additional_details) end def build_author(author) diff --git a/ee/app/services/ee/projects/deploy_tokens/create_service.rb b/ee/app/services/ee/projects/deploy_tokens/create_service.rb index aa44759e04f2fe..d841938c172fb2 100644 --- a/ee/app/services/ee/projects/deploy_tokens/create_service.rb +++ b/ee/app/services/ee/projects/deploy_tokens/create_service.rb @@ -29,7 +29,10 @@ def audit_event_service(deploy_token, result) author: current_user, scope: project, target: deploy_token, - message: message + message: message, + additional_details: { + action: :custom + } } ::Gitlab::Audit::Auditor.audit(audit_context) end diff --git a/ee/app/services/ee/projects/deploy_tokens/destroy_service.rb b/ee/app/services/ee/projects/deploy_tokens/destroy_service.rb index 85658cf59cd64c..08514fb8f6dfe1 100644 --- a/ee/app/services/ee/projects/deploy_tokens/destroy_service.rb +++ b/ee/app/services/ee/projects/deploy_tokens/destroy_service.rb @@ -22,7 +22,10 @@ def audit_event_service(deploy_token) author: current_user, scope: project, target: deploy_token, - message: message + message: message, + additional_details: { + action: :custom + } } ::Gitlab::Audit::Auditor.audit(audit_context) end diff --git a/ee/lib/gitlab/audit/auditor.rb b/ee/lib/gitlab/audit/auditor.rb index b4968bd8b3142b..bb63b573c56d72 100644 --- a/ee/lib/gitlab/audit/auditor.rb +++ b/ee/lib/gitlab/audit/auditor.rb @@ -11,6 +11,7 @@ class Auditor # @option context [User, Project, Group] :scope the scope which audit event belongs to # @option context [Object] :target the target object being audited # @option context [String] :message the message describing the action + # @option context [Hash] :additional_details the additional details we want to merge into audit event details. # @option context [Time] :created_at the time that the event occurred (defaults to the current time) # # @example Using block (useful when events are emitted deep in the call stack) @@ -60,6 +61,7 @@ def initialize(context = {}) @target = @context.fetch(:target) @created_at = @context.fetch(:created_at, DateTime.current) @message = @context.fetch(:message, '') + @additional_details = @context.fetch(:additional_details, {}) end def multiple_audit @@ -98,7 +100,8 @@ def build_event(message) scope: @scope, target: @target, created_at: @created_at, - message: message + message: message, + additional_details: @additional_details ).execute end diff --git a/ee/spec/lib/gitlab/audit/auditor_spec.rb b/ee/spec/lib/gitlab/audit/auditor_spec.rb index 62b527bd2d99c2..8a3603c3b2f945 100644 --- a/ee/spec/lib/gitlab/audit/auditor_spec.rb +++ b/ee/spec/lib/gitlab/audit/auditor_spec.rb @@ -154,6 +154,40 @@ end end + context 'when overriding the additional_details' do + additional_details = { action: :custom } + let(:context) do + { name: name, + author: author, + scope: scope, + target: target, + created_at: Time.zone.now, + additional_details: additional_details } + end + + it 'logs audit events to database' do + freeze_time do + audit! + + expect(AuditEvent.last.details).to include(additional_details) + end + end + + it 'logs audit events to file' do + freeze_time do + expect(::Gitlab::AuditJsonLogger).to receive(:build).and_return(logger) + + audit! + + expect(logger).to have_received(:info).exactly(2).times.with( + hash_including( + 'details' => hash_including('action' => 'custom') + ) + ) + end + end + end + context 'when event is only streamed' do let(:context) do { name: name, author: author, scope: scope, target: target, created_at: 3.weeks.ago, stream_only: true } diff --git a/ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb b/ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb index 74232a379209fa..7141a9018162d1 100644 --- a/ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb +++ b/ee/spec/requests/ee/projects/deploy_tokens_controller_spec.rb @@ -33,6 +33,10 @@ MESSAGE expect(AuditEvent.last.details[:custom_message]).to eq(expected_message) + expect(AuditEvent.last.details).to include({ + custom_message: expected_message, + action: :custom + }) end before do diff --git a/ee/spec/services/audit_events/build_service_spec.rb b/ee/spec/services/audit_events/build_service_spec.rb index a4c9085faf8002..c3ff73da0e72e9 100644 --- a/ee/spec/services/audit_events/build_service_spec.rb +++ b/ee/spec/services/audit_events/build_service_spec.rb @@ -9,13 +9,15 @@ let(:target) { build_stubbed(:project) } let(:ip_address) { '192.168.8.8' } let(:message) { 'Added an interesting field from project Gotham' } + let(:additional_details) { { action: :custom } } subject(:service) do described_class.new( author: author, scope: scope, target: target, - message: message + message: message, + additional_details: additional_details ) end @@ -47,7 +49,8 @@ target_details: target.name, custom_message: message, ip_address: ip_address, - entity_path: scope.full_path + entity_path: scope.full_path, + action: :custom ) expect(event.ip_address).to eq(ip_address) @@ -122,7 +125,8 @@ target_id: target.id, target_type: target.class.name, target_details: target.name, - custom_message: message + custom_message: message, + action: :custom ) expect(event.ip_address).to be_nil diff --git a/ee/spec/services/ee/projects/deploy_tokens/create_service_spec.rb b/ee/spec/services/ee/projects/deploy_tokens/create_service_spec.rb index 7e6528f13a66f1..a83aa215fb1346 100644 --- a/ee/spec/services/ee/projects/deploy_tokens/create_service_spec.rb +++ b/ee/spec/services/ee/projects/deploy_tokens/create_service_spec.rb @@ -26,7 +26,10 @@ with token_id: #{subject[:deploy_token].id} with scopes: #{subject[:deploy_token].scopes}. MESSAGE - expect(AuditEvent.last.details[:custom_message]).to eq(expected_message) + expect(AuditEvent.last.details).to include({ + custom_message: expected_message, + action: :custom + }) end it_behaves_like 'sends correct event type in audit event stream' do @@ -42,7 +45,10 @@ expected_message = "Attempted to create project deploy token but failed with message: Scopes can't be blank" - expect(AuditEvent.last.details[:custom_message]).to eq(expected_message) + expect(AuditEvent.last.details).to include({ + custom_message: expected_message, + action: :custom + }) end it_behaves_like 'sends correct event type in audit event stream' do diff --git a/ee/spec/services/ee/projects/deploy_tokens/destroy_service_spec.rb b/ee/spec/services/ee/projects/deploy_tokens/destroy_service_spec.rb index f848930fdca66b..a3e89bea526bb7 100644 --- a/ee/spec/services/ee/projects/deploy_tokens/destroy_service_spec.rb +++ b/ee/spec/services/ee/projects/deploy_tokens/destroy_service_spec.rb @@ -25,7 +25,10 @@ with token_id: #{deploy_token.id} with scopes: #{deploy_token.scopes}. MESSAGE - expect(AuditEvent.last.details[:custom_message]).to eq(expected_message) + expect(AuditEvent.last.details).to include({ + custom_message: expected_message, + action: :custom + }) end before do -- GitLab