Merge branch 'osw-record-repo-push-events' into 'master'

Track repository pushes as audit events

See merge request gitlab-org/gitlab-ee!15667

app/services/audit_event_service.rb

@@ -21,6 +21,10 @@ class AuditEventService
     log_security_event_to_database
   end
 
+  def log_security_event_to_file
+    file_logger.info(base_payload.merge(formatted_details))
+  end
+
   private
 
   def base_payload
@@ -39,10 +43,6 @@ class AuditEventService
     @details.merge(@details.slice(:from, :to).transform_values(&:to_s))
   end
 
-  def log_security_event_to_file
-    file_logger.info(base_payload.merge(formatted_details))
-  end
-
   def log_security_event_to_database
     SecurityEvent.create(base_payload.merge(details: @details))
   end

config/sidekiq_queues.yml

@@ -102,6 +102,7 @@
   - [create_github_webhook, 2]
   - [geo, 1]
   - [repository_update_mirror, 1]
+  - [repository_push_audit_event, 1]
   - [new_epic, 2]
   - [project_import_schedule, 1]
   - [project_update_repository_storage, 1]

ee/app/models/ee/project.rb

@@ -237,6 +237,10 @@ module EE
         (::Feature.enabled?(feature) && feature_available?(feature))
     end
 
+    def push_audit_events_enabled?
+      ::Feature.enabled?(:repository_push_audit_event, self)
+    end
+
     def feature_available?(feature, user = nil)
       if ::ProjectFeature::FEATURES.include?(feature)
         super

ee/app/services/ee/audit_event_service.rb

@@ -84,13 +84,15 @@ module EE
     end
 
     def security_event
+      prepare_security_event
+
+      super if enabled?
+    end
+
+    def prepare_security_event
       if admin_audit_log_enabled?
         add_security_event_admin_details!
-
-        return super
       end
-
-      super if audit_events_enabled? || entity_audit_events_enabled?
     end
 
     def unauth_security_event
@@ -115,6 +117,12 @@ module EE
       for_custom_model('group', @entity.full_path)
     end
 
+    def enabled?
+      admin_audit_log_enabled? ||
+        audit_events_enabled? ||
+        entity_audit_events_enabled?
+    end
+
     def entity_audit_events_enabled?
       @entity.respond_to?(:feature_available?) && @entity.feature_available?(:audit_events)
     end

ee/app/services/ee/audit_events/bulk_insert_service.rb

+# frozen_string_literal: true
+
+module EE
+  module AuditEvents
+    class BulkInsertService
+      BATCH_SIZE = 100
+
+      # service_collection - An array of audit event services that must respond to:
+      # - enabled?
+      # - attributes (Hash of AuditEvent attributes)
+      # - write_log
+      def initialize(service_collection)
+        @service_collection = service_collection
+      end
+
+      def execute
+        collection = @service_collection.select(&:enabled?)
+
+        return if collection.empty?
+
+        collection.in_groups_of(BATCH_SIZE, false) do |services|
+          ::Gitlab::Database.bulk_insert(::AuditEvent.table_name, services.map(&:attributes))
+
+          services.each(&:log_security_event_to_file)
+        end
+      end
+    end
+  end
+end

ee/app/services/ee/audit_events/repository_push_audit_event_service.rb

+# frozen_string_literal: true
+
+module EE
+  module AuditEvents
+    class RepositoryPushAuditEventService < ::AuditEventService
+      def initialize(author, project, target_ref, from, to)
+        super(author, project, {
+          updated_ref: ::Gitlab::Git.ref_name(target_ref),
+          author_name: author.name,
+          from: Commit.truncate_sha(from),
+          to: Commit.truncate_sha(to),
+          target_details: project.full_path
+        })
+      end
+
+      def attributes
+        base_payload.merge(type: SecurityEvent.to_s,
+                           created_at: DateTime.now,
+                           updated_at: DateTime.now,
+                           details: @details.to_yaml)
+      end
+
+      def enabled?
+        super && @entity.push_audit_events_enabled?
+      end
+    end
+  end
+end

ee/app/workers/all_queues.yml

@@ -68,3 +68,4 @@
 - project_update_repository_storage
 - rebase
 - repository_update_mirror
+- repository_push_audit_event

ee/app/workers/ee/post_receive.rb

@@ -13,8 +13,14 @@ module EE
     def after_project_changes_hooks(post_received, user, refs, changes)
       super
 
+      project = post_received.project
+
+      if audit_push?(project)
+        ::RepositoryPushAuditEventWorker.perform_async(changes, project.id, user.id)
+      end
+
       if ::Gitlab::Geo.primary?
-        ::Geo::RepositoryUpdatedService.new(post_received.project.repository, refs: refs, changes: changes).execute
+        ::Geo::RepositoryUpdatedService.new(project.repository, refs: refs, changes: changes).execute
       end
     end
 
@@ -25,5 +31,9 @@ module EE
         ::Geo::RepositoryUpdatedService.new(post_received.project.wiki.repository).execute
       end
     end
+
+    def audit_push?(project)
+      project.push_audit_events_enabled? && !::Gitlab::Database.read_only?
+    end
   end
 end

ee/app/workers/repository_push_audit_event_worker.rb

+# frozen_string_literal: true
+
+class RepositoryPushAuditEventWorker
+  include ApplicationWorker
+
+  def perform(changes, project_id, user_id)
+    project = Project.find(project_id)
+    user = User.find(user_id)
+
+    changes.map! do |change|
+      before, after, ref = change['before'], change['after'], change['ref']
+
+      service = EE::AuditEvents::RepositoryPushAuditEventService
+        .new(user, project, ref, before, after)
+        .tap { |event| event.prepare_security_event }
+
+      # Checking if it's enabled and reusing the changes array
+      # is mostly a memory optimization.
+      service if service.enabled?
+    end.compact!
+
+    EE::AuditEvents::BulkInsertService.new(changes).execute
+  end
+end

ee/changelogs/unreleased/osw-record-repo-push-events.yml

+---
+title: Track repository pushes as audit events
+merge_request: 15667
+author:
+type: added

ee/lib/audit/details.rb

@@ -2,7 +2,7 @@
 
 module Audit
   class Details
-    ACTIONS = %i[add remove failed_login change custom_message].freeze
+    ACTIONS = %i[add remove failed_login change updated_ref custom_message].freeze
 
     def self.humanize(*args)
       new(*args).humanize
@@ -32,6 +32,12 @@ module Audit
         "Removed #{target_name}"
       when :failed_login
         "Failed to login with #{oauth_label} authentication"
+      when :updated_ref
+        target_ref = @details[:updated_ref]
+        from_sha = @details[:from]
+        to_sha = @details[:to]
+
+        "Updated ref #{target_ref} from #{from_sha} to #{to_sha}"
       when :custom_message
         detail_value
       else

ee/spec/lib/audit/details_spec.rb

@@ -124,5 +124,23 @@ describe Audit::Details do
         expect(string).to eq('Changed email from a@b.com to c@b.com')
       end
     end
+
+    context 'updated ref' do
+      let(:action) do
+        {
+          updated_ref: 'master',
+          author_name: 'Hackerman',
+          from: 'b6bce79c',
+          to: 'a7bce79c',
+          target_details: 'group/project'
+        }
+      end
+
+      it 'humanizes the action' do
+        string = described_class.humanize(action)
+
+        expect(string).to eq('Updated ref master from b6bce79c to a7bce79c')
+      end
+    end
   end
 end

ee/spec/services/audit_event_service_spec.rb

@@ -56,6 +56,29 @@ describe AuditEventService do
     end
   end
 
+  describe '#enabled?' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:admin_audit_log, :audit_events, :extended_audit_events, :result) do
+      true  | false | false | true
+      false | true  | false | true
+      false | false | true  | true
+      false | false | false | false
+    end
+
+    with_them do
+      before do
+        stub_licensed_features(admin_audit_log: admin_audit_log,
+                               audit_events: audit_events,
+                               extended_audit_events: extended_audit_events)
+      end
+
+      it 'returns the correct result when feature is available' do
+        expect(service.enabled?).to eq(result)
+      end
+    end
+  end
+
   describe '#entity_audit_events_enabled??' do
     context 'entity is a project' do
       let(:service) { described_class.new(user, project, { action: :destroy }) }

ee/spec/services/ee/audit_events/bulk_insert_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe EE::AuditEvents::BulkInsertService do
+  let(:user) { create(:user) }
+  let(:entity) { create(:project) }
+  let(:entity_type) { 'Project' }
+  let(:target_ref) { 'refs/heads/master' }
+  let(:from) { 'b6bce79c3a8cb367877b53e315799b69acb700d7' }
+  let(:to) { 'a7bce79c3a8cb367877b53e315799b69acb700fo' }
+  let!(:collection) do
+    Array.new(3).map do
+      EE::AuditEvents::RepositoryPushAuditEventService.new(user, entity, target_ref, from, to)
+    end
+  end
+  let(:timestamp) { Time.new(2019, 10, 10) }
+  let(:attrs) do
+    {
+      author_id: user.id,
+      entity_id: entity.id,
+      entity_type: entity_type,
+      type: 'SecurityEvent',
+      created_at: timestamp,
+      updated_at: timestamp,
+      details: {
+        updated_ref: 'master',
+        author_name: user.name,
+        from: 'b6bce79c',
+        to: 'a7bce79c',
+        target_details: entity.full_path
+      }
+    }
+  end
+
+  let(:service) { described_class.new(collection) }
+
+  describe '#execute' do
+    it 'persists audit events' do
+      Timecop.freeze(timestamp) { service.execute }
+
+      events_attributes = SecurityEvent.all.map { |event| event.attributes.deep_symbolize_keys }
+
+      expect(SecurityEvent.count).to eq(3)
+      expect(events_attributes).to all(include(attrs))
+    end
+
+    it 'writes logs' do
+      collection.each do |service| # rubocop:disable RSpec/IteratedExpectation
+        expect(service).to receive(:log_security_event_to_file).and_call_original
+      end
+
+      service.execute
+    end
+  end
+end

ee/spec/services/ee/audit_events/repository_push_audit_event_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe EE::AuditEvents::RepositoryPushAuditEventService do
+  let(:user) { create(:user) }
+  let(:entity) { create(:project) }
+  let(:entity_type) { 'Project' }
+  let(:target_ref) { 'refs/heads/master' }
+  let(:from) { 'b6bce79c3a8cb367877b53e315799b69acb700d7' }
+  let(:to) { 'a7bce79c3a8cb367877b53e315799b69acb700fo' }
+  let(:service) { described_class.new(user, entity, target_ref, from, to) }
+
+  describe '#attributes' do
+    let(:timestamp) { Time.new(2019, 10, 10) }
+    let(:attrs) do
+      {
+        author_id: user.id,
+        entity_id: entity.id,
+        entity_type: entity_type,
+        type: 'SecurityEvent',
+        created_at: timestamp,
+        updated_at: timestamp,
+        details: {
+          updated_ref: updated_ref,
+          author_name: user.name,
+          from: 'b6bce79c',
+          to: 'a7bce79c',
+          target_details: entity.full_path
+        }.to_yaml
+      }
+    end
+
+    context 'when branch push' do
+      let(:target_ref) { 'refs/heads/master' }
+      let(:updated_ref) { 'master' }
+
+      it 'returns audit event attributes' do
+        Timecop.freeze(timestamp) do
+          expect(service.attributes).to eq(attrs)
+        end
+      end
+    end
+
+    context 'when tag push' do
+      let(:target_ref) { 'refs/tags/v1.0' }
+      let(:updated_ref) { 'v1.0' }
+
+      it 'returns audit event attributes' do
+        Timecop.freeze(timestamp) do
+          expect(service.attributes).to eq(attrs)
+        end
+      end
+    end
+  end
+
+  describe '#enabled?' do
+    let(:target_ref) { 'refs/tags/v1.0' }
+
+    subject { service.enabled? }
+
+    context 'when not licensed and not enabled' do
+      before do
+        stub_licensed_features(audit_events: false,
+                               extended_audit_events: false,
+                               admin_audit_log: false)
+
+        stub_feature_flags(repository_push_audit_event: false)
+      end
+
+      it { is_expected.to be(false) }
+    end
+
+    context 'when licensed but not enabled' do
+      before do
+        stub_licensed_features(audit_events: true,
+                               extended_audit_events: false,
+                               admin_audit_log: false)
+
+        stub_feature_flags(repository_push_audit_event: false)
+      end
+
+      it { is_expected.to be(false) }
+    end
+
+    context 'when licensed and enabled' do
+      before do
+        stub_licensed_features(audit_events: true,
+                               extended_audit_events: false,
+                               admin_audit_log: false)
+
+        stub_feature_flags(repository_push_audit_event: true)
+      end
+
+      it { is_expected.to be(true) }
+    end
+  end
+end

ee/spec/workers/post_receive_spec.rb

@@ -21,6 +21,7 @@ describe PostReceive do
       let(:fake_hook_data) { Hash.new(event_name: 'repository_update') }
 
       before do
+        allow(RepositoryPushAuditEventWorker).to receive(:perform_async)
         allow_any_instance_of(Gitlab::DataBuilder::Repository).to receive(:update).and_return(fake_hook_data)
         # silence hooks so we can isolate
         allow_any_instance_of(Key).to receive(:post_create_hook).and_return(true)
@@ -34,6 +35,48 @@ describe PostReceive do
         end
       end
 
+      context 'when DB is readonly' do
+        before do
+          allow(Gitlab::Database).to receive(:read_only?) { true }
+        end
+
+        it 'does not call RepositoryPushAuditEventWorker' do
+          expect(::RepositoryPushAuditEventWorker).not_to receive(:perform_async)
+
+          described_class.new.perform(gl_repository, key_id, base64_changes)
+        end
+      end
+
+      context 'when DB is not readonly' do
+        before do
+          allow(Gitlab::Database).to receive(:read_only?) { false }
+        end
+
+        it 'calls RepositoryPushAuditEventWorker' do
+          expected_changes = [
+            { after: '789012', before: '123456', ref: 'refs/heads/tést' },
+            { after: '210987', before: '654321', ref: 'refs/tags/tag' }
+          ]
+
+          expect(::RepositoryPushAuditEventWorker).to receive(:perform_async)
+            .with(expected_changes, project.id, project.owner.id)
+
+          described_class.new.perform(gl_repository, key_id, base64_changes)
+        end
+
+        context 'when feature flag is off' do
+          before do
+            stub_feature_flags(repository_push_audit_event: false)
+          end
+
+          it 'does not call RepositoryPushAuditEventWorker' do
+            expect(::RepositoryPushAuditEventWorker).not_to receive(:perform_async)
+
+            described_class.new.perform(gl_repository, key_id, base64_changes)
+          end
+        end
+      end
+
       it 'calls Geo::RepositoryUpdatedService when running on a Geo primary node' do
         allow(Gitlab::Geo).to receive(:primary?) { true }
 

ee/spec/workers/repository_push_audit_event_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe RepositoryPushAuditEventWorker do
+  describe '#perform' do
+    let(:project) { create(:project, :repository) }
+    let(:user) { create(:user) }
+    let(:changes) do
+      [
+        {
+          'before' => '123456',
+          'after' => '789012',
+          'ref' => 'refs/heads/tést'
+        },
+        {
+          'before' => '654321',
+          'after' => '210987',
+          'ref' => 'refs/tags/tag'
+        }
+      ]
+    end
+
+    def event_attributes(details = {})
+      {
+        author_id: user.id,
+        entity_id: project.id,
+        entity_type: 'Project',
+        details: details
+      }
+    end
+
+    subject { described_class.new.perform(changes, project.id, user.id) }
+
+    it 'audits events according to push changes' do
+      subject
+
+      branch_event = event_attributes(updated_ref: 'tést',
+                                      author_name: user.name,
+                                      from: '123456',
+                                      to: '789012',
+                                      target_details: project.full_path)
+
+      tag_event = event_attributes(updated_ref: 'tag',
+                                   author_name: user.name,
+                                   from: '654321',
+                                   to: '210987',
+                                   target_details: project.full_path)
+
+      events = SecurityEvent.all.map do |event|
+        event
+          .attributes
+          .deep_symbolize_keys
+          .slice(:author_id, :entity_id, :entity_type, :details)
+      end
+
+      expect(events).to match_array([branch_event, tag_event])
+    end
+
+    context 'when feature is not available' do
+      let(:changes) do
+        [{
+          'before' => '654321',
+          'after' => '210987',
+          'ref' => 'refs/tags/tag'
+        }]
+      end
+
+      it 'does not create events' do
+        expect_next_instance_of(EE::AuditEvents::RepositoryPushAuditEventService) do |instance|
+          expect(instance).to receive(:enabled?) { false }
+        end
+
+        expect { subject }.not_to change(SecurityEvent, :count)
+      end
+    end
+  end
+end

spec/services/audit_event_service_spec.rb

@@ -47,4 +47,16 @@ describe AuditEventService do
       expect(details[:target_id]).to eq(1)
     end
   end
+
+  describe '#log_security_event_to_file' do
+    it 'logs security event to file' do
+      expect(service).to receive(:file_logger).and_return(logger)
+      expect(logger).to receive(:info).with(author_id: user.id,
+                                            entity_type: 'Project',
+                                            entity_id: project.id,
+                                            action: :destroy)
+
+      service.log_security_event_to_file
+    end
+  end
 end