More work, forgot to include security when creating attachment

remotes/upstream/api-rewrite
Mike Koch 7 years ago
parent c58867577f
commit e54d5278ac

@ -10,6 +10,7 @@ use BusinessLogic\Emails\EmailTemplateRetriever;
use BusinessLogic\Emails\MailgunEmailSender; use BusinessLogic\Emails\MailgunEmailSender;
use BusinessLogic\Security\BanRetriever; use BusinessLogic\Security\BanRetriever;
use BusinessLogic\Security\UserContextBuilder; use BusinessLogic\Security\UserContextBuilder;
use BusinessLogic\Security\UserToTicketChecker;
use BusinessLogic\Settings\ApiChecker; use BusinessLogic\Settings\ApiChecker;
use BusinessLogic\Tickets\Autoassigner; use BusinessLogic\Tickets\Autoassigner;
use BusinessLogic\Tickets\TicketRetriever; use BusinessLogic\Tickets\TicketRetriever;
@ -96,6 +97,7 @@ class ApplicationContext {
$this->get[ModsForHeskSettingsGateway::class]); $this->get[ModsForHeskSettingsGateway::class]);
// Attachments // Attachments
$this->get[UserToTicketChecker::class] = new UserToTicketChecker($this->get[UserGateway::class]);
$this->get[FileWriter::class] = new FileWriter(); $this->get[FileWriter::class] = new FileWriter();
$this->get[AttachmentGateway::class] = new AttachmentGateway(); $this->get[AttachmentGateway::class] = new AttachmentGateway();
$this->get[AttachmentHandler::class] = new AttachmentHandler($this->get[TicketGateway::class], $this->get[AttachmentHandler::class] = new AttachmentHandler($this->get[TicketGateway::class],

@ -4,6 +4,8 @@ namespace BusinessLogic\Attachments;
use BusinessLogic\Exceptions\ValidationException; use BusinessLogic\Exceptions\ValidationException;
use BusinessLogic\Security\UserContext;
use BusinessLogic\Security\UserToTicketChecker;
use BusinessLogic\Tickets\Attachment; use BusinessLogic\Tickets\Attachment;
use BusinessLogic\Tickets\Ticket; use BusinessLogic\Tickets\Ticket;
use BusinessLogic\ValidationModel; use BusinessLogic\ValidationModel;
@ -21,24 +23,35 @@ class AttachmentHandler {
/* @var $fileWriter FileWriter */ /* @var $fileWriter FileWriter */
private $fileWriter; private $fileWriter;
function __construct($ticketGateway, $attachmentGateway, $fileWriter) { /* @var $userToTicketChecker UserToTicketChecker */
private $userToTicketChecker;
function __construct($ticketGateway, $attachmentGateway, $fileWriter, $userToTicketChecker) {
$this->ticketGateway = $ticketGateway; $this->ticketGateway = $ticketGateway;
$this->attachmentGateway = $attachmentGateway; $this->attachmentGateway = $attachmentGateway;
$this->fileWriter = $fileWriter; $this->fileWriter = $fileWriter;
$this->userToTicketChecker = $userToTicketChecker;
} }
/** /**
* @param $createAttachmentModel CreateAttachmentForTicketModel * @param $createAttachmentModel CreateAttachmentForTicketModel
* @param $userContext UserContext
* @param $heskSettings array * @param $heskSettings array
* @return TicketAttachment the newly created attachment * @return TicketAttachment the newly created attachment
* @throws \Exception
*/ */
function createAttachmentForTicket($createAttachmentModel, $heskSettings) { function createAttachmentForTicket($createAttachmentModel, $userContext, $heskSettings) {
$this->validate($createAttachmentModel, $heskSettings); $this->validate($createAttachmentModel, $heskSettings);
$decodedAttachment = base64_decode($createAttachmentModel->attachmentContents); $decodedAttachment = base64_decode($createAttachmentModel->attachmentContents);
$ticket = $this->ticketGateway->getTicketById($createAttachmentModel->ticketId, $heskSettings); $ticket = $this->ticketGateway->getTicketById($createAttachmentModel->ticketId, $heskSettings);
if (!$this->userToTicketChecker->isTicketWritableToUser($userContext, $ticket, $createAttachmentModel->isEditing, $heskSettings)) {
throw new \Exception("User does not have access to ticket {$ticket->id} being created / edited!");
}
$cleanedFileName = $this->cleanFileName($createAttachmentModel->displayName); $cleanedFileName = $this->cleanFileName($createAttachmentModel->displayName);
$fileParts = pathinfo($cleanedFileName); $fileParts = pathinfo($cleanedFileName);

@ -15,4 +15,7 @@ class CreateAttachmentModel {
/* @var $attachmentContents string */ /* @var $attachmentContents string */
public $attachmentContents; public $attachmentContents;
/* @var $isEditing bool */
public $isEditing;
} }

@ -7,6 +7,7 @@ use BusinessLogic\Attachments\AttachmentHandler;
use BusinessLogic\Attachments\CreateAttachmentForTicketModel; use BusinessLogic\Attachments\CreateAttachmentForTicketModel;
use BusinessLogic\Exceptions\ApiFriendlyException; use BusinessLogic\Exceptions\ApiFriendlyException;
use BusinessLogic\Helpers; use BusinessLogic\Helpers;
use BusinessLogic\Security\UserToTicketChecker;
use Controllers\JsonRetriever; use Controllers\JsonRetriever;
class StaffTicketAttachmentsController { class StaffTicketAttachmentsController {
@ -23,7 +24,7 @@ class StaffTicketAttachmentsController {
} }
function post($ticketId) { function post($ticketId) {
global $hesk_settings, $applicationContext; global $hesk_settings, $applicationContext, $userContext;
$this->verifyAttachmentsAreEnabled($hesk_settings); $this->verifyAttachmentsAreEnabled($hesk_settings);

@ -5,6 +5,8 @@ namespace BusinessLogic\Attachments;
use BusinessLogic\Exceptions\ValidationException; use BusinessLogic\Exceptions\ValidationException;
use BusinessLogic\Security\UserContext;
use BusinessLogic\Security\UserToTicketChecker;
use BusinessLogic\Tickets\Ticket; use BusinessLogic\Tickets\Ticket;
use DataAccess\Attachments\AttachmentGateway; use DataAccess\Attachments\AttachmentGateway;
use DataAccess\Files\FileWriter; use DataAccess\Files\FileWriter;
@ -25,9 +27,15 @@ class AttachmentHandlerTest extends TestCase {
/* @var $attachmentGateway \PHPUnit_Framework_MockObject_MockObject */ /* @var $attachmentGateway \PHPUnit_Framework_MockObject_MockObject */
private $attachmentGateway; private $attachmentGateway;
/* @var $userToTicketChecker \PHPUnit_Framework_MockObject_MockObject */
private $userToTicketChecker;
/* @var $fileWriter \PHPUnit_Framework_MockObject_MockObject */ /* @var $fileWriter \PHPUnit_Framework_MockObject_MockObject */
private $fileWriter; private $fileWriter;
/* @var $userContext UserContext */
private $userContext;
/* @var $heskSettings array */ /* @var $heskSettings array */
private $heskSettings; private $heskSettings;
@ -35,6 +43,8 @@ class AttachmentHandlerTest extends TestCase {
$this->ticketGateway = $this->createMock(TicketGateway::class); $this->ticketGateway = $this->createMock(TicketGateway::class);
$this->attachmentGateway = $this->createMock(AttachmentGateway::class); $this->attachmentGateway = $this->createMock(AttachmentGateway::class);
$this->fileWriter = $this->createMock(FileWriter::class); $this->fileWriter = $this->createMock(FileWriter::class);
$this->userToTicketChecker = $this->createMock(UserToTicketChecker::class);
$this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
$this->heskSettings = array( $this->heskSettings = array(
'attach_dir' => 'attachments', 'attach_dir' => 'attachments',
'attachments' => array( 'attachments' => array(
@ -43,12 +53,16 @@ class AttachmentHandlerTest extends TestCase {
) )
); );
$this->attachmentHandler = new AttachmentHandler($this->ticketGateway, $this->attachmentGateway, $this->fileWriter); $this->attachmentHandler = new AttachmentHandler($this->ticketGateway,
$this->attachmentGateway,
$this->fileWriter,
$this->userToTicketChecker);
$this->createAttachmentForTicketModel = new CreateAttachmentForTicketModel(); $this->createAttachmentForTicketModel = new CreateAttachmentForTicketModel();
$this->createAttachmentForTicketModel->attachmentContents = base64_encode('string'); $this->createAttachmentForTicketModel->attachmentContents = base64_encode('string');
$this->createAttachmentForTicketModel->displayName = 'DisplayName.txt'; $this->createAttachmentForTicketModel->displayName = 'DisplayName.txt';
$this->createAttachmentForTicketModel->ticketId = 1; $this->createAttachmentForTicketModel->ticketId = 1;
$this->createAttachmentForTicketModel->type = AttachmentType::MESSAGE; $this->createAttachmentForTicketModel->type = AttachmentType::MESSAGE;
$this->userContext = new UserContext();
} }
function testThatValidateThrowsAnExceptionWhenTheAttachmentBodyIsNull() { function testThatValidateThrowsAnExceptionWhenTheAttachmentBodyIsNull() {
@ -60,7 +74,7 @@ class AttachmentHandlerTest extends TestCase {
$this->expectExceptionMessageRegExp('/CONTENTS_EMPTY/'); $this->expectExceptionMessageRegExp('/CONTENTS_EMPTY/');
//-- Act //-- Act
$this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->heskSettings); $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->userContext, $this->heskSettings);
} }
function testThatValidateThrowsAnExceptionWhenTheAttachmentBodyIsEmpty() { function testThatValidateThrowsAnExceptionWhenTheAttachmentBodyIsEmpty() {
@ -72,7 +86,7 @@ class AttachmentHandlerTest extends TestCase {
$this->expectExceptionMessageRegExp('/CONTENTS_EMPTY/'); $this->expectExceptionMessageRegExp('/CONTENTS_EMPTY/');
//-- Act //-- Act
$this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->heskSettings); $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->userContext, $this->heskSettings);
} }
function testThatValidateThrowsAnExceptionWhenTheAttachmentBodyIsInvalidBase64() { function testThatValidateThrowsAnExceptionWhenTheAttachmentBodyIsInvalidBase64() {
@ -84,7 +98,7 @@ class AttachmentHandlerTest extends TestCase {
$this->expectExceptionMessageRegExp('/CONTENTS_NOT_BASE_64/'); $this->expectExceptionMessageRegExp('/CONTENTS_NOT_BASE_64/');
//-- Act //-- Act
$this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->heskSettings); $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->userContext, $this->heskSettings);
} }
function testThatValidateThrowsAnExceptionWhenTheDisplayNameIsNull() { function testThatValidateThrowsAnExceptionWhenTheDisplayNameIsNull() {
@ -96,7 +110,7 @@ class AttachmentHandlerTest extends TestCase {
$this->expectExceptionMessageRegExp('/DISPLAY_NAME_EMPTY/'); $this->expectExceptionMessageRegExp('/DISPLAY_NAME_EMPTY/');
//-- Act //-- Act
$this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->heskSettings); $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->userContext, $this->heskSettings);
} }
function testThatValidateThrowsAnExceptionWhenTheDisplayNameIsEmpty() { function testThatValidateThrowsAnExceptionWhenTheDisplayNameIsEmpty() {
@ -108,7 +122,7 @@ class AttachmentHandlerTest extends TestCase {
$this->expectExceptionMessageRegExp('/DISPLAY_NAME_EMPTY/'); $this->expectExceptionMessageRegExp('/DISPLAY_NAME_EMPTY/');
//-- Act //-- Act
$this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->heskSettings); $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->userContext, $this->heskSettings);
} }
function testThatValidateThrowsAnExceptionWhenTheTicketIdIsNull() { function testThatValidateThrowsAnExceptionWhenTheTicketIdIsNull() {
@ -120,7 +134,7 @@ class AttachmentHandlerTest extends TestCase {
$this->expectExceptionMessageRegExp('/TICKET_ID_MISSING/'); $this->expectExceptionMessageRegExp('/TICKET_ID_MISSING/');
//-- Act //-- Act
$this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->heskSettings); $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->userContext, $this->heskSettings);
} }
function testThatValidateThrowsAnExceptionWhenTheTicketIdIsANonPositiveInteger() { function testThatValidateThrowsAnExceptionWhenTheTicketIdIsANonPositiveInteger() {
@ -132,7 +146,7 @@ class AttachmentHandlerTest extends TestCase {
$this->expectExceptionMessageRegExp('/TICKET_ID_MISSING/'); $this->expectExceptionMessageRegExp('/TICKET_ID_MISSING/');
//-- Act //-- Act
$this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->heskSettings); $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->userContext, $this->heskSettings);
} }
function testThatValidateThrowsAnExceptionWhenTheFileExtensionIsNotPermitted() { function testThatValidateThrowsAnExceptionWhenTheFileExtensionIsNotPermitted() {
@ -145,7 +159,7 @@ class AttachmentHandlerTest extends TestCase {
$this->expectExceptionMessageRegExp('/EXTENSION_NOT_PERMITTED/'); $this->expectExceptionMessageRegExp('/EXTENSION_NOT_PERMITTED/');
//-- Act //-- Act
$this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->heskSettings); $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->userContext, $this->heskSettings);
} }
function testThatValidateThrowsAnExceptionWhenTheFileSizeIsLargerThanMaxPermitted() { function testThatValidateThrowsAnExceptionWhenTheFileSizeIsLargerThanMaxPermitted() {
@ -158,7 +172,7 @@ class AttachmentHandlerTest extends TestCase {
$this->expectExceptionMessageRegExp('/FILE_SIZE_TOO_LARGE/'); $this->expectExceptionMessageRegExp('/FILE_SIZE_TOO_LARGE/');
//-- Act //-- Act
$this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->heskSettings); $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->userContext, $this->heskSettings);
} }
function testItSavesATicketWithTheProperProperties() { function testItSavesATicketWithTheProperProperties() {
@ -179,7 +193,7 @@ class AttachmentHandlerTest extends TestCase {
//-- Act //-- Act
$actual = $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->heskSettings); $actual = $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->userContext, $this->heskSettings);
//-- Assert //-- Assert
self::assertThat($actual->id, self::equalTo(50)); self::assertThat($actual->id, self::equalTo(50));
@ -208,9 +222,11 @@ class AttachmentHandlerTest extends TestCase {
//-- Act //-- Act
$actual = $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->heskSettings); $actual = $this->attachmentHandler->createAttachmentForTicket($this->createAttachmentForTicketModel, $this->userContext, $this->heskSettings);
//-- Assert //-- Assert
self::assertThat($actual->fileSize, self::equalTo(1024)); self::assertThat($actual->fileSize, self::equalTo(1024));
} }
//-- TODO Test UserToTicketChecker
} }

Loading…
Cancel
Save