From a3c213fec23df0134811e070902c735e6a2bf4b4 Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Mon, 16 Jan 2023 00:01:05 -0600 Subject: [PATCH 1/5] Refactor PersonalAccessTokens::CreateService audit events --- .../types/personal_access_token_created.yml | 9 +++++++ .../personal_access_tokens/create_service.rb | 26 +++++++++---------- .../create_service_audit_log_spec.rb | 21 ++++++++++----- 3 files changed, 35 insertions(+), 21 deletions(-) create mode 100644 config/audit_events/types/personal_access_token_created.yml diff --git a/config/audit_events/types/personal_access_token_created.yml b/config/audit_events/types/personal_access_token_created.yml new file mode 100644 index 00000000000000..31ebf5163bc242 --- /dev/null +++ b/config/audit_events/types/personal_access_token_created.yml @@ -0,0 +1,9 @@ +--- +name: personal_access_token_created +description: Event triggered when a user creates a personal access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374113 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108952 +feature_category: compliance_management +milestone: '15.8' +saved_to_database: true +streamed: true diff --git a/ee/app/services/ee/personal_access_tokens/create_service.rb b/ee/app/services/ee/personal_access_tokens/create_service.rb index 5bb4f21927a34b..8ce19d1ca8ca39 100644 --- a/ee/app/services/ee/personal_access_tokens/create_service.rb +++ b/ee/app/services/ee/personal_access_tokens/create_service.rb @@ -5,30 +5,28 @@ module PersonalAccessTokens module CreateService def execute super.tap do |response| - log_audit_event(response.payload[:personal_access_token], response) + send_audit_event(response) end end private - def log_audit_event(token, response) - audit_event_service(token, response).for_user(full_path: target_user.username, entity_id: target_user.id).security_event - end - - def audit_event_service(token, response) + def send_audit_event(response) message = if response.success? - "Created personal access token with id #{token.id}" + "Created personal access token with id #{response.payload[:personal_access_token].id}" else "Attempted to create personal access token but failed with message: #{response.message}" end - ::AuditEventService.new( - current_user, - target_user, - action: :custom, - custom_message: message, - ip_address: ip_address - ) + audit_context = { + name: 'personal_access_token_created', + author: current_user, + scope: current_user, + target: target_user, + message: message + } + + ::Gitlab::Audit::Auditor.audit(audit_context) end end end diff --git a/ee/spec/services/personal_access_tokens/create_service_audit_log_spec.rb b/ee/spec/services/personal_access_tokens/create_service_audit_log_spec.rb index 8f2eda8a67120e..bf04da6e9eb431 100644 --- a/ee/spec/services/personal_access_tokens/create_service_audit_log_spec.rb +++ b/ee/spec/services/personal_access_tokens/create_service_audit_log_spec.rb @@ -11,7 +11,7 @@ context 'when non-admin user' do context 'when user creates their own token' do it 'creates AuditEvent with success message' do - expect_to_log(user, user, /Created personal access token with id \d+/) + expect_to_audit(user, user, /Created personal access token with id \d+/) described_class.new(current_user: user, target_user: user, params: params).execute end @@ -21,7 +21,7 @@ let(:other_user) { create(:user) } it 'creates AuditEvent with failure message' do - expect_to_log(user, other_user, 'Attempted to create personal access token but failed with message: Not permitted to create') + expect_to_audit(user, other_user, 'Attempted to create personal access token but failed with message: Not permitted to create') described_class.new(current_user: user, target_user: other_user, params: params).execute end @@ -32,14 +32,14 @@ let(:admin) { create(:user, :admin) } it 'with admin mode enabled', :enable_admin_mode do - expect_to_log(admin, user, /Created personal access token with id \d+/) + expect_to_audit(admin, user, /Created personal access token with id \d+/) described_class.new(current_user: admin, target_user: user, params: params).execute end context 'with admin mode disabled' do it 'creates audit logs with failure message' do - expect_to_log(admin, user, 'Attempted to create personal access token but failed with message: Not permitted to create') + expect_to_audit(admin, user, 'Attempted to create personal access token but failed with message: Not permitted to create') described_class.new(current_user: admin, target_user: user, params: params).execute end @@ -47,9 +47,16 @@ end end - def expect_to_log(current_user, target_user, message) - expect(::AuditEventService).to receive(:new) - .with(current_user, target_user, action: :custom, custom_message: message, ip_address: nil) + def expect_to_audit(current_user, target_user, message) + audit_context = { + name: 'personal_access_token_created', + author: current_user, + scope: current_user, + target: target_user, + message: message + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) .and_call_original end end -- GitLab From cc16be3d048596bb5d2c55e6e2a704f0910d2d8a Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Mon, 16 Jan 2023 00:19:09 -0600 Subject: [PATCH 2/5] Refactor PersonalAccessTokens::RevokeService audit events --- .../personal_access_tokens/revoke_service.rb | 25 ++++++++----------- .../types/personal_access_token_created.yml | 0 .../types/personal_access_token_revoked.yml | 9 +++++++ .../revoke_service_audit_log_spec.rb | 12 ++++++--- 4 files changed, 29 insertions(+), 17 deletions(-) rename {config => ee/config}/audit_events/types/personal_access_token_created.yml (100%) create mode 100644 ee/config/audit_events/types/personal_access_token_revoked.yml diff --git a/ee/app/services/ee/personal_access_tokens/revoke_service.rb b/ee/app/services/ee/personal_access_tokens/revoke_service.rb index 8aacccc5b7e6de..ae6f451e2a291f 100644 --- a/ee/app/services/ee/personal_access_tokens/revoke_service.rb +++ b/ee/app/services/ee/personal_access_tokens/revoke_service.rb @@ -8,7 +8,7 @@ module RevokeService def execute super.tap do |response| - log_audit_event(token, response) + send_audit_event(token, response) end end @@ -28,25 +28,22 @@ def managed_user_revocation_allowed? can?(current_user, :admin_group_credentials_inventory, group) end - def log_audit_event(token, response) - return unless token.present? - - audit_event_service(token, response).for_user(full_path: token.user.username, entity_id: token.user.id).security_event - end - - def audit_event_service(token, response) + def send_audit_event(token, response) message = if response.success? "Revoked personal access token with id #{token.id}" else "Attempted to revoke personal access token with id #{token.id} but failed with message: #{response.message}" end - ::AuditEventService.new( - current_user, - token.user, - action: :custom, - custom_message: message - ) + audit_context = { + name: 'personal_access_token_revoked', + author: current_user, + scope: current_user, + target: token.user, + message: message + } + + ::Gitlab::Audit::Auditor.audit(audit_context) end end end diff --git a/config/audit_events/types/personal_access_token_created.yml b/ee/config/audit_events/types/personal_access_token_created.yml similarity index 100% rename from config/audit_events/types/personal_access_token_created.yml rename to ee/config/audit_events/types/personal_access_token_created.yml diff --git a/ee/config/audit_events/types/personal_access_token_revoked.yml b/ee/config/audit_events/types/personal_access_token_revoked.yml new file mode 100644 index 00000000000000..dace6db5c22609 --- /dev/null +++ b/ee/config/audit_events/types/personal_access_token_revoked.yml @@ -0,0 +1,9 @@ +--- +name: personal_access_token_revoked +description: Event triggered when a personal access token is revoked +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374113 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108952 +feature_category: compliance_management +milestone: '15.8' +saved_to_database: true +streamed: true diff --git a/ee/spec/services/personal_access_tokens/revoke_service_audit_log_spec.rb b/ee/spec/services/personal_access_tokens/revoke_service_audit_log_spec.rb index dce06452af5a27..47abea54c9023a 100644 --- a/ee/spec/services/personal_access_tokens/revoke_service_audit_log_spec.rb +++ b/ee/spec/services/personal_access_tokens/revoke_service_audit_log_spec.rb @@ -11,9 +11,15 @@ let(:service) { described_class.new(user, token: token) } it 'creates audit events' do - expect(::AuditEventService) - .to receive(:new) - .with(user, user, action: :custom, custom_message: "Revoked personal access token with id #{token.id}") + audit_context = { + name: 'personal_access_token_revoked', + author: user, + scope: user, + target: user, + message: "Revoked personal access token with id #{token.id}" + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) .and_call_original subject -- GitLab From 5bc4cb6471e9786343e38965a6482ab55cdeca4f Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Mon, 16 Jan 2023 09:17:44 -0600 Subject: [PATCH 3/5] Update Security::TokenRevocationService spec for audit events --- .../security/token_revocation_service_spec.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/ee/spec/services/security/token_revocation_service_spec.rb b/ee/spec/services/security/token_revocation_service_spec.rb index e7ad52c7a7c758..edbc463906244f 100644 --- a/ee/spec/services/security/token_revocation_service_spec.rb +++ b/ee/spec/services/security/token_revocation_service_spec.rb @@ -68,14 +68,15 @@ it 'returns success' do expect(PersonalAccessTokens::RevokeService).to receive(:new).once.and_call_original - expect(::AuditEventService) - .to receive(:new) - .with( - User.security_bot, - glpat_token.user, - action: :custom, - custom_message: "Revoked personal access token with id #{glpat_token.id}" - ).and_call_original + audit_context = { + name: 'personal_access_token_revoked', + author: User.security_bot, + scope: User.security_bot, + target: glpat_token.user, + message: "Revoked personal access token with id #{glpat_token.id}" + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context).and_call_original expect(SystemNoteService) .to receive(:change_vulnerability_state) -- GitLab From fdaae4c1ee385cd090dac33d1f4f12b11951a36c Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Mon, 16 Jan 2023 10:15:51 -0600 Subject: [PATCH 4/5] Don't audit when revoking a nil token --- ee/app/services/ee/personal_access_tokens/revoke_service.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ee/app/services/ee/personal_access_tokens/revoke_service.rb b/ee/app/services/ee/personal_access_tokens/revoke_service.rb index ae6f451e2a291f..a83d5dd9c610ee 100644 --- a/ee/app/services/ee/personal_access_tokens/revoke_service.rb +++ b/ee/app/services/ee/personal_access_tokens/revoke_service.rb @@ -29,6 +29,8 @@ def managed_user_revocation_allowed? end def send_audit_event(token, response) + return unless token + message = if response.success? "Revoked personal access token with id #{token.id}" else @@ -39,7 +41,7 @@ def send_audit_event(token, response) name: 'personal_access_token_revoked', author: current_user, scope: current_user, - target: token.user, + target: token&.user, message: message } -- GitLab From 28ef806f98fde16d4bc10aa56c281a18d70a10b9 Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Fri, 20 Jan 2023 12:37:32 -0600 Subject: [PATCH 5/5] Change milestone from 15.8 to 15.9 --- ee/config/audit_events/types/personal_access_token_created.yml | 2 +- ee/config/audit_events/types/personal_access_token_revoked.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/config/audit_events/types/personal_access_token_created.yml b/ee/config/audit_events/types/personal_access_token_created.yml index 31ebf5163bc242..9b03dcd32b8ab6 100644 --- a/ee/config/audit_events/types/personal_access_token_created.yml +++ b/ee/config/audit_events/types/personal_access_token_created.yml @@ -4,6 +4,6 @@ description: Event triggered when a user creates a personal access token introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374113 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108952 feature_category: compliance_management -milestone: '15.8' +milestone: '15.9' saved_to_database: true streamed: true diff --git a/ee/config/audit_events/types/personal_access_token_revoked.yml b/ee/config/audit_events/types/personal_access_token_revoked.yml index dace6db5c22609..57c700aff89917 100644 --- a/ee/config/audit_events/types/personal_access_token_revoked.yml +++ b/ee/config/audit_events/types/personal_access_token_revoked.yml @@ -4,6 +4,6 @@ description: Event triggered when a personal access token is revoked introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374113 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108952 feature_category: compliance_management -milestone: '15.8' +milestone: '15.9' saved_to_database: true streamed: true -- GitLab