Merge branch 'pl-alert-management-status-service-auth' into 'master'

Check user permissions when updating alert status

See merge request gitlab-org/gitlab!31379

app/graphql/mutations/alert_management/update_alert_status.rb

@@ -11,7 +11,6 @@ module Mutations
 
       def resolve(args)
         alert = authorized_find!(project_path: args[:project_path], iid: args[:iid])
-
         result = update_status(alert, args[:status])
 
         prepare_response(result)
@@ -20,7 +19,9 @@ module Mutations
       private
 
       def update_status(alert, status)
-        ::AlertManagement::UpdateAlertStatusService.new(alert, status).execute
+        ::AlertManagement::UpdateAlertStatusService
+          .new(alert, current_user, status)
+          .execute
       end
 
       def prepare_response(result)

app/services/alert_management/update_alert_status_service.rb

@@ -5,14 +5,17 @@ module AlertManagement
     include Gitlab::Utils::StrongMemoize
 
     # @param alert [AlertManagement::Alert]
+    # @param user [User]
     # @param status [Integer] Must match a value from AlertManagement::Alert::STATUSES
-    def initialize(alert, status)
+    def initialize(alert, user, status)
       @alert = alert
+      @user = user
       @status = status
     end
 
     def execute
-      return error('Invalid status') unless status_key
+      return error_no_permissions unless allowed?
+      return error_invalid_status unless status_key
 
       if alert.update(status_event: status_event)
         success
@@ -23,7 +26,13 @@ module AlertManagement
 
     private
 
-    attr_reader :alert, :status
+    attr_reader :alert, :user, :status
+
+    delegate :project, to: :alert
+
+    def allowed?
+      user.can?(:update_alert_management_alert, project)
+    end
 
     def status_key
       strong_memoize(:status_key) do
@@ -39,6 +48,14 @@ module AlertManagement
       ServiceResponse.success(payload: { alert: alert })
     end
 
+    def error_no_permissions
+      error(_('You have no permissions'))
+    end
+
+    def error_invalid_status
+      error(_('Invalid status'))
+    end
+
     def error(message)
       ServiceResponse.error(payload: { alert: alert }, message: message)
     end

locale/gitlab.pot

@@ -11618,6 +11618,9 @@ msgstr ""
 msgid "Invalid start or end time format"
 msgstr ""
 
+msgid "Invalid status"
+msgstr ""
+
 msgid "Invalid two-factor code."
 msgstr ""
 

spec/factories/alert_management/alerts.rb

@@ -3,6 +3,7 @@ require 'ffaker'
 
 FactoryBot.define do
   factory :alert_management_alert, class: 'AlertManagement::Alert' do
+    triggered
     project
     title { FFaker::Lorem.sentence }
     started_at { Time.current }
@@ -35,6 +36,11 @@ FactoryBot.define do
       ended_at { nil }
     end
 
+    trait :triggered do
+      status { AlertManagement::Alert::STATUSES[:triggered] }
+      without_ended_at
+    end
+
     trait :acknowledged do
       status { AlertManagement::Alert::STATUSES[:acknowledged] }
       without_ended_at

spec/graphql/mutations/alert_management/update_alert_status_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 describe Mutations::AlertManagement::UpdateAlertStatus do
   let_it_be(:current_user) { create(:user) }
-  let_it_be(:alert) { create(:alert_management_alert, status: 'triggered') }
+  let_it_be(:alert) { create(:alert_management_alert, :triggered) }
   let_it_be(:project) { alert.project }
   let(:new_status) { Types::AlertManagement::StatusEnum.values['ACKNOWLEDGED'].value }
   let(:args) { { status: new_status, project_path: project.full_path, iid: alert.iid } }
@@ -53,7 +53,7 @@ describe Mutations::AlertManagement::UpdateAlertStatus do
           it 'returns the alert with errors' do
             expect(resolve).to eq(
               alert: alert,
-              errors: ['Invalid status']
+              errors: [_('Invalid status')]
             )
           end
         end

spec/services/alert_management/update_alert_status_service_spec.rb

@@ -3,27 +3,64 @@
 require 'spec_helper'
 
 describe AlertManagement::UpdateAlertStatusService do
-  let_it_be(:alert) { create(:alert_management_alert, status: 'triggered') }
+  let(:project) { alert.project }
+  let_it_be(:user) { build(:user) }
+
+  let_it_be(:alert, reload: true) do
+    create(:alert_management_alert, :triggered)
+  end
+
+  let(:service) { described_class.new(alert, user, new_status) }
 
   describe '#execute' do
-    subject(:execute) { described_class.new(alert, new_status).execute }
+    shared_examples 'update failure' do |error_message|
+      it 'returns an error' do
+        expect(response).to be_error
+        expect(response.message).to eq(error_message)
+        expect(response.payload[:alert]).to eq(alert)
+      end
+
+      it 'does not update the status' do
+        expect { response }.not_to change { alert.status }
+      end
+    end
 
     let(:new_status) { Types::AlertManagement::StatusEnum.values['ACKNOWLEDGED'].value }
+    let(:can_update) { true }
+
+    subject(:response) { service.execute }
+
+    before do
+      allow(user).to receive(:can?)
+        .with(:update_alert_management_alert, project)
+        .and_return(can_update)
+    end
+
+    it 'returns success' do
+      expect(response).to be_success
+      expect(response.payload[:alert]).to eq(alert)
+    end
 
     it 'updates the status' do
-      expect { execute }.to change { alert.acknowledged? }.to(true)
+      expect { response }.to change { alert.acknowledged? }.to(true)
     end
 
-    context 'with unknown status' do
-      let(:new_status) { 'random_status' }
+    context 'when user has no permissions' do
+      let(:can_update) { false }
 
-      it 'returns an error' do
-        expect(execute.status).to eq(:error)
+      include_examples 'update failure', _('You have no permissions')
     end
 
-      it 'does not update the status' do
-        expect { execute }.not_to change { alert.status }
+    context 'with no status' do
+      let(:new_status) { nil }
+
+      include_examples 'update failure', _('Invalid status')
     end
+
+    context 'with unknown status' do
+      let(:new_status) { -1 }
+
+      include_examples 'update failure', _('Invalid status')
     end
   end
 end