Merge branch '8631-archiving-a-project-should-create-an-audit-event' into 'master'

Resolve "Archiving a project should create an audit event"

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

app/controllers/projects_controller.rb

@@ -29,6 +29,7 @@ class ProjectsController < Projects::ApplicationController
 
   # Authorize
   before_action :authorize_admin_project!, only: [:edit, :update, :housekeeping, :download_export, :export, :remove_export, :generate_new_export]
+  before_action :authorize_archive_project!, only: [:archive, :unarchive]
   before_action :event_filter, only: [:show, :activity]
 
   layout :determine_layout
@@ -164,8 +165,6 @@ class ProjectsController < Projects::ApplicationController
   end
 
   def archive
-    return access_denied! unless can?(current_user, :archive_project, @project)
-
     ::Projects::UpdateService.new(@project, current_user, archived: true).execute
 
     respond_to do |format|
@@ -174,8 +173,6 @@ class ProjectsController < Projects::ApplicationController
   end
 
   def unarchive
-    return access_denied! unless can?(current_user, :archive_project, @project)
-
     ::Projects::UpdateService.new(@project, current_user, archived: false).execute
 
     respond_to do |format|

doc/administration/audit_events.md

@@ -75,6 +75,8 @@ From there, you can see the following actions:
 - User was removed from project
 - Project export was downloaded
 - Project repository was downloaded
+- Project was archived
+- Project was unarchived
 
 ### Instance events **(PREMIUM ONLY)**
 

ee/app/controllers/ee/projects_controller.rb

@@ -6,7 +6,9 @@ module EE
     extend ::Gitlab::Utils::Override
 
     prepended do
-      before_action :log_audit_event, only: [:download_export]
+      before_action :log_download_export_audit_event, only: [:download_export]
+      before_action :log_archive_audit_event, only: [:archive]
+      before_action :log_unarchive_audit_event, only: [:unarchive]
     end
 
     override :project_params_attributes
@@ -82,13 +84,25 @@ module EE
       project&.feature_available?(:merge_pipelines)
     end
 
-    def log_audit_event
+    def log_audit_event(message:)
       AuditEvents::CustomAuditEventService.new(
         current_user,
         project,
         request.remote_ip,
-        'Export file download started'
+        message
       ).for_project.security_event
     end
+
+    def log_download_export_audit_event
+      log_audit_event(message: 'Export file download started')
+    end
+
+    def log_archive_audit_event
+      log_audit_event(message: 'Project archived')
+    end
+
+    def log_unarchive_audit_event
+      log_audit_event(message: 'Project unarchived')
+    end
   end
 end

ee/changelogs/unreleased/8631-archiving-a-project-should-create-an-audit-event.yml

+---
+title: Add audit event for archiving & unarchiving projects
+merge_request: 15362
+author:
+type: added

ee/spec/controllers/projects_controller_spec.rb

@@ -335,4 +335,60 @@ describe ProjectsController do
       end
     end
   end
+
+  context 'Archive & Unarchive actions' do
+    let(:group) { create(:group) }
+    let(:project) { create(:project, group: group) }
+    let(:archived_project) { create(:project, :archived, group: group) }
+
+    describe 'POST #archive' do
+      let(:request) { post :archive, params: { namespace_id: project.namespace, id: project } }
+
+      context 'for a user with the ability to archive a project' do
+        before do
+          group.add_owner(user)
+        end
+
+        it 'logs the audit event' do
+          expect { request }.to change { SecurityEvent.count }.by(1)
+          expect(SecurityEvent.last.details[:custom_message]).to eq('Project archived')
+        end
+      end
+
+      context 'for a user that does not have the ability to archive a project' do
+        before do
+          project.add_maintainer(user)
+        end
+
+        it 'does not log the audit event' do
+          expect { request }.not_to change { SecurityEvent.count }
+        end
+      end
+    end
+
+    describe 'POST #unarchive' do
+      let(:request) { post :unarchive, params: { namespace_id: archived_project.namespace, id: archived_project } }
+
+      context 'for a user with the ability to unarchive a project' do
+        before do
+          group.add_owner(user)
+        end
+
+        it 'logs the audit event' do
+          expect { request }.to change { SecurityEvent.count }.by(1)
+          expect(SecurityEvent.last.details[:custom_message]).to eq('Project unarchived')
+        end
+      end
+
+      context 'for a user that does not have the ability to unarchive a project' do
+        before do
+          project.add_maintainer(user)
+        end
+
+        it 'does not log the audit event' do
+          expect { request }.not_to change { SecurityEvent.count }
+        end
+      end
+    end
+  end
 end

spec/controllers/projects_controller_spec.rb

@@ -318,6 +318,102 @@ describe ProjectsController do
     end
   end
 
+  describe 'POST #archive' do
+    let(:group) { create(:group) }
+    let(:project) { create(:project, group: group) }
+
+    before do
+      sign_in(user)
+    end
+
+    context 'for a user with the ability to archive a project' do
+      before do
+        group.add_owner(user)
+
+        post :archive, params: {
+          namespace_id: project.namespace.path,
+          id: project.path
+        }
+      end
+
+      it 'archives the project' do
+        expect(project.reload.archived?).to be_truthy
+      end
+
+      it 'redirects to projects path' do
+        expect(response).to have_gitlab_http_status(302)
+        expect(response).to redirect_to(project_path(project))
+      end
+    end
+
+    context 'for a user that does not have the ability to archive a project' do
+      before do
+        project.add_maintainer(user)
+
+        post :archive, params: {
+          namespace_id: project.namespace.path,
+          id: project.path
+        }
+      end
+
+      it 'does not archive the project' do
+        expect(project.reload.archived?).to be_falsey
+      end
+
+      it 'returns 404' do
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+  end
+
+  describe 'POST #unarchive' do
+    let(:group) { create(:group) }
+    let(:project) { create(:project, :archived, group: group) }
+
+    before do
+      sign_in(user)
+    end
+
+    context 'for a user with the ability to unarchive a project' do
+      before do
+        group.add_owner(user)
+
+        post :unarchive, params: {
+          namespace_id: project.namespace.path,
+          id: project.path
+        }
+      end
+
+      it 'unarchives the project' do
+        expect(project.reload.archived?).to be_falsey
+      end
+
+      it 'redirects to projects path' do
+        expect(response).to have_gitlab_http_status(302)
+        expect(response).to redirect_to(project_path(project))
+      end
+    end
+
+    context 'for a user that does not have the ability to unarchive a project' do
+      before do
+        project.add_maintainer(user)
+
+        post :unarchive, params: {
+          namespace_id: project.namespace.path,
+          id: project.path
+        }
+      end
+
+      it 'does not unarchive the project' do
+        expect(project.reload.archived?).to be_truthy
+      end
+
+      it 'returns 404' do
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+  end
+
   describe '#housekeeping' do
     let(:group) { create(:group) }
     let(:project) { create(:project, group: group) }