Commit debeab0f authored by James Lopez's avatar James Lopez

Merge branch '229914-deprecate-audit-event-type' into 'master'

Stop writing to type column in audit_events table

See merge request gitlab-org/gitlab!39082
parents 3a5cc77e 0eeac118
...@@ -7,6 +7,7 @@ class AuditEvent < ApplicationRecord ...@@ -7,6 +7,7 @@ class AuditEvent < ApplicationRecord
PARALLEL_PERSISTENCE_COLUMNS = [:author_name, :entity_path, :target_details].freeze PARALLEL_PERSISTENCE_COLUMNS = [:author_name, :entity_path, :target_details].freeze
ignore_column :type, remove_with: '13.6', remove_after: '2020-11-22'
ignore_column :updated_at, remove_with: '13.4', remove_after: '2020-09-22' ignore_column :updated_at, remove_with: '13.4', remove_after: '2020-09-22'
serialize :details, Hash # rubocop:disable Cop/ActiveRecordSerialize serialize :details, Hash # rubocop:disable Cop/ActiveRecordSerialize
...@@ -29,6 +30,14 @@ class AuditEvent < ApplicationRecord ...@@ -29,6 +30,14 @@ class AuditEvent < ApplicationRecord
# https://gitlab.com/groups/gitlab-org/-/epics/2765 # https://gitlab.com/groups/gitlab-org/-/epics/2765
after_validation :parallel_persist after_validation :parallel_persist
# Note: After loading records, do not attempt to type cast objects it finds.
# We are in the process of deprecating STI (i.e. SecurityEvent) out of AuditEvent.
#
# https://gitlab.com/gitlab-org/gitlab/-/issues/216845
def self.inheritance_column
:_type_disabled
end
def self.order_by(method) def self.order_by(method)
case method.to_s case method.to_s
when 'created_asc' when 'created_asc'
......
# frozen_string_literal: true
class SecurityEvent < AuditEvent
end
...@@ -37,7 +37,7 @@ class AuditEventService ...@@ -37,7 +37,7 @@ class AuditEventService
# Writes event to a file and creates an event record in DB # Writes event to a file and creates an event record in DB
# #
# @return [SecurityEvent] persited if saves and non-persisted if fails # @return [AuditEvent] persited if saves and non-persisted if fails
def security_event def security_event
log_security_event_to_file log_security_event_to_file
log_security_event_to_database log_security_event_to_database
...@@ -81,7 +81,7 @@ class AuditEventService ...@@ -81,7 +81,7 @@ class AuditEventService
def log_security_event_to_database def log_security_event_to_database
return if Gitlab::Database.read_only? return if Gitlab::Database.read_only?
SecurityEvent.create(base_payload.merge(details: @details)) AuditEvent.create(base_payload.merge(details: @details))
end end
end end
......
...@@ -11,7 +11,7 @@ module AuditEvents ...@@ -11,7 +11,7 @@ module AuditEvents
end end
def execute def execute
SecurityEvent.new(payload) AuditEvent.new(payload)
end end
private private
......
...@@ -129,7 +129,7 @@ module EE ...@@ -129,7 +129,7 @@ module EE
# Creates an event record in DB # Creates an event record in DB
# #
# @return [SecurityEvent, nil] if record is persisted or nil if audit events # @return [AuditEvent, nil] if record is persisted or nil if audit events
# features are not enabled # features are not enabled
def unauth_security_event def unauth_security_event
return unless audit_events_enabled? return unless audit_events_enabled?
...@@ -145,7 +145,7 @@ module EE ...@@ -145,7 +145,7 @@ module EE
payload[:ip_address] = ip_address if admin_audit_log_enabled? payload[:ip_address] = ip_address if admin_audit_log_enabled?
SecurityEvent.create(payload) ::AuditEvent.create(payload)
end end
# Builds the @details attribute for user # Builds the @details attribute for user
......
...@@ -14,8 +14,7 @@ module EE ...@@ -14,8 +14,7 @@ module EE
end end
def attributes def attributes
base_payload.merge(type: SecurityEvent.to_s, base_payload.merge(created_at: DateTime.current,
created_at: DateTime.current,
details: @details.to_yaml) details: @details.to_yaml)
end end
......
...@@ -11,7 +11,7 @@ module Audit ...@@ -11,7 +11,7 @@ module Audit
# @option options [Object] :model object being audited # @option options [Object] :model object being audited
# @option options [Boolean] :skip_changes whether to record from/to values # @option options [Boolean] :skip_changes whether to record from/to values
# #
# @return [SecurityEvent, nil] the resulting object or nil if there is no # @return [AuditEvent, nil] the resulting object or nil if there is no
# change detected # change detected
def audit_changes(column, options = {}) def audit_changes(column, options = {})
column = options[:column] || column column = options[:column] || column
......
...@@ -18,7 +18,7 @@ RSpec.describe Admin::ApplicationsController do ...@@ -18,7 +18,7 @@ RSpec.describe Admin::ApplicationsController do
expect do expect do
post :create, params: { doorkeeper_application: create_params } post :create, params: { doorkeeper_application: create_params }
end.to change { SecurityEvent.count }.by(1) end.to change { AuditEvent.count }.by(1)
end end
end end
end end
...@@ -19,7 +19,7 @@ RSpec.describe Admin::ImpersonationsController do ...@@ -19,7 +19,7 @@ RSpec.describe Admin::ImpersonationsController do
end end
it 'creates an audit log record' do it 'creates an audit log record' do
expect { delete :destroy }.to change { SecurityEvent.count }.by(1) expect { delete :destroy }.to change { AuditEvent.count }.by(1)
end end
end end
end end
......
...@@ -98,7 +98,7 @@ RSpec.describe Admin::UsersController do ...@@ -98,7 +98,7 @@ RSpec.describe Admin::UsersController do
end end
it 'creates an audit log record' do it 'creates an audit log record' do
expect { post :impersonate, params: { id: user.username } }.to change { SecurityEvent.count }.by(1) expect { post :impersonate, params: { id: user.username } }.to change { AuditEvent.count }.by(1)
end end
end end
end end
...@@ -27,7 +27,7 @@ RSpec.describe Ldap::OmniauthCallbacksController do ...@@ -27,7 +27,7 @@ RSpec.describe Ldap::OmniauthCallbacksController do
it 'logs a failure event', retry: 0 do it 'logs a failure event', retry: 0 do
stub_licensed_features(extended_audit_events: true) stub_licensed_features(extended_audit_events: true)
expect { post provider }.to change(SecurityEvent, :count).by(1) expect { post provider }.to change(AuditEvent, :count).by(1)
end end
end end
end end
...@@ -19,7 +19,7 @@ RSpec.describe Oauth::ApplicationsController do ...@@ -19,7 +19,7 @@ RSpec.describe Oauth::ApplicationsController do
application = build(:oauth_application) application = build(:oauth_application)
application_attributes = application.attributes.merge("scopes" => []) application_attributes = application.attributes.merge("scopes" => [])
expect { post :create, params: { doorkeeper_application: application_attributes } }.to change { SecurityEvent.count }.by(1) expect { post :create, params: { doorkeeper_application: application_attributes } }.to change { AuditEvent.count }.by(1)
end end
end end
end end
......
...@@ -13,7 +13,7 @@ RSpec.describe Profiles::KeysController do ...@@ -13,7 +13,7 @@ RSpec.describe Profiles::KeysController do
key = build(:key) key = build(:key)
expect { post :create, params: { key: key.attributes } }.to change { SecurityEvent.count }.by(1) expect { post :create, params: { key: key.attributes } }.to change { AuditEvent.count }.by(1)
end end
end end
end end
...@@ -10,7 +10,7 @@ RSpec.describe Projects::RepositoriesController do ...@@ -10,7 +10,7 @@ RSpec.describe Projects::RepositoriesController do
it 'logs the audit event' do it 'logs the audit event' do
expect do expect do
get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master" }, format: "zip" get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master" }, format: "zip"
end.to change { SecurityEvent.count }.by(1) end.to change { AuditEvent.count }.by(1)
end end
end end
......
...@@ -430,7 +430,7 @@ RSpec.describe ProjectsController do ...@@ -430,7 +430,7 @@ RSpec.describe ProjectsController do
context 'when project export is enabled' do context 'when project export is enabled' do
it 'logs the audit event' do it 'logs the audit event' do
expect { request }.to change { SecurityEvent.count }.by(1) expect { request }.to change { AuditEvent.count }.by(1)
end end
end end
...@@ -440,7 +440,7 @@ RSpec.describe ProjectsController do ...@@ -440,7 +440,7 @@ RSpec.describe ProjectsController do
end end
it 'does not log an audit event' do it 'does not log an audit event' do
expect { request }.not_to change { SecurityEvent.count } expect { request }.not_to change { AuditEvent.count }
end end
end end
end end
...@@ -459,8 +459,8 @@ RSpec.describe ProjectsController do ...@@ -459,8 +459,8 @@ RSpec.describe ProjectsController do
end end
it 'logs the audit event' do it 'logs the audit event' do
expect { request }.to change { SecurityEvent.count }.by(1) expect { request }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details[:custom_message]).to eq('Project archived') expect(AuditEvent.last.details[:custom_message]).to eq('Project archived')
end end
end end
...@@ -470,7 +470,7 @@ RSpec.describe ProjectsController do ...@@ -470,7 +470,7 @@ RSpec.describe ProjectsController do
end end
it 'does not log the audit event' do it 'does not log the audit event' do
expect { request }.not_to change { SecurityEvent.count } expect { request }.not_to change { AuditEvent.count }
end end
end end
end end
...@@ -484,8 +484,8 @@ RSpec.describe ProjectsController do ...@@ -484,8 +484,8 @@ RSpec.describe ProjectsController do
end end
it 'logs the audit event' do it 'logs the audit event' do
expect { request }.to change { SecurityEvent.count }.by(1) expect { request }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details[:custom_message]).to eq('Project unarchived') expect(AuditEvent.last.details[:custom_message]).to eq('Project unarchived')
end end
end end
...@@ -495,7 +495,7 @@ RSpec.describe ProjectsController do ...@@ -495,7 +495,7 @@ RSpec.describe ProjectsController do
end end
it 'does not log the audit event' do it 'does not log the audit event' do
expect { request }.not_to change { SecurityEvent.count } expect { request }.not_to change { AuditEvent.count }
end end
end end
end end
......
...@@ -15,7 +15,7 @@ RSpec.describe 'Login' do ...@@ -15,7 +15,7 @@ RSpec.describe 'Login' do
user = create(:user, password: 'not-the-default') user = create(:user, password: 'not-the-default')
expect { gitlab_sign_in(user) } expect { gitlab_sign_in(user) }
.to change { SecurityEvent.where(entity_id: -1).count }.from(0).to(1) .to change { AuditEvent.where(entity_id: -1).count }.from(0).to(1)
end end
it 'creates a security event for an invalid OAuth login' do it 'creates a security event for an invalid OAuth login' do
...@@ -29,7 +29,7 @@ RSpec.describe 'Login' do ...@@ -29,7 +29,7 @@ RSpec.describe 'Login' do
user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml') user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')
expect { gitlab_sign_in_via('saml', user, 'wrong-uid') } expect { gitlab_sign_in_via('saml', user, 'wrong-uid') }
.to change { SecurityEvent.where(entity_id: -1).count }.from(0).to(1) .to change { AuditEvent.where(entity_id: -1).count }.from(0).to(1)
end end
describe 'smartcard authentication' do describe 'smartcard authentication' do
......
...@@ -23,7 +23,7 @@ RSpec.describe Audit::Changes do ...@@ -23,7 +23,7 @@ RSpec.describe Audit::Changes do
it 'does not call the audit event service' do it 'does not call the audit event service' do
user.update!(email: 'scrooge.mcduck@gitlab.com') user.update!(email: 'scrooge.mcduck@gitlab.com')
expect { audit! }.not_to change { SecurityEvent.count } expect { audit! }.not_to change { AuditEvent.count }
end end
end end
...@@ -33,7 +33,7 @@ RSpec.describe Audit::Changes do ...@@ -33,7 +33,7 @@ RSpec.describe Audit::Changes do
it 'does not call the audit event service' do it 'does not call the audit event service' do
user.update!(name: 'Scrooge McDuck') user.update!(name: 'Scrooge McDuck')
expect { audit! }.not_to change { SecurityEvent.count } expect { audit! }.not_to change { AuditEvent.count }
end end
end end
end end
......
...@@ -16,7 +16,7 @@ RSpec.describe EE::Audit::GroupChangesAuditor do ...@@ -16,7 +16,7 @@ RSpec.describe EE::Audit::GroupChangesAuditor do
it 'does not call the audit event service' do it 'does not call the audit event service' do
group.update!(runners_token: 'new token') group.update!(runners_token: 'new token')
expect { foo_instance.execute }.not_to change { SecurityEvent.count } expect { foo_instance.execute }.not_to change { AuditEvent.count }
end end
end end
...@@ -24,16 +24,16 @@ RSpec.describe EE::Audit::GroupChangesAuditor do ...@@ -24,16 +24,16 @@ RSpec.describe EE::Audit::GroupChangesAuditor do
it 'creates and event when the visibility change' do it 'creates and event when the visibility change' do
group.update!(visibility_level: 20) group.update!(visibility_level: 20)
expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1) expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details[:change]).to eq 'visibility' expect(AuditEvent.last.details[:change]).to eq 'visibility'
end end
it 'creates an event for project creation level change' do it 'creates an event for project creation level change' do
group.update!(project_creation_level: 0) group.update!(project_creation_level: 0)
expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1) expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
event = SecurityEvent.last event = AuditEvent.last
expect(event.details[:from]).to eq 'Maintainers' expect(event.details[:from]).to eq 'Maintainers'
expect(event.details[:to]).to eq 'No one' expect(event.details[:to]).to eq 'No one'
expect(event.details[:change]).to eq 'project_creation_level' expect(event.details[:change]).to eq 'project_creation_level'
...@@ -58,9 +58,9 @@ RSpec.describe EE::Audit::GroupChangesAuditor do ...@@ -58,9 +58,9 @@ RSpec.describe EE::Audit::GroupChangesAuditor do
group.update_attribute(column, value) group.update_attribute(column, value)
expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1) expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
event = SecurityEvent.last event = AuditEvent.last
expect(event.details[:from]).to eq data expect(event.details[:from]).to eq data
expect(event.details[:to]).to eq value expect(event.details[:to]).to eq value
expect(event.details[:change]).to eq column.to_s expect(event.details[:change]).to eq column.to_s
......
...@@ -29,7 +29,7 @@ RSpec.describe EE::Audit::ProjectChangesAuditor do ...@@ -29,7 +29,7 @@ RSpec.describe EE::Audit::ProjectChangesAuditor do
it 'does not call the audit event service' do it 'does not call the audit event service' do
project.update!(description: 'new description') project.update!(description: 'new description')
expect { foo_instance.execute }.not_to change { SecurityEvent.count } expect { foo_instance.execute }.not_to change { AuditEvent.count }
end end
end end
...@@ -37,22 +37,22 @@ RSpec.describe EE::Audit::ProjectChangesAuditor do ...@@ -37,22 +37,22 @@ RSpec.describe EE::Audit::ProjectChangesAuditor do
it 'creates an event when the visibility change' do it 'creates an event when the visibility change' do
project.update!(visibility_level: 20) project.update!(visibility_level: 20)
expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1) expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details[:change]).to eq 'visibility' expect(AuditEvent.last.details[:change]).to eq 'visibility'
end end
it 'creates an event when the name change' do it 'creates an event when the name change' do
project.update!(name: 'new name') project.update!(name: 'new name')
expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1) expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details[:change]).to eq 'name' expect(AuditEvent.last.details[:change]).to eq 'name'
end end
it 'creates an event when the path change' do it 'creates an event when the path change' do
project.update!(path: 'newpath') project.update!(path: 'newpath')
expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1) expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details[:change]).to eq 'path' expect(AuditEvent.last.details[:change]).to eq 'path'
end end
it 'creates an event when the namespace change' do it 'creates an event when the namespace change' do
...@@ -60,30 +60,30 @@ RSpec.describe EE::Audit::ProjectChangesAuditor do ...@@ -60,30 +60,30 @@ RSpec.describe EE::Audit::ProjectChangesAuditor do
project.update!(namespace: new_namespace) project.update!(namespace: new_namespace)
expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1) expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details[:change]).to eq 'namespace' expect(AuditEvent.last.details[:change]).to eq 'namespace'
end end
it 'creates an event when the repository size limit changes' do it 'creates an event when the repository size limit changes' do
project.update!(repository_size_limit: 100) project.update!(repository_size_limit: 100)
expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1) expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details[:change]).to eq 'repository_size_limit' expect(AuditEvent.last.details[:change]).to eq 'repository_size_limit'
end end
it 'creates an event when the packages enabled setting changes' do it 'creates an event when the packages enabled setting changes' do
project.update!(packages_enabled: false) project.update!(packages_enabled: false)
expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1) expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details[:change]).to eq 'packages_enabled' expect(AuditEvent.last.details[:change]).to eq 'packages_enabled'
end end
it 'creates an event when the merge requests author approval changes' do it 'creates an event when the merge requests author approval changes' do
project.update!(merge_requests_author_approval: true) project.update!(merge_requests_author_approval: true)
aggregate_failures do aggregate_failures do
expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1) expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details).to include( expect(AuditEvent.last.details).to include(
change: 'prevent merge request approval from authors', change: 'prevent merge request approval from authors',
from: true, from: true,
to: false to: false
...@@ -95,8 +95,8 @@ RSpec.describe EE::Audit::ProjectChangesAuditor do ...@@ -95,8 +95,8 @@ RSpec.describe EE::Audit::ProjectChangesAuditor do
project.update!(merge_requests_disable_committers_approval: false) project.update!(merge_requests_disable_committers_approval: false)
aggregate_failures do aggregate_failures do
expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1) expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details).to include( expect(AuditEvent.last.details).to include(
change: 'prevent merge request approval from reviewers', change: 'prevent merge request approval from reviewers',
from: true, from: true,
to: false to: false
......
...@@ -21,9 +21,9 @@ RSpec.describe EE::Audit::ProjectFeatureChangesAuditor do ...@@ -21,9 +21,9 @@ RSpec.describe EE::Audit::ProjectFeatureChangesAuditor do
new_value = ProjectFeature::DISABLED new_value = ProjectFeature::DISABLED
features.update_attribute(column, new_value) features.update_attribute(column, new_value)
expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1) expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
event = SecurityEvent.last event = AuditEvent.last
expect(event.details[:from]).to eq ::Gitlab::VisibilityLevel.level_name(previous_value) expect(event.details[:from]).to eq ::Gitlab::VisibilityLevel.level_name(previous_value)
expect(event.details[:to]).to eq ::Gitlab::VisibilityLevel.level_name(new_value) expect(event.details[:to]).to eq ::Gitlab::VisibilityLevel.level_name(new_value)
expect(event.details[:change]).to eq column expect(event.details[:change]).to eq column
......
...@@ -8,7 +8,6 @@ RSpec.describe Gitlab::BackgroundMigration::FixRubyObjectInAuditEvents, :migrati ...@@ -8,7 +8,6 @@ RSpec.describe Gitlab::BackgroundMigration::FixRubyObjectInAuditEvents, :migrati
it 'cleans up ruby/object in details field', :aggregate_failures do it 'cleans up ruby/object in details field', :aggregate_failures do
tainted_audit_event = audit_events.create!( tainted_audit_event = audit_events.create!(
author_id: -1, author_id: -1,
type: 'SecurityEvent',
entity_id: 1, entity_id: 1,
entity_type: 'User', entity_type: 'User',
details: "---\n:failed_login: STANDARD\n:author_name: hacker\n" \ details: "---\n:failed_login: STANDARD\n:author_name: hacker\n" \
...@@ -18,7 +17,6 @@ RSpec.describe Gitlab::BackgroundMigration::FixRubyObjectInAuditEvents, :migrati ...@@ -18,7 +17,6 @@ RSpec.describe Gitlab::BackgroundMigration::FixRubyObjectInAuditEvents, :migrati
clean_audit_event = audit_events.create!( clean_audit_event = audit_events.create!(
author_id: 1, author_id: 1,
type: 'SecurityEvent',
entity_id: 1, entity_id: 1,
entity_type: 'User', entity_type: 'User',
details: "---\n:failed_login: STANDARD\n:author_name: homer\n" \ details: "---\n:failed_login: STANDARD\n:author_name: homer\n" \
......
...@@ -11,7 +11,6 @@ RSpec.describe ScheduleFixRubyObjectInAuditEvents do ...@@ -11,7 +11,6 @@ RSpec.describe ScheduleFixRubyObjectInAuditEvents do
audit_events.create!( audit_events.create!(
author_id: -1, author_id: -1,
type: 'SecurityEvent',
entity_id: 1, entity_id: 1,
entity_type: 'User', entity_type: 'User',
details: "---\n:failed_login: STANDARD\n:author_name: hacker\n" \ details: "---\n:failed_login: STANDARD\n:author_name: hacker\n" \
...@@ -21,7 +20,6 @@ RSpec.describe ScheduleFixRubyObjectInAuditEvents do ...@@ -21,7 +20,6 @@ RSpec.describe ScheduleFixRubyObjectInAuditEvents do
audit_events.create!( audit_events.create!(
author_id: 1, author_id: 1,
type: 'SecurityEvent',
entity_id: 1, entity_id: 1,
entity_type: 'User', entity_type: 'User',
details: "---\n:failed_login: STANDARD\n:author_name: homer\n" \ details: "---\n:failed_login: STANDARD\n:author_name: homer\n" \
......
...@@ -25,7 +25,7 @@ RSpec.describe API::Ci::Pipelines do ...@@ -25,7 +25,7 @@ RSpec.describe API::Ci::Pipelines do
end end
it 'does not log an audit event' do it 'does not log an audit event' do
expect { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) }.not_to change { SecurityEvent.count } expect { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) }.not_to change { AuditEvent.count }
end end
end end
end end
......
...@@ -16,7 +16,7 @@ RSpec.describe API::Repositories do ...@@ -16,7 +16,7 @@ RSpec.describe API::Repositories do
it 'logs the audit event' do it 'logs the audit event' do
expect do expect do
get api(route, current_user) get api(route, current_user)
end.to change { SecurityEvent.count }.by(1) end.to change { AuditEvent.count }.by(1)
end end
it 'sends the archive' do it 'sends the archive' do
......
...@@ -12,6 +12,6 @@ RSpec.describe ::Applications::CreateService do ...@@ -12,6 +12,6 @@ RSpec.describe ::Applications::CreateService do
it 'creates an audit log' do it 'creates an audit log' do
stub_licensed_features(extended_audit_events: true) stub_licensed_features(extended_audit_events: true)
expect { subject.execute(request) }.to change { SecurityEvent.count }.by(1) expect { subject.execute(request) }.to change { AuditEvent.count }.by(1)
end end
end end
...@@ -68,15 +68,15 @@ RSpec.describe AuditEventService do ...@@ -68,15 +68,15 @@ RSpec.describe AuditEventService do
end end
it 'does not create an event' do it 'does not create an event' do
expect(SecurityEvent).not_to receive(:create) expect(AuditEvent).not_to receive(:create)
expect { service.security_event }.not_to change(SecurityEvent, :count) expect { service.security_event }.not_to change(AuditEvent, :count)
end end
end end
context 'licensed' do context 'licensed' do
it 'creates an event' do it 'creates an event' do
expect { service.security_event }.to change(SecurityEvent, :count).by(1) expect { service.security_event }.to change(AuditEvent, :count).by(1)
end end
context 'on a read-only instance' do context 'on a read-only instance' do
...@@ -85,7 +85,7 @@ RSpec.describe AuditEventService do ...@@ -85,7 +85,7 @@ RSpec.describe AuditEventService do
end end
it 'does not create an event' do it 'does not create an event' do
expect { service.security_event }.not_to change(SecurityEvent, :count) expect { service.security_event }.not_to change(AuditEvent, :count)
end end
end end
end end
......
...@@ -15,7 +15,7 @@ RSpec.describe ::Ci::DestroyPipelineService do ...@@ -15,7 +15,7 @@ RSpec.describe ::Ci::DestroyPipelineService do
end end
it 'does not log an audit event' do it 'does not log an audit event' do
expect { subject }.not_to change { SecurityEvent.count } expect { subject }.not_to change { AuditEvent.count }
end end
end end
end end
...@@ -21,7 +21,6 @@ RSpec.describe EE::AuditEvents::BulkInsertService do ...@@ -21,7 +21,6 @@ RSpec.describe EE::AuditEvents::BulkInsertService do
author_id: user.id, author_id: user.id,
entity_id: entity.id, entity_id: entity.id,
entity_type: entity_type, entity_type: entity_type,
type: 'SecurityEvent',
created_at: timestamp, created_at: timestamp,
details: { details: {
updated_ref: 'master', updated_ref: 'master',
...@@ -39,9 +38,9 @@ RSpec.describe EE::AuditEvents::BulkInsertService do ...@@ -39,9 +38,9 @@ RSpec.describe EE::AuditEvents::BulkInsertService do
it 'persists audit events' do it 'persists audit events' do
Timecop.freeze(timestamp) { service.execute } Timecop.freeze(timestamp) { service.execute }
events_attributes = SecurityEvent.all.map { |event| event.attributes.deep_symbolize_keys } events_attributes = AuditEvent.all.map { |event| event.attributes.deep_symbolize_keys }
expect(SecurityEvent.count).to eq(3) expect(AuditEvent.count).to eq(3)
expect(events_attributes).to all(include(attrs)) expect(events_attributes).to all(include(attrs))
end end
......
...@@ -24,8 +24,8 @@ RSpec.describe EE::AuditEvents::ImpersonationAuditEventService do ...@@ -24,8 +24,8 @@ RSpec.describe EE::AuditEvents::ImpersonationAuditEventService do
ip_address: ip_address, ip_address: ip_address,
custom_message: message) custom_message: message)
expect { service.security_event }.to change(SecurityEvent, :count).by(1) expect { service.security_event }.to change(AuditEvent, :count).by(1)
security_event = SecurityEvent.last security_event = AuditEvent.last
expect(security_event.details).to eq(custom_message: message, expect(security_event.details).to eq(custom_message: message,
ip_address: ip_address, ip_address: ip_address,
......
...@@ -35,9 +35,9 @@ RSpec.describe EE::AuditEvents::ProtectedBranchAuditEventService do ...@@ -35,9 +35,9 @@ RSpec.describe EE::AuditEvents::ProtectedBranchAuditEventService do
action => 'protected_branch', action => 'protected_branch',
ip_address: '127.0.0.1') ip_address: '127.0.0.1')
expect { service.security_event }.to change(SecurityEvent, :count).by(1) expect { service.security_event }.to change(AuditEvent, :count).by(1)
security_event = SecurityEvent.last security_event = AuditEvent.last
expect(security_event.details).to eq(action => 'protected_branch', expect(security_event.details).to eq(action => 'protected_branch',
author_name: author.name, author_name: author.name,
...@@ -73,7 +73,7 @@ RSpec.describe EE::AuditEvents::ProtectedBranchAuditEventService do ...@@ -73,7 +73,7 @@ RSpec.describe EE::AuditEvents::ProtectedBranchAuditEventService do
it "doesn't create an event or log to a file" do it "doesn't create an event or log to a file" do
expect(service).not_to receive(:file_logger) expect(service).not_to receive(:file_logger)
expect { service.security_event }.not_to change(SecurityEvent, :count) expect { service.security_event }.not_to change(AuditEvent, :count)
end end
end end
end end
......
...@@ -23,7 +23,6 @@ RSpec.describe EE::AuditEvents::RepositoryPushAuditEventService do ...@@ -23,7 +23,6 @@ RSpec.describe EE::AuditEvents::RepositoryPushAuditEventService do
author_name: user.name, author_name: user.name,
entity_id: entity.id, entity_id: entity.id,
entity_type: entity_type, entity_type: entity_type,
type: 'SecurityEvent',
created_at: timestamp, created_at: timestamp,
ip_address: '127.0.0.1', ip_address: '127.0.0.1',
details: { details: {
......
...@@ -15,7 +15,7 @@ RSpec.describe Members::DestroyService do ...@@ -15,7 +15,7 @@ RSpec.describe Members::DestroyService do
shared_examples_for 'logs an audit event' do shared_examples_for 'logs an audit event' do
specify do specify do
expect { event }.to change { SecurityEvent.count }.by(1) expect { event }.to change { AuditEvent.count }.by(1)
end end
end end
......
...@@ -22,7 +22,7 @@ RSpec.describe Members::UpdateService do ...@@ -22,7 +22,7 @@ RSpec.describe Members::UpdateService do
specify do specify do
expect do expect do
described_class.new(current_user, params).execute(member, permission: permission) described_class.new(current_user, params).execute(member, permission: permission)
end.to change { SecurityEvent.count }.by(1) end.to change { AuditEvent.count }.by(1)
end end
end end
......
...@@ -89,14 +89,14 @@ RSpec.describe ProtectedBranches::CreateService do ...@@ -89,14 +89,14 @@ RSpec.describe ProtectedBranches::CreateService do
end end
it 'adds a security audit event entry' do it 'adds a security audit event entry' do
expect { service.execute }.to change(::SecurityEvent, :count).by(1) expect { service.execute }.to change(::AuditEvent, :count).by(1)
end end
context 'with invalid params' do context 'with invalid params' do
let(:params) { nil } let(:params) { nil }
it "doesn't add a security audit event entry" do it "doesn't add a security audit event entry" do
expect { service.execute }.not_to change(::SecurityEvent, :count) expect { service.execute }.not_to change(::AuditEvent, :count)
end end
end end
end end
......
...@@ -12,7 +12,7 @@ RSpec.describe ProtectedBranches::DestroyService do ...@@ -12,7 +12,7 @@ RSpec.describe ProtectedBranches::DestroyService do
subject(:service) { described_class.new(project, user) } subject(:service) { described_class.new(project, user) }
it 'adds a security audit event entry' do it 'adds a security audit event entry' do
expect { service.execute(protected_branch) }.to change(::SecurityEvent, :count).by(1) expect { service.execute(protected_branch) }.to change(::AuditEvent, :count).by(1)
end end
context 'when destroy fails' do context 'when destroy fails' do
...@@ -21,7 +21,7 @@ RSpec.describe ProtectedBranches::DestroyService do ...@@ -21,7 +21,7 @@ RSpec.describe ProtectedBranches::DestroyService do
end end
it "doesn't add a security audit event entry" do it "doesn't add a security audit event entry" do
expect { service.execute(protected_branch) }.not_to change(::SecurityEvent, :count) expect { service.execute(protected_branch) }.not_to change(::AuditEvent, :count)
end end
end end
end end
......
...@@ -20,7 +20,7 @@ RSpec.describe ProtectedBranches::UpdateService do ...@@ -20,7 +20,7 @@ RSpec.describe ProtectedBranches::UpdateService do
subject(:service) { described_class.new(project, user, params) } subject(:service) { described_class.new(project, user, params) }
it 'adds a security audit event entry' do it 'adds a security audit event entry' do
expect { service.execute(protected_branch) }.to change(::SecurityEvent, :count).by(1) expect { service.execute(protected_branch) }.to change(::AuditEvent, :count).by(1)
end end
context 'with invalid params' do context 'with invalid params' do
...@@ -32,7 +32,7 @@ RSpec.describe ProtectedBranches::UpdateService do ...@@ -32,7 +32,7 @@ RSpec.describe ProtectedBranches::UpdateService do
end end
it "doesn't add a security audit event entry" do it "doesn't add a security audit event entry" do
expect { service.execute(protected_branch) }.not_to change(::SecurityEvent, :count) expect { service.execute(protected_branch) }.not_to change(::AuditEvent, :count)
end end
end end
end end
......
...@@ -12,7 +12,7 @@ RSpec.describe Emails::CreateService do ...@@ -12,7 +12,7 @@ RSpec.describe Emails::CreateService do
it 'registers a security event' do it 'registers a security event' do
stub_licensed_features(extended_audit_events: true) stub_licensed_features(extended_audit_events: true)
expect { service.execute }.to change { SecurityEvent.count }.by(1) expect { service.execute }.to change { AuditEvent.count }.by(1)
end end
end end
end end
...@@ -12,7 +12,7 @@ RSpec.describe Emails::DestroyService do ...@@ -12,7 +12,7 @@ RSpec.describe Emails::DestroyService do
it 'registers a security event' do it 'registers a security event' do
stub_licensed_features(extended_audit_events: true) stub_licensed_features(extended_audit_events: true)
expect { service.execute(email) }.to change { SecurityEvent.count }.by(1) expect { service.execute(email) }.to change { AuditEvent.count }.by(1)
end end
end end
end end
...@@ -50,6 +50,6 @@ RSpec.describe GroupSaml::Identity::DestroyService do ...@@ -50,6 +50,6 @@ RSpec.describe GroupSaml::Identity::DestroyService do
it 'logs an audit event' do it 'logs an audit event' do
expect do expect do
subject.execute subject.execute
end.to change { SecurityEvent.count }.by(1) end.to change { AuditEvent.count }.by(1)
end end
end end
...@@ -12,9 +12,9 @@ RSpec.describe Keys::CreateService do ...@@ -12,9 +12,9 @@ RSpec.describe Keys::CreateService do
it 'creates' do it 'creates' do
stub_licensed_features(extended_audit_events: true) stub_licensed_features(extended_audit_events: true)
expect { subject.execute }.to change { SecurityEvent.count }.by(1) expect { subject.execute }.to change { AuditEvent.count }.by(1)
event = SecurityEvent.last event = AuditEvent.last
expect(event.author_name).to eq(admin.name) expect(event.author_name).to eq(admin.name)
expect(event.entity_id).to eq(user.id) expect(event.entity_id).to eq(user.id)
......
...@@ -23,7 +23,7 @@ RSpec.describe Projects::ImportService do ...@@ -23,7 +23,7 @@ RSpec.describe Projects::ImportService do
end end
it 'does audit' do it 'does audit' do
expect { subject.execute }.to change { SecurityEvent.count }.by(1) expect { subject.execute }.to change { AuditEvent.count }.by(1)
end end
end end
...@@ -34,7 +34,7 @@ RSpec.describe Projects::ImportService do ...@@ -34,7 +34,7 @@ RSpec.describe Projects::ImportService do
end end
it 'does not audit' do it 'does not audit' do
expect { subject.execute }.not_to change { SecurityEvent.count } expect { subject.execute }.not_to change { AuditEvent.count }
end end
end end
end end
...@@ -48,7 +48,7 @@ RSpec.describe Projects::ImportService do ...@@ -48,7 +48,7 @@ RSpec.describe Projects::ImportService do
end end
it 'does not audit' do it 'does not audit' do
expect { subject.execute }.not_to change { SecurityEvent.count } expect { subject.execute }.not_to change { AuditEvent.count }
end end
end end
...@@ -58,7 +58,7 @@ RSpec.describe Projects::ImportService do ...@@ -58,7 +58,7 @@ RSpec.describe Projects::ImportService do
end end
it 'does not audit' do it 'does not audit' do
expect { subject.execute }.not_to change { SecurityEvent.count } expect { subject.execute }.not_to change { AuditEvent.count }
end end
end end
end end
......
...@@ -57,8 +57,8 @@ RSpec.shared_examples 'logs the custom audit event' do ...@@ -57,8 +57,8 @@ RSpec.shared_examples 'logs the custom audit event' do
ip_address: ip_address, ip_address: ip_address,
custom_message: custom_message) custom_message: custom_message)
expect { service.security_event }.to change(SecurityEvent, :count).by(1) expect { service.security_event }.to change(AuditEvent, :count).by(1)
security_event = SecurityEvent.last security_event = AuditEvent.last
expect(security_event.details).to eq(custom_message: custom_message, expect(security_event.details).to eq(custom_message: custom_message,
ip_address: ip_address, ip_address: ip_address,
...@@ -97,9 +97,9 @@ RSpec.shared_examples 'logs the release audit event' do ...@@ -97,9 +97,9 @@ RSpec.shared_examples 'logs the release audit event' do
target_id: target_id, target_id: target_id,
target_type: target_type) target_type: target_type)
expect { service.security_event }.to change(SecurityEvent, :count).by(1) expect { service.security_event }.to change(AuditEvent, :count).by(1)
security_event = SecurityEvent.last security_event = AuditEvent.last
expect(security_event.details).to eq(custom_message: custom_message, expect(security_event.details).to eq(custom_message: custom_message,
ip_address: ip_address, ip_address: ip_address,
......
...@@ -47,7 +47,7 @@ RSpec.describe RepositoryPushAuditEventWorker do ...@@ -47,7 +47,7 @@ RSpec.describe RepositoryPushAuditEventWorker do
to: '210987', to: '210987',
target_details: project.full_path) target_details: project.full_path)
events = SecurityEvent.all.map do |event| events = AuditEvent.all.map do |event|
event event
.attributes .attributes
.deep_symbolize_keys .deep_symbolize_keys
...@@ -71,7 +71,7 @@ RSpec.describe RepositoryPushAuditEventWorker do ...@@ -71,7 +71,7 @@ RSpec.describe RepositoryPushAuditEventWorker do
expect(instance).to receive(:enabled?) { false } expect(instance).to receive(:enabled?) { false }
end end
expect { subject }.not_to change(SecurityEvent, :count) expect { subject }.not_to change(AuditEvent, :count)
end end
end end
end end
......
...@@ -130,8 +130,8 @@ RSpec.describe SessionsController do ...@@ -130,8 +130,8 @@ RSpec.describe SessionsController do
end end
it 'creates an audit log record' do it 'creates an audit log record' do
expect { post(:create, params: { user: user_params }) }.to change { SecurityEvent.count }.by(1) expect { post(:create, params: { user: user_params }) }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details[:with]).to eq('standard') expect(AuditEvent.last.details[:with]).to eq('standard')
end end
include_examples 'user login request with unique ip limit', 302 do include_examples 'user login request with unique ip limit', 302 do
...@@ -396,8 +396,8 @@ RSpec.describe SessionsController do ...@@ -396,8 +396,8 @@ RSpec.describe SessionsController do
end end
it "creates an audit log record" do it "creates an audit log record" do
expect { authenticate_2fa(login: user.username, otp_attempt: user.current_otp) }.to change { SecurityEvent.count }.by(1) expect { authenticate_2fa(login: user.username, otp_attempt: user.current_otp) }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details[:with]).to eq("two-factor") expect(AuditEvent.last.details[:with]).to eq("two-factor")
end end
end end
...@@ -433,8 +433,8 @@ RSpec.describe SessionsController do ...@@ -433,8 +433,8 @@ RSpec.describe SessionsController do
it "creates an audit log record" do it "creates an audit log record" do
allow(U2fRegistration).to receive(:authenticate).and_return(true) allow(U2fRegistration).to receive(:authenticate).and_return(true)
expect { authenticate_2fa_u2f(login: user.username, device_response: "{}") }.to change { SecurityEvent.count }.by(1) expect { authenticate_2fa_u2f(login: user.username, device_response: "{}") }.to change { AuditEvent.count }.by(1)
expect(SecurityEvent.last.details[:with]).to eq("two-factor-via-u2f-device") expect(AuditEvent.last.details[:with]).to eq("two-factor-via-u2f-device")
end end
end end
end end
......
# frozen_string_literal: true # frozen_string_literal: true
FactoryBot.define do FactoryBot.define do
factory :audit_event, class: 'SecurityEvent', aliases: [:user_audit_event] do factory :audit_event, class: 'AuditEvent', aliases: [:user_audit_event] do
user user
transient { target_user { create(:user) } } transient { target_user { create(:user) } }
......
...@@ -7,7 +7,10 @@ RSpec.describe AuditEventPartitioned do ...@@ -7,7 +7,10 @@ RSpec.describe AuditEventPartitioned do
let(:partitioned_table) { described_class } let(:partitioned_table) { described_class }
it 'has the same columns as the source table' do it 'has the same columns as the source table' do
expect(partitioned_table.column_names).to match_array(source_table.column_names) column_names_from_source_table = column_names(source_table)
column_names_from_partioned_table = column_names(partitioned_table)
expect(column_names_from_partioned_table).to match_array(column_names_from_source_table)
end end
it 'has the same null constraints as the source table' do it 'has the same null constraints as the source table' do
...@@ -30,6 +33,14 @@ RSpec.describe AuditEventPartitioned do ...@@ -30,6 +33,14 @@ RSpec.describe AuditEventPartitioned do
expect(event_from_partitioned_table).to eq(event_from_source_table) expect(event_from_partitioned_table).to eq(event_from_source_table)
end end
def column_names(table)
table.connection.select_all(<<~SQL)
SELECT c.column_name
FROM information_schema.columns c
WHERE c.table_name = '#{table.table_name}'
SQL
end
def null_constraints(table) def null_constraints(table)
table.connection.select_all(<<~SQL) table.connection.select_all(<<~SQL)
SELECT c.column_name, c.is_nullable SELECT c.column_name, c.is_nullable
......
...@@ -624,7 +624,7 @@ RSpec.describe API::Ci::Pipelines do ...@@ -624,7 +624,7 @@ RSpec.describe API::Ci::Pipelines do
end end
it 'does not log an audit event' do it 'does not log an audit event' do
expect { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) }.not_to change { SecurityEvent.count } expect { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) }.not_to change { AuditEvent.count }
end end
context 'when the pipeline has jobs' do context 'when the pipeline has jobs' do
......
...@@ -22,7 +22,7 @@ RSpec.describe AuditEventService do ...@@ -22,7 +22,7 @@ RSpec.describe AuditEventService do
entity_type: "Project", entity_type: "Project",
action: :destroy) action: :destroy)
expect { service.security_event }.to change(SecurityEvent, :count).by(1) expect { service.security_event }.to change(AuditEvent, :count).by(1)
end end
it 'formats from and to fields' do it 'formats from and to fields' do
...@@ -44,9 +44,9 @@ RSpec.describe AuditEventService do ...@@ -44,9 +44,9 @@ RSpec.describe AuditEventService do
action: :create, action: :create,
target_id: 1) target_id: 1)
expect { service.security_event }.to change(SecurityEvent, :count).by(1) expect { service.security_event }.to change(AuditEvent, :count).by(1)
details = SecurityEvent.last.details details = AuditEvent.last.details
expect(details[:from]).to be true expect(details[:from]).to be true
expect(details[:to]).to be false expect(details[:to]).to be false
expect(details[:action]).to eq(:create) expect(details[:action]).to eq(:create)
......
...@@ -29,7 +29,7 @@ RSpec.describe ::Ci::DestroyPipelineService do ...@@ -29,7 +29,7 @@ RSpec.describe ::Ci::DestroyPipelineService do
end end
it 'does not log an audit event' do it 'does not log an audit event' do
expect { subject }.not_to change { SecurityEvent.count } expect { subject }.not_to change { AuditEvent.count }
end end
context 'when the pipeline has jobs' do context 'when the pipeline has jobs' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment