diff --git a/config/feature_flags/gitlab_com_derisk/sync_audit_events_to_new_tables.yml b/config/feature_flags/gitlab_com_derisk/sync_audit_events_to_new_tables.yml deleted file mode 100644 index c01dca0d0a3fc0a4010eaea269561c7e71367971..0000000000000000000000000000000000000000 --- a/config/feature_flags/gitlab_com_derisk/sync_audit_events_to_new_tables.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -name: sync_audit_events_to_new_tables -feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/454175 -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/158202 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/470481 -milestone: '17.3' -group: group::compliance -type: gitlab_com_derisk -default_enabled: false diff --git a/ee/spec/lib/gitlab/audit/auditor_spec.rb b/ee/spec/lib/gitlab/audit/auditor_spec.rb index 414871bcc87937c37137fa6f37ece01a7dc356c4..46322669c9880a477c59c9da3e82be70e284674e 100644 --- a/ee/spec/lib/gitlab/audit/auditor_spec.rb +++ b/ee/spec/lib/gitlab/audit/auditor_spec.rb @@ -81,48 +81,6 @@ end end - shared_examples 'when sync_audit_events_to_new_tables is disabled does not create audit event' do - before do - stub_feature_flags(sync_audit_events_to_new_tables: false) - end - - context "when entity type is 'Project'" do - let(:scope) { build_stubbed(:project) } - let(:target) { scope } - - it 'does not sync audit event into "AuditEvents::ProjectAuditEvent"' do - expect { audit! }.to change { AuditEvents::ProjectAuditEvent.count }.by(0) - end - end - - context "when entity type is 'Group'" do - let(:scope) { build_stubbed(:group) } - let(:target) { scope } - - it 'does not sync audit event into "AuditEvents::GroupAuditEvent"' do - expect { audit! }.to change { AuditEvents::GroupAuditEvent.count }.by(0) - end - end - - context "when entity type is 'User'" do - let(:scope) { build_stubbed(:user) } - let(:target) { scope } - - it 'does not sync audit event into "AuditEvents::UserAuditEvent"' do - expect { audit! }.to change { AuditEvents::UserAuditEvent.count }.by(0) - end - end - - context "when entity type is 'Gitlab::Audit::InstanceScope'" do - let(:scope) { Gitlab::Audit::InstanceScope.new } - let(:target) { build_stubbed(:user) } - - it 'syncs audit event into "AuditEvents::InstanceAuditEvent"' do - expect { audit! }.to change { ::AuditEvents::InstanceAuditEvent.count }.by(0) - end - end - end - shared_examples 'when audit event is invalid' do let(:scope) { Gitlab::Audit::InstanceScope.new } let(:target) { build_stubbed(:user) } @@ -391,63 +349,55 @@ it_behaves_like 'only streamed' end - it_behaves_like "when sync_audit_events_to_new_tables is disabled does not create audit event" + context "when entity type is 'Project'" do + let(:scope) { build_stubbed(:project) } + let(:target) { scope } + let(:expected_count) { 2 } - context 'when sync_audit_events_to_new_tables is enabled' do - before do - stub_feature_flags(sync_audit_events_to_new_tables: true) - end + it_behaves_like 'common audit event attributes', 'AuditEvents::ProjectAuditEvent', 'project' + end - context "when entity type is 'Project'" do - let(:scope) { build_stubbed(:project) } - let(:target) { scope } - let(:expected_count) { 2 } + context "when entity type is 'Group'" do + let(:scope) { build_stubbed(:group) } + let(:target) { scope } + let(:expected_count) { 2 } - it_behaves_like 'common audit event attributes', 'AuditEvents::ProjectAuditEvent', 'project' - end + it_behaves_like 'common audit event attributes', 'AuditEvents::GroupAuditEvent', 'group' + end + + context "when entity type is 'User'" do + let(:scope) { build_stubbed(:user) } + let(:target) { scope } + let(:expected_count) { 2 } - context "when entity type is 'Group'" do - let(:scope) { build_stubbed(:group) } - let(:target) { scope } - let(:expected_count) { 2 } + it_behaves_like 'common audit event attributes', 'AuditEvents::UserAuditEvent', 'user' + end - it_behaves_like 'common audit event attributes', 'AuditEvents::GroupAuditEvent', 'group' - end + context "when entity type is 'Gitlab::Audit::InstanceScope'" do + let(:scope) { Gitlab::Audit::InstanceScope.new } + let(:target) { build_stubbed(:user) } - context "when entity type is 'User'" do - let(:scope) { build_stubbed(:user) } - let(:target) { scope } - let(:expected_count) { 2 } + it 'syncs audit event into "AuditEvents::InstanceAuditEvent"', :aggregate_failures do + expect { audit! }.to change { ::AuditEvents::InstanceAuditEvent.count }.by(2) - it_behaves_like 'common audit event attributes', 'AuditEvents::UserAuditEvent', 'user' - end + created_audit_events = ::AuditEvents::InstanceAuditEvent.order(:id).limit(2) + audit_events = AuditEvent.order(:id).limit(2) - context "when entity type is 'Gitlab::Audit::InstanceScope'" do - let(:scope) { Gitlab::Audit::InstanceScope.new } - let(:target) { build_stubbed(:user) } - - it 'syncs audit event into "AuditEvents::InstanceAuditEvent"', :aggregate_failures do - expect { audit! }.to change { ::AuditEvents::InstanceAuditEvent.count }.by(2) - - created_audit_events = ::AuditEvents::InstanceAuditEvent.order(:id).limit(2) - audit_events = AuditEvent.order(:id).limit(2) - - created_audit_events.zip(audit_events).each do |created_event, audit_event| - expect(created_event).to have_attributes( - id: audit_event.id, - author_id: author.id, - target_id: target.id, - event_name: name, - author_name: author.name, - entity_path: "gitlab_instance", - target_details: target.name, - target_type: "User") - end + created_audit_events.zip(audit_events).each do |created_event, audit_event| + expect(created_event).to have_attributes( + id: audit_event.id, + author_id: author.id, + target_id: target.id, + event_name: name, + author_name: author.name, + entity_path: "gitlab_instance", + target_details: target.name, + target_type: "User") end end - - it_behaves_like 'when audit event is invalid' end + + it_behaves_like 'when audit event is invalid' end context 'when recording single event' do @@ -476,60 +426,52 @@ it_behaves_like 'logs event to database' - it_behaves_like "when sync_audit_events_to_new_tables is disabled does not create audit event" - - context 'when sync_audit_events_to_new_tables is enabled' do - before do - stub_feature_flags(sync_audit_events_to_new_tables: true) - end - - context "when entity type is 'Project'" do - let(:scope) { build_stubbed(:project) } - let(:target) { scope } - let(:expected_count) { 1 } + context "when entity type is 'Project'" do + let(:scope) { build_stubbed(:project) } + let(:target) { scope } + let(:expected_count) { 1 } - it_behaves_like 'common audit event attributes', 'AuditEvents::ProjectAuditEvent', 'project' - end + it_behaves_like 'common audit event attributes', 'AuditEvents::ProjectAuditEvent', 'project' + end - context "when entity type is 'Group'" do - let(:scope) { build_stubbed(:group) } - let(:target) { scope } - let(:expected_count) { 1 } + context "when entity type is 'Group'" do + let(:scope) { build_stubbed(:group) } + let(:target) { scope } + let(:expected_count) { 1 } - it_behaves_like 'common audit event attributes', 'AuditEvents::GroupAuditEvent', 'group' - end + it_behaves_like 'common audit event attributes', 'AuditEvents::GroupAuditEvent', 'group' + end - context "when entity type is 'User'" do - let(:scope) { build_stubbed(:user) } - let(:target) { scope } - let(:expected_count) { 1 } + context "when entity type is 'User'" do + let(:scope) { build_stubbed(:user) } + let(:target) { scope } + let(:expected_count) { 1 } - it_behaves_like 'common audit event attributes', 'AuditEvents::UserAuditEvent', 'user' - end + it_behaves_like 'common audit event attributes', 'AuditEvents::UserAuditEvent', 'user' + end - context "when entity type is 'Gitlab::Audit::InstanceScope'" do - let(:scope) { Gitlab::Audit::InstanceScope.new } - let(:target) { build_stubbed(:user) } + context "when entity type is 'Gitlab::Audit::InstanceScope'" do + let(:scope) { Gitlab::Audit::InstanceScope.new } + let(:target) { build_stubbed(:user) } - it 'syncs audit event into "AuditEvents::InstanceAuditEvent"', :aggregate_failures do - expect { audit! }.to change { ::AuditEvents::InstanceAuditEvent.count }.by(1) + it 'syncs audit event into "AuditEvents::InstanceAuditEvent"', :aggregate_failures do + expect { audit! }.to change { ::AuditEvents::InstanceAuditEvent.count }.by(1) - expect(::AuditEvents::InstanceAuditEvent.last).to have_attributes( - id: AuditEvent.last.id, - author_id: author.id, - target_id: target.id, - event_name: name, - author_name: author.name, - entity_path: "gitlab_instance", - target_details: target.name, - target_type: "User" - ) - end + expect(::AuditEvents::InstanceAuditEvent.last).to have_attributes( + id: AuditEvent.last.id, + author_id: author.id, + target_id: target.id, + event_name: name, + author_name: author.name, + entity_path: "gitlab_instance", + target_details: target.name, + target_type: "User" + ) end - - it_behaves_like 'when audit event is invalid' end + it_behaves_like 'when audit event is invalid' + it 'does not bulk insert and uses save to insert' do expect(AuditEvent).not_to receive(:bulk_insert!) expect_next_instance_of(AuditEvent) do |instance| diff --git a/ee/spec/services/audit_event_service_spec.rb b/ee/spec/services/audit_event_service_spec.rb index 780fddf63b4751a7f715a96dec4b7f1e72515262..e91f8ef87c8764660e1fc63adb59c728e0ef62e1 100644 --- a/ee/spec/services/audit_event_service_spec.rb +++ b/ee/spec/services/audit_event_service_spec.rb @@ -94,140 +94,83 @@ expect { service.security_event }.to change(AuditEvent, :count).by(1) end - context 'when sync_audit_events_to_new_tables is disabled' do - before do - stub_feature_flags(sync_audit_events_to_new_tables: false) - end - - context 'entity is a project' do - let(:service) { described_class.new(user, project, { action: :destroy }) } + context 'entity is a project' do + let(:service) { described_class.new(user, project, { action: :destroy }) } - it 'does not create audit event in project audit events' do - expect do - service.security_event - end.to change(AuditEvents::ProjectAuditEvent, :count).by(0).and change(AuditEvent, :count).by(1) - end - end + it 'creates audit event in project audit events' do + expect do + service.security_event + end.to change(AuditEvents::ProjectAuditEvent, :count).by(1).and change(AuditEvent, :count).by(1) - context 'entity is a group' do - let(:group) { create(:group) } - let(:service) { described_class.new(user, group, { action: :destroy }) } + event = AuditEvents::ProjectAuditEvent.last + audit_event = AuditEvent.last - it 'does not create audit event in group audit events' do - expect do - service.security_event - end.to change(AuditEvents::GroupAuditEvent, :count).by(0).and change(AuditEvent, :count).by(1) - end + expect(event).to have_attributes( + id: audit_event.id, + project_id: project.id, + author_id: user.id, + author_name: user.name) end + end - context 'entity is a user' do - before do - stub_licensed_features(extended_audit_events: true) - end - - let(:service) { described_class.new(user, user, { action: :destroy }) } + context 'entity is a group' do + let(:group) { create(:group) } + let(:service) { described_class.new(user, group, { action: :destroy }) } - it 'does not create audit event in user audit events' do - expect do - service.security_event - end.to change(AuditEvents::UserAuditEvent, :count).by(0).and change(AuditEvent, :count).by(1) - end - end + it 'creates audit event in group audit events' do + expect do + service.security_event + end.to change(AuditEvents::GroupAuditEvent, :count).by(1).and change(AuditEvent, :count).by(1) - context 'entity is instance scope' do - let(:service) { described_class.new(user, ::Gitlab::Audit::InstanceScope.new, { action: :destroy }) } + event = AuditEvents::GroupAuditEvent.last + audit_event = AuditEvent.last - it 'does not create audit event in instance audit events' do - expect do - service.security_event - end.to change(AuditEvents::InstanceAuditEvent, :count).by(0).and change(AuditEvent, :count).by(1) - end + expect(event).to have_attributes( + id: audit_event.id, + group_id: group.id, + author_id: user.id, + author_name: user.name) end end - context 'when sync_audit_events_to_new_tables is enabled' do + context 'entity is a user' do before do - stub_feature_flags(sync_audit_events_to_new_tables: true) + stub_licensed_features(extended_audit_events: true) end - context 'entity is a project' do - let(:service) { described_class.new(user, project, { action: :destroy }) } - - it 'creates audit event in project audit events' do - expect do - service.security_event - end.to change(AuditEvents::ProjectAuditEvent, :count).by(1).and change(AuditEvent, :count).by(1) + let(:service) { described_class.new(user, user, { action: :destroy }) } - event = AuditEvents::ProjectAuditEvent.last - audit_event = AuditEvent.last - - expect(event).to have_attributes( - id: audit_event.id, - project_id: project.id, - author_id: user.id, - author_name: user.name) - end - end - - context 'entity is a group' do - let(:group) { create(:group) } - let(:service) { described_class.new(user, group, { action: :destroy }) } - - it 'creates audit event in group audit events' do - expect do - service.security_event - end.to change(AuditEvents::GroupAuditEvent, :count).by(1).and change(AuditEvent, :count).by(1) - - event = AuditEvents::GroupAuditEvent.last - audit_event = AuditEvent.last - - expect(event).to have_attributes( - id: audit_event.id, - group_id: group.id, - author_id: user.id, - author_name: user.name) - end - end - - context 'entity is a user' do - before do - stub_licensed_features(extended_audit_events: true) - end - - let(:service) { described_class.new(user, user, { action: :destroy }) } - - it 'creates audit event in user audit events' do - expect do - service.security_event - end.to change(AuditEvents::UserAuditEvent, :count).by(1).and change(AuditEvent, :count).by(1) + it 'creates audit event in user audit events' do + expect do + service.security_event + end.to change(AuditEvents::UserAuditEvent, :count).by(1).and change(AuditEvent, :count).by(1) - event = AuditEvents::UserAuditEvent.last - audit_event = AuditEvent.last + event = AuditEvents::UserAuditEvent.last + audit_event = AuditEvent.last - expect(event).to have_attributes( - id: audit_event.id, - user_id: user.id, - author_id: user.id, - author_name: user.name) - end + expect(event).to have_attributes( + id: audit_event.id, + user_id: user.id, + author_id: user.id, + author_name: user.name) end + end - context 'entity is instance scope' do - let(:service) { described_class.new(user, ::Gitlab::Audit::InstanceScope.new, { action: :destroy }) } + context 'entity is instance scope' do + let(:service) { described_class.new(user, ::Gitlab::Audit::InstanceScope.new, { action: :destroy }) } - it 'creates audit event in user audit events' do - expect do - service.security_event - end.to change(AuditEvents::InstanceAuditEvent, :count).by(1).and change(AuditEvent, :count).by(1) + it 'creates audit event in user audit events' do + expect do + service.security_event + end.to change(AuditEvents::InstanceAuditEvent, :count).by(1).and change(AuditEvent, :count).by(1) - event = AuditEvents::InstanceAuditEvent.last - audit_event = AuditEvent.last + event = AuditEvents::InstanceAuditEvent.last + audit_event = AuditEvent.last - expect(event).to have_attributes( - id: audit_event.id, - author_id: user.id, - author_name: user.name) - end + expect(event).to have_attributes( + id: audit_event.id, + author_id: user.id, + author_name: user.name) end end diff --git a/lib/gitlab/audit/logging.rb b/lib/gitlab/audit/logging.rb index a712c1e589a3d57e140b6dda43e2e7a4e21b8718..b07d2dfa9eceb8ef3a3dec5fa777d9f623904206 100644 --- a/lib/gitlab/audit/logging.rb +++ b/lib/gitlab/audit/logging.rb @@ -12,7 +12,6 @@ module Logging def log_to_new_tables(events, audit_operation) return if events.blank? - return unless Feature.enabled?(:sync_audit_events_to_new_tables, Feature.current_request) events.each { |event| log_event(event) } rescue ActiveRecord::RecordInvalid => e diff --git a/spec/services/audit_event_service_spec.rb b/spec/services/audit_event_service_spec.rb index 349705354c7d969c05f43173cff05aa8b094a157..7528b3daf74db065656d1aa89cc1df2eecd70c06 100644 --- a/spec/services/audit_event_service_spec.rb +++ b/spec/services/audit_event_service_spec.rb @@ -23,33 +23,17 @@ expect { service.security_event }.to change(AuditEvent, :count).by(1) end - context 'when sync_audit_events_to_new_tables is disabled does create not audit events' do - before do - stub_feature_flags(sync_audit_events_to_new_tables: false) - end - - it 'does not create audit event in project audit events' do - expect { service.security_event }.to change(AuditEvents::ProjectAuditEvent, :count).by(0) - end - end + it 'creates audit event in project audit events' do + expect { service.security_event }.to change(AuditEvents::ProjectAuditEvent, :count).by(1) - context 'when sync_audit_events_to_new_tables is enabled it creates audit events' do - before do - stub_feature_flags(sync_audit_events_to_new_tables: true) - end - - it 'creates audit event in project audit events' do - expect { service.security_event }.to change(AuditEvents::ProjectAuditEvent, :count).by(1) + event = AuditEvents::ProjectAuditEvent.last + audit_event = AuditEvent.last - event = AuditEvents::ProjectAuditEvent.last - audit_event = AuditEvent.last - - expect(event).to have_attributes( - id: audit_event.id, - project_id: project.id, - author_id: user.id, - author_name: user.name) - end + expect(event).to have_attributes( + id: audit_event.id, + project_id: project.id, + author_id: user.id, + author_name: user.name) end it 'formats from and to fields' do