From bdb70deabc919c0d6b04cfd1f5ddd5fbb16720cd Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Fri, 31 Mar 2023 04:49:54 -0500 Subject: [PATCH 1/9] Add audit event detail spec for PasswordsController --- ee/app/controllers/ee/passwords_controller.rb | 17 ++++++++-------- .../types/password_reset_requested.yml | 10 ++++++++++ .../controllers/passwords_controller_spec.rb | 20 ++++++++++++++++++- 3 files changed, 37 insertions(+), 10 deletions(-) create mode 100644 ee/config/audit_events/types/password_reset_requested.yml diff --git a/ee/app/controllers/ee/passwords_controller.rb b/ee/app/controllers/ee/passwords_controller.rb index b686286e1178ef..e864dac64c27c7 100644 --- a/ee/app/controllers/ee/passwords_controller.rb +++ b/ee/app/controllers/ee/passwords_controller.rb @@ -11,16 +11,15 @@ module PasswordsController private def log_audit_event - ::AuditEventService.new( - current_user, - resource, - action: :custom, - custom_message: 'Ask for password reset', + ::Gitlab::Audit::Auditor.audit({ + name: "password_reset_requested", + author: ::Gitlab::Audit::UnauthenticatedAuthor.new, + scope: resource, + target: ::User.new, + target_details: resource_params[:email], + message: "Ask for password reset", ip_address: request.remote_ip - ).for_user( - full_path: resource_params[:email], - entity_id: nil - ).unauth_security_event + }) end end end diff --git a/ee/config/audit_events/types/password_reset_requested.yml b/ee/config/audit_events/types/password_reset_requested.yml new file mode 100644 index 00000000000000..970a446287e115 --- /dev/null +++ b/ee/config/audit_events/types/password_reset_requested.yml @@ -0,0 +1,10 @@ +--- +name: password_reset_requested +description: Event triggered when a user requests a password reset using a registered + email address +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374107 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/114548 +feature_category: compliance_management +milestone: '15.11' +saved_to_database: true +streamed: false diff --git a/ee/spec/controllers/passwords_controller_spec.rb b/ee/spec/controllers/passwords_controller_spec.rb index 75ad364e3073af..36c12ad08a4cd1 100644 --- a/ee/spec/controllers/passwords_controller_spec.rb +++ b/ee/spec/controllers/passwords_controller_spec.rb @@ -13,6 +13,24 @@ subject { post :create, params: { user: { email: user.email } } } - it { expect { subject }.to change { AuditEvent.count }.by(1) } + it do + expect { subject }.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.last + expect(audit_event.attributes).to include({ + "entity_id" => user.id, + "entity_type" => "User", + "entity_path" => nil, + "author_name" => "An unauthenticated user", + "target_type" => "User", + "target_details" => user.email + }) + expect(audit_event.details).to include({ + custom_message: "Ask for password reset", + author_name: "An unauthenticated user", + target_type: "User", + target_details: user.email + }) + end end end -- GitLab From 71cbfa9f40ecc429020c6a2da181407193b65dac Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Fri, 31 Mar 2023 07:18:16 -0500 Subject: [PATCH 2/9] Refactor audit events for OmniauthCallbacksController --- .../ee/omniauth_callbacks_controller.rb | 25 +++++++++++++++---- .../types/omniauth_login_failed.yml | 9 +++++++ .../ee/omniauth_callbacks_controller_spec.rb | 17 +++++++++++++ 3 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 ee/config/audit_events/types/omniauth_login_failed.yml diff --git a/ee/app/controllers/ee/omniauth_callbacks_controller.rb b/ee/app/controllers/ee/omniauth_callbacks_controller.rb index 9691f33bd0d5a7..940f2bbb07b2eb 100644 --- a/ee/app/controllers/ee/omniauth_callbacks_controller.rb +++ b/ee/app/controllers/ee/omniauth_callbacks_controller.rb @@ -22,11 +22,26 @@ def openid_connect override :log_failed_login def log_failed_login(author, provider) - ::AuditEventService.new( - author, - nil, - with: provider - ).for_failed_login.unauth_security_event + # ::AuditEventService.new( + # author, + # with: provider + # nil, + # ).for_failed_login.unauth_security_event + + unauth_author = ::Gitlab::Audit::UnauthenticatedAuthor.new(name: author) + user = ::User.new(id: unauth_author.id, name: author) + ::Gitlab::Audit::Auditor.audit({ + name: "omniauth_login_failed", + author: unauth_author, + scope: user, + target: user, + additional_details: { + failed_login: provider.upcase, + author_name: user.name, + target_details: user.name + }, + message: "#{provider.upcase} login failed" + }) end override :after_sign_up_path diff --git a/ee/config/audit_events/types/omniauth_login_failed.yml b/ee/config/audit_events/types/omniauth_login_failed.yml new file mode 100644 index 00000000000000..9415084861f949 --- /dev/null +++ b/ee/config/audit_events/types/omniauth_login_failed.yml @@ -0,0 +1,9 @@ +--- +name: omniauth_login_failed +description: Event triggered when an OmniAuth login fails +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374107 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/114548 +feature_category: compliance_management +milestone: '15.11' +saved_to_database: true +streamed: false diff --git a/ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb b/ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb index 9461ce3635b0e7..c688eae419555b 100644 --- a/ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb +++ b/ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb @@ -30,7 +30,24 @@ it 'audits provider failed login when licensed' do stub_licensed_features(extended_audit_events: true) + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including({ + name: "omniauth_login_failed" + })).and_call_original + expect { subject.failure }.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.last + expect(audit_event.attributes).to include({ + "author_name" => user.username, + "entity_type" => "User", + "target_details" => user.username + }) + expect(audit_event.details).to include({ + failed_login: "LDAP", + author_name: user.username, + target_details: user.username, + custom_message: "LDAP login failed" + }) end it 'does not audit provider failed login when unlicensed' do -- GitLab From 6743b57d6c62a930196cd210c14c45390eb1c65f Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Sun, 2 Apr 2023 09:21:36 -0500 Subject: [PATCH 3/9] Refactor RegistrationsController audit events --- .../ee/omniauth_callbacks_controller.rb | 6 ------ ee/app/controllers/ee/passwords_controller.rb | 17 +++++++++-------- .../controllers/ee/registrations_controller.rb | 14 ++++++++------ ..._requested.yml => registration_created.yml} | 5 ++--- .../ee/registrations_controller_spec.rb | 18 ++++++++++++++++-- .../controllers/passwords_controller_spec.rb | 1 - 6 files changed, 35 insertions(+), 26 deletions(-) rename ee/config/audit_events/types/{password_reset_requested.yml => registration_created.yml} (65%) diff --git a/ee/app/controllers/ee/omniauth_callbacks_controller.rb b/ee/app/controllers/ee/omniauth_callbacks_controller.rb index 940f2bbb07b2eb..091c759eae11a1 100644 --- a/ee/app/controllers/ee/omniauth_callbacks_controller.rb +++ b/ee/app/controllers/ee/omniauth_callbacks_controller.rb @@ -22,12 +22,6 @@ def openid_connect override :log_failed_login def log_failed_login(author, provider) - # ::AuditEventService.new( - # author, - # with: provider - # nil, - # ).for_failed_login.unauth_security_event - unauth_author = ::Gitlab::Audit::UnauthenticatedAuthor.new(name: author) user = ::User.new(id: unauth_author.id, name: author) ::Gitlab::Audit::Auditor.audit({ diff --git a/ee/app/controllers/ee/passwords_controller.rb b/ee/app/controllers/ee/passwords_controller.rb index e864dac64c27c7..b686286e1178ef 100644 --- a/ee/app/controllers/ee/passwords_controller.rb +++ b/ee/app/controllers/ee/passwords_controller.rb @@ -11,15 +11,16 @@ module PasswordsController private def log_audit_event - ::Gitlab::Audit::Auditor.audit({ - name: "password_reset_requested", - author: ::Gitlab::Audit::UnauthenticatedAuthor.new, - scope: resource, - target: ::User.new, - target_details: resource_params[:email], - message: "Ask for password reset", + ::AuditEventService.new( + current_user, + resource, + action: :custom, + custom_message: 'Ask for password reset', ip_address: request.remote_ip - }) + ).for_user( + full_path: resource_params[:email], + entity_id: nil + ).unauth_security_event end end end diff --git a/ee/app/controllers/ee/registrations_controller.rb b/ee/app/controllers/ee/registrations_controller.rb index 163c22dd151782..223cdef4e312c9 100644 --- a/ee/app/controllers/ee/registrations_controller.rb +++ b/ee/app/controllers/ee/registrations_controller.rb @@ -99,12 +99,14 @@ def ensure_can_remove_self def log_audit_event(user) return unless user&.persisted? - ::AuditEventService.new( - user, - user, - action: :custom, - custom_message: _('Instance access request') - ).for_user.security_event + ::Gitlab::Audit::Auditor.audit({ + name: "registration_created", + author: user, + scope: user, + target: user, + target_details: user.username, + message: _("Instance access request") + }) end override :after_sign_up_path diff --git a/ee/config/audit_events/types/password_reset_requested.yml b/ee/config/audit_events/types/registration_created.yml similarity index 65% rename from ee/config/audit_events/types/password_reset_requested.yml rename to ee/config/audit_events/types/registration_created.yml index 970a446287e115..39d940b3588472 100644 --- a/ee/config/audit_events/types/password_reset_requested.yml +++ b/ee/config/audit_events/types/registration_created.yml @@ -1,7 +1,6 @@ --- -name: password_reset_requested -description: Event triggered when a user requests a password reset using a registered - email address +name: registration_created +description: Event triggered when a user registers for instance access introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374107 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/114548 feature_category: compliance_management diff --git a/ee/spec/controllers/ee/registrations_controller_spec.rb b/ee/spec/controllers/ee/registrations_controller_spec.rb index 1162d221da2c0a..ed228194492f92 100644 --- a/ee/spec/controllers/ee/registrations_controller_spec.rb +++ b/ee/spec/controllers/ee/registrations_controller_spec.rb @@ -120,14 +120,28 @@ end it 'logs the audit event info', :aggregate_failures do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including({ + name: "registration_created" + })).and_call_original + subject created_user = User.find_by(email: new_user_email) audit_event = AuditEvent.where(author_id: created_user.id).last + expect(audit_event.entity).to eq(created_user) + expect(audit_event.author).to eq(created_user) expect(audit_event.ip_address).to eq(created_user.current_sign_in_ip) - expect(audit_event.details[:target_details]).to eq(created_user.username) - expect(audit_event.details[:custom_message]).to eq('Instance access request') + expect(audit_event.attributes).to include({ + "target_details" => created_user.username, + "target_id" => created_user.id, + "target_type" => "User", + "entity_path" => created_user.full_path + }) + expect(audit_event.details).to include({ + target_details: created_user.username, + custom_message: "Instance access request" + }) end context 'with invalid user' do diff --git a/ee/spec/controllers/passwords_controller_spec.rb b/ee/spec/controllers/passwords_controller_spec.rb index 36c12ad08a4cd1..efc33cf92dd0aa 100644 --- a/ee/spec/controllers/passwords_controller_spec.rb +++ b/ee/spec/controllers/passwords_controller_spec.rb @@ -20,7 +20,6 @@ expect(audit_event.attributes).to include({ "entity_id" => user.id, "entity_type" => "User", - "entity_path" => nil, "author_name" => "An unauthenticated user", "target_type" => "User", "target_details" => user.email -- GitLab From 7fb63936c442adb70cf15e27f5bb557f8f900193 Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Fri, 9 Jun 2023 13:53:42 -0500 Subject: [PATCH 4/9] Rollback PasswordsControllerSpec changes --- .../controllers/passwords_controller_spec.rb | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/ee/spec/controllers/passwords_controller_spec.rb b/ee/spec/controllers/passwords_controller_spec.rb index efc33cf92dd0aa..75ad364e3073af 100644 --- a/ee/spec/controllers/passwords_controller_spec.rb +++ b/ee/spec/controllers/passwords_controller_spec.rb @@ -13,23 +13,6 @@ subject { post :create, params: { user: { email: user.email } } } - it do - expect { subject }.to change { AuditEvent.count }.by(1) - - audit_event = AuditEvent.last - expect(audit_event.attributes).to include({ - "entity_id" => user.id, - "entity_type" => "User", - "author_name" => "An unauthenticated user", - "target_type" => "User", - "target_details" => user.email - }) - expect(audit_event.details).to include({ - custom_message: "Ask for password reset", - author_name: "An unauthenticated user", - target_type: "User", - target_details: user.email - }) - end + it { expect { subject }.to change { AuditEvent.count }.by(1) } end end -- GitLab From c5b15f50cdfbadeab69182d4d75adab4adf5c4c0 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Mon, 12 Jun 2023 16:41:55 +0000 Subject: [PATCH 5/9] Update audit event yml with correct MR+milestone --- ee/config/audit_events/types/omniauth_login_failed.yml | 4 ++-- ee/config/audit_events/types/registration_created.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ee/config/audit_events/types/omniauth_login_failed.yml b/ee/config/audit_events/types/omniauth_login_failed.yml index 9415084861f949..9ca07e25e50e35 100644 --- a/ee/config/audit_events/types/omniauth_login_failed.yml +++ b/ee/config/audit_events/types/omniauth_login_failed.yml @@ -2,8 +2,8 @@ name: omniauth_login_failed description: Event triggered when an OmniAuth login fails introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374107 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/114548 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123080 feature_category: compliance_management -milestone: '15.11' +milestone: '16.1' saved_to_database: true streamed: false diff --git a/ee/config/audit_events/types/registration_created.yml b/ee/config/audit_events/types/registration_created.yml index 39d940b3588472..a0b876c083d407 100644 --- a/ee/config/audit_events/types/registration_created.yml +++ b/ee/config/audit_events/types/registration_created.yml @@ -2,8 +2,8 @@ name: registration_created description: Event triggered when a user registers for instance access introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374107 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/114548 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123080 feature_category: compliance_management -milestone: '15.11' +milestone: '16.1' saved_to_database: true streamed: false -- GitLab From 17a7c7a349b5df45a8920b81a57c1b70194ab6c2 Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Tue, 13 Jun 2023 11:45:14 -0500 Subject: [PATCH 6/9] Update omniauth/registration audit configs for streaming --- ee/config/audit_events/types/omniauth_login_failed.yml | 2 +- ee/config/audit_events/types/registration_created.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/config/audit_events/types/omniauth_login_failed.yml b/ee/config/audit_events/types/omniauth_login_failed.yml index 9ca07e25e50e35..de30bf004a79ec 100644 --- a/ee/config/audit_events/types/omniauth_login_failed.yml +++ b/ee/config/audit_events/types/omniauth_login_failed.yml @@ -6,4 +6,4 @@ introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123080 feature_category: compliance_management milestone: '16.1' saved_to_database: true -streamed: false +streamed: true diff --git a/ee/config/audit_events/types/registration_created.yml b/ee/config/audit_events/types/registration_created.yml index a0b876c083d407..f3f9473676200d 100644 --- a/ee/config/audit_events/types/registration_created.yml +++ b/ee/config/audit_events/types/registration_created.yml @@ -6,4 +6,4 @@ introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123080 feature_category: compliance_management milestone: '16.1' saved_to_database: true -streamed: false +streamed: true -- GitLab From 6fd926f42b3f97e00d085b816269e2fb7ddb1fd6 Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Wed, 21 Jun 2023 08:10:31 -0500 Subject: [PATCH 7/9] Update controller tests to use have_attributes --- .../ee/omniauth_callbacks_controller_spec.rb | 28 ++++++++++--------- .../ee/registrations_controller_spec.rb | 28 ++++++++++--------- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb b/ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb index c688eae419555b..2008e981fc6204 100644 --- a/ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb +++ b/ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb @@ -28,26 +28,28 @@ ) end - it 'audits provider failed login when licensed' do + it 'audits provider failed login when licensed', :aggregate_failures do stub_licensed_features(extended_audit_events: true) + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including({ name: "omniauth_login_failed" })).and_call_original expect { subject.failure }.to change { AuditEvent.count }.by(1) - audit_event = AuditEvent.last - expect(audit_event.attributes).to include({ - "author_name" => user.username, - "entity_type" => "User", - "target_details" => user.username - }) - expect(audit_event.details).to include({ - failed_login: "LDAP", - author_name: user.username, - target_details: user.username, - custom_message: "LDAP login failed" - }) + expect(AuditEvent.last).to have_attributes( + attributes: hash_including({ + "author_name" => user.username, + "entity_type" => "User", + "target_details" => user.username + }), + details: hash_including({ + failed_login: "LDAP", + author_name: user.username, + target_details: user.username, + custom_message: "LDAP login failed" + }) + ) end it 'does not audit provider failed login when unlicensed' do diff --git a/ee/spec/controllers/ee/registrations_controller_spec.rb b/ee/spec/controllers/ee/registrations_controller_spec.rb index ed228194492f92..09921af8240fe0 100644 --- a/ee/spec/controllers/ee/registrations_controller_spec.rb +++ b/ee/spec/controllers/ee/registrations_controller_spec.rb @@ -129,19 +129,21 @@ created_user = User.find_by(email: new_user_email) audit_event = AuditEvent.where(author_id: created_user.id).last - expect(audit_event.entity).to eq(created_user) - expect(audit_event.author).to eq(created_user) - expect(audit_event.ip_address).to eq(created_user.current_sign_in_ip) - expect(audit_event.attributes).to include({ - "target_details" => created_user.username, - "target_id" => created_user.id, - "target_type" => "User", - "entity_path" => created_user.full_path - }) - expect(audit_event.details).to include({ - target_details: created_user.username, - custom_message: "Instance access request" - }) + expect(audit_event).to have_attributes( + entity: created_user, + author: created_user, + ip_address: created_user.current_sign_in_ip, + attributes: hash_including({ + "target_details" => created_user.username, + "target_id" => created_user.id, + "target_type" => "User", + "entity_path" => created_user.full_path + }), + details: hash_including({ + target_details: created_user.username, + custom_message: "Instance access request" + }) + ) end context 'with invalid user' do -- GitLab From 8a17020253cc681983aa511d3c02dbe0f7dc46a0 Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Mon, 17 Jul 2023 19:42:11 +0000 Subject: [PATCH 8/9] update milestone to 16.2 --- ee/config/audit_events/types/omniauth_login_failed.yml | 2 +- ee/config/audit_events/types/registration_created.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/config/audit_events/types/omniauth_login_failed.yml b/ee/config/audit_events/types/omniauth_login_failed.yml index de30bf004a79ec..963103bde97aa9 100644 --- a/ee/config/audit_events/types/omniauth_login_failed.yml +++ b/ee/config/audit_events/types/omniauth_login_failed.yml @@ -4,6 +4,6 @@ description: Event triggered when an OmniAuth login fails introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374107 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123080 feature_category: compliance_management -milestone: '16.1' +milestone: '16.2' saved_to_database: true streamed: true diff --git a/ee/config/audit_events/types/registration_created.yml b/ee/config/audit_events/types/registration_created.yml index f3f9473676200d..d9feb3a0144384 100644 --- a/ee/config/audit_events/types/registration_created.yml +++ b/ee/config/audit_events/types/registration_created.yml @@ -4,6 +4,6 @@ description: Event triggered when a user registers for instance access introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374107 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123080 feature_category: compliance_management -milestone: '16.1' +milestone: '16.2' saved_to_database: true streamed: true -- GitLab From 6c204ad743207404b8c6aa443f652422e9688906 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 18 Jul 2023 12:38:05 +0000 Subject: [PATCH 9/9] update milestone to 16.3 --- ee/config/audit_events/types/omniauth_login_failed.yml | 2 +- ee/config/audit_events/types/registration_created.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/config/audit_events/types/omniauth_login_failed.yml b/ee/config/audit_events/types/omniauth_login_failed.yml index 963103bde97aa9..5355354e5bb49c 100644 --- a/ee/config/audit_events/types/omniauth_login_failed.yml +++ b/ee/config/audit_events/types/omniauth_login_failed.yml @@ -4,6 +4,6 @@ description: Event triggered when an OmniAuth login fails introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374107 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123080 feature_category: compliance_management -milestone: '16.2' +milestone: '16.3' saved_to_database: true streamed: true diff --git a/ee/config/audit_events/types/registration_created.yml b/ee/config/audit_events/types/registration_created.yml index d9feb3a0144384..4a228d15cf80a2 100644 --- a/ee/config/audit_events/types/registration_created.yml +++ b/ee/config/audit_events/types/registration_created.yml @@ -4,6 +4,6 @@ description: Event triggered when a user registers for instance access introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374107 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123080 feature_category: compliance_management -milestone: '16.2' +milestone: '16.3' saved_to_database: true streamed: true -- GitLab