From d461059cf06da8bdb2064be7d5f296512f18e580 Mon Sep 17 00:00:00 2001 From: Mike Koch Date: Sun, 9 Apr 2017 22:14:13 -0400 Subject: [PATCH] Wrapped up retrieving ticket attachments --- api/ApplicationContext.php | 10 ++++++++- .../Attachments/AttachmentRetriever.php | 22 ++++++++++++++++--- .../StaffTicketAttachmentsController.php | 13 ++++++++--- .../Attachments/AttachmentRetrieverTest.php | 18 +++++++++++++-- api/index.php | 2 +- 5 files changed, 55 insertions(+), 10 deletions(-) diff --git a/api/ApplicationContext.php b/api/ApplicationContext.php index a9107a74..a170a5ec 100644 --- a/api/ApplicationContext.php +++ b/api/ApplicationContext.php @@ -2,6 +2,7 @@ // Responsible for loading in all necessary classes. AKA a poor man's DI solution. use BusinessLogic\Attachments\AttachmentHandler; +use BusinessLogic\Attachments\AttachmentRetriever; use BusinessLogic\Categories\CategoryRetriever; use BusinessLogic\Emails\BasicEmailSender; use BusinessLogic\Emails\EmailSenderHelper; @@ -21,6 +22,7 @@ use BusinessLogic\Tickets\TrackingIdGenerator; use BusinessLogic\Tickets\VerifiedEmailChecker; use DataAccess\Attachments\AttachmentGateway; use DataAccess\Categories\CategoryGateway; +use DataAccess\Files\FileReader; use DataAccess\Files\FileWriter; use DataAccess\Logging\LoggingGateway; use DataAccess\Security\BanGateway; @@ -99,9 +101,15 @@ class ApplicationContext { // Attachments $this->get[UserToTicketChecker::class] = new UserToTicketChecker($this->get[UserGateway::class]); $this->get[FileWriter::class] = new FileWriter(); + $this->get[FileReader::class] = new FileReader(); $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[FileWriter::class], + $this->get[UserToTicketChecker::class]); + $this->get[AttachmentRetriever::class] = new AttachmentRetriever($this->get[AttachmentGateway::class], + $this->get[FileReader::class], + $this->get[TicketGateway::class], + $this->get[UserToTicketChecker::class]); } } \ No newline at end of file diff --git a/api/BusinessLogic/Attachments/AttachmentRetriever.php b/api/BusinessLogic/Attachments/AttachmentRetriever.php index 00961d7f..c4889206 100644 --- a/api/BusinessLogic/Attachments/AttachmentRetriever.php +++ b/api/BusinessLogic/Attachments/AttachmentRetriever.php @@ -3,8 +3,10 @@ namespace BusinessLogic\Attachments; +use BusinessLogic\Security\UserToTicketChecker; use DataAccess\Attachments\AttachmentGateway; use DataAccess\Files\FileReader; +use DataAccess\Tickets\TicketGateway; class AttachmentRetriever { /* @var $attachmentGateway AttachmentGateway */ @@ -13,13 +15,27 @@ class AttachmentRetriever { /* @var $fileReader FileReader */ private $fileReader; - function __construct($attachmentGateway, $fileReader) { + /* @var $ticketGateway TicketGateway */ + private $ticketGateway; + + /* @var $userToTicketChecker UserToTicketChecker */ + private $userToTicketChecker; + + function __construct($attachmentGateway, $fileReader, $ticketGateway, $userToTicketChecker) { $this->attachmentGateway = $attachmentGateway; $this->fileReader = $fileReader; + $this->ticketGateway = $ticketGateway; + $this->userToTicketChecker = $userToTicketChecker; } - function getAttachmentContentsForTicket($id, $heskSettings) { - $attachment = $this->attachmentGateway->getAttachmentById($id, $heskSettings); + function getAttachmentContentsForTicket($ticketId, $attachmentId, $userContext, $heskSettings) { + $ticket = $this->ticketGateway->getTicketById($ticketId, $heskSettings); + + if (!$this->userToTicketChecker->isTicketWritableToUser($userContext, $ticket, false, $heskSettings)) { + throw new \Exception("User does not have access to attachment {$attachmentId}!"); + } + + $attachment = $this->attachmentGateway->getAttachmentById($attachmentId, $heskSettings); $contents = base64_encode($this->fileReader->readFromFile( $attachment->savedName, $heskSettings['attach_dir'])); diff --git a/api/Controllers/Attachments/StaffTicketAttachmentsController.php b/api/Controllers/Attachments/StaffTicketAttachmentsController.php index caa8b8ed..d06ddb75 100644 --- a/api/Controllers/Attachments/StaffTicketAttachmentsController.php +++ b/api/Controllers/Attachments/StaffTicketAttachmentsController.php @@ -4,6 +4,7 @@ namespace Controllers\Attachments; use BusinessLogic\Attachments\AttachmentHandler; +use BusinessLogic\Attachments\AttachmentRetriever; use BusinessLogic\Attachments\CreateAttachmentForTicketModel; use BusinessLogic\Exceptions\ApiFriendlyException; use BusinessLogic\Helpers; @@ -11,10 +12,15 @@ use BusinessLogic\Security\UserToTicketChecker; use Controllers\JsonRetriever; class StaffTicketAttachmentsController { - function get($attachmentId) { - global $hesk_settings, $applicationContext; + function get($ticketId, $attachmentId) { + global $hesk_settings, $applicationContext, $userContext; $this->verifyAttachmentsAreEnabled($hesk_settings); + + /* @var $attachmentRetriever AttachmentRetriever */ + $attachmentRetriever = $applicationContext->get[AttachmentRetriever::class]; + + $attachmentRetriever->getAttachmentContentsForTicket($ticketId, $attachmentId, $userContext, $hesk_settings); } private function verifyAttachmentsAreEnabled($heskSettings) { @@ -33,7 +39,8 @@ class StaffTicketAttachmentsController { $createAttachmentForTicketModel = $this->createModel(JsonRetriever::getJsonData(), $ticketId); - $createdAttachment = $attachmentHandler->createAttachmentForTicket($createAttachmentForTicketModel, $hesk_settings); + $createdAttachment = $attachmentHandler->createAttachmentForTicket( + $createAttachmentForTicketModel, $userContext, $hesk_settings); return output($createdAttachment, 201); } diff --git a/api/Tests/BusinessLogic/Attachments/AttachmentRetrieverTest.php b/api/Tests/BusinessLogic/Attachments/AttachmentRetrieverTest.php index 4790262c..d1b62e8a 100644 --- a/api/Tests/BusinessLogic/Attachments/AttachmentRetrieverTest.php +++ b/api/Tests/BusinessLogic/Attachments/AttachmentRetrieverTest.php @@ -4,8 +4,11 @@ namespace BusinessLogic\Attachments; +use BusinessLogic\Security\UserContext; +use BusinessLogic\Security\UserToTicketChecker; use DataAccess\Attachments\AttachmentGateway; use DataAccess\Files\FileReader; +use DataAccess\Tickets\TicketGateway; use PHPUnit\Framework\TestCase; class AttachmentRetrieverTest extends TestCase { @@ -15,6 +18,12 @@ class AttachmentRetrieverTest extends TestCase { /* @var $fileReader \PHPUnit_Framework_MockObject_MockObject */ private $fileReader; + /* @var $ticketGateway \PHPUnit_Framework_MockObject_MockObject */ + private $ticketGateway; + + /* @var $userToTicketChecker \PHPUnit_Framework_MockObject_MockObject */ + private $userToTicketChecker; + /* @var $attachmentRetriever AttachmentRetriever */ private $attachmentRetriever; @@ -24,9 +33,14 @@ class AttachmentRetrieverTest extends TestCase { protected function setUp() { $this->attachmentGateway = $this->createMock(AttachmentGateway::class); $this->fileReader = $this->createMock(FileReader::class); + $this->ticketGateway = $this->createMock(TicketGateway::class); + $this->userToTicketChecker = $this->createMock(UserToTicketChecker::class); $this->heskSettings = array('attach_dir' => 'attachments'); - $this->attachmentRetriever = new AttachmentRetriever($this->attachmentGateway, $this->fileReader); + $this->attachmentRetriever = new AttachmentRetriever($this->attachmentGateway, $this->fileReader, + $this->ticketGateway, $this->userToTicketChecker); + + $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true); } function testItGetsTheMetadataFromTheGateway() { @@ -43,7 +57,7 @@ class AttachmentRetrieverTest extends TestCase { ->willReturn($attachmentContents); //-- Act - $actualContents = $this->attachmentRetriever->getAttachmentContentsForTicket(4, $this->heskSettings); + $actualContents = $this->attachmentRetriever->getAttachmentContentsForTicket(0, 4, new UserContext(), $this->heskSettings); //-- Assert self::assertThat($actualContents, self::equalTo($expectedContents)); diff --git a/api/index.php b/api/index.php index b4c1ef29..1968e797 100644 --- a/api/index.php +++ b/api/index.php @@ -155,7 +155,7 @@ Link::all(array( // Attachments '/v1/staff/tickets/{i}/attachments' => \Controllers\Attachments\StaffTicketAttachmentsController::class, - '/v1/staff/attachments/{i}' => \Controllers\Attachments\StaffTicketAttachmentsController::class, + '/v1/staff/tickets/{i}/attachments/{i}' => \Controllers\Attachments\StaffTicketAttachmentsController::class, // Any URL that doesn't match goes to the 404 handler '404' => 'handle404'