Merge branch '7531-audit-mr-approval-permission' into 'master'

Audit project MR approval permission changes

See merge request gitlab-org/gitlab!25959

doc/administration/audit_events.md

@@ -79,6 +79,8 @@ From there, you can see the following actions:
 - Release was added to a project
 - Release was updated
 - Release milestone associations changed
+- Permission to approve merge requests by committers was updated ([introduced](https://gitlab.com/gitlab-org/gitlab/issues/7531) in GitLab 12.9)
+- Permission to approve merge requests by authors was updated ([introduced](https://gitlab.com/gitlab-org/gitlab/issues/7531) in GitLab 12.9)
 
 ### Instance events **(PREMIUM ONLY)**
 

ee/changelogs/unreleased/7531-audit-mr-approval-permission.yml

+---
+title: Audit project MR approval permission changes
+merge_request: 25959
+author:
+type: added

ee/lib/ee/audit/project_changes_auditor.rb

@@ -11,9 +11,14 @@ module EE
         audit_changes(:repository_size_limit, as: 'repository_size_limit', model: model)
         audit_changes(:packages_enabled, as: 'packages_enabled', model: model)
 
+        audit_changes(:merge_requests_author_approval, as: 'prevent merge request approval from authors', model: model)
+        audit_changes(:merge_requests_disable_committers_approval, as: 'prevent merge request approval from reviewers', model: model)
+
         audit_project_feature_changes
       end
 
+      private
+
       def audit_project_feature_changes
         ::EE::Audit::ProjectFeatureChangesAuditor.new(@current_user, model.project_feature, model).execute
       end
@@ -40,6 +45,11 @@ module EE
             from: model.old_path_with_namespace,
             to: model.full_path
           }
+        when :merge_requests_author_approval
+          {
+            from: !model.previous_changes[column].first,
+            to: !model.previous_changes[column].last
+          }
         else
           {
             from: model.previous_changes[column].first,

ee/spec/features/projects/audit_events_spec.rb

@@ -129,6 +129,40 @@ describe 'Projects > Audit Events', :js do
     end
   end
 
+  describe 'changing merge request approval permission for authors and reviewers' do
+    before do
+      project.add_developer(pete)
+    end
+
+    it "appears in the project's audit events" do
+      visit edit_project_path(project)
+
+      page.within('#js-merge-request-approval-settings') do
+        uncheck 'project_merge_requests_author_approval'
+        check 'project_merge_requests_disable_committers_approval'
+        click_button 'Save changes'
+      end
+
+      wait_for('Save is completed') do
+        page.has_content?('was successfully updated', wait: 0)
+      end
+
+      page.within('.qa-project-sidebar') do
+        find(:link, text: 'Settings').click
+        click_link 'Audit Events'
+      end
+
+      wait_for_all_requests
+
+      page.within('#audits') do
+        expect(page).to have_content(project.owner.name)
+        expect(page).to have_content('Change prevent merge request approval from authors')
+        expect(page).to have_content('Change prevent merge request approval from reviewers')
+        expect(page).to have_content(project.name)
+      end
+    end
+  end
+
   it_behaves_like 'audit event contains custom message' do
     let(:audit_events_url) { project_audit_events_path(project) }
   end

ee/spec/lib/ee/audit/project_changes_auditor_spec.rb

@@ -4,9 +4,21 @@ require 'spec_helper'
 
 describe EE::Audit::ProjectChangesAuditor do
   describe '.audit_changes' do
-    let!(:user) { create(:user) }
-    let!(:project) { create(:project, visibility_level: 0) }
-    let(:foo_instance) { described_class.new(user, project) }
+    let(:user) { create(:user) }
+    let(:project) do
+      create(
+        :project,
+        visibility_level: 0,
+        name: 'interesting name',
+        path: 'interesting-path',
+        repository_size_limit: 10,
+        packages_enabled: true,
+        merge_requests_author_approval: false,
+        merge_requests_disable_committers_approval: true
+      )
+    end
+
+    subject(:foo_instance) { described_class.new(user, project) }
 
     before do
       project.reload
@@ -65,6 +77,32 @@ describe EE::Audit::ProjectChangesAuditor do
         expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1)
         expect(SecurityEvent.last.details[:change]).to eq 'packages_enabled'
       end
+
+      it 'creates an event when the merge requests author approval changes' do
+        project.update!(merge_requests_author_approval: true)
+
+        aggregate_failures do
+          expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1)
+          expect(SecurityEvent.last.details).to include(
+            change: 'prevent merge request approval from authors',
+            from: true,
+            to: false
+          )
+        end
+      end
+
+      it 'creates an event when the merge requests committers approval changes' do
+        project.update!(merge_requests_disable_committers_approval: false)
+
+        aggregate_failures do
+          expect { foo_instance.execute }.to change { SecurityEvent.count }.by(1)
+          expect(SecurityEvent.last.details).to include(
+            change: 'prevent merge request approval from reviewers',
+            from: true,
+            to: false
+          )
+        end
+      end
     end
   end
 end