diff --git a/Controller/ApiController.php b/Controller/ApiController.php index 91777b1..5d0b1a8 100755 --- a/Controller/ApiController.php +++ b/Controller/ApiController.php @@ -90,8 +90,8 @@ final class ApiController extends Controller $request->getData('name') === null || $request->getFiles() !== null ? '' : $request->getData('name'), $request->getFiles(), $request->getHeader()->getAccount(), - __DIR__ . '/../../../Modules/Media/Files' . ((string) ($request->getData('path') ?? '')), - (string) ($request->getData('virtualPath') ?? ''), + __DIR__ . '/../../../Modules/Media/Files' . \urldecode((string) ($request->getData('path') ?? '')), + \urldecode((string) ($request->getData('virtualpath') ?? '')), (string) ($request->getData('password') ?? ''), (string) ($request->getData('encrypt') ?? ''), (int) ($request->getData('pathsettings') ?? PathSettings::RANDOM_PATH) @@ -143,7 +143,7 @@ final class ApiController extends Controller if ($pathSettings === PathSettings::RANDOM_PATH) { $outputDir = self::createMediaPath($basePath); } elseif ($pathSettings === PathSettings::FILE_PATH) { - $outputDir = \rtrim($basePath, '/\\') . $virtualPath; + $outputDir = \rtrim($basePath, '/\\'); $absolute = true; } else { return $mediaCreated; @@ -255,9 +255,8 @@ final class ApiController extends Controller private static function normalizeDbPath(string $path) : string { $realpath = \realpath(__DIR__ . '/../../../'); - if ($realpath === false) { - throw new \Exception(); + throw new \Exception(); // @codeCoverageIgnore } return \str_replace('\\', '/', @@ -308,22 +307,22 @@ final class ApiController extends Controller /** @var Media $media */ $media = MediaMapper::get($id); $media->setName((string) ($request->getData('name') ?? $media->getName())); + $media->setDescription((string) ($request->getData('description') ?? $media->getDescription())); + $media->setPath((string) ($request->getData('path') ?? $media->getPath())); $media->setVirtualPath(\urldecode((string) ($request->getData('virtualpath') ?? $media->getVirtualPath()))); - if ($id == 0) { - $path = \urldecode($request->getData('path')); + // @todo: implement a security check to ensure the user is allowed to write to the file. Right now you could overwrite ANY file with a malicious $path + if ($id === 0 + && $media instanceof NullMedia + && \is_file(__DIR__ . '/../Files' . ($path = \urldecode($request->getData('path')))) + ) { + $name = \explode('.', \basename($path)); - if ($media instanceof NullMedia - && \is_file(__DIR__ . '/../Files' . $path) - ) { - $name = \explode('.', \basename($path)); - - $media->setName($name[0]); - $media->setExtension($name[1] ?? ''); - $media->setVirtualPath(\dirname($path)); - $media->setPath('/Modules/Media/Files/' . \ltrim($path, '\\/')); - $media->setAbsolute(false); - } + $media->setName($name[0]); + $media->setExtension($name[1] ?? ''); + $media->setVirtualPath(\dirname($path)); + $media->setPath('/Modules/Media/Files/' . \ltrim($path, '\\/')); + $media->setAbsolute(false); } if ($request->getData('content') !== null) { @@ -429,6 +428,8 @@ final class ApiController extends Controller /** * Method to create media collection from request. * + * This doesn't create a database entry only the collection model. + * * @param string $name Collection name * @param string $description Description * @param Media[] $media Media files to create the collection from @@ -440,13 +441,9 @@ final class ApiController extends Controller */ public function createMediaCollectionFromMedia(string $name, string $description, array $media, int $account) : Collection { - if (empty($media)) { - return new NullCollection(); - } - - // is allowed to create media file - if (!$this->app->accountManager->get($account)->hasPermission( - PermissionType::CREATE, $this->app->orgId, null, self::MODULE_NAME, PermissionState::COLLECTION, null) + if (empty($media) + || !$this->app->accountManager->get($account)->hasPermission( + PermissionType::CREATE, $this->app->orgId, null, self::MODULE_NAME, PermissionState::COLLECTION, null) ) { return new NullCollection(); } @@ -481,7 +478,7 @@ final class ApiController extends Controller { $path = \urldecode((string) ($request->getData('path') ?? '')); $virtualPath = \urldecode((string) ($request->getData('virtualpath') ?? '')); - $fileName = (string) ($request->getData('fileName') ?? ($request->getData('name') ?? '')); + $fileName = (string) ($request->getData('filename') ?? ($request->getData('name') ?? '')); $fileName .= \strripos($fileName, '.') === false ? '.txt' : ''; $outputDir = ''; @@ -495,7 +492,7 @@ final class ApiController extends Controller $created = Directory::create($outputDir, 0775, true); if (!$created) { - throw new \Exception('Couldn\'t create outputdir: "' . $outputDir . '"'); + throw new \Exception('Couldn\'t create outputdir: "' . $outputDir . '"'); // @codeCoverageIgnore } } diff --git a/Models/MediaMapper.php b/Models/MediaMapper.php index 6c372e6..d76d194 100755 --- a/Models/MediaMapper.php +++ b/Models/MediaMapper.php @@ -126,7 +126,7 @@ class MediaMapper extends DataMapperAbstract $query->where(self::$table . '_' . $depth . '.media_virtual', '=', $virtualPath); if ($hidden === false) { - $query->where(self::$table . '_' . $depth . '.media_hidden', '=', (int) $hidden); + $query->andWhere(self::$table . '_' . $depth . '.media_hidden', '=', (int) $hidden); } return self::getAllByQuery($query, RelationType::ALL, $depth); diff --git a/tests/Controller/Api/ApiControllerCollectionTrait.php b/tests/Controller/Api/ApiControllerCollectionTrait.php index 23fe6d0..7e1db84 100755 --- a/tests/Controller/Api/ApiControllerCollectionTrait.php +++ b/tests/Controller/Api/ApiControllerCollectionTrait.php @@ -19,6 +19,11 @@ use phpOMS\Message\Http\HttpResponse; use phpOMS\System\File\Local\Directory; use phpOMS\Uri\HttpUri; use phpOMS\Utils\TestUtils; +use phpOMS\Message\Http\RequestStatusCode; +use Modules\Media\Models\MediaMapper; +use Modules\Media\Models\Media; +use Modules\Admin\Models\NullAccount; +use Modules\Media\Models\NullCollection; trait ApiControllerCollectionTrait { @@ -26,7 +31,7 @@ trait ApiControllerCollectionTrait * @covers Modules\Media\Controller\ApiController * @group module */ - public function testApiCollectionCreate() : void + public function testApiCollectionCreateWitRandomPath() : void { $response = new HttpResponse(); $request = new HttpRequest(new HttpUri('')); @@ -86,4 +91,72 @@ trait ApiControllerCollectionTrait self::assertEquals('Test Collection', $collection->getName()); self::assertCount(2, $collection->getSources()); } + + /** + * @covers Modules\Media\Controller\ApiController + * @group module + */ + public function testApiCollectionCreateInvalid() : void + { + $response = new HttpResponse(); + $request = new HttpRequest(new HttpUri('')); + + $request->getHeader()->setAccount(1); + + $this->module->apiCollectionCreate($request, $response); + self::assertEquals(RequestStatusCode::R_400, $response->getHeader()->getStatusCode()); + } + + /** + * @covers Modules\Media\Controller\ApiController + * @group module + */ + public function testApiCollectionCreateWithPath() : void + { + $response = new HttpResponse(); + $request = new HttpRequest(new HttpUri('')); + + $request->getHeader()->setAccount(1); + $request->setData('name', 'Test Collection'); + $request->setData('path', '/test/path'); + + $this->module->apiCollectionCreate($request, $response); + + $collection = $response->get('')['response']; + self::assertTrue(\is_dir(__DIR__ . '/../../../Files/test/path')); + + Directory::delete(__DIR__ . '/../../../Files/test/path'); + } + + /** + * @covers Modules\Media\Controller\ApiController + * @group module + */ + public function testApiCollectionFromMedia() : void + { + $media = new Media(); + $media->setCreatedBy(new NullAccount(1)); + $media->setDescription('desc'); + $media->setDescriptionRaw('descRaw'); + $media->setPath('some/path'); + $media->setSize(11); + $media->setExtension('png'); + $media->setName('Media for collection'); + $id = MediaMapper::create($media); + + self::assertGreaterThan(0, $media->getId()); + self::assertEquals($id, $media->getId()); + + $collection = $this->module->createMediaCollectionFromMedia('Collection With Media', '', [$media], 1); + + self::assertEquals('Collection With Media', $collection->getName()); + self::assertCount(1, $collection->getSources()); + + self::assertInstanceOf( + NullCollection::class, + $this->module->createMediaCollectionFromMedia('Collection With Media', '', [], 1) + ); + + Directory::delete(__DIR__ . '/../../../Files/test/path'); + } } diff --git a/tests/Controller/Api/ApiControllerMediaTrait.php b/tests/Controller/Api/ApiControllerMediaTrait.php index ea26c02..434f9ff 100755 --- a/tests/Controller/Api/ApiControllerMediaTrait.php +++ b/tests/Controller/Api/ApiControllerMediaTrait.php @@ -14,12 +14,14 @@ declare(strict_types=1); namespace Modules\Media\tests\Controller\Api; +use Modules\Media\Models\PathSettings; use Modules\Media\Models\UploadStatus; use phpOMS\Message\Http\HttpRequest; use phpOMS\Message\Http\HttpResponse; use phpOMS\System\File\Local\Directory; use phpOMS\Uri\HttpUri; use phpOMS\Utils\TestUtils; +use Modules\Media\Models\MediaMapper; trait ApiControllerMediaTrait { @@ -64,7 +66,7 @@ trait ApiControllerMediaTrait * @covers Modules\Media\Controller\ApiController * @group module */ - public function testApiMediaUpload() : void + public function testApiMediaUploadRandomPath() : void { $response = new HttpResponse(); $request = new HttpRequest(new HttpUri('')); @@ -107,4 +109,152 @@ trait ApiControllerMediaTrait $media = $response->get('')['response']; self::assertCount(2, $media); } + + /** + * @covers Modules\Media\Controller\ApiController + * @group module + */ + public function testApiMediaUploadDefinedPath() : void + { + $response = new HttpResponse(); + $request = new HttpRequest(new HttpUri('')); + + $request->getHeader()->setAccount(1); + $request->setData('name', 'Test Upload'); + $request->setData('pathsettings', PathSettings::FILE_PATH); + $request->setData('path', '/../tests/Controller/test/path'); // change path from Media/Files to this path + $request->setData('virtualpath', '/other/path'); // Upload should be to this subdir in this path + + if (!\is_dir(__DIR__ . '/temp')) { + \mkdir(__DIR__ . '/temp'); + } + + \copy(__DIR__ . '/testFile1.txt', __DIR__ . '/temp/testFile1.txt'); + \copy(__DIR__ . '/testFile2.txt', __DIR__ . '/temp/testFile2.txt'); + + $files = [ + [ + 'error' => \UPLOAD_ERR_OK, + 'type' => 'txt', + 'name' => 'testFile1.txt', + 'tmp_name' => __DIR__ . '/temp/testFile1.txt', + 'size' => \filesize(__DIR__ . '/testFile1.txt'), + ], + [ + 'error' => \UPLOAD_ERR_OK, + 'type' => 'txt', + 'name' => 'testFile2.txt', + 'tmp_name' => __DIR__ . '/temp/testFile2.txt', + 'size' => \filesize(__DIR__ . '/testFile2.txt'), + ], + ]; + + TestUtils::setMember($request, 'files', $files); + + $this->module->apiMediaUpload($request, $response); + + if (\is_dir(__DIR__ . '/temp')) { + Directory::delete(__DIR__ . '/temp'); + } + + $media = $response->get('')['response']; + self::assertTrue(\is_file(__DIR__ . '/../test/path/testFile1.txt')); + self::assertTrue(\is_file(__DIR__ . '/../test/path/testFile2.txt')); + + Directory::delete(__DIR__ . '/../test'); + } + + /** + * @covers Modules\Media\Controller\ApiController + * @group module + */ + public function testUploadFilesInvalidPathSetting() : void + { + self::assertEquals( + [], + $this->module->uploadFiles('test', ['test'], 1, '/test', '', '', '', 99) + ); + } + + /** + * @covers Modules\Media\Controller\ApiController + * @group module + */ + public function testApiMediaUpdate() : void + { + $response = new HttpResponse(); + $request = new HttpRequest(new HttpUri('')); + + $request->getHeader()->setAccount(1); + $request->setData('name', 'Test Media'); + $request->setData('pathsettings', PathSettings::FILE_PATH); + $request->setData('path', '/../tests/Controller/test/path'); // change path from Media/Files to this path + $request->setData('virtualpath', '/other/path'); // Upload should be to this subdir in this path + + if (!\is_dir(__DIR__ . '/temp')) { + \mkdir(__DIR__ . '/temp'); + } + + \copy(__DIR__ . '/testFile1.txt', __DIR__ . '/temp/testFile1.txt'); + \copy(__DIR__ . '/testFile2.txt', __DIR__ . '/temp/testFile2.txt'); + + $files = [ + [ + 'error' => \UPLOAD_ERR_OK, + 'type' => 'txt', + 'name' => 'testFile1.txt', + 'tmp_name' => __DIR__ . '/temp/testFile1.txt', + 'size' => \filesize(__DIR__ . '/testFile1.txt'), + ], + [ + 'error' => \UPLOAD_ERR_OK, + 'type' => 'txt', + 'name' => 'testFile2.txt', + 'tmp_name' => __DIR__ . '/temp/testFile2.txt', + 'size' => \filesize(__DIR__ . '/testFile2.txt'), + ], + ]; + + TestUtils::setMember($request, 'files', $files); + $this->module->apiMediaUpload($request, $response); + + $id = \reset($response->get('')['response']); + $response = new HttpResponse(); + $request = new HttpRequest(new HttpUri('')); + + $request->getHeader()->setAccount(1); + $request->setData('id', $id); + $request->setData('name', 'Test Changed'); + $request->setData('content', 'Test Content'); + $this->module->apiMediaUpdate($request, $response); + + $media = MediaMapper::get($id); + self::assertEquals('Test Changed', $media->getName()); + self::assertEquals('Test Content', \file_get_contents(__DIR__ . '/../test/path/testFile1.txt')); + + Directory::delete(__DIR__ . '/../test'); + } + + /** + * @covers Modules\Media\Controller\ApiController + * @group module + */ + public function testApiMediaCreateWithPath() : void + { + $response = new HttpResponse(); + $request = new HttpRequest(new HttpUri('')); + + $request->getHeader()->setAccount(1); + $request->setData('name', 'Created File'); + $request->setData('content', 'file content'); + $request->setData('filename', 'created.md'); + $request->setData('path', '/../tests/Controller/test/path'); + + $this->module->apiMediaCreate($request, $response); + + self::assertCount(1, $response->get('')['response']); + self::assertTrue(\is_file(__DIR__ . '/../test/path/created.md')); + + Directory::delete(__DIR__ . '/../test'); + } } diff --git a/tests/Controller/ApiControllerTest.php b/tests/Controller/ApiControllerTest.php index 4e14d6f..35b49b9 100755 --- a/tests/Controller/ApiControllerTest.php +++ b/tests/Controller/ApiControllerTest.php @@ -23,6 +23,10 @@ use phpOMS\Dispatcher\Dispatcher; use phpOMS\Event\EventManager; use phpOMS\Module\ModuleManager; use phpOMS\Router\WebRouter; +use phpOMS\Account\Account; +use phpOMS\Utils\TestUtils; +use phpOMS\Account\PermissionType; +use Modules\Admin\Models\AccountPermission; /** * @internal @@ -42,7 +46,7 @@ class ApiControllerTest extends \PHPUnit\Framework\TestCase $this->app->dbPool = $GLOBALS['dbpool']; $this->app->orgId = 1; - $this->app->appName = 'Backend'; + $this->app->appName = 'Api'; $this->app->accountManager = new AccountManager($GLOBALS['session']); $this->app->appSettings = new CoreSettings($this->app->dbPool->get()); $this->app->moduleManager = new ModuleManager($this->app, __DIR__ . '/../../../Modules'); @@ -52,6 +56,24 @@ class ApiControllerTest extends \PHPUnit\Framework\TestCase $this->app->router = new WebRouter(); + $account = new Account(); + TestUtils::setMember($account, 'id', 1); + + $permission = new AccountPermission(); + $permission->setUnit(1); + $permission->setApp('Api'); + $permission->setPermission( + PermissionType::READ + | PermissionType::CREATE + | PermissionType::MODIFY + | PermissionType::DELETE + | PermissionType::PERMISSION + ); + + $account->addPermission($permission); + + $this->app->accountManager->add($account); + $this->module = $this->app->moduleManager->get('Media'); } diff --git a/tests/Models/MediaMapperTest.php b/tests/Models/MediaMapperTest.php index 3372643..c017e16 100755 --- a/tests/Models/MediaMapperTest.php +++ b/tests/Models/MediaMapperTest.php @@ -126,11 +126,11 @@ class MediaMapperTest extends \PHPUnit\Framework\TestCase $media->setCreatedBy(new NullAccount(1)); $media->setDescription('desc'); $media->setPath('https://avatars0.githubusercontent.com/u/16034994'); - $media->setVirtualPath('/test/path'); + $media->setVirtualPath('/mediamappertest/path'); $media->setAbsolute(true); $media->setSize(11); $media->setExtension('png'); - $media->setName('Absolute path'); + $media->setName('With virtual path'); $id = MediaMapper::create($media); self::assertGreaterThan(0, $media->getId());