From f35cd7fbe9d94b8353147c420853b369e82d46ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Denny=20Korsuk=C3=A9witz?= <db@znuny.com>
Date: Mon, 11 Nov 2019 16:47:51 +0100
Subject: [PATCH] Fixes #2687 - Article deletion for agents limited to 10
 minutes after article creation.

---
 .../ticket_zoom/article_action/delete.coffee  | 10 +++-
 app/controllers/ticket_articles_controller.rb | 17 ++++--
 spec/requests/ticket/article_spec.rb          | 53 ++++++++++++++++++-
 3 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee
index 830f10c2b..3a610c10e 100644
--- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee
+++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee
@@ -18,7 +18,15 @@ class Delete
 
     callback = ->
       article = App.TicketArticle.find(article.id)
-      article.destroy()
+      article.destroy(
+        fail: (article, details) ->
+          ui.log 'errors', details
+          ui.notify(
+            type:    'error'
+            msg:     App.i18n.translateContent(details.error_human || details.error || 'Unable to delete article!')
+            timeout: 6000
+        )
+      )
 
     new App.ControllerConfirm(
       message: 'Sure?'
diff --git a/app/controllers/ticket_articles_controller.rb b/app/controllers/ticket_articles_controller.rb
index afb502f54..52e72a472 100644
--- a/app/controllers/ticket_articles_controller.rb
+++ b/app/controllers/ticket_articles_controller.rb
@@ -134,24 +134,31 @@ class TicketArticlesController < ApplicationController
     render json: article.attributes_with_association_names, status: :ok
   end
 
-  # DELETE /articles/1
+  # DELETE /api/v1/ticket_articles/:id
   def destroy
     article = Ticket::Article.find(params[:id])
     access!(article, 'delete')
 
     if current_user.permissions?('admin')
       article.destroy!
-      head :ok
+      render json: {}, status: :ok
       return
     end
 
-    if current_user.permissions?('ticket.agent') && article.created_by_id == current_user.id && article.type.name == 'note'
+    article_deletable =
+      current_user.permissions?('ticket.agent') &&
+      article.created_by_id == current_user.id &&
+      !article.type.communication?
+
+    raise Exceptions::NotAuthorized, 'Not authorized (admin permission required)!' if !article_deletable
+
+    if article_deletable && article.created_at >= 10.minutes.ago
       article.destroy!
-      head :ok
+      render json: {}, status: :ok
       return
     end
 
-    raise Exceptions::NotAuthorized, 'Not authorized (admin permission required)!'
+    raise Exceptions::NotAuthorized, 'Articles can only be deleted within 10 minutes after creation.'
   end
 
   # POST /ticket_attachment_upload_clone_by_article
diff --git a/spec/requests/ticket/article_spec.rb b/spec/requests/ticket/article_spec.rb
index 1d18bd601..19917c233 100644
--- a/spec/requests/ticket/article_spec.rb
+++ b/spec/requests/ticket/article_spec.rb
@@ -1,9 +1,9 @@
 require 'rails_helper'
 
-RSpec.describe 'Ticket Article', type: :request do
+RSpec.describe 'Ticket Article API endpoints', type: :request do
 
   let(:admin_user) do
-    create(:admin_user)
+    create(:admin_user, groups: Group.all)
   end
   let!(:group) { create(:group) }
 
@@ -479,4 +479,53 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
       expect(json_response['attachments'].count).to eq(0)
     end
   end
+
+  describe 'DELETE /api/v1/ticket_articles/:id' do
+
+    let!(:article) { create(:ticket_article, sender_name: 'Agent', type_name: 'note', updated_by_id: agent_user.id, created_by_id: agent_user.id  ) }
+
+    context 'by Admin user' do
+      before do
+        authenticated_as(admin_user)
+      end
+
+      it 'always succeeds' do
+        expect { delete "/api/v1/ticket_articles/#{article.id}", params: {}, as: :json }.to change { Ticket::Article.exists?(id: article.id) }
+      end
+    end
+
+    context 'by Agent user' do
+      before do
+        # this is needed, role needs full rights for the new group
+        # so that agent can delete the article
+        group_ids_access_map = Group.all.pluck(:id).each_with_object({}) { |group_id, result| result[group_id] = 'full'.freeze }
+        role = Role.find_by(name: 'Agent')
+        role.group_ids_access_map = group_ids_access_map
+        role.save!
+      end
+
+      context 'within 10 minutes of creation' do
+        before do
+
+          authenticated_as(agent_user)
+          travel 8.minutes
+        end
+
+        it 'succeeds' do
+          expect { delete "/api/v1/ticket_articles/#{article.id}", params: {}, as: :json }.to change { Ticket::Article.exists?(id: article.id) }
+        end
+      end
+
+      context '10+ minutes after creation' do
+        before do
+          authenticated_as(agent_user)
+          travel 10.minutes
+        end
+
+        it 'fails' do
+          expect { delete "/api/v1/ticket_articles/#{article.id}", params: {}, as: :json }.to change { Ticket::Article.exists?(id: article.id) }.to(false)
+        end
+      end
+    end
+  end
 end