diff --git a/.github/workflows/rector.yml b/.github/workflows/rector.yml index fff0208..ff3bcde 100644 --- a/.github/workflows/rector.yml +++ b/.github/workflows/rector.yml @@ -64,5 +64,5 @@ jobs: - name: Analyze for refactoring run: | - composer global require --dev rector/rector:^0.12.10 + composer global require --dev rector/rector:dev-main rector process --dry-run --no-progress-bar diff --git a/.gitignore b/.gitignore index 11192f3..4fd0807 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ -vendor/ +/vendor/ build/ phpunit*.xml phpunit diff --git a/UPGRADING.md b/UPGRADING.md index 3a9889a..382828c 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -5,6 +5,7 @@ * Switches to `Tatter\Preferences` for managing persistent settings; read more below * Drops `Tatter\Audits` as a dependency and adds it as a suggestion; read more below +* Access rights are now handled via Config file; see [Tatter\Permits](https://github.com/tattersoftware/codeigniter4-permits) for more information ### `Settings` Migration @@ -29,7 +30,7 @@ php spark migrate --all ``` Example model file in **app/Models/FileModel.php**: -``` +```php [ + __DIR__ . '/src/Helpers', + ], + // May load view files directly when detecting classes StringClassNameToClassConstantRector::class, diff --git a/src/Config/Registrar.php b/src/Config/Registrar.php index 34a4cb2..a489f99 100644 --- a/src/Config/Registrar.php +++ b/src/Config/Registrar.php @@ -17,4 +17,21 @@ public static function Pager() ], ]; } + + /** + * Adds necessary configuration values for Permits + * to identify the owner(s) of files. + * + * @return array + */ + public static function Permits() + { + return [ + 'files' => [ + 'userKey' => 'user_id', + 'pivotKey' => 'file_id', + 'pivotTable' => 'files_users', + ], + ]; + } } diff --git a/src/Controllers/Files.php b/src/Controllers/Files.php index 4b3f18e..4d7922a 100644 --- a/src/Controllers/Files.php +++ b/src/Controllers/Files.php @@ -23,7 +23,7 @@ class Files extends Controller protected FilesConfig $config; /** - * The model to use, may be a child of this library's. + * The model to use. */ protected FileModel $model; @@ -156,7 +156,6 @@ public function user($userId = null) } $this->setData([ - 'access' => $this->model->mayAdmin() ? 'manage' : 'display', 'title' => 'User Files', 'userName' => 'User', ]); diff --git a/src/Entities/File.php b/src/Entities/File.php index 37bb218..fba67cd 100644 --- a/src/Entities/File.php +++ b/src/Entities/File.php @@ -18,7 +18,7 @@ class File extends Entity /** * Resolved path to the default thumbnail */ - protected static ?string $defaultThumbnail; + protected static ?string $defaultThumbnail = null; /** * Returns the absolute path to the configured default thumbnail diff --git a/src/Models/FileModel.php b/src/Models/FileModel.php index d4e08b1..5f1964e 100644 --- a/src/Models/FileModel.php +++ b/src/Models/FileModel.php @@ -34,12 +34,6 @@ class FileModel extends Model 'size' => 'permit_empty|is_natural', ]; - // Permits - protected $mode = 04660; - protected $userKey = 'user_id'; - protected $pivotKey = 'file_id'; - protected $usersPivot = 'files_users'; - //-------------------------------------------------------------------- /** diff --git a/src/Structures/FileObject.php b/src/Structures/FileObject.php index 0a620e2..ff34377 100644 --- a/src/Structures/FileObject.php +++ b/src/Structures/FileObject.php @@ -16,7 +16,7 @@ class FileObject extends File /** * Base file name to override disk version */ - protected ?string $basename; + protected ?string $basename = null; /** * Returns the full path to this file @@ -35,7 +35,7 @@ public function setBasename(?string $basename = null): self * * @param string $suffix Optional suffix to omit from the base name returned */ - public function getBasename($suffix = null): string + public function getBasename($suffix = ''): string { if ($this->basename) { return basename($this->basename, $suffix); diff --git a/tests/_support/FeatureTestCase.php b/tests/_support/FeatureTestCase.php index 70684bc..2f41f48 100644 --- a/tests/_support/FeatureTestCase.php +++ b/tests/_support/FeatureTestCase.php @@ -2,9 +2,7 @@ namespace Tests\Support; -use CodeIgniter\Config\Factories; use CodeIgniter\Test\FeatureTestTrait; -use Tests\Support\Models\UserModel; /** * @internal @@ -22,9 +20,6 @@ protected function setUp(): void { parent::setUp(); - // Make sure we use the correct UserModel for permissions - Factories::injectMock('models', UserModel::class, new UserModel()); - // Make sure everything is published once $this->publishAll(); } diff --git a/tests/_support/Models/UserModel.php b/tests/_support/Models/UserModel.php deleted file mode 100644 index 152dba4..0000000 --- a/tests/_support/Models/UserModel.php +++ /dev/null @@ -1,26 +0,0 @@ -root); - - // "vendor" gets ignored by .gitignore so rename it after copying to VFS - $path = $this->root->url() . DIRECTORY_SEPARATOR . 'vendor' . DIRECTORY_SEPARATOR; - rename($this->root->url() . '/storage', $path); - $this->testPath = $path . 'image.jpg'; + // Make sure all files are on a single page + $_REQUEST['perPage'] = 200; // Force Files config to the virtual path + $path = self::$root->url() . DIRECTORY_SEPARATOR; $config = config('Files'); $config->setPath($path); Factories::injectMock('config', 'Files', $config); + + // Copy the files to VFS (if necessary) + $this->testPath = $config->getPath() . 'image.jpg'; + if (! is_file($this->testPath)) { + vfsStream::copyFromFileSystem(SUPPORTPATH . 'VFS', self::$root); + } + + // Set up the model + $this->model = model(FileModel::class); // @phpstan-ignore-line + } + + protected function tearDown(): void + { + parent::tearDown(); + + ImposterFactory::reset(); + } + + /** + * Creates a test User with some files. + * + * @return array Tuple of [User, File] + */ + protected function createUserWithFile(): array + { + $user = ImposterFactory::fake(); + $user->id = ImposterFactory::add($user); + + $file = fake(FileModel::class); + $this->model->addToUser($file->id, $user->id); + + return [$user, $file]; } } diff --git a/tests/_support/VFS/storage/image.jpg b/tests/_support/VFS/image.jpg similarity index 100% rename from tests/_support/VFS/storage/image.jpg rename to tests/_support/VFS/image.jpg diff --git a/tests/feature/DisplayTest.php b/tests/feature/DisplayTest.php index 4b2155b..eacd523 100644 --- a/tests/feature/DisplayTest.php +++ b/tests/feature/DisplayTest.php @@ -8,7 +8,8 @@ */ final class DisplayTest extends FeatureTestCase { - protected $refresh = true; + protected $refresh = true; + protected $refreshVfs = true; public function testNoFiles() { @@ -144,8 +145,6 @@ public function testSearches(string $keyword) public function testPages() { - $_REQUEST['perPage'] = 2; - for ($i = 0; $i < 2; $i++) { $file = fake(FileModel::class); } @@ -156,7 +155,8 @@ public function testPages() ]); // Last file should be on the next page - $result = $this->get('files'); + $_REQUEST['perPage'] = 2; + $result = $this->get('files'); $result->assertStatus(200); $result->assertDontSee($file->filename); diff --git a/tests/feature/PermissionsTest.php b/tests/feature/PermissionsTest.php index 70c0760..53af0a3 100644 --- a/tests/feature/PermissionsTest.php +++ b/tests/feature/PermissionsTest.php @@ -1,10 +1,8 @@ model = model(FileModel::class); // @phpstan-ignore-line - - if (! $this->seeded) { - // Set permissions - model(PermitModel::class)->insertBatch([ - [ - 'name' => 'listFiles', - 'user_id' => self::USER_ID, - ], - [ - 'name' => 'listFiles', - 'user_id' => self::PROCTOR_ID, - ], - [ - 'name' => 'listFiles', - 'user_id' => self::SUPER_ID, - ], - [ - 'name' => 'readFiles', - 'user_id' => self::SUPER_ID, - ], - [ - 'name' => 'adminFiles', - 'user_id' => self::ADMIN_ID, - ], - ]); - - foreach ([self::USER_ID, self::SUPER_ID, self::ADMIN_ID, self::PROCTOR_ID] as $userId) { - $this->createUserFiles($userId); - } + $this->model->setPermits([ + 'list' => Permits::ANYBODY, + ]); - $this->seeded = true; - } + [$user, $file] = $this->createUserWithFile(); - // Make sure all files are on a single page - $_REQUEST['perPage'] = 200; + $result = $this->withSession()->get('files'); + $result->assertStatus(200); + $result->assertSee($file->filename); } - /** - * Creates random files for a user. - * - * @param int $userId User ID to own the files - * @param int $count Number of files to create - */ - protected function createUserFiles(int $userId, int $count = 2) + public function testListsOwnFiles() { - // Create files and assign them to the user - for ($i = 0; $i < abs($count); $i++) { - $file = fake(FileModel::class); + $this->model->setPermits([ + 'list' => Permits::NOBODY, + ]); - $this->model->addToUser($file->id, $userId); - } + [$user, $userFile] = $this->createUserWithFile(); + [$owner, $ownerFile] = $this->createUserWithFile(); + service('auth')->login($owner); + + $result = $this->withSession()->get('files'); + $result->assertStatus(200); + $result->assertSee($ownerFile->filename); + $result->assertDontSee($userFile->filename); } - /** - * Injects a permission mode into the shared FileModel. - * - * @param int $mode Octal mode - */ - protected function setMode(int $mode) + public function testAdminListsFiles() { - $this->model->setMode($mode); + $this->model->setPermits([ + 'list' => Permits::NOBODY, + ]); - Factories::injectMock('models', FileModel::class, $this->model); - } + [$user, $userFile] = $this->createUserWithFile(); + [$admin, $adminFile] = $this->createUserWithFile(); + $admin->permissions = ['files.admin']; + service('auth')->login($admin); - //-------------------------------------------------------------------- + $result = $this->withSession()->get('files'); - public function testDefaultAccessListsAllFiles() - { - $this->assertSame(04660, $this->model->getMode()); - $result = $this->get('files'); $result->assertStatus(200); - - $files = $this->model->getForUser(self::SUPER_ID); - $result->assertSee($files[0]->filename); - - $files = $this->model->getForUser(self::ADMIN_ID); - $result->assertSee($files[0]->filename); + $result->assertSee($adminFile->filename); + $result->assertSee($userFile->filename); } public function testDenyListRedirects() { - $this->setMode(00660); + $this->model->setPermits([ + 'list' => Permits::NOBODY, + ]); $result = $this->get('files'); $result->assertStatus(302); @@ -134,17 +70,19 @@ public function testDenyListRedirects() public function testDenyAjaxReturnsError() { - $this->setMode(00660); + $this->model->setPermits([ + 'list' => Permits::NOBODY, + ]); $result = $this->withHeaders(['X-Requested-With' => 'XMLHttpRequest'])->get('files'); $result->assertStatus(403); $result->assertJSONFragment(['error' => lang('Permits.notPermitted')]); } - public function testAuthenticatedAddOnlyEmptyFile() + public function testUploadMissingFile() { - $this->setMode(04664); - service('auth')->login(self::ADMIN_ID); + [$user, ] = $this->createUserWithFile(); + service('auth')->login($user); $result = $this->withSession() ->withHeaders(['X-Requested-With' => 'XMLHttpRequest']) @@ -152,10 +90,10 @@ public function testAuthenticatedAddOnlyEmptyFile() $result->assertStatus(400); } - public function testAuthenticatedAddOnlyWithInvalidFile() + public function testUploadInvalidFile() { - $this->setMode(04664); - service('auth')->login(self::ADMIN_ID); + [$user, ] = $this->createUserWithFile(); + service('auth')->login($user); $_FILES = [ 'file' => [ @@ -170,6 +108,7 @@ public function testAuthenticatedAddOnlyWithInvalidFile() $result = $this->withSession() ->withHeaders(['X-Requested-With' => 'XMLHttpRequest']) ->post('files/upload'); + $result->assertSee('The file uploaded with success.(0)'); } @@ -209,69 +148,4 @@ public function testAuthenticatedAddOnlyWithValidFile() $this->assertSame('', $result->response()->getBody()); } */ - public function testProctorListsAllFiles() - { - $this->createUserFiles(self::ADMIN_ID); - - $this->setMode(00660); - service('auth')->login(self::PROCTOR_ID); - - $result = $this->withSession()->get('files'); - $result->assertStatus(200); - - $files = $this->model->getForUser(self::ADMIN_ID); - $result->assertSee($files[0]->filename); - } - - public function testAuthenticatedListOwnOnly() - { - $this->setMode(00660); - service('auth')->login(self::PROCTOR_ID); - - $fileOwnByProctor = fake(FileModel::class); - $this->model->addToUser($fileOwnByProctor->id, self::PROCTOR_ID); - - $fileOwnByProctor2 = fake(FileModel::class); - $this->model->addToUser($fileOwnByProctor2->id, self::PROCTOR_ID); - - $fileOwnByAdmin = fake(FileModel::class); - $this->model->addToUser($fileOwnByAdmin->id, self::ADMIN_ID); - - $result = $this->withSession()->get('files/user/' . self::PROCTOR_ID); - $result->assertStatus(200); - - $files = $this->model->getForUser(self::PROCTOR_ID); - $result->assertSee($files[0]->filename); - $result->assertSee($files[1]->filename); - - $files = $this->model->getForUser(self::ADMIN_ID); - $result->assertDontSee($files[0]->filename); - } - - /** - * @dataProvider accessProvider - */ - public function testAdminAccess(int $mode) - { - $this->setMode($mode); - service('auth')->login(self::ADMIN_ID); - - $result = $this->withSession()->get('files'); - $result->assertStatus(200); - - $files = $this->model->getForUser(self::ADMIN_ID); - $result->assertSee($files[0]->filename); - $files = $this->model->getForUser(self::USER_ID); - $result->assertSee($files[0]->filename); - } - - public function accessProvider() - { - return [ - ['read' => 00444], - ['write' => 00222], - ['execute' => 00111], - ['read write execute' => 00777], - ]; - } } diff --git a/tests/feature/UserTest.php b/tests/feature/UserTest.php index af913a9..2221a7f 100644 --- a/tests/feature/UserTest.php +++ b/tests/feature/UserTest.php @@ -1,6 +1,5 @@ login($userId); + [$user, $file] = $this->createUserWithFile(); + service('auth')->login($user); - /** @var FileModel $model */ - $model = model(FileModel::class); - $model->addToUser($file->id, $userId); - - $result = $this->get('files/user/' . $userId); + $result = $this->get('files/user/' . $user->id); $result->assertSee('Manage My Files', 'h1'); $result->assertSee($file->filename); } public function testShowsOtherFiles() { - $file = fake(FileModel::class); - $userId = 13; - service('auth')->login($userId); - - /** @var FileModel $model */ - $model = model(FileModel::class); - $model->addToUser($file->id, $userId); + [$user, $file] = $this->createUserWithFile(); + service('auth')->login($user); $result = $this->get('files/user/1000'); $result->assertSee('Browse User Files', 'h1'); diff --git a/tests/misc/ControllerTest.php b/tests/misc/ControllerTest.php index 174ee57..bdba921 100644 --- a/tests/misc/ControllerTest.php +++ b/tests/misc/ControllerTest.php @@ -22,6 +22,8 @@ final class ControllerTest extends TestCase */ protected $controller; + protected $refreshVfs = true; + protected function setUp(): void { parent::setUp(); @@ -29,13 +31,6 @@ protected function setUp(): void $this->controller(Files::class); } - protected function tearDown(): void - { - parent::tearDown(); - - $this->controller = null; - } - public function testSetPreferencesUsesValidInput() { $_REQUEST = [ diff --git a/tests/misc/EntityTest.php b/tests/misc/EntityTest.php index 3202d46..20eceb6 100644 --- a/tests/misc/EntityTest.php +++ b/tests/misc/EntityTest.php @@ -1,7 +1,6 @@ createFromPath($this->testPath); + $file = $this->model->createFromPath($this->testPath); $expected = config('Files')->getPath() . $file->localname; diff --git a/tests/misc/ModelTest.php b/tests/misc/ModelTest.php index 2688817..f45ce1d 100644 --- a/tests/misc/ModelTest.php +++ b/tests/misc/ModelTest.php @@ -9,14 +9,7 @@ */ final class ModelTest extends TestCase { - protected FileModel $model; - - protected function setUp(): void - { - parent::setUp(); - - $this->model = model(FileModel::class); // @phpstan-ignore-line - } + protected $refreshVfs = true; public function testAddToUser() {