diff --git a/doc/administration/audit_event_streaming/audit_event_types.md b/doc/administration/audit_event_streaming/audit_event_types.md index 7bb1f8b82429891c0fafe2beb1ccc625ca20f19c..9b8ee55c7f913b00a354b286dcf97b7dc58783b1 100644 --- a/doc/administration/audit_event_streaming/audit_event_types.md +++ b/doc/administration/audit_event_streaming/audit_event_types.md @@ -88,6 +88,7 @@ Every audit event is associated with an event type. The association with the eve | [`destroy_compliance_framework`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74292) | Triggered on successful compliance framework deletion | **{check-circle}** Yes | **{check-circle}** Yes | `compliance_management` | GitLab [14.6](https://gitlab.com/gitlab-org/gitlab/-/issues/340649) | | [`destroy_event_streaming_destination`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74632) | Event triggered when an external audit event destination is deleted | **{check-circle}** Yes | **{check-circle}** Yes | `audit_events` | GitLab [14.6](https://gitlab.com/gitlab-org/gitlab/-/issues/344664) | | [`destroy_instance_event_streaming_destination`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125846) | Event triggered when an instance level external audit event destination is deleted | **{check-circle}** Yes | **{check-circle}** Yes | `audit_events` | GitLab [16.2](https://gitlab.com/gitlab-org/gitlab/-/issues/404730) | +| [`email_confirmation_sent`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129261) | Triggered when users add or change and email address and it needs to be confirmed. | **{dotted-circle}** No | **{check-circle}** Yes | `user_profile` | GitLab [16.3](https://gitlab.com/gitlab-org/gitlab/-/issues/377625) | | [`email_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/114546) | Event triggered when an email is created | **{check-circle}** Yes | **{check-circle}** Yes | `compliance_management` | GitLab [15.11](https://gitlab.com/gitlab-org/gitlab/-/issues/374107) | | [`email_destroyed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/114546) | Event triggered when an email is destroyed | **{check-circle}** Yes | **{check-circle}** Yes | `compliance_management` | GitLab [15.11](https://gitlab.com/gitlab-org/gitlab/-/issues/374107) | | [`environment_protected`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108247) | This event is triggered when a protected environment is created. | **{check-circle}** Yes | **{dotted-circle}** No | `environment_management` | GitLab [15.8](https://gitlab.com/gitlab-org/gitlab/-/issues/216164) | diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index b840b73570a774e08d0e3fc5f7af7a2492816d2c..19cc1fef12650a949a090bc521b4776ded50b824 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -635,6 +635,24 @@ def password_required?(*) super end + # override, from Devise::Confirmable + def send_confirmation_instructions + super + + ::Gitlab::Audit::Auditor.audit({ + name: 'email_confirmation_sent', + author: self, + scope: self, + message: "Confirmation instructions sent to: #{unconfirmed_email}", + target: self, + additional_details: { + target_type: "Email", + current_email: email, + unconfirmed_email: unconfirmed_email + } + }) + end + private def gitlab_com_member? diff --git a/ee/app/services/ee/emails/base_service.rb b/ee/app/services/ee/emails/base_service.rb index d978054e87f2733762433b0c75f404744646e7ad..42bcaaa763e49d955d430ebdcba0a6fb3a1826d4 100644 --- a/ee/app/services/ee/emails/base_service.rb +++ b/ee/app/services/ee/emails/base_service.rb @@ -19,6 +19,16 @@ def log_audit_event(options = {}) message: "Email destroyed", additional_details: { remove: "email" } } + when :confirm + { + name: "email_confirmation_sent", + message: "Confirmation instructions sent to: #{options[:unconfirmed_email]}", + additional_details: { + current_email: @current_user.email, + target_type: "Email", + unconfirmed_email: options[:unconfirmed_email] + } + } end ::Gitlab::Audit::Auditor.audit(audit_context.deep_merge({ diff --git a/ee/app/services/ee/emails/confirm_service.rb b/ee/app/services/ee/emails/confirm_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..1cd888c2b0efe8beba576b8aedaa3e0d2ee0fd9f --- /dev/null +++ b/ee/app/services/ee/emails/confirm_service.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module EE + module Emails + module ConfirmService + extend ::Gitlab::Utils::Override + + override :execute + def execute(email) + super.tap do + log_audit_event(action: :confirm, target: email, unconfirmed_email: email.email) + end + end + end + end +end diff --git a/ee/config/audit_events/types/email_confirmation_sent.yml b/ee/config/audit_events/types/email_confirmation_sent.yml new file mode 100644 index 0000000000000000000000000000000000000000..ee8c0f8f3f790fe2b0d43617ce3f6412d621425e --- /dev/null +++ b/ee/config/audit_events/types/email_confirmation_sent.yml @@ -0,0 +1,9 @@ +--- +name: email_confirmation_sent +description: Triggered when users add or change and email address and it needs to be confirmed. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/377625 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129261 +feature_category: user_profile +milestone: '16.3' +saved_to_database: false +streamed: true diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index b0760bb3cce351f501b8e3fac89df3b272b70f9a..e3e9e615e37ec45da496a96c689647cd945deb92 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -3155,4 +3155,28 @@ expect(subject).to eql(details_hash) end end + + describe 'audits' do + context 'audit events' do + it 'audits the confirmation request' do + user = create :user + unconfirmed_email = 'first-unconfirmed-email@example.com' + + expect(::Gitlab::Audit::Auditor).to(receive(:audit).with(hash_including({ + author: user, + scope: user, + target: user, + name: 'email_confirmation_sent', + message: "Confirmation instructions sent to: #{unconfirmed_email}", + additional_details: hash_including({ + current_email: user.email, + target_type: 'Email', + unconfirmed_email: unconfirmed_email + }) + })).and_call_original) + + user.update!(email: unconfirmed_email) + end + end + end end diff --git a/ee/spec/services/ee/users/create_service_spec.rb b/ee/spec/services/ee/users/create_service_spec.rb index 1b0bd062f91c647c4d402f878a03065271706773..0bf693f8631d37fa3240dfc9dc8101b1758e0dd5 100644 --- a/ee/spec/services/ee/users/create_service_spec.rb +++ b/ee/spec/services/ee/users/create_service_spec.rb @@ -17,39 +17,55 @@ subject(:service) { described_class.new(current_user, params) } describe '#execute' do - let(:operation) { service.execute } + context "when licenesed" do + before do + stub_licensed_features(extended_audit_events: true) + end - context 'audit events' do - include_examples 'audit event logging' do - let(:audit_event_name) { 'user_created' } + context 'audit events' do + it 'logs the audit event info' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including( + name: 'user_created' + )).and_call_original - let(:fail_condition!) do - expect_any_instance_of(User) - .to receive(:save).and_return(false) - end + # user creation will also send confirmaiton instructions which is also audited + allow(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including(name: 'email_confirmation_sent')) - let(:attributes) do - { + user = service.execute + + expect(AuditEvent.last).to have_attributes( author_id: current_user.id, - entity_id: @resource.id, + entity_id: user.id, entity_type: 'User', details: { add: 'user', author_class: 'User', author_name: current_user.name, - custom_message: "User #{@resource.username} created", - target_id: @resource.id, + custom_message: "User #{user.username} created", + target_id: user.id, target_type: 'User', - target_details: @resource.full_path, + target_details: user.full_path, registration_details: { - id: @resource.id, - name: @resource.name, - username: @resource.username, - email: @resource.email, - access_level: @resource.access_level + id: user.id, + name: user.name, + username: user.username, + email: user.email, + access_level: user.access_level } } - } + ) + end + + it 'does not log audit event if operation fails' do + expect_any_instance_of(User).to receive(:save).and_return(false) + + expect { service.execute }.not_to change { AuditEvent.count } + end + + it 'does not log audit event if operation results in no change' do + service.execute + + expect { service.execute }.not_to change(AuditEvent, :count) end end @@ -57,9 +73,25 @@ let(:current_user) { nil } it 'does not log any audit event' do - expect { operation }.not_to change(AuditEvent, :count) + allow(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including(name: 'email_created')) + + expect { service.execute }.not_to change(AuditEvent, :count) end end end end + + context 'when not licensed' do + before do + stub_licensed_features( + admin_audit_log: false, + audit_events: false, + extended_audit_events: false + ) + end + + it 'does not log audit event' do + expect { service.execute }.not_to change(AuditEvent, :count) + end + end end diff --git a/ee/spec/services/emails/confirm_service_spec.rb b/ee/spec/services/emails/confirm_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..96b8ee81da3e253697fc95a6f27a94adcea9ba82 --- /dev/null +++ b/ee/spec/services/emails/confirm_service_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Emails::ConfirmService, feature_category: :user_management do + describe '#execute' do + it 'records an audit event when confirmation is sent' do + user = create(:user) + unconfirmed_email = 'foobar@gitlab.com' + + expect(::Gitlab::Audit::Auditor).to(receive(:audit).with(hash_including({ + name: 'email_confirmation_sent', + message: 'Confirmation instructions sent to: foobar@gitlab.com', + additional_details: hash_including({ + current_email: user.email, + target_type: 'Email', + unconfirmed_email: unconfirmed_email + }) + })).and_call_original) + + described_class.new(user).execute(user.emails.create!(email: unconfirmed_email)) + end + end +end