add tests

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') ?? '')),
+            __DIR__ . '/../../../Modules/Media/Files' . \urldecode((string) ($request->getData('path') ?? '')),
-            (string) ($request->getData('virtualPath') ?? ''),
+            \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) {
+        // @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
-            $path = \urldecode($request->getData('path'));
+        if ($id === 0
+            && $media instanceof NullMedia
+            && \is_file(__DIR__ . '/../Files' . ($path = \urldecode($request->getData('path'))))
+        ) {
+            $name = \explode('.', \basename($path));
 
-            if ($media instanceof NullMedia
+            $media->setName($name[0]);
-                && \is_file(__DIR__ . '/../Files' . $path)
+            $media->setExtension($name[1] ?? '');
-            ) {
+            $media->setVirtualPath(\dirname($path));
-                $name = \explode('.', \basename($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)) {
+        if (empty($media)
-            return new NullCollection();
+            || !$this->app->accountManager->get($account)->hasPermission(
-        }
+                PermissionType::CREATE, $this->app->orgId, null, self::MODULE_NAME, PermissionState::COLLECTION, null)
-
-        // 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)
         ) {
             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
             }
         }
 

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);

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');
+    }
 }

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');
+    }
 }

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');
     }
 

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());