diff --git a/doc/administration/audit_event_streaming/audit_event_types.md b/doc/administration/audit_event_streaming/audit_event_types.md index b69f181539495c48807db9fc9b0a50710a990141..b41ac728ac7f943a824b3bae88888352532ae564 100644 --- a/doc/administration/audit_event_streaming/audit_event_types.md +++ b/doc/administration/audit_event_streaming/audit_event_types.md @@ -148,6 +148,9 @@ Every audit event is associated with an event type. The association with the eve | [`issue_closed_by_project_bot`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485) | Triggered when an issue is closed using a project access token | **{check-circle}** Yes | **{check-circle}** Yes | `team_planning` | GitLab [16.1](https://gitlab.com/gitlab-org/gitlab/-/issues/323299) | | [`issue_created_by_project_bot`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485) | Triggered when an issue is created using a project access token | **{check-circle}** Yes | **{check-circle}** Yes | `team_planning` | GitLab [16.1](https://gitlab.com/gitlab-org/gitlab/-/issues/323299) | | [`issue_reopened_by_project_bot`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485) | Triggered when an issue is reopened using a project access token | **{check-circle}** Yes | **{check-circle}** Yes | `team_planning` | GitLab [16.1](https://gitlab.com/gitlab-org/gitlab/-/issues/323299) | +| [`login_failed_with_otp_authentication`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129595) | Triggered when the login fails due to an incorrect OTP | **{check-circle}** Yes | **{check-circle}** Yes | `system_access` | GitLab [16.4](https://gitlab.com/gitlab-org/gitlab/-/issues/377758) | +| [`login_failed_with_standard_authentication`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129595) | Triggered when login to GitLab fails with standard authentication like password. | **{check-circle}** Yes | **{check-circle}** Yes | `system_access` | GitLab [16.4](https://gitlab.com/gitlab-org/gitlab/-/issues/377758) | +| [`login_failed_with_webauthn_authentication`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129595) | Triggered when login fails via WebAuthn device | **{check-circle}** Yes | **{check-circle}** Yes | `system_access` | GitLab [16.4](https://gitlab.com/gitlab-org/gitlab/-/issues/377758) | | [`member_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/109711) | Event triggered when a membership is created | **{check-circle}** Yes | **{check-circle}** Yes | `compliance_management` | GitLab [15.9](https://gitlab.com/gitlab-org/gitlab/-/issues/374112) | | [`member_destroyed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/109711) | Event triggered when a membership is destroyed | **{check-circle}** Yes | **{check-circle}** Yes | `compliance_management` | GitLab [15.9](https://gitlab.com/gitlab-org/gitlab/-/issues/374112) | | [`member_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/109711) | Event triggered when a membership is updated | **{check-circle}** Yes | **{check-circle}** Yes | `compliance_management` | GitLab [15.9](https://gitlab.com/gitlab-org/gitlab/-/issues/374112) | diff --git a/ee/app/controllers/concerns/ee/authenticates_with_two_factor.rb b/ee/app/controllers/concerns/ee/authenticates_with_two_factor.rb index 7a3c606bbccb61d65fbd15bb6f33eb2254122189..d121fd3629d8ee2e015723c1d74d4af522f6fe4f 100644 --- a/ee/app/controllers/concerns/ee/authenticates_with_two_factor.rb +++ b/ee/app/controllers/concerns/ee/authenticates_with_two_factor.rb @@ -6,11 +6,7 @@ module AuthenticatesWithTwoFactor override :log_failed_two_factor def log_failed_two_factor(user, method) - ::AuditEventService.new( - user, - user, - with: method - ).for_failed_login.unauth_security_event + Audit::UnauthenticatedSecurityEventAuditor.new(user, method).execute end end end diff --git a/ee/app/controllers/ee/sessions_controller.rb b/ee/app/controllers/ee/sessions_controller.rb index efdcb7fda0f6551c882bc3ccb09efdc57556e5e5..afbf07d0a7ec984c2a17f51ed1b6b864c34aba3a 100644 --- a/ee/app/controllers/ee/sessions_controller.rb +++ b/ee/app/controllers/ee/sessions_controller.rb @@ -75,8 +75,7 @@ def geo_return_to_after_logout override :log_failed_login def log_failed_login login = request.filtered_parameters.dig('user', 'login') - audit_event_service = ::AuditEventService.new(login, nil) - audit_event_service.for_failed_login.unauth_security_event + Audit::UnauthenticatedSecurityEventAuditor.new(login).execute super end diff --git a/ee/app/services/ee/audit_event_service.rb b/ee/app/services/ee/audit_event_service.rb index cf807a128ce5cef8c55b9bda9dff46482452f4fe..23c5580afbca9096e9721ff3a4b6c98607768021 100644 --- a/ee/app/services/ee/audit_event_service.rb +++ b/ee/app/services/ee/audit_event_service.rb @@ -68,21 +68,6 @@ def for_member(member) self end - # Builds the @details attribute for a failed login - # - # @return [AuditEventService] - def for_failed_login - auth = @details[:with] || 'STANDARD' - - @details = { - failed_login: auth.upcase, - author_name: @author.name, - target_details: @author.name - } - - self - end - # Builds the @details attribute for changes # # @param model [Object] the target model being audited diff --git a/ee/config/audit_events/types/login_failed_with_otp_authentication.yml b/ee/config/audit_events/types/login_failed_with_otp_authentication.yml new file mode 100644 index 0000000000000000000000000000000000000000..73244f773a4398e092aab92dc40a4647fcad90a6 --- /dev/null +++ b/ee/config/audit_events/types/login_failed_with_otp_authentication.yml @@ -0,0 +1,9 @@ +--- +name: login_failed_with_otp_authentication +description: Triggered when the login fails due to an incorrect OTP +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/377758 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129595 +feature_category: system_access +milestone: '16.4' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/login_failed_with_standard_authentication.yml b/ee/config/audit_events/types/login_failed_with_standard_authentication.yml new file mode 100644 index 0000000000000000000000000000000000000000..3fca60e9fd89c854076155508d5e41d1980e21c6 --- /dev/null +++ b/ee/config/audit_events/types/login_failed_with_standard_authentication.yml @@ -0,0 +1,9 @@ +--- +name: login_failed_with_standard_authentication +description: Triggered when login to GitLab fails with standard authentication like password. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/377758 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129595 +feature_category: system_access +milestone: '16.4' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/login_failed_with_webauthn_authentication.yml b/ee/config/audit_events/types/login_failed_with_webauthn_authentication.yml new file mode 100644 index 0000000000000000000000000000000000000000..1231fc06397d75ca511a1a0ff56f50cb947c5a46 --- /dev/null +++ b/ee/config/audit_events/types/login_failed_with_webauthn_authentication.yml @@ -0,0 +1,9 @@ +--- +name: login_failed_with_webauthn_authentication +description: Triggered when login fails via WebAuthn device +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/377758 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129595 +feature_category: system_access +milestone: '16.4' +saved_to_database: true +streamed: true diff --git a/ee/lib/audit/unauthenticated_security_event_auditor.rb b/ee/lib/audit/unauthenticated_security_event_auditor.rb new file mode 100644 index 0000000000000000000000000000000000000000..8b4892e1979e3e14fc05e09a567a98c4da58038f --- /dev/null +++ b/ee/lib/audit/unauthenticated_security_event_auditor.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Audit + class UnauthenticatedSecurityEventAuditor + def initialize(user, authentication_method = 'STANDARD') + if user.instance_of?(String) + @author = ::Gitlab::Audit::UnauthenticatedAuthor.new(name: user) + @scope = Gitlab::Audit::InstanceScope.new + else + @author = @scope = user + end + + @authentication_method = authentication_method + end + + def execute + context = { + name: "login_failed_with_#{@authentication_method.downcase}_authentication", + scope: @scope, + author: @author, + target: @author, + message: "Failed to login with #{@authentication_method} authentication", + additional_details: { + failed_login: @authentication_method + } + } + + ::Gitlab::Audit::Auditor.audit(context) + end + end +end diff --git a/ee/lib/ee/api/internal/base.rb b/ee/lib/ee/api/internal/base.rb index ce9bd597ea40672562d83dfd43abebec9772a1d0..b90e4d1b3a8b352a3457d13c396a9a52409b51e4 100644 --- a/ee/lib/ee/api/internal/base.rb +++ b/ee/lib/ee/api/internal/base.rb @@ -48,8 +48,7 @@ def two_factor_manual_otp_check { success: true } else user.increment_failed_attempts! - ::AuditEventService.new(user, user, with: "OTP") - .for_failed_login.unauth_security_event + Audit::UnauthenticatedSecurityEventAuditor.new(user, 'OTP').execute ::Gitlab::AppLogger.info( message: 'Failed OTP login', user_id: user.id, diff --git a/ee/spec/controllers/ee/sessions_controller_spec.rb b/ee/spec/controllers/ee/sessions_controller_spec.rb index 5358e1cfe2a4866f80a45bfc57a4a8b7ec92fc0f..a97e33f7a46e266a9d2fe111d83548e9a2923642 100644 --- a/ee/spec/controllers/ee/sessions_controller_spec.rb +++ b/ee/spec/controllers/ee/sessions_controller_spec.rb @@ -66,6 +66,30 @@ it_behaves_like 'a valid oauth authentication redirect' end end + + context 'when login fails' do + before do + @request.env["warden.options"] = { action: 'unauthenticated' } + end + + it 'creates a failed authentication audit event' do + audit_context = { + name: "login_failed_with_standard_authentication", + message: "Failed to login with STANDARD authentication", + target: be_an_instance_of(Gitlab::Audit::UnauthenticatedAuthor), + scope: be_an_instance_of(Gitlab::Audit::InstanceScope), + author: be_an_instance_of(Gitlab::Audit::UnauthenticatedAuthor), + additional_details: { + failed_login: 'STANDARD' + } + } + + expect(Audit::UnauthenticatedSecurityEventAuditor).to receive(:new).with('foo@bar.com').and_call_original + expect(Gitlab::Audit::Auditor).to receive(:audit).with(audit_context).and_call_original + + get(:new, params: { user: { login: 'foo@bar.com' } }) + end + end end describe '#create' do diff --git a/ee/spec/features/users/login_spec.rb b/ee/spec/features/users/login_spec.rb index 6f5df38b5fef3a1f0e718b3efd11afecf910b0f4..0a656a5cee1a6aff344c6e981062a89a031900b8 100644 --- a/ee/spec/features/users/login_spec.rb +++ b/ee/spec/features/users/login_spec.rb @@ -15,7 +15,7 @@ user = create(:user) expect { gitlab_sign_in(user, password: 'incorrect-password') } - .to change { AuditEvent.where(entity_id: -1).count }.from(0).to(1) + .to change { AuditEvent.where(entity_type: 'Gitlab::Audit::InstanceScope').count }.from(0).to(1) end it 'creates a security event for an invalid OAuth login' do diff --git a/ee/spec/services/audit_event_service_spec.rb b/ee/spec/services/audit_event_service_spec.rb index 5ba11ca60a8d7710c846080aad4c6debf76f577d..5c085d0e32ecbda86b4d2b57ff8c29186eb83d56 100644 --- a/ee/spec/services/audit_event_service_spec.rb +++ b/ee/spec/services/audit_event_service_spec.rb @@ -285,68 +285,6 @@ end end - describe '#for_failed_login' do - let(:author_name) { 'testuser' } - let(:service) { described_class.new(author_name, nil) } - let(:event) { service.for_failed_login.unauth_security_event } - - before do - stub_licensed_features(extended_audit_events: true) - end - - it 'has the right type' do - expect(event.entity_type).to eq('User') - end - - it 'has the right author' do - expect(event.details[:author_name]).to eq(author_name) - expect(event.author_name).to eq(author_name) - end - - it 'has the right target_details' do - expect(event.details[:target_details]).to eq(author_name) - end - - it 'has the right auth method for OAUTH' do - oauth_service = described_class.new(author_name, nil, ip_address: request_ip_address, with: 'ldap') - event = oauth_service.for_failed_login.unauth_security_event - - expect(event.details[:failed_login]).to eq('LDAP') - end - - context 'admin audit log licensed' do - before do - stub_licensed_features(extended_audit_events: true, admin_audit_log: true) - end - - it 'has the right IP address' do - expect(event.ip_address).to eq(request_ip_address) - expect(event.details[:ip_address]).to eq(request_ip_address) - end - end - - context 'admin audit log unlicensed' do - before do - stub_licensed_features(extended_audit_events: true, admin_audit_log: false) - end - - it 'does not have the ip_address' do - expect(event.ip_address).to be_nil - expect(event.details).not_to have_key(:ip_address) - end - end - - context 'on a read-only instance' do - before do - allow(Gitlab::Database).to receive(:read_only?).and_return(true) - end - - it 'does not create an event record in the database' do - expect { service.for_failed_login.unauth_security_event }.not_to change(AuditEvent, :count) - end - end - end - describe '#for_user' do let(:current_user) { create(:user) } let(:user) { create(:user) } diff --git a/ee/spec/support/shared_examples/controllers/sessions_shared_examples.rb b/ee/spec/support/shared_examples/controllers/sessions_shared_examples.rb index aa36ded11fc8e448995d58ecdcaf3cd0a9f77203..22cf725828aad83dda9e139938dd81e7630b63f6 100644 --- a/ee/spec/support/shared_examples/controllers/sessions_shared_examples.rb +++ b/ee/spec/support/shared_examples/controllers/sessions_shared_examples.rb @@ -2,13 +2,20 @@ RSpec.shared_examples 'an auditable failed authentication' do it 'log an audit event', :aggregate_failures do - audit_event_service = instance_spy(AuditEventService) - allow(AuditEventService).to receive(:new).and_return(audit_event_service) + audit_context = { + name: "login_failed_with_#{method.downcase}_authentication", + message: "Failed to login with #{method} authentication", + target: user, + scope: user, + author: user, + additional_details: { + failed_login: method + } + } - operation + expect(Audit::UnauthenticatedSecurityEventAuditor).to receive(:new).with(user, method).and_call_original + expect(Gitlab::Audit::Auditor).to receive(:audit).with(audit_context).and_call_original - expect(AuditEventService).to have_received(:new).with(user, user, with: method) - expect(audit_event_service).to have_received(:for_failed_login) - expect(audit_event_service).to have_received(:unauth_security_event) + operation end end