From d67485af1348446c17e8f85dce53c940bc2113e7 Mon Sep 17 00:00:00 2001 From: Mike Koch Date: Wed, 12 Apr 2017 22:00:56 -0400 Subject: [PATCH] Ticket attachments can be deleted --- api/ApplicationContext.php | 5 +- .../Attachments/AttachmentHandler.php | 33 +++++++- .../Security/UserToTicketChecker.php | 4 +- .../StaffTicketAttachmentsController.php | 11 +++ api/DataAccess/Files/FileDeleter.php | 17 +++++ .../Attachments/AttachmentHandlerTest.php | 76 ++++++++++++++++++- 6 files changed, 141 insertions(+), 5 deletions(-) create mode 100644 api/DataAccess/Files/FileDeleter.php diff --git a/api/ApplicationContext.php b/api/ApplicationContext.php index a170a5ec..602604af 100644 --- a/api/ApplicationContext.php +++ b/api/ApplicationContext.php @@ -22,6 +22,7 @@ use BusinessLogic\Tickets\TrackingIdGenerator; use BusinessLogic\Tickets\VerifiedEmailChecker; use DataAccess\Attachments\AttachmentGateway; use DataAccess\Categories\CategoryGateway; +use DataAccess\Files\FileDeleter; use DataAccess\Files\FileReader; use DataAccess\Files\FileWriter; use DataAccess\Logging\LoggingGateway; @@ -102,11 +103,13 @@ class ApplicationContext { $this->get[UserToTicketChecker::class] = new UserToTicketChecker($this->get[UserGateway::class]); $this->get[FileWriter::class] = new FileWriter(); $this->get[FileReader::class] = new FileReader(); + $this->get[FileDeleter::class] = new FileDeleter(); $this->get[AttachmentGateway::class] = new AttachmentGateway(); $this->get[AttachmentHandler::class] = new AttachmentHandler($this->get[TicketGateway::class], $this->get[AttachmentGateway::class], $this->get[FileWriter::class], - $this->get[UserToTicketChecker::class]); + $this->get[UserToTicketChecker::class], + $this->get[FileDeleter::class]); $this->get[AttachmentRetriever::class] = new AttachmentRetriever($this->get[AttachmentGateway::class], $this->get[FileReader::class], $this->get[TicketGateway::class], diff --git a/api/BusinessLogic/Attachments/AttachmentHandler.php b/api/BusinessLogic/Attachments/AttachmentHandler.php index f11d98f6..343c13c1 100644 --- a/api/BusinessLogic/Attachments/AttachmentHandler.php +++ b/api/BusinessLogic/Attachments/AttachmentHandler.php @@ -3,6 +3,7 @@ namespace BusinessLogic\Attachments; +use BusinessLogic\Exceptions\ApiFriendlyException; use BusinessLogic\Exceptions\ValidationException; use BusinessLogic\Security\UserContext; use BusinessLogic\Security\UserToTicketChecker; @@ -10,6 +11,7 @@ use BusinessLogic\Tickets\Attachment; use BusinessLogic\Tickets\Ticket; use BusinessLogic\ValidationModel; use DataAccess\Attachments\AttachmentGateway; +use DataAccess\Files\FileDeleter; use DataAccess\Files\FileWriter; use DataAccess\Tickets\TicketGateway; @@ -23,14 +25,18 @@ class AttachmentHandler { /* @var $fileWriter FileWriter */ private $fileWriter; + /* @var $fileDeleter FileDeleter */ + private $fileDeleter; + /* @var $userToTicketChecker UserToTicketChecker */ private $userToTicketChecker; - function __construct($ticketGateway, $attachmentGateway, $fileWriter, $userToTicketChecker) { + function __construct($ticketGateway, $attachmentGateway, $fileWriter, $userToTicketChecker, $fileDeleter) { $this->ticketGateway = $ticketGateway; $this->attachmentGateway = $attachmentGateway; $this->fileWriter = $fileWriter; $this->userToTicketChecker = $userToTicketChecker; + $this->fileDeleter = $fileDeleter; } @@ -75,6 +81,31 @@ class AttachmentHandler { return $ticketAttachment; } + function deleteAttachmentFromTicket($ticketId, $attachmentId, $userContext, $heskSettings) { + $ticket = $this->ticketGateway->getTicketById($ticketId, $heskSettings); + + if (!$this->userToTicketChecker->isTicketWritableToUser($userContext, $ticket, true, $heskSettings)) { + throw new \Exception("User does not have access to ticket {$ticketId} being created / edited!"); + } + + $indexToRemove = -1; + for ($i = 0; $i < count($ticket->attachments); $i++) { + $attachment = $ticket->attachments[$i]; + if ($attachment->id === $attachmentId) { + $indexToRemove = $i; + $this->fileDeleter->deleteFile($attachment->savedName, $heskSettings['attach_dir']); + } + } + + if ($indexToRemove === -1) { + throw new ApiFriendlyException("Attachment not found for ticket!", "Attachment not found", 404); + } + + $attachments = $ticket->attachments; + unset($attachments[$indexToRemove]); + $this->ticketGateway->updateAttachmentsForTicket($ticketId, $attachments, $heskSettings); + } + /** * @param $createAttachmentModel CreateAttachmentForTicketModel * @param $heskSettings array diff --git a/api/BusinessLogic/Security/UserToTicketChecker.php b/api/BusinessLogic/Security/UserToTicketChecker.php index 0c3b27bf..777bd200 100644 --- a/api/BusinessLogic/Security/UserToTicketChecker.php +++ b/api/BusinessLogic/Security/UserToTicketChecker.php @@ -30,7 +30,9 @@ class UserToTicketChecker { $categoryManagerId = $this->userGateway->getManagerForCategory($ticket->categoryId, $heskSettings); $hasAccess = $hasAccess && - (in_array(UserPrivilege::CAN_EDIT_TICKETS, $user->permissions) || $categoryManagerId == $user->id); + ($user->admin === true + || in_array(UserPrivilege::CAN_EDIT_TICKETS, $user->permissions) + || $categoryManagerId == $user->id); } return $hasAccess; diff --git a/api/Controllers/Attachments/StaffTicketAttachmentsController.php b/api/Controllers/Attachments/StaffTicketAttachmentsController.php index ad63b7c0..3ca6ca74 100644 --- a/api/Controllers/Attachments/StaffTicketAttachmentsController.php +++ b/api/Controllers/Attachments/StaffTicketAttachmentsController.php @@ -55,4 +55,15 @@ class StaffTicketAttachmentsController { return $model; } + + function delete($ticketId, $attachmentId) { + global $applicationContext, $hesk_settings, $userContext; + + /* @var $attachmentHandler AttachmentHandler */ + $attachmentHandler = $applicationContext->get[AttachmentHandler::class]; + + $attachmentHandler->deleteAttachmentFromTicket($ticketId, $attachmentId, $userContext, $hesk_settings); + + return http_response_code(204); + } } \ No newline at end of file diff --git a/api/DataAccess/Files/FileDeleter.php b/api/DataAccess/Files/FileDeleter.php new file mode 100644 index 00000000..a625d764 --- /dev/null +++ b/api/DataAccess/Files/FileDeleter.php @@ -0,0 +1,17 @@ +ticketGateway = $this->createMock(TicketGateway::class); $this->attachmentGateway = $this->createMock(AttachmentGateway::class); $this->fileWriter = $this->createMock(FileWriter::class); + $this->fileDeleter = $this->createMock(FileDeleter::class); $this->userToTicketChecker = $this->createMock(UserToTicketChecker::class); - $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); $this->heskSettings = array( 'attach_dir' => 'attachments', 'attachments' => array( @@ -56,7 +60,8 @@ class AttachmentHandlerTest extends TestCase { $this->attachmentHandler = new AttachmentHandler($this->ticketGateway, $this->attachmentGateway, $this->fileWriter, - $this->userToTicketChecker); + $this->userToTicketChecker, + $this->fileDeleter); $this->createAttachmentForTicketModel = new CreateAttachmentForTicketModel(); $this->createAttachmentForTicketModel->attachmentContents = base64_encode('string'); $this->createAttachmentForTicketModel->displayName = 'DisplayName.txt'; @@ -67,6 +72,7 @@ class AttachmentHandlerTest extends TestCase { function testThatValidateThrowsAnExceptionWhenTheAttachmentBodyIsNull() { //-- Arrange + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); $this->createAttachmentForTicketModel->attachmentContents = null; //-- Assert @@ -79,6 +85,7 @@ class AttachmentHandlerTest extends TestCase { function testThatValidateThrowsAnExceptionWhenTheAttachmentBodyIsEmpty() { //-- Arrange + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); $this->createAttachmentForTicketModel->attachmentContents = ''; //-- Assert @@ -91,6 +98,7 @@ class AttachmentHandlerTest extends TestCase { function testThatValidateThrowsAnExceptionWhenTheAttachmentBodyIsInvalidBase64() { //-- Arrange + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); $this->createAttachmentForTicketModel->attachmentContents = 'invalid base 64'; //-- Assert @@ -103,6 +111,7 @@ class AttachmentHandlerTest extends TestCase { function testThatValidateThrowsAnExceptionWhenTheDisplayNameIsNull() { //-- Arrange + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); $this->createAttachmentForTicketModel->displayName = null; //-- Assert @@ -115,6 +124,7 @@ class AttachmentHandlerTest extends TestCase { function testThatValidateThrowsAnExceptionWhenTheDisplayNameIsEmpty() { //-- Arrange + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); $this->createAttachmentForTicketModel->displayName = ''; //-- Assert @@ -127,6 +137,7 @@ class AttachmentHandlerTest extends TestCase { function testThatValidateThrowsAnExceptionWhenTheTicketIdIsNull() { //-- Arrange + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); $this->createAttachmentForTicketModel->ticketId = null; //-- Assert @@ -139,6 +150,7 @@ class AttachmentHandlerTest extends TestCase { function testThatValidateThrowsAnExceptionWhenTheTicketIdIsANonPositiveInteger() { //-- Arrange + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); $this->createAttachmentForTicketModel->ticketId = 0; //-- Assert @@ -151,6 +163,7 @@ class AttachmentHandlerTest extends TestCase { function testThatValidateThrowsAnExceptionWhenTheFileExtensionIsNotPermitted() { //-- Arrange + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); $this->heskSettings['attachments']['allowed_types'] = array('.gif'); $this->createAttachmentForTicketModel->ticketId = 0; @@ -164,6 +177,7 @@ class AttachmentHandlerTest extends TestCase { function testThatValidateThrowsAnExceptionWhenTheFileSizeIsLargerThanMaxPermitted() { //-- Arrange + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); $this->createAttachmentForTicketModel->attachmentContents = base64_encode("msg"); $this->heskSettings['attachments']['max_size'] = 1; @@ -177,6 +191,7 @@ class AttachmentHandlerTest extends TestCase { function testItSavesATicketWithTheProperProperties() { //-- Arrange + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); $this->createAttachmentForTicketModel->ticketId = 1; $ticket = new Ticket(); $ticket->trackingId = 'ABC-DEF-1234'; @@ -205,6 +220,7 @@ class AttachmentHandlerTest extends TestCase { function testItSavesTheFileToTheFileSystem() { //-- Arrange + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); $this->createAttachmentForTicketModel->ticketId = 1; $ticket = new Ticket(); $ticket->trackingId = 'ABC-DEF-1234'; @@ -229,4 +245,60 @@ class AttachmentHandlerTest extends TestCase { } //-- TODO Test UserToTicketChecker + + function testDeleteThrowsAnExceptionWhenTheUserDoesNotHaveAccessToTheTicket() { + //-- Arrange + $ticketId = 1; + $ticket = new Ticket(); + $this->ticketGateway->method('getTicketById') + ->with($ticketId, $this->heskSettings)->willReturn($ticket); + $this->userToTicketChecker->method('isTicketWritableToUser') + ->with($this->userContext, $ticket, true, $this->heskSettings) + ->willReturn(false); + + //-- Assert + $this->expectException(\Exception::class); + $this->expectExceptionMessage("User does not have access to ticket {$ticketId} being created / edited!"); + + //-- Act + $this->attachmentHandler->deleteAttachmentFromTicket($ticketId, 1, $this->userContext, $this->heskSettings); + } + + function testDeleteActuallyDeletesTheFile() { + //-- Arrange + $ticketId = 1; + $ticket = new Ticket(); + $attachment = new Attachment(); + $attachment->id = 5; + $attachment->savedName = 'foobar.txt'; + $this->heskSettings['attach_dir'] = 'attach-dir'; + $ticket->attachments = array($attachment); + $this->ticketGateway->method('getTicketById')->willReturn($ticket); + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); + + //-- Assert + $this->fileDeleter->expects($this->once())->method('deleteFile')->with('foobar.txt', 'attach-dir'); + + //-- Act + $this->attachmentHandler->deleteAttachmentFromTicket($ticketId, 5, $this->userContext, $this->heskSettings); + } + + function testDeleteUpdatesTheTicketItselfAndSavesIt() { + //-- Arrange + $ticketId = 1; + $ticket = new Ticket(); + $attachment = new Attachment(); + $attachment->id = 5; + $attachment->savedName = 'foobar.txt'; + $this->heskSettings['attach_dir'] = 'attach-dir'; + $ticket->attachments = array($attachment); + $this->ticketGateway->method('getTicketById')->willReturn($ticket); + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); + + //-- Assert + $this->ticketGateway->expects($this->once())->method('updateAttachmentsForTicket'); + + //-- Act + $this->attachmentHandler->deleteAttachmentFromTicket($ticketId, 5, $this->userContext, $this->heskSettings); + } }