Merge branch 'expose-project-audit-events-on-group' into 'master'

Show project-level audit events in group

See merge request gitlab-org/gitlab!27785

ee/app/controllers/admin/audit_logs_controller.rb

@@ -27,7 +27,8 @@ class Admin::AuditLogsController < Admin::ApplicationController
   private
 
   def audit_log_events
-    events = AuditLogFinder.new(audit_logs_params).execute
+    level = Gitlab::Audit::Levels::Instance.new
+    events = AuditLogFinder.new(level: level, params: audit_logs_params).execute
     events = events.page(params[:page]).per(PER_PAGE).without_count
 
     Gitlab::Audit::Events::Preloader.preload!(events)

ee/app/controllers/groups/audit_events_controller.rb

@@ -11,17 +11,14 @@ class Groups::AuditEventsController < Groups::ApplicationController
   layout 'group_settings'
 
   def index
-    events = AuditLogFinder.new(audit_logs_params).execute.page(params[:page])
+    level = Gitlab::Audit::Levels::Group.new(group: group)
 
-    @events = Gitlab::Audit::Events::Preloader.preload!(events)
-  end
+    events = AuditLogFinder
+      .new(level: level, params: audit_logs_params)
+      .execute
+      .page(params[:page])
+      .without_count
 
-  private
-
-  def audit_logs_params
-    super.merge(
-      entity_type: group.class.name,
-      entity_id: group.id
-    )
+    @events = Gitlab::Audit::Events::Preloader.preload!(events)
   end
 end

ee/app/controllers/projects/audit_events_controller.rb

@@ -12,20 +12,18 @@ class Projects::AuditEventsController < Projects::ApplicationController
   layout 'project_settings'
 
   def index
-    events = AuditLogFinder.new(audit_logs_params).execute.page(params[:page])
+    level = Gitlab::Audit::Levels::Project.new(project: project)
+
+    events = AuditLogFinder
+      .new(level: level, params: audit_logs_params)
+      .execute
+      .page(params[:page])
 
     @events = Gitlab::Audit::Events::Preloader.preload!(events)
   end
 
   private
 
-  def audit_logs_params
-    super.merge(
-      entity_type: project.class.name,
-      entity_id: project.id
-    )
-  end
-
   def check_audit_events_available!
     render_404 unless @project.feature_available?(:audit_events) || LicenseHelper.show_promotions?(current_user)
   end

ee/app/finders/audit_log_finder.rb

@@ -4,11 +4,14 @@ class AuditLogFinder
   include CreatedAtFilter
   include FinderMethods
 
+  InvalidLevelTypeError = Class.new(StandardError)
+
   VALID_ENTITY_TYPES = %w[Project User Group].freeze
 
   # Instantiates a new finder
   #
-  # @param [Hash] params
+  # @param [Levels::Project, Levels::Group, Levels::Instance] level that results should be scoped to
+  # @param [Hash] params for filtering and sorting
   # @option params [String] :entity_type
   # @option params [Integer] :entity_id
   # @option params [DateTime] :created_after from created_at date
@@ -16,15 +19,16 @@ class AuditLogFinder
   # @option params [String] :sort order by field_direction (e.g. created_asc)
   #
   # @return [AuditLogFinder]
-  def initialize(params = {})
+  def initialize(level:, params: {})
+    @level = level
     @params = params
   end
 
-  # Filters and sorts records according to `params`
+  # Filters and sorts records
   #
   # @return [AuditEvent::ActiveRecord_Relation]
   def execute
-    audit_events = AuditEvent.all
+    audit_events = init_collection
     audit_events = by_entity(audit_events)
     audit_events = by_created_at(audit_events)
 
@@ -33,7 +37,17 @@ class AuditLogFinder
 
   private
 
-  attr_reader :params
+  attr_reader :level, :params
+
+  def init_collection
+    raise InvalidLevelTypeError unless valid_level_type?
+
+    level.apply
+  end
+
+  def valid_level_type?
+    level.class.name.include?('Gitlab::Audit::Levels')
+  end
 
   def by_entity(audit_events)
     return audit_events unless valid_entity_type?

ee/app/models/ee/audit_event.rb

@@ -5,6 +5,10 @@ module EE
     extend ActiveSupport::Concern
     extend ::Gitlab::Utils::Override
 
+    prepended do
+      scope :by_entity, -> (entity_type, entity_id) { by_entity_type(entity_type).by_entity_id(entity_id) }
+    end
+
     def entity
       lazy_entity
     end

ee/app/views/shared/audit_events/_event_table.html.haml

@@ -26,4 +26,4 @@
         .table-section.section-20
           = event.date
 
-  = paginate events, theme: "gitlab"
+  = paginate_without_count events

ee/lib/api/audit_events.rb

@@ -25,7 +25,8 @@ module API
         use :pagination
       end
       get do
-        audit_events = AuditLogFinder.new(params).execute
+        level = ::Gitlab::Audit::Levels::Instance.new
+        audit_events = AuditLogFinder.new(level: level, params: params).execute
 
         present paginate(audit_events), with: EE::API::Entities::AuditEvent
       end
@@ -37,9 +38,10 @@ module API
         requires :id, type: Integer, desc: 'The ID of audit event'
       end
       get ':id' do
+        level = ::Gitlab::Audit::Levels::Instance.new
         # rubocop: disable CodeReuse/ActiveRecord
         # This is not `find_by!` from ActiveRecord
-        audit_event = AuditLogFinder.new.find_by!(id: params[:id])
+        audit_event = AuditLogFinder.new(level: level).find_by!(id: params[:id])
         # rubocop: enable CodeReuse/ActiveRecord
 
         present audit_event, with: EE::API::Entities::AuditEvent

ee/lib/ee/api/groups.rb

@@ -50,9 +50,8 @@ module EE
             forbidden! unless group.feature_available?(:audit_events)
           end
 
-          def audit_log_finder_params(group)
-            audit_log_finder_params = params.slice(:created_after, :created_before)
-            audit_log_finder_params.merge(entity_type: group.class.name, entity_id: group.id)
+          def audit_log_finder_params
+            params.slice(:created_after, :created_before)
           end
 
           override :delete_group
@@ -102,7 +101,11 @@ module EE
               use :pagination
             end
             get '/' do
-              audit_events = AuditLogFinder.new(audit_log_finder_params(user_group)).execute
+              level = ::Gitlab::Audit::Levels::Group.new(group: user_group)
+              audit_events = AuditLogFinder.new(
+                level: level,
+                params: audit_log_finder_params
+              ).execute
 
               present paginate(audit_events), with: EE::API::Entities::AuditEvent
             end
@@ -114,9 +117,10 @@ module EE
               requires :audit_event_id, type: Integer, desc: 'The ID of the audit event'
             end
             get '/:audit_event_id' do
+              level = ::Gitlab::Audit::Levels::Group.new(group: user_group)
               # rubocop: disable CodeReuse/ActiveRecord
               # This is not `find_by!` from ActiveRecord
-              audit_event = AuditLogFinder.new(audit_log_finder_params(user_group))
+              audit_event = AuditLogFinder.new(level: level, params: audit_log_finder_params)
                 .find_by!(id: params[:audit_event_id])
               # rubocop: enable CodeReuse/ActiveRecord
 

ee/lib/gitlab/audit/levels/group.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Audit
+    module Levels
+      class Group
+        def initialize(group:)
+          @group = group
+        end
+
+        def apply
+          if Feature.enabled?(:audit_log_group_level, @group)
+            projects = ::Project.for_group_and_its_subgroups(@group)
+
+            AuditEvent.by_entity('Group', @group)
+              .or(AuditEvent.by_entity('Project', projects))
+          else
+            AuditEvent.by_entity('Group', @group)
+          end
+        end
+      end
+    end
+  end
+end

ee/lib/gitlab/audit/levels/instance.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Audit
+    module Levels
+      class Instance
+        def apply
+          AuditEvent.all
+        end
+      end
+    end
+  end
+end

ee/lib/gitlab/audit/levels/project.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Audit
+    module Levels
+      class Project
+        def initialize(project:)
+          @project = project
+        end
+
+        def apply
+          AuditEvent.by_entity('Project', @project)
+        end
+      end
+    end
+  end
+end

ee/spec/controllers/groups/audit_events_controller_spec.rb

@@ -21,21 +21,31 @@ describe Groups::AuditEventsController do
       end
 
       context 'when audit_events feature is available' do
-        let(:audit_logs_params) { ActionController::Parameters.new(entity_type: ::Group.name, entity_id: group.id, sort: '').permit! }
+        let(:level) { Gitlab::Audit::Levels::Group.new(group: group) }
+        let(:audit_logs_params) { ActionController::Parameters.new(sort: '').permit! }
 
         before do
           stub_licensed_features(audit_events: true)
+
+          allow(Gitlab::Audit::Levels::Group).to receive(:new).and_return(level)
+          allow(AuditLogFinder).to receive(:new).and_call_original
         end
 
         it 'renders index with 200 status code' do
-          expect(AuditLogFinder).to receive(:new).with(audit_logs_params).and_call_original
-
           request
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(response).to render_template(:index)
         end
 
+        it 'invokes AuditLogFinder with correct arguments' do
+          request
+
+          expect(AuditLogFinder).to have_received(:new).with(
+            level: level, params: audit_logs_params
+          )
+        end
+
         context 'ordering' do
           shared_examples 'orders by id descending' do
             it 'orders by id descending' do
@@ -75,6 +85,14 @@ describe Groups::AuditEventsController do
             it_behaves_like 'orders by id descending'
           end
         end
+
+        context 'pagination' do
+          it 'paginates audit events, without casting a count query' do
+            request
+
+            expect(assigns(:events)).to be_kind_of(Kaminari::PaginatableWithoutCount)
+          end
+        end
       end
     end
 

ee/spec/controllers/projects/audit_events_controller_spec.rb

@@ -21,21 +21,31 @@ describe Projects::AuditEventsController do
       end
 
       context 'when audit_events feature is available' do
-        let(:audit_logs_params) { ActionController::Parameters.new(entity_type: ::Project.name, entity_id: project.id, sort: '').permit! }
+        let(:level) { Gitlab::Audit::Levels::Project.new(project: project) }
+        let(:audit_logs_params) { ActionController::Parameters.new(sort: '').permit! }
 
         before do
           stub_licensed_features(audit_events: true)
+
+          allow(Gitlab::Audit::Levels::Project).to receive(:new).and_return(level)
+          allow(AuditLogFinder).to receive(:new).and_call_original
         end
 
         it 'renders index with 200 status code' do
-          expect(AuditLogFinder).to receive(:new).with(audit_logs_params).and_call_original
-
           request
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(response).to render_template(:index)
         end
 
+        it 'invokes AuditLogFinder with correct arguments' do
+          request
+
+          expect(AuditLogFinder).to have_received(:new).with(
+            level: level, params: audit_logs_params
+          )
+        end
+
         context 'ordering' do
           shared_examples 'orders by id descending' do
             it 'orders by id descending' do

ee/spec/finders/audit_log_finder_spec.rb

@@ -3,18 +3,84 @@
 require 'spec_helper'
 
 describe AuditLogFinder do
-  let_it_be(:user_audit_event) { create(:user_audit_event, created_at: 3.days.ago) }
-  let_it_be(:project_audit_event) { create(:project_audit_event, created_at: 2.days.ago) }
-  let_it_be(:group_audit_event) { create(:group_audit_event, created_at: 1.day.ago) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:subgroup) { create(:group, parent: group) }
+  let_it_be(:project) { create(:project, namespace: group) }
+  let_it_be(:subproject) { create(:project, namespace: subgroup) }
 
-  describe '#execute' do
-    subject { described_class.new(params).execute }
+  let_it_be(:user_audit_event) { create(:user_audit_event, created_at: 3.days.ago) }
+  let_it_be(:project_audit_event) { create(:project_audit_event, entity_id: project.id, created_at: 2.days.ago) }
+  let_it_be(:subproject_audit_event) { create(:project_audit_event, entity_id: subproject.id, created_at: 2.days.ago) }
+  let_it_be(:group_audit_event) { create(:group_audit_event, entity_id: group.id, created_at: 1.day.ago) }
 
-    context 'no filtering' do
+  let(:level) { Gitlab::Audit::Levels::Instance.new }
   let(:params) { {} }
 
+  subject(:finder) { described_class.new(level: level, params: params) }
+
+  describe '#execute' do
+    subject { finder.execute }
+
+    shared_examples 'no filtering' do
       it 'finds all the events' do
-        expect(subject.count).to eq(3)
+        expect(subject.count).to eq(4)
+      end
+    end
+
+    context 'filtering by level' do
+      context 'when project level' do
+        let(:level) { Gitlab::Audit::Levels::Project.new(project: project) }
+
+        it 'finds all project events' do
+          expect(subject).to contain_exactly(project_audit_event)
+        end
+      end
+
+      context 'when group level' do
+        context 'when audit_log_group_level feature enabled' do
+          before do
+            stub_feature_flags(audit_log_group_level: true)
+          end
+
+          let(:level) { Gitlab::Audit::Levels::Group.new(group: group) }
+
+          it 'finds all group and project events' do
+            expect(subject).to contain_exactly(project_audit_event, subproject_audit_event, group_audit_event)
+          end
+        end
+
+        context 'when audit_log_group_level feature disabled' do
+          before do
+            stub_feature_flags(audit_log_group_level: false)
+          end
+
+          let(:level) { Gitlab::Audit::Levels::Group.new(group: group) }
+
+          it 'finds all group events' do
+            expect(subject).to contain_exactly(group_audit_event)
+          end
+        end
+      end
+
+      context 'when instance level' do
+        let(:level) { Gitlab::Audit::Levels::Instance.new }
+
+        it 'finds all instance level events' do
+          expect(subject).to contain_exactly(
+            project_audit_event,
+            subproject_audit_event,
+            group_audit_event,
+            user_audit_event
+          )
+        end
+      end
+
+      context 'when invalid level' do
+        let(:level) { 'an invalid level' }
+
+        it 'raises exception' do
+          expect { subject }.to raise_error(described_class::InvalidLevelTypeError)
+        end
       end
     end
 
@@ -22,9 +88,7 @@ describe AuditLogFinder do
       context 'no entity_type provided' do
         let(:params) { { entity_id: 1 } }
 
-        it 'ignores entity_id and returns all events' do
-          expect(subject.count).to eq(3)
-        end
+        it_behaves_like 'no filtering'
       end
 
       context 'invalid entity_id' do
@@ -105,17 +169,13 @@ describe AuditLogFinder do
         context 'blank entity_type' do
           let(:params) { { entity_type: '' } }
 
-          it 'finds all the events with blank entity_type' do
-            expect(subject.count).to eq(3)
-          end
+          it_behaves_like 'no filtering'
         end
 
         context 'invalid entity_type' do
           let(:params) { { entity_type: 'Invalid Entity Type' } }
 
-          it 'finds all the events with invalid entity_type' do
-            expect(subject.count).to eq(3)
-          end
+          it_behaves_like 'no filtering'
         end
       end
     end
@@ -148,10 +208,9 @@ describe AuditLogFinder do
   end
 
   describe '#find_by!' do
-    let(:params) { {} }
     let(:id) { user_audit_event.id }
 
-    subject { described_class.new(params).find_by!(id: id) }
+    subject { finder.find_by!(id: id) }
 
     it { is_expected.to eq(user_audit_event) }
 

ee/spec/lib/gitlab/audit/levels/group_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Audit::Levels::Group do
+  describe '#apply' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:subgroup) { create(:group, parent: group) }
+    let_it_be(:project) { create(:project, namespace: group) }
+    let_it_be(:subproject) { create(:project, namespace: subgroup) }
+
+    let_it_be(:project_audit_event) { create(:project_audit_event, entity_id: project.id) }
+    let_it_be(:subproject_audit_event) { create(:project_audit_event, entity_id: subproject.id) }
+    let_it_be(:group_audit_event) { create(:group_audit_event, entity_id: group.id) }
+
+    subject { described_class.new(group: group).apply }
+
+    context 'when audit_log_group_level feature enabled' do
+      before do
+        stub_feature_flags(audit_log_group_level: true)
+      end
+
+      it 'finds all group and project events' do
+        expect(subject).to contain_exactly(project_audit_event, subproject_audit_event, group_audit_event)
+      end
+    end
+
+    context 'when audit_log_group_level feature disabled' do
+      before do
+        stub_feature_flags(audit_log_group_level: false)
+      end
+
+      it 'finds all group events' do
+        expect(subject).to contain_exactly(group_audit_event)
+      end
+    end
+  end
+end

ee/spec/lib/gitlab/audit/levels/instance_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Audit::Levels::Instance do
+  describe '#apply' do
+    let_it_be(:audit_events) do
+      [
+        create(:project_audit_event),
+        create(:group_audit_event),
+        create(:user_audit_event)
+      ]
+    end
+
+    subject { described_class.new.apply }
+
+    it 'finds all events' do
+      expect(subject).to match_array(audit_events)
+    end
+  end
+end

ee/spec/lib/gitlab/audit/levels/project_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Audit::Levels::Project do
+  describe '#apply' do
+    let_it_be(:project) { create(:project) }
+
+    let_it_be(:project_audit_event) { create(:project_audit_event, entity_id: project.id) }
+
+    subject { described_class.new(project: project).apply }
+
+    it 'finds all project events' do
+      expect(subject).to contain_exactly(project_audit_event)
+    end
+  end
+end

ee/spec/models/audit_event_spec.rb

@@ -13,6 +13,21 @@ RSpec.describe AuditEvent, type: :model do
     it { is_expected.to validate_presence_of(:entity_type) }
   end
 
+  describe '.by_entity' do
+    let_it_be(:project_event_1) { create(:project_audit_event) }
+    let_it_be(:project_event_2) { create(:project_audit_event) }
+    let_it_be(:user_event) { create(:user_audit_event) }
+
+    let(:entity_type) { 'Project' }
+    let(:entity_id) { project_event_1.entity_id }
+
+    subject(:event) { described_class.by_entity(entity_type, entity_id) }
+
+    it 'returns the correct audit events' do
+      expect(event).to contain_exactly(project_event_1)
+    end
+  end
+
   describe '.order_by' do
     let_it_be(:event_1) { create(:audit_event) }
     let_it_be(:event_2) { create(:audit_event) }