diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bb803342..426e7738e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Log the project folder setup and terms of service signed for the OpenProject user to the audit log - Log users connecting and disconnecting their OpenProject account to the audit log - Log files linked to and unlinked from OpenProject work packages to the audit log +- Handle exceeding post_max_size and upload_max_filesize [#1190](https://github.com/nextcloud/integration_openproject/pull/1190) ### Fixed - Keep the admin settings when Nextcloud replays the migrations to check the database schema (`occ db:schema:check`, every `occ upgrade` on Nextcloud 35) diff --git a/lib/Controller/DirectUploadController.php b/lib/Controller/DirectUploadController.php index d0c7bef49..cb14803da 100644 --- a/lib/Controller/DirectUploadController.php +++ b/lib/Controller/DirectUploadController.php @@ -8,10 +8,8 @@ namespace OCA\OpenProject\Controller; use \OCP\AppFramework\ApiController; -use DateTime; use InvalidArgumentException; use OC\Files\Filesystem; -use OC\Files\Node\Folder; use OC\ForbiddenException; use OC\User\NoUserException; use OCA\OpenProject\Exception\OpenprojectFileNotUploadedException; @@ -35,6 +33,7 @@ use OCP\IUserManager; use OCP\IUserSession; use OCP\Lock\LockedException; +use OCP\Util; use Sabre\DAV\Exception\Conflict; class DirectUploadController extends ApiController { @@ -78,6 +77,17 @@ class DirectUploadController extends ApiController { private $l; + private const PHP_FILE_UPLOAD_ERRORS = [ + UPLOAD_ERR_OK => 'The file was uploaded', + UPLOAD_ERR_INI_SIZE => 'The uploaded file exceeds the upload_max_filesize directive in php.ini', + UPLOAD_ERR_FORM_SIZE => 'The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML form', + UPLOAD_ERR_PARTIAL => 'The file was only partially uploaded', + UPLOAD_ERR_NO_FILE => 'No file was uploaded', + UPLOAD_ERR_NO_TMP_DIR => 'Missing a temporary folder', + UPLOAD_ERR_CANT_WRITE => 'Could not write file to disk', + UPLOAD_ERR_EXTENSION => 'A PHP extension stopped the file upload', + ]; + public function __construct( string $appName, IRequest $request, @@ -154,93 +164,46 @@ public function directUpload(string $token):DataResponse { try { if (strlen($token) !== 64 || !preg_match('/^[a-zA-Z0-9]*/', $token)) { $this->databaseService->deleteToken($token); - throw new NotFoundException('invalid token'); + throw new OpenprojectUnauthorizedUserException('invalid token'); } - $tokenInfo = $this->directUploadService->getTokenInfo($token); + + $postMaxSizeInBytes = (int)Util::computerFileSize(ini_get('post_max_size')); + $contentLength = (int)$this->request->getHeader('Content-Length'); + if ($postMaxSizeInBytes > 0 && $contentLength > $postMaxSizeInBytes) { + throw new OpenprojectFileNotUploadedException( + 'File was not uploaded. The request exceeds the maximum post size.', + ); + } + $directUploadFile = $this->request->getUploadedFile('file'); if (empty($directUploadFile)) { - throw new OpenprojectFileNotUploadedException( - 'File was not uploaded. post_max_size exceeded?' + throw new \InvalidArgumentException( + 'No file is present to upload.' ); } - $fileName = trim($directUploadFile['name']); - $this->scanForInvalidCharacters($fileName, "\\/"); - if (empty($directUploadFile['tmp_name']) || $directUploadFile['error'] === 1) { + + if (array_key_exists('error', $directUploadFile) && $directUploadFile['error'] !== UPLOAD_ERR_OK) { throw new OpenprojectFileNotUploadedException( - 'File was not uploaded. upload_max_filesize exceeded?' + self::PHP_FILE_UPLOAD_ERRORS[$directUploadFile['error']], ); } - $tmpPath = $directUploadFile['tmp_name']; + + $fileName = trim($directUploadFile['name']); + $this->scanForInvalidCharacters($fileName, "\\/"); if (Filesystem::isFileBlacklisted($fileName)) { throw new ForbiddenException('invalid file name'); } + $overwrite = $this->request->getParam('overwrite'); if (isset($overwrite)) { $acceptedOverwriteValues = ['true','false']; $overwrite = strtolower($overwrite); - if (in_array($overwrite, $acceptedOverwriteValues)) { - $overwrite = $overwrite === 'true'; - } else { + if (!in_array($overwrite, $acceptedOverwriteValues)) { throw new \InvalidArgumentException('invalid overwrite value'); } - } else { - $overwrite = null; - } - $user = $this->userManager->get($tokenInfo['user_id']); - $this->userSession->setUser($user); - $userFolder = $this->rootFolder->getUserFolder($user->getUID()); - $nodes = $userFolder->getById($tokenInfo['folder_id']); - if (empty($nodes)) { - throw new NotFoundException('folder not found or not enough permissions'); - } - /** - * @var Folder $folderNode - */ - $folderNode = array_shift($nodes); - if (!$folderNode->isCreatable() && !$overwrite) { - throw new ForbiddenException('not enough permissions'); - } - $freeSpace = $folderNode->getFreeSpace(); - - // this is also true if we try to overwrite - // to overwrite a file we need enough free quota for the new data - // otherwise `putContent()` fails, - // if freeSpace is smaller than 0 then treat it as a unlimited space - // and don't throw an error - if ($directUploadFile['size'] > $freeSpace && $freeSpace >= 0) { - throw new NotEnoughSpaceException('insufficient quota'); + $overwrite = $overwrite === 'true'; } - if ($folderNode->nodeExists($fileName) && $overwrite) { - /** - * @var File $file - */ - $file = $folderNode->get($fileName); - if ($file->getType() === FileInfo::TYPE_FOLDER) { - throw new Conflict('overwrite is not allowed on non-files'); - } - if (!$file->isUpdateable()) { - throw new ForbiddenException('not enough permissions'); - } - // overwrite the file - $file->putContent(fopen($tmpPath, 'r')); - $fileId = $file->getId(); - return new DataResponse([ - 'file_name' => $fileName, - 'file_id' => $fileId - ], Http::STATUS_OK); - } elseif ($folderNode->nodeExists($fileName) && $overwrite === false) { - // get unique name for duplicate file with number suffix - $fileName = $folderNode->getNonExistingName($fileName); - } elseif ($folderNode->nodeExists($fileName)) { - throw new Conflict('conflict, file name already exists'); - } - $fileInfo = $folderNode->newFile($fileName, fopen($tmpPath, 'r')); - $fileId = $fileInfo->getId(); - // setting the creation time for the uploaded file - $creationTime = (new DateTime())->getTimestamp(); - $fileInfo->getStorage()->getCache()->update($fileId, [ - 'creation_time' => $creationTime - ]); + $result = $this->directUploadService->uploadFile($token, $directUploadFile, $overwrite); } catch (OpenprojectUnauthorizedUserException $e) { return new DataResponse([ 'error' => $this->l->t($e->getMessage()) @@ -269,8 +232,7 @@ public function directUpload(string $token):DataResponse { ], Http::STATUS_INSUFFICIENT_STORAGE); } catch (OpenprojectFileNotUploadedException $e) { return new DataResponse([ - 'error' => $this->l->t($e->getMessage()), - 'upload_limit' => \OCP\Util::uploadLimit() + 'error' => $this->l->t($e->getMessage()) ], Http::STATUS_REQUEST_ENTITY_TOO_LARGE); } catch (InvalidContentException $e) { // files_antivirus throws this exception return new DataResponse([ @@ -285,10 +247,11 @@ public function directUpload(string $token):DataResponse { 'error' => $this->l->t($e->getMessage()) ], Http::STATUS_INTERNAL_SERVER_ERROR); } + $statusCode = $overwrite ? Http::STATUS_OK : Http::STATUS_CREATED; return new DataResponse([ - 'file_name' => $fileName, - 'file_id' => $fileId - ], Http::STATUS_CREATED); + 'file_name' => $result['file_name'], + 'file_id' => $result['file_id'], + ], $statusCode); } /** diff --git a/lib/Service/DirectUploadService.php b/lib/Service/DirectUploadService.php index fb319f52e..02fe6ba7c 100644 --- a/lib/Service/DirectUploadService.php +++ b/lib/Service/DirectUploadService.php @@ -8,12 +8,20 @@ namespace OCA\OpenProject\Service; use DateTime; +use OC\Files\Node\Folder; +use OC\ForbiddenException; use OCA\OpenProject\Exception\OpenprojectUnauthorizedUserException; use OCP\DB\Exception; +use OCP\Files\File; +use OCP\Files\FileInfo; +use OCP\Files\IRootFolder; +use OCP\Files\NotEnoughSpaceException; use OCP\Files\NotFoundException; use OCP\IL10N; use OCP\IUserManager; +use OCP\IUserSession; use OCP\Security\ISecureRandom; +use Sabre\DAV\Exception\Conflict; class DirectUploadService { @@ -40,16 +48,30 @@ class DirectUploadService { /** @var string time of token expiration */ private string $expiryTime = '+1 hour'; + /** + * @var IUserSession + */ + private IUserSession $userSession; + + /** + * @var IRootFolder + */ + private IRootFolder $rootFolder; + public function __construct( - IUserManager $userManager, - IL10N $l, - ISecureRandom $secureRandom, - DatabaseService $databaseService + IUserManager $userManager, + IL10N $l, + ISecureRandom $secureRandom, + DatabaseService $databaseService, + IUserSession $userSession, + IRootFolder $rootFolder, ) { $this->l = $l; $this->userManager = $userManager; $this->secureRandom = $secureRandom; $this->databaseService = $databaseService; + $this->userSession = $userSession; + $this->rootFolder = $rootFolder; } /** @@ -100,4 +122,83 @@ public function getTokenInfo(string $token): ?array { } return $tokenInfo; } + + /** + * + * uploads a file + * + * @param string $token + * @param mixed $directUploadFile + * @param bool|null $overwrite + * + * @return array{file_id: int, file_name: string} + * + * @throws NotFoundException + * @throws ForbiddenException + * @throws NotEnoughSpaceException + * @throws Conflict + */ + public function uploadFile(string $token, mixed $directUploadFile, ?bool $overwrite) : array { + $tokenInfo = $this->getTokenInfo($token); + $user = $this->userManager->get($tokenInfo['user_id']); + $this->userSession->setUser($user); + $userFolder = $this->rootFolder->getUserFolder($user->getUID()); + $nodes = $userFolder->getById($tokenInfo['folder_id']); + if (empty($nodes)) { + throw new NotFoundException('folder not found or not enough permissions'); + } + /** + * @var Folder $folderNode + */ + $folderNode = array_shift($nodes); + if (!$folderNode->isCreatable() && !$overwrite) { + throw new ForbiddenException('not enough permissions'); + } + $freeSpace = $folderNode->getFreeSpace(); + + // this is also true if we try to overwrite + // to overwrite a file we need enough free quota for the new data + // otherwise `putContent()` fails, + // if freeSpace is smaller than 0 then treat it as a unlimited space + // and don't throw an error + if ($directUploadFile['size'] > $freeSpace && $freeSpace >= 0) { + throw new NotEnoughSpaceException('insufficient quota'); + } + $fileName = trim($directUploadFile['name']); + $tmpPath = $directUploadFile['tmp_name']; + if ($folderNode->nodeExists($fileName) && $overwrite) { + /** + * @var File $file + */ + $file = $folderNode->get($fileName); + if ($file->getType() === FileInfo::TYPE_FOLDER) { + throw new Conflict('overwrite is not allowed on non-files'); + } + if (!$file->isUpdateable()) { + throw new ForbiddenException('not enough permissions'); + } + // overwrite the file + $file->putContent(fopen($tmpPath, 'r')); + return [ + 'file_id' => $file->getId(), + 'file_name' => $fileName, + ]; + } elseif ($folderNode->nodeExists($fileName) && $overwrite === false) { + // get unique name for duplicate file with number suffix + $fileName = $folderNode->getNonExistingName($fileName); + } elseif ($folderNode->nodeExists($fileName)) { + throw new Conflict('conflict, file name already exists'); + } + $fileInfo = $folderNode->newFile($fileName, fopen($tmpPath, 'r')); + $fileId = $fileInfo->getId(); + // setting the creation time for the uploaded file + $creationTime = (new DateTime())->getTimestamp(); + $fileInfo->getStorage()->getCache()->update($fileId, [ + 'creation_time' => $creationTime + ]); + return [ + 'file_id' => $fileId, + 'file_name' => $fileName, + ]; + } } diff --git a/tests/acceptance/features/api/directUpload.feature b/tests/acceptance/features/api/directUpload.feature index 78233d8da..00f99e32a 100644 --- a/tests/acceptance/features/api/directUpload.feature +++ b/tests/acceptance/features/api/directUpload.feature @@ -70,18 +70,17 @@ Feature: API endpoint for direct upload When an anonymous user sends a multipart form data POST request to the "direct-upload/%last-created-direct-upload-token%" endpoint with: | file_name | "" | | data | some data | - Then the HTTP status code should be "413" + # An empty file name means no file is received, so this is a bad request (400), not content too large (413) + Then the HTTP status code should be "400" And the data of the response should match """" { "type": "object", "required": [ - "error", - "upload_limit" + "error" ], "properties": { - "error": {"type": "string", "pattern": "^File was not uploaded\\. post_max_size exceeded\\?$"}, - "upload_limit": {"type": "integer"} + "error": {"type": "string", "pattern": "No file is present to upload."} } } """ @@ -260,20 +259,22 @@ Feature: API endpoint for direct upload When an anonymous user sends a multipart form data POST request to the "direct-upload/%last-created-direct-upload-token%" endpoint with: | file_name | "" | | data | some data | - Then the HTTP status code should be "413" + Then the HTTP status code should be "400" When an anonymous user sends a multipart form data POST request to the "direct-upload/%last-created-direct-upload-token%" endpoint with: | file_name | file.txt | | data | some data | - Then the HTTP status code should be "401" + Then the HTTP status code should be "201" And the data of the response should match """" { "type": "object", "required": [ - "error" + "file_name", + "file_id" ], "properties": { - "error": {"type": "string", "pattern": "^unauthorized$"} + "file_name": {"type": "string", "pattern": "^file\\.txt$"}, + "file_id": {"type" : "integer"} } } """ diff --git a/tests/lib/Controller/DirectUploadControllerTest.php b/tests/lib/Controller/DirectUploadControllerTest.php index c0c25701e..a0c995076 100644 --- a/tests/lib/Controller/DirectUploadControllerTest.php +++ b/tests/lib/Controller/DirectUploadControllerTest.php @@ -7,12 +7,16 @@ namespace OCA\OpenProject\Controller; +use OCA\OpenProject\Service\DatabaseService; +use OCA\OpenProject\Service\DirectUploadService; use OCP\Files\Folder; -use OCP\Files\ForbiddenException as FileAccessForbiddenException; -use OCP\Files\InvalidContentException; +use OCP\Files\IRootFolder; use OCP\Files\IUserFolder; use OCP\IL10N; use OCP\IRequest; +use OCP\IUser; +use OCP\IUserManager; +use OCP\IUserSession; use PHPUnit\Framework\TestCase; use function PHPUnit\Framework\assertSame; @@ -22,6 +26,7 @@ class DirectUploadControllerTest extends TestCase { * @var IL10N */ private $l; + private const TOKEN = 'WampxL5Z97CndGwB7qLPfotosDT5mXk7oFyGLa64nmY35ANtkzT7zDQwYyXrbdC3'; /** * @return mixed @@ -36,14 +41,14 @@ public function getFolderMock(): mixed { /** * @return void */ - public function testprepareDirectUpload() { + public function testPrepareDirectUpload() { $folderMock = $this->getFolderMock(); $folderMock->method('getById')->willReturn($this->getNodeMock('dir')); $directUploadController = $this->createDirectUploadController($folderMock); $result = $directUploadController->prepareDirectUpload(123); assertSame( [ - 'token' => 'WampxL5Z97CndGwB7qLPfotosDT5mXk7oFyGLa64nmY35ANtkzT7zDQwYyXrbdC3', + 'token' => self::TOKEN, 'expires_on' => 1671537939 ], $result->getData() @@ -54,7 +59,7 @@ public function testprepareDirectUpload() { /** * @return void */ - public function testprepareDirectUploadTypeFile(): void { + public function testPrepareDirectUploadTypeFile(): void { $folderMock = $this->getFolderMock(); $folderMock->method('getById')->willReturn($this->getNodeMock('file')); $directUploadController = $this->createDirectUploadController($folderMock); @@ -68,7 +73,7 @@ public function testprepareDirectUploadTypeFile(): void { assertSame(404, $result->getStatus()); } - public function testprepareDirectUploadException(): void { + public function testPrepareDirectUploadException(): void { $folderMock = $this->getFolderMock(); $folderMock->method('getById') ->will($this->throwException(new \Exception('something bad happened'))); @@ -113,28 +118,7 @@ public function testDirectUploadInvalidToken(string $token):void { ], $result->getData() ); - assertSame(404, $result->getStatus()); - } - - public function testDirectUploadNotEnoughSpace():void { - $nodeMock = $this->getNodeMock('folder'); - $nodeMock[0]->method('getFreeSpace')->willReturn(100); - - $userFolderMock = $this->getFolderMock(); - $userFolderMock->method('getById')->willReturn($nodeMock); - $directUploadController = $this->createDirectUploadController( - $userFolderMock, 101 - ); - $result = $directUploadController->directUpload( - 'WampxL5Z97CndGwB7qLPfotosDT5mXk7oFyGLa64nmY35ANtkzT7zDQwYyXrbdC3' - ); - assertSame( - [ - 'error' => 'insufficient quota' - ], - $result->getData() - ); - assertSame(507, $result->getStatus()); + assertSame(401, $result->getStatus()); } /** @@ -142,104 +126,82 @@ public function testDirectUploadNotEnoughSpace():void { */ public function fileNotUploadedDataProvider() { return [ - ['', 1], - ['some name', 1], - ['', 0], + [UPLOAD_ERR_INI_SIZE, 'The uploaded file exceeds the upload_max_filesize directive in php.ini'], + [UPLOAD_ERR_FORM_SIZE, 'The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML form'], + [UPLOAD_ERR_PARTIAL, 'The file was only partially uploaded'], + [UPLOAD_ERR_NO_FILE, 'No file was uploaded'], + [UPLOAD_ERR_NO_TMP_DIR, 'Missing a temporary folder'], + [UPLOAD_ERR_CANT_WRITE, 'Could not write file to disk'], + [UPLOAD_ERR_EXTENSION, 'A PHP extension stopped the file upload'], ]; } /** - * @param string $tmpName * @param int $error + * @param string $expectedErrorMessage * @return void * @dataProvider fileNotUploadedDataProvider */ - public function testDirectUploadFileNotUploaded(string $tmpName, int $error):void { + public function testDirectUploadFileNotUploaded(int $error, string $expectedErrorMessage):void { $nodeMock = $this->getNodeMock('folder'); $userFolderMock = $this->getFolderMock(); $userFolderMock->method('getById')->willReturn($nodeMock); $directUploadController = $this->createDirectUploadController( - $userFolderMock, 100, $tmpName, $error + $userFolderMock, 100, '', $error ); $result = $directUploadController->directUpload( - 'WampxL5Z97CndGwB7qLPfotosDT5mXk7oFyGLa64nmY35ANtkzT7zDQwYyXrbdC3' + self::TOKEN, ); $resultArray = $result->getData(); assertSame( - 'File was not uploaded. upload_max_filesize exceeded?', + $expectedErrorMessage, $resultArray['error'] ); - self::assertIsNumeric($resultArray['upload_limit']); assertSame(413, $result->getStatus()); } - /** - * @return array> - */ - public function newFileExceptionsDataProvider() { - return [ - [new InvalidContentException('Virus detected'), 'Virus detected', 415], - [new FileAccessForbiddenException('Access denied by the access control', false), 'Access denied by the access control', 403], - [new \Exception('could not upload'), 'could not upload', 500], - ]; - } - /** * @return void - * @dataProvider newFileExceptionsDataProvider */ - public function testDirectUploadException( - \Exception $exception, string $expectedErrorMessage, int $expectedStatusCode - ):void { + public function testDirectUploadNoFile():void { $nodeMock = $this->getNodeMock('folder'); - $tmpFileName = '/tmp/integration_openproject_unit_test'; - touch($tmpFileName); - $nodeMock[0]->method('newFile')->will($this->throwException($exception)); + $userFolderMock = $this->getFolderMock(); $userFolderMock->method('getById')->willReturn($nodeMock); $directUploadController = $this->createDirectUploadController( - $userFolderMock, 0, $tmpFileName); + folderMock: $userFolderMock, noUploadedFile: true + ); $result = $directUploadController->directUpload( - 'WampxL5Z97CndGwB7qLPfotosDT5mXk7oFyGLa64nmY35ANtkzT7zDQwYyXrbdC3' + self::TOKEN, ); $resultArray = $result->getData(); assertSame( - $expectedErrorMessage, + 'No file is present to upload.', $resultArray['error'] ); - assertSame($expectedStatusCode, $result->getStatus()); + assertSame(400, $result->getStatus()); } - public function testNegativeFreeSpace(): void { - $cacheMock = $this->getMockBuilder('\OCP\Files\Cache\ICache')->getMock(); - $cacheMock->method('update')->willReturn(true); - $storageMock = $this->getMockBuilder('\OCP\Files\Storage\IStorage')->disableOriginalConstructor()->getMock(); - $storageMock->method('getCache')->willReturn($cacheMock); - - $fileMock = $this->getMockBuilder('\OC\Files\Node\File')->disableOriginalConstructor()->getMock(); - $fileMock->method('getId')->willReturn(123); - $fileMock->method('getStorage')->willReturn($storageMock); + /** + * @return void + */ + public function testDirectUploadPostMaxSizeExceeded():void { $nodeMock = $this->getNodeMock('folder'); - $tmpFileName = '/tmp/integration_openproject_unit_test'; - touch($tmpFileName); - $nodeMock[0]->method('getFreeSpace')->willReturn(-3); - $nodeMock[0]->method('newFile')->willReturn($fileMock); + $userFolderMock = $this->getFolderMock(); $userFolderMock->method('getById')->willReturn($nodeMock); $directUploadController = $this->createDirectUploadController( - $userFolderMock, 101, $tmpFileName + folderMock: $userFolderMock, contentLength: (string)PHP_INT_MAX ); $result = $directUploadController->directUpload( - 'WampxL5Z97CndGwB7qLPfotosDT5mXk7oFyGLa64nmY35ANtkzT7zDQwYyXrbdC3' + self::TOKEN, ); $resultArray = $result->getData(); assertSame( - [ - 'file_name' => 'file.txt', - 'file_id' => 123 - ], - $resultArray + 'File was not uploaded. The request exceeds the maximum post size.', + $resultArray['error'] ); + assertSame(413, $result->getStatus()); } @@ -248,30 +210,33 @@ public function testNegativeFreeSpace(): void { * @param int $uploadedFileSize * @param string $uploadedFileTmpName * @param int $uploadedFileError + * @param string $contentLength + * @param bool $noUploadedFile * @return DirectUploadController */ private function createDirectUploadController( mixed $folderMock, int $uploadedFileSize = 9999, string $uploadedFileTmpName = '/tmp/andjashd', - int $uploadedFileError = 0 + int $uploadedFileError = 0, + string $contentLength = '100', + bool $noUploadedFile = false, ): DirectUploadController { - $storageMock = $this->getMockBuilder('\OCP\Files\IRootFolder')->getMock(); + $storageMock = $this->createMock(IRootFolder::class); $storageMock->method('getUserFolder')->willReturn($folderMock); - $userMock = $this->getMockBuilder('\OCP\IUser')->getMock(); + $userMock = $this->createMock(IUser::class); $userMock->method('getUID')->willReturn('testUser'); - $userSessionMock = $this->getMockBuilder('\OCP\IUserSession')->getMock(); + $userSessionMock = $this->createMock(IUserSession::class); $userSessionMock->method('getUser')->willReturn($userMock); - $directUploadServiceMock = $this->getMockBuilder( - 'OCA\OpenProject\Service\DirectUploadService' - )->disableOriginalConstructor()->getMock(); + $directUploadServiceMock = $this->createMock( + DirectUploadService::class); $directUploadServiceMock->method('getTokenForDirectUpload') ->willReturn([ - 'token' => 'WampxL5Z97CndGwB7qLPfotosDT5mXk7oFyGLa64nmY35ANtkzT7zDQwYyXrbdC3', + 'token' => self::TOKEN, 'expires_on' => 1671537939 ]); @@ -289,12 +254,17 @@ private function createDirectUploadController( 'folder_id' => 123 ] ); - $userManagerMock = $this->getMockBuilder('OCP\IUserManager')->disableOriginalConstructor()->getMock(); + $userManagerMock = $this->createMock(IUserManager::class); $userManagerMock->method('get')->willReturn($userMock); - $requestMock = $this->getMockBuilder(IRequest::class)->disableOriginalConstructor()->getMock(); + $requestMock = $this->createMock(IRequest::class); + $requestMock->method('getHeader')->willReturnCallback( + function (string $name) use ($contentLength) { + return $name === 'Content-Length' ? $contentLength : ''; + } + ); - $requestMock->method('getUploadedFile')->willReturn([ + $requestMock->method('getUploadedFile')->willReturn($noUploadedFile ? null : [ 'name' => 'file.txt', 'tmp_name' => $uploadedFileTmpName, 'size' => $uploadedFileSize, @@ -307,7 +277,7 @@ private function createDirectUploadController( $userSessionMock, $userManagerMock, $directUploadServiceMock, - $this->getMockBuilder('OCA\OpenProject\Service\DatabaseService')->disableOriginalConstructor()->getMock(), + $this->createMock(DatabaseService::class), $this->l, 'testUser', ); diff --git a/tests/lib/Service/DirectUploadServiceTest.php b/tests/lib/Service/DirectUploadServiceTest.php index 630b784cf..0cb717084 100644 --- a/tests/lib/Service/DirectUploadServiceTest.php +++ b/tests/lib/Service/DirectUploadServiceTest.php @@ -7,34 +7,65 @@ namespace OCA\OpenProject\Service; +use OC\Files\Node\File; +use OC\Files\Node\Folder; +use OCP\Files\Cache\ICache; +use OCP\Files\ForbiddenException as FileAccessForbiddenException; +use OCP\Files\InvalidContentException; +use OCP\Files\IRootFolder; +use OCP\Files\IUserFolder; +use OCP\Files\NotEnoughSpaceException; use OCP\Files\NotFoundException; +use OCP\Files\Storage\IStorage; use OCP\IL10N; use OCP\IUser; use OCP\IUserManager; +use OCP\IUserSession; use OCP\Security\ISecureRandom; use PHPUnit\Framework\TestCase; +use function PHPUnit\Framework\assertSame; class DirectUploadServiceTest extends TestCase { - protected function getDirectUploadService(int $expiresOn): DirectUploadService { - $databaseServiceMock = $this->getMockBuilder(DatabaseService::class)->disableOriginalConstructor()->getMock(); + + private const TOKEN = 'ziPpdeFW4qoTg7AzEc4E9bnREkF97f5B2q65M4t3iex58E7ENZK4GomwEZCPjeNa'; + private const TMP_NAME = '/tmp/integration_openproject_unit_test'; + + /** + * @return mixed + */ + public function getFolderMock(): mixed { + if (interface_exists(IUserFolder::class)) { + return $this->createMock(IUserFolder::class); + } + return $this->createMock(Folder::class); + } + + protected function getDirectUploadService(int $expiresOn, mixed $userFolderMock = null): DirectUploadService { + $databaseServiceMock = $this->createMock(DatabaseService::class); $databaseServiceMock->method('getTokenInfoFromDB')->willReturn([ 'user_id' => 'testUser', 'expires_on' => $expiresOn, 'folder_id' => '123' ]); - $userMock = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); + $userMock = $this->createMock(IUser::class); $userMock->method('isEnabled')->willReturn(true); - $userManagerMock = $this->getMockBuilder(IUserManager::class)->disableOriginalConstructor()->getMock(); + $userManagerMock = $this->createMock(IUserManager::class); $userManagerMock->method('userExists')->willReturn(true); $userManagerMock->method('get')->willReturn($userMock); + $userSessionMock = $this->createMock(IUserSession::class); + $userSessionMock->method('getUser')->willReturn($userMock); + $storageMock = $this->createMock(IRootFolder::class); + $storageMock->method('getUserFolder')->willReturn($userFolderMock ?? $this->getFolderMock()); return new DirectUploadService( $userManagerMock, $this->createMock(IL10N::class), $this->createMock(ISecureRandom::class), - $databaseServiceMock + $databaseServiceMock, + $userSessionMock, + $storageMock ); } @@ -45,8 +76,107 @@ protected function getDirectUploadService(int $expiresOn): DirectUploadService { */ public function testGetTokenInfoExpiredTime() { //send an already expired time - $direcUploadService = $this->getDirectUploadService(1671537979); + $directUploadService = $this->getDirectUploadService(1671537979); $this->expectException(NotFoundException::class); - $direcUploadService->getTokenInfo('ziPpdeFW4qoTg7AzEc4E9bnREkF97f5B2q65M4t3iex58E7ENZK4GomwEZCPjeNa'); + $directUploadService->getTokenInfo(self::TOKEN); + } + + /** + * + * @param string $type + * @param int $id + * @return array + */ + private function getNodeMock(string $type, int $id = 123): array { + $ownerMock = $this->createMock(IUser::class); + $ownerMock->method('getDisplayName')->willReturn('Test User'); + $ownerMock->method('getUID')->willReturn('3df8ff78-49cb-4d60-8d8b-171b29591fd3'); + + $cacheMock = $this->createMock(ICache::class); + $cacheMock->method('update')->willReturn(true); + $storageMock = $this->createMock(IStorage::class); + $storageMock->method('getCache')->willReturn($cacheMock); + + $fileMock = $this->createMock(File::class); + $fileMock->method('getId')->willReturn(123); + $fileMock->method('getStorage')->willReturn($storageMock); + + $folderMock = $this->getFolderMock(); + $folderMock->method('getId')->willReturn($id); + $folderMock->method('getType')->willReturn($type); + $folderMock->method('isCreatable')->willReturn(true); + $folderMock->method('nodeExists')->willReturn(false); + $folderMock->method('newFile')->willReturn($fileMock); + return [$folderMock]; + } + + public function testDirectUploadNotEnoughSpace(): void { + $nodeMock = $this->getNodeMock('folder'); + $nodeMock[0]->method('getFreeSpace')->willReturn(100); + + $userFolderMock = $this->getFolderMock(); + $userFolderMock->method('getById')->willReturn($nodeMock); + + $directUploadServiceMock = $this->getDirectUploadService(time() + 1000, $userFolderMock); + $this->expectException(NotEnoughSpaceException::class); + $this->expectExceptionMessage('insufficient quota'); + $directUploadServiceMock->uploadFile(self::TOKEN, [ 'name' => 'file.txt', 'tmp_name' => self::TMP_NAME, 'size' => 101], false); + } + + public function testNegativeFreeSpace(): void { + $cacheMock = $this->createMock(ICache::class); + $cacheMock->method('update')->willReturn(true); + $storageMock = $this->createMock(IStorage::class); + $storageMock->method('getCache')->willReturn($cacheMock); + + $fileMock = $this->createMock(File::class); + $fileMock->method('getId')->willReturn(123); + $fileMock->method('getStorage')->willReturn($storageMock); + $nodeMock = $this->getNodeMock('folder'); + touch(self::TMP_NAME); + $nodeMock[0]->method('getFreeSpace')->willReturn(-3); + $nodeMock[0]->method('newFile')->willReturn($fileMock); + $userFolderMock = $this->getFolderMock(); + $userFolderMock->method('getById')->willReturn($nodeMock); + + $directUploadServiceMock = $this->getDirectUploadService(time() + 1000, $userFolderMock); + $result = $directUploadServiceMock->uploadFile(self::TOKEN, [ 'name' => 'file.txt', 'tmp_name' => self::TMP_NAME, 'size' => 101], false); + assertSame( + [ + 'file_id' => 123, + 'file_name' => 'file.txt', + ], + $result + ); + } + + /** + * @return array + */ + public function newFileExceptionsDataProvider() { + return [ + [new InvalidContentException('Virus detected'), 'Virus detected'], + [new FileAccessForbiddenException('Access denied by the access control', false), 'Access denied by the access control'], + [new \Exception('could not upload'), 'could not upload'], + ]; + } + + /** + * @return void + * @dataProvider newFileExceptionsDataProvider + */ + public function testDirectUploadException( + \Exception $exception, string $expectedErrorMessage + ):void { + $nodeMock = $this->getNodeMock('folder'); + touch(self::TMP_NAME); + $nodeMock[0]->method('newFile')->willThrowException($exception); + $userFolderMock = $this->getFolderMock(); + $userFolderMock->method('getById')->willReturn($nodeMock); + + $directUploadServiceMock = $this->getDirectUploadService(time() + 1000, $userFolderMock); + $this->expectException($exception::class); + $this->expectExceptionMessage($expectedErrorMessage); + $directUploadServiceMock->uploadFile(self::TOKEN, [ 'name' => 'file.txt', 'tmp_name' => self::TMP_NAME, 'size' => 0], false); } }