From 5bd4d92093e4d7667ce065eb11a128f62c1d6c0d Mon Sep 17 00:00:00 2001 From: eugielimpin Date: Tue, 20 Jun 2023 16:41:33 +0800 Subject: [PATCH 1/6] Add user_access_locked audit event Create a user_access_locked audit event when the user's access to the instance is locked. This can happen, for example, when the user exceeds the allowed number of failed login attempts. EE: true Changelog: added --- ee/app/models/ee/user.rb | 18 +++++++ .../audit_events/types/user_access_locked.yml | 8 ++++ ee/spec/models/ee/user_spec.rb | 48 +++++++++++++++++++ 3 files changed, 74 insertions(+) create mode 100644 ee/config/audit_events/types/user_access_locked.yml diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index 8c31e51348fcff..e3180a33a44048 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -578,6 +578,24 @@ def code_suggestions_disabled_by_group? groups.roots.joins(:namespace_settings).where(namespace_settings: { code_suggestions: false }).any? end + override :lock_access! + def lock_access!(opts = {}) + unless access_locked? + reason = nil + reason = 'excessive failed login attempts' if attempts_exceeded? + + ::Gitlab::Audit::Auditor.audit( + name: 'user_access_locked', + author: ::User.admin_bot, + scope: self, + target: self, + message: ['User access locked', reason].compact.join(' - ') + ) + end + + super + end + protected override :password_required? diff --git a/ee/config/audit_events/types/user_access_locked.yml b/ee/config/audit_events/types/user_access_locked.yml new file mode 100644 index 00000000000000..33d1dbda20c090 --- /dev/null +++ b/ee/config/audit_events/types/user_access_locked.yml @@ -0,0 +1,8 @@ +name: user_access_locked +description: Event triggered when user access to the instance is locked +introduced_by_issue: https://gitlab.com/gitlab-org/modelops/anti-abuse/team-tasks/-/issues/244 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/124169 +feature_category: 'system_access' +milestone: '16.2' +saved_to_database: true +streamed: true diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index 6b0767000490ce..f0725ea5e7925b 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -2761,4 +2761,52 @@ end end end + + describe '#lock_access!' do + let_it_be(:user) { create(:user) } + let_it_be(:gitlab_admin_bot) { described_class.admin_bot } + + subject { user.lock_access! } + + before do + stub_licensed_features(admin_audit_log: true) + end + + it 'logs a user_access_locked audit event' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with( + hash_including( + name: 'user_access_locked', + author: gitlab_admin_bot, + scope: user, + target: user, + message: 'User access locked' + ) + ).and_call_original + expect { subject }.to change { AuditEvent.count }.by(1) + end + + context 'when reason for access lock is excessive failed login attempts' do + before do + allow(user).to receive(:attempts_exceeded?).and_return(true) + end + + it 'logs a user_access_locked audit event with the correct message' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with( + hash_including(message: 'User access locked - excessive failed login attempts') + ) + + subject + end + end + + context 'when user access is already locked' do + before do + user.lock_access! + end + + it 'does not log an audit event' do + expect { subject }.not_to change { AuditEvent.count } + end + end + end end -- GitLab From d8bb2247f2716180ccbb5bdde3d88149c4de9194 Mon Sep 17 00:00:00 2001 From: eugielimpin Date: Wed, 21 Jun 2023 11:44:00 +0800 Subject: [PATCH 2/6] Refactor: override a hook method instead --- app/models/user.rb | 5 +++++ ee/app/models/ee/user.rb | 34 ++++++++++++++++------------------ ee/spec/models/ee/user_spec.rb | 3 ++- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 96cdbb192bcd7c..42348fe87b0fb9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2063,6 +2063,7 @@ def read_only_attribute?(attribute) # override, from Devise def lock_access!(opts = {}) Gitlab::AppLogger.info("Account Locked: username=#{username}") + audit_lock_access super end @@ -2608,6 +2609,10 @@ def ci_namespace_mirrors_for_group_members(level) def prefix_for_feed_token FEED_TOKEN_PREFIX end + + # method overriden in EE + def audit_lock_access + end end User.prepend_mod_with('User') diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index e3180a33a44048..eb12df91d3bbc0 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -578,24 +578,6 @@ def code_suggestions_disabled_by_group? groups.roots.joins(:namespace_settings).where(namespace_settings: { code_suggestions: false }).any? end - override :lock_access! - def lock_access!(opts = {}) - unless access_locked? - reason = nil - reason = 'excessive failed login attempts' if attempts_exceeded? - - ::Gitlab::Audit::Auditor.audit( - name: 'user_access_locked', - author: ::User.admin_bot, - scope: self, - target: self, - message: ['User access locked', reason].compact.join(' - ') - ) - end - - super - end - protected override :password_required? @@ -704,5 +686,21 @@ def perform_user_cap_check def should_delay_delete?(*args) super && !has_paid_namespace?(exclude_trials: true) end + + override :audit_lock_access + def audit_lock_access + unless access_locked? + reason = nil + reason = 'excessive failed login attempts' if attempts_exceeded? + + ::Gitlab::Audit::Auditor.audit( + name: 'user_access_locked', + author: ::User.admin_bot, + scope: self, + target: self, + message: ['User access locked', reason].compact.join(' - ') + ) + end + end end end diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index f0725ea5e7925b..2efadcc4d0b313 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -2763,9 +2763,10 @@ end describe '#lock_access!' do - let_it_be(:user) { create(:user) } let_it_be(:gitlab_admin_bot) { described_class.admin_bot } + let(:user) { create(:user) } + subject { user.lock_access! } before do -- GitLab From 38d560aed94a21bdea195d3925248c4b36d21336 Mon Sep 17 00:00:00 2001 From: eugielimpin Date: Wed, 21 Jun 2023 11:45:55 +0800 Subject: [PATCH 3/6] Define empty method in a single line --- app/models/user.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 42348fe87b0fb9..2c24972004933e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2611,8 +2611,7 @@ def prefix_for_feed_token end # method overriden in EE - def audit_lock_access - end + def audit_lock_access; end end User.prepend_mod_with('User') -- GitLab From 64a9c71d951e0f5a294d8dba1c923965dbbbfe54 Mon Sep 17 00:00:00 2001 From: eugielimpin Date: Wed, 21 Jun 2023 11:54:07 +0800 Subject: [PATCH 4/6] Document user_access_locked audit event type --- doc/administration/audit_events.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index e924da39145d25..c3f5d60b161c03 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -375,6 +375,7 @@ The following user actions on a GitLab instance generate instance audit events: - A user's personal access token was successfully or unsuccessfully created or revoked. - A user's two-factor authentication was disabled. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/238177) in GitLab 15.1. +- User access is locked. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/124169) in GitLab 16.2. #### User management -- GitLab From f1283b3857ef8eb9fdba52d17527c280bde321f4 Mon Sep 17 00:00:00 2001 From: eugielimpin Date: Thu, 22 Jun 2023 10:46:46 +0800 Subject: [PATCH 5/6] Use let_it_be_with_reload instead of let --- ee/spec/models/ee/user_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index 2efadcc4d0b313..19500ee57daf0d 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -2764,8 +2764,7 @@ describe '#lock_access!' do let_it_be(:gitlab_admin_bot) { described_class.admin_bot } - - let(:user) { create(:user) } + let_it_be_with_reload(:user) { create(:user) } subject { user.lock_access! } -- GitLab From 35363cbdfd3f57603caa76daf2a67fedf0643a20 Mon Sep 17 00:00:00 2001 From: Phillip Wells Date: Thu, 22 Jun 2023 02:54:02 +0000 Subject: [PATCH 6/6] Move version history to the top of the section --- doc/administration/audit_events.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index c3f5d60b161c03..a80d3421d348fb 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -362,6 +362,9 @@ GitLab generates audit events when a cluster agent token is created or revoked. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/276250) in GitLab 13.6, audit events for when a user is approved using the Admin Area. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/276921) in GitLab 13.6, audit events for when a user's personal access token is successfully or unsuccessfully created or revoked. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/298783) in GitLab 13.9, audit events for when a user requests access to an instance or is rejected using the Admin Area. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/238177) in + GitLab 15.1, audit events when a user's two-factor authentication is disabled. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/124169) in GitLab 16.2, audit events when a user's access is locked. The following user actions on a GitLab instance generate instance audit events: @@ -373,9 +376,8 @@ The following user actions on a GitLab instance generate instance audit events: - Grant OAuth access. - Failed second-factor authentication attempt. - A user's personal access token was successfully or unsuccessfully created or revoked. -- A user's two-factor authentication was disabled. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/238177) in - GitLab 15.1. -- User access is locked. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/124169) in GitLab 16.2. +- A user's two-factor authentication was disabled. +- A user's access is locked. #### User management -- GitLab