From f68569221bec9374f0cae16f1e2e733a48239cfc Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Tue, 30 May 2023 12:35:39 +0530 Subject: [PATCH 1/3] Audit events for google cloud logging configurations This commit adds audit events for create, update and delete actions on google cloud logging configurations EE: true Changelog: added --- .../base.rb | 24 ++++++++++++++ .../create.rb | 4 ++- .../destroy.rb | 4 ++- .../update.rb | 4 ++- .../create_spec.rb | 33 +++++++++++++++++-- .../destroy_spec.rb | 29 ++++++++++++++++ .../update_spec.rb | 33 +++++++++++++++++-- 7 files changed, 122 insertions(+), 9 deletions(-) create mode 100644 ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/base.rb diff --git a/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/base.rb b/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/base.rb new file mode 100644 index 00000000000000..0f20b36025970e --- /dev/null +++ b/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/base.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Mutations + module AuditEvents + module GoogleCloudLoggingConfigurations + class Base < BaseMutation + private + + def audit(config, action:, extra_context: {}) + audit_context = { + name: "google_cloud_logging_configuration_#{action}", + author: current_user, + scope: config.group, + target: config.group, + message: "#{action.capitalize} Google Cloud logging configuration with project id: " \ + "#{config.google_project_id_name} and log id: #{config.log_id_name}" + } + + ::Gitlab::Audit::Auditor.audit(audit_context.merge(extra_context)) + end + end + end + end +end diff --git a/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/create.rb b/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/create.rb index 7141ca5e3953b9..09b731d8d15e09 100644 --- a/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/create.rb +++ b/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/create.rb @@ -3,7 +3,7 @@ module Mutations module AuditEvents module GoogleCloudLoggingConfigurations - class Create < BaseMutation + class Create < Base graphql_name 'GoogleCloudLoggingConfigurationCreate' authorize :admin_external_audit_events @@ -53,6 +53,8 @@ def resolve(group_path:, google_project_id_name:, client_email:, private_key:, l config = ::AuditEvents::GoogleCloudLoggingConfiguration.new(config_attributes) if config.save + audit(config, action: :created) + { google_cloud_logging_configuration: config, errors: [] } else { google_cloud_logging_configuration: nil, errors: Array(config.errors) } diff --git a/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/destroy.rb b/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/destroy.rb index c2be51b47a4ead..0c9f7e180b1e3c 100644 --- a/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/destroy.rb +++ b/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/destroy.rb @@ -3,7 +3,7 @@ module Mutations module AuditEvents module GoogleCloudLoggingConfigurations - class Destroy < BaseMutation + class Destroy < Base graphql_name 'GoogleCloudLoggingConfigurationDestroy' authorize :admin_external_audit_events @@ -16,6 +16,8 @@ def resolve(id:) config = authorized_find!(id) if config.destroy + audit(config, action: :deleted) + { errors: [] } else { errors: Array(config.errors) } diff --git a/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/update.rb b/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/update.rb index ff67d11798b763..ccd94caaafdd81 100644 --- a/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/update.rb +++ b/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/update.rb @@ -3,7 +3,7 @@ module Mutations module AuditEvents module GoogleCloudLoggingConfigurations - class Update < BaseMutation + class Update < Base graphql_name 'GoogleCloudLoggingConfigurationUpdate' authorize :admin_external_audit_events @@ -48,6 +48,8 @@ def resolve(id:, google_project_id_name: nil, client_email: nil, private_key: ni }.compact if config.update(config_attributes) + audit(config, action: :updated) + { google_cloud_logging_configuration: config, errors: [] } else { google_cloud_logging_configuration: nil, errors: Array(config.errors) } diff --git a/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/create_spec.rb b/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/create_spec.rb index fe7db0ffe12c9a..f832fc0ecb6819 100644 --- a/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/create_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/create_spec.rb @@ -10,7 +10,7 @@ let_it_be(:google_project_id_name) { 'test-project' } let_it_be(:client_email) { 'test-email@example.com' } let_it_be(:private_key) { OpenSSL::PKey::RSA.new(4096).to_pem } - let_it_be(:default_log_id_name) { 'audit_events' } + let_it_be(:log_id_name) { 'audit_events' } let(:current_user) { owner } let(:mutation) { graphql_mutation(:google_cloud_logging_configuration_create, input) } @@ -27,11 +27,34 @@ subject(:mutate) { post_graphql_mutation(mutation, current_user: owner) } + shared_examples 'creates an audit event' do + before do + allow(Gitlab::Audit::Auditor).to receive(:audit) + end + + it 'audits the creation' do + subject + + expect(Gitlab::Audit::Auditor).to have_received(:audit) do |args| + expect(args[:name]).to eq('google_cloud_logging_configuration_created') + expect(args[:author]).to eq(current_user) + expect(args[:scope]).to eq(group) + expect(args[:target]).to eq(group) + expect(args[:message]).to eq("Created Google Cloud logging configuration with project id: " \ + "#{google_project_id_name} and log id: #{log_id_name}") + end + end + end + shared_examples 'a mutation that does not create a configuration' do - it 'does not destroy the configuration' do + it 'does not create the configuration' do expect { mutate } .not_to change { AuditEvents::GoogleCloudLoggingConfiguration.count } end + + it 'does not create audit event' do + expect { mutate }.not_to change { AuditEvent.count } + end end shared_examples 'an unauthorized mutation that does not create a configuration' do @@ -63,10 +86,12 @@ expect(config.group).to eq(group) expect(config.google_project_id_name).to eq(google_project_id_name) expect(config.client_email).to eq(client_email) - expect(config.log_id_name).to eq(default_log_id_name) + expect(config.log_id_name).to eq(log_id_name) expect(config.private_key).to eq(private_key) end + it_behaves_like 'creates an audit event', 'audit_events' + context 'when overriding log id name' do let_it_be(:log_id_name) { 'test-log-id' } @@ -91,6 +116,8 @@ expect(config.log_id_name).to eq(log_id_name) expect(config.private_key).to eq(private_key) end + + it_behaves_like 'creates an audit event' end context 'when there is error while saving' do diff --git a/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/destroy_spec.rb b/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/destroy_spec.rb index 8a257637e01cb7..bedb48d5297b91 100644 --- a/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/destroy_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/destroy_spec.rb @@ -15,6 +15,17 @@ subject(:mutate) { post_graphql_mutation(mutation, current_user: owner) } + shared_examples 'a mutation that does not destroy a configuration' do + it 'does not destroy the configuration' do + expect { mutate } + .not_to change { AuditEvents::GoogleCloudLoggingConfiguration.count } + end + + it 'does not create audit event' do + expect { mutate }.not_to change { AuditEvent.count } + end + end + context 'when feature is licensed' do before do stub_licensed_features(external_audit_events: true) @@ -23,6 +34,7 @@ context 'when current user is a group owner' do before do group.add_owner(owner) + allow(Gitlab::Audit::Auditor).to receive(:audit) end it 'destroys the configuration' do @@ -30,6 +42,19 @@ .to change { AuditEvents::GoogleCloudLoggingConfiguration.count }.by(-1) end + it 'audits the deletion' do + subject + + expect(Gitlab::Audit::Auditor).to have_received(:audit) do |args| + expect(args[:name]).to eq('google_cloud_logging_configuration_deleted') + expect(args[:author]).to eq(current_user) + expect(args[:scope]).to eq(group) + expect(args[:target]).to eq(group) + expect(args[:message]).to eq("Deleted Google Cloud logging configuration with project id: " \ + "#{config.google_project_id_name} and log id: #{config.log_id_name}") + end + end + context 'when there is an error during destroy' do before do allow_next_instance_of(Mutations::AuditEvents::GoogleCloudLoggingConfigurations::Destroy) do |mutation| @@ -59,6 +84,7 @@ end it_behaves_like 'a mutation on an unauthorized resource' + it_behaves_like 'a mutation that does not destroy a configuration' end context 'when current user is a group developer' do @@ -67,6 +93,7 @@ end it_behaves_like 'a mutation on an unauthorized resource' + it_behaves_like 'a mutation that does not destroy a configuration' end context 'when current user is a group guest' do @@ -75,6 +102,7 @@ end it_behaves_like 'a mutation on an unauthorized resource' + it_behaves_like 'a mutation that does not destroy a configuration' end end @@ -84,5 +112,6 @@ end it_behaves_like 'a mutation on an unauthorized resource' + it_behaves_like 'a mutation that does not destroy a configuration' end end diff --git a/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/update_spec.rb b/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/update_spec.rb index 19ccffdeeb719c..242b5d6ce119af 100644 --- a/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/update_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/update_spec.rb @@ -30,6 +30,16 @@ subject(:mutate) { post_graphql_mutation(mutation, current_user: owner) } + shared_examples 'a mutation that does not update the configuration' do + it 'does not update the configuration' do + expect { mutate }.not_to change { config.reload.attributes } + end + + it 'does not create audit event' do + expect { mutate }.not_to change { AuditEvent.count } + end + end + context 'when feature is licensed' do before do stub_licensed_features(external_audit_events: true) @@ -38,6 +48,8 @@ context 'when current user is a group owner' do before do group.add_owner(owner) + allow(Gitlab::Audit::Auditor).to receive(:audit) + allow(Gitlab::Audit::Auditor).to receive(:audit) end it 'updates the configuration' do @@ -51,6 +63,19 @@ expect(config.log_id_name).to eq(updated_log_id_name) end + it 'audits the update' do + subject + + expect(Gitlab::Audit::Auditor).to have_received(:audit) do |args| + expect(args[:name]).to eq('google_cloud_logging_configuration_updated') + expect(args[:author]).to eq(current_user) + expect(args[:scope]).to eq(group) + expect(args[:target]).to eq(group) + expect(args[:message]).to eq("Updated Google Cloud logging configuration with project id: " \ + "#{updated_google_project_id_name} and log id: #{updated_log_id_name}") + end + end + context 'when no fields are provided for update' do let(:input) do { @@ -58,9 +83,7 @@ } end - it 'does not update the configuration' do - expect { mutate }.not_to change { config.reload.attributes } - end + it_behaves_like 'a mutation that does not update the configuration' end context 'when there is error while updating' do @@ -92,6 +115,7 @@ end it_behaves_like 'a mutation on an unauthorized resource' + it_behaves_like 'a mutation that does not update the configuration' end context 'when current user is a group developer' do @@ -100,6 +124,7 @@ end it_behaves_like 'a mutation on an unauthorized resource' + it_behaves_like 'a mutation that does not update the configuration' end context 'when current user is a group guest' do @@ -108,6 +133,7 @@ end it_behaves_like 'a mutation on an unauthorized resource' + it_behaves_like 'a mutation that does not update the configuration' end end @@ -117,5 +143,6 @@ end it_behaves_like 'a mutation on an unauthorized resource' + it_behaves_like 'a mutation that does not update the configuration' end end -- GitLab From 78e28118140e46bb0df14c5c387d02080cef85cf Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Tue, 30 May 2023 12:41:39 +0530 Subject: [PATCH 2/3] Extra allow block fix --- .../google_cloud_logging_configurations/update_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/update_spec.rb b/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/update_spec.rb index 242b5d6ce119af..004561e9d6619a 100644 --- a/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/update_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/audit_events/google_cloud_logging_configurations/update_spec.rb @@ -49,7 +49,6 @@ before do group.add_owner(owner) allow(Gitlab::Audit::Auditor).to receive(:audit) - allow(Gitlab::Audit::Auditor).to receive(:audit) end it 'updates the configuration' do -- GitLab From 3d4cfbf59db0055a85724d0b33146858e1c137bb Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Fri, 2 Jun 2023 13:13:41 +0530 Subject: [PATCH 3/3] Remove extra context --- .../audit_events/google_cloud_logging_configurations/base.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/base.rb b/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/base.rb index 0f20b36025970e..152f67c8d1e373 100644 --- a/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/base.rb +++ b/ee/app/graphql/mutations/audit_events/google_cloud_logging_configurations/base.rb @@ -6,7 +6,7 @@ module GoogleCloudLoggingConfigurations class Base < BaseMutation private - def audit(config, action:, extra_context: {}) + def audit(config, action:) audit_context = { name: "google_cloud_logging_configuration_#{action}", author: current_user, @@ -16,7 +16,7 @@ def audit(config, action:, extra_context: {}) "#{config.google_project_id_name} and log id: #{config.log_id_name}" } - ::Gitlab::Audit::Auditor.audit(audit_context.merge(extra_context)) + ::Gitlab::Audit::Auditor.audit(audit_context) end end end -- GitLab