Skip to content

Commit f3b874f

Browse files
krulis-martinkrulis-martin
committed
Removing isPublic flag from uploaded file entity. Access to files will be ensured by file links.
1 parent 6c76069 commit f3b874f

6 files changed

Lines changed: 38 additions & 92 deletions

File tree

‎app/V1Module/security/Policies/UploadedFilePermissionPolicy.php‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,6 @@ public function getAssociatedClass()
3030
return UploadedFile::class;
3131
}
3232

33-
public function isPublic(Identity $identity, UploadedFile $file)
34-
{
35-
return $file->isPublic();
36-
}
37-
3833
public function isAttachmentFile(Identity $identity, UploadedFile $file)
3934
{
4035
return $file instanceof AttachmentFile;

‎app/config/permissions.neon‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,6 @@ permissions:
10761076
or:
10771077
- file.isSolutionInSupervisedOrObservedGroup
10781078
- file.isReferenceSolutionInSupervisedOrObservedSubGroup
1079-
- file.isPublic
10801079
- file.isOwner
10811080
- file.isRelatedToAssignment # deprecated (will be removed with Attachment files)
10821081
- file.isAuthorOfFileExercises

‎app/model/entity/AttachmentFile.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public function getAssignmentsAndIReallyMeanAllOkay()
7575
*/
7676
public function __construct($name, DateTime $uploadedAt, $fileSize, ?User $user, Exercise $exercise)
7777
{
78-
parent::__construct($name, $uploadedAt, $fileSize, $user, true);
78+
parent::__construct($name, $uploadedAt, $fileSize, $user);
7979
$this->exercises = new ArrayCollection();
8080
$this->assignments = new ArrayCollection();
8181

‎app/model/entity/UploadedFile.php‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,6 @@ public function getFileExtension(): string
4545
*/
4646
protected $uploadedAt;
4747

48-
/**
49-
* @ORM\Column(type="boolean")
50-
* If true, the file is accessible to all logged-in users.
51-
* This is useful for files associated with entries like exercises.
52-
*/
53-
protected $isPublic;
54-
5548
/**
5649
* @ORM\Column(type="integer")
5750
*/
@@ -82,20 +75,17 @@ public function getUserIdEvenIfDeleted(): ?string
8275
* @param DateTime $uploadedAt Time of the upload
8376
* @param int $fileSize Size of the file
8477
* @param User|null $user The user who uploaded the file
85-
* @param bool $isPublic
8678
*/
8779
public function __construct(
8880
string $name,
8981
DateTime $uploadedAt,
9082
int $fileSize,
91-
?User $user,
92-
$isPublic = false
83+
?User $user
9384
) {
9485
$this->name = $name;
9586
$this->uploadedAt = $uploadedAt;
9687
$this->fileSize = $fileSize;
9788
$this->user = $user;
98-
$this->isPublic = $isPublic;
9989
}
10090

10191
public function jsonSerialize(): mixed
@@ -106,20 +96,9 @@ public function jsonSerialize(): mixed
10696
"size" => $this->fileSize,
10797
"uploadedAt" => $this->uploadedAt->getTimestamp(),
10898
"userId" => $this->getUserId(),
109-
"isPublic" => $this->isPublic
11099
];
111100
}
112101

113-
public function isPublic()
114-
{
115-
return $this->isPublic;
116-
}
117-
118-
public function setPublic(bool $isPublic): void
119-
{
120-
$this->isPublic = $isPublic;
121-
}
122-
123102
/**
124103
* Retrieve a corresponding file object from file storage.
125104
* @param FileStorageManager $manager the storage that retrieves the file

‎tests/Presenters/UploadedFilesPresenter.phpt‎

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,8 @@ class TestUploadedFilesPresenter extends Tester\TestCase
9595

9696
public function testUserCannotAccessDetail()
9797
{
98-
$token = PresenterTestHelper::login($this->container, $this->otherUserLogin);
99-
$file = current($this->presenter->uploadedFiles->findBy(["isPublic" => false]));
98+
PresenterTestHelper::login($this->container, $this->otherUserLogin);
99+
$file = current($this->presenter->uploadedFiles->findAll());
100100
$request = new Nette\Application\Request(
101101
$this->presenterPath,
102102
'GET',
@@ -112,8 +112,7 @@ class TestUploadedFilesPresenter extends Tester\TestCase
112112

113113
public function testDetail()
114114
{
115-
$token = PresenterTestHelper::loginDefaultAdmin($this->container);
116-
115+
PresenterTestHelper::loginDefaultAdmin($this->container);
117116
$file = current($this->presenter->uploadedFiles->findAll());
118117

119118
$request = new Nette\Application\Request(
@@ -437,32 +436,6 @@ class TestUploadedFilesPresenter extends Tester\TestCase
437436
Assert::equal($file->getName(), $response->getName());
438437
}
439438

440-
public function testOutsiderCannotAccessAttachmentFiles()
441-
{
442-
$token = PresenterTestHelper::login($this->container, $this->otherUserLogin);
443-
444-
/** @var EntityManagerInterface $em */
445-
$em = $this->container->getByType(EntityManagerInterface::class);
446-
$file = current($em->getRepository(AttachmentFile::class)->findAll());
447-
448-
$mockStorage = Mockery::mock(FileStorageManager::class);
449-
$mockStorage->shouldReceive("getAttachmentFile")->withArgs([$file])->andReturn(null)->once();
450-
$this->presenter->fileStorage = $mockStorage;
451-
452-
$request = new Nette\Application\Request(
453-
$this->presenterPath,
454-
'GET',
455-
[
456-
'action' => 'download',
457-
'id' => $file->getId()
458-
]
459-
);
460-
461-
Assert::exception(function () use ($request) {
462-
$this->presenter->run($request);
463-
}, NotFoundException::class, "Not Found - File not found in the storage");
464-
}
465-
466439
public function testDownloadResultArchive()
467440
{
468441
PresenterTestHelper::loginDefaultAdmin($this->container);

‎tests/helpers/FileStorage.phpt‎

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ class TestFileStorage extends Tester\TestCase
125125
self::rmdirRecursive($hashDir);
126126
}
127127
@mkdir($hashDir);
128-
$storage = new LocalHashFileStorage([ 'root' => $hashDir ]);
128+
$storage = new LocalHashFileStorage(['root' => $hashDir]);
129129

130130
foreach ($files as $file) {
131131
$hash = sha1($file);
@@ -147,7 +147,7 @@ class TestFileStorage extends Tester\TestCase
147147
self::rmdirRecursive($rootDir);
148148
}
149149
@mkdir($rootDir);
150-
$storage = new LocalFileStorage(new TmpFilesHelper($this->tmpDir), [ 'root' => $rootDir ]);
150+
$storage = new LocalFileStorage(new TmpFilesHelper($this->tmpDir), ['root' => $rootDir]);
151151

152152
foreach ($files as $file => $contents) {
153153
if (str_contains($file, '/')) {
@@ -190,7 +190,7 @@ class TestFileStorage extends Tester\TestCase
190190
{
191191
$contents = "Lorem ipsum et sepsum!";
192192
$hash = sha1($contents);
193-
$hashStorage = $this->prepareHashStorage([ $contents ]);
193+
$hashStorage = $this->prepareHashStorage([$contents]);
194194
$file = $hashStorage->fetchOrThrow($hash);
195195
Assert::type(IImmutableFile::class, $file);
196196
Assert::equal($hash, $file->getStoragePath());
@@ -204,7 +204,7 @@ class TestFileStorage extends Tester\TestCase
204204
{
205205
$contents = "Lorem ipsum et sepsum!";
206206
$hash = sha1($contents);
207-
$hashStorage = $this->prepareHashStorage([ $contents ]);
207+
$hashStorage = $this->prepareHashStorage([$contents]);
208208
$file = $hashStorage->fetch(sha1('no aint here'));
209209
Assert::null($file);
210210
Assert::exception(function () use ($hashStorage) {
@@ -263,7 +263,7 @@ class TestFileStorage extends Tester\TestCase
263263
{
264264
$contents = "Lorem ipsum et sepsum!";
265265
$hash = sha1($contents);
266-
$hashStorage = $this->prepareHashStorage([ $contents ]);
266+
$hashStorage = $this->prepareHashStorage([$contents]);
267267
Assert::type(IImmutableFile::class, $hashStorage->fetch($hash));
268268
Assert::true($hashStorage->delete($hash));
269269
Assert::null($hashStorage->fetch($hash));
@@ -275,7 +275,7 @@ class TestFileStorage extends Tester\TestCase
275275
{
276276
$entry = 'foo/bar';
277277
$data = 'abcde';
278-
$zip = $this->createZipFile([ $entry => $data ]);
278+
$zip = $this->createZipFile([$entry => $data]);
279279
$file = new ArchivedImmutableFile($zip, $entry);
280280
Assert::equal("$zip#$entry", $file->getStoragePath());
281281
Assert::equal(strlen($data), $file->getSize());
@@ -287,7 +287,7 @@ class TestFileStorage extends Tester\TestCase
287287

288288
public function testZipFileStorageFetch()
289289
{
290-
$zip = $this->createZipFile([ 'a.txt' => 'AAAAA', 'b.txt' => 'BBBB' ]);
290+
$zip = $this->createZipFile(['a.txt' => 'AAAAA', 'b.txt' => 'BBBB']);
291291
$storage = new ZipFileStorage(new TmpFilesHelper($this->tmpDir), $zip);
292292
$fileA = $storage->fetch('a.txt');
293293
Assert::equal('AAAAA', $fileA->getContents());
@@ -300,7 +300,7 @@ class TestFileStorage extends Tester\TestCase
300300

301301
public function testZipFileStorageFetchNonexist()
302302
{
303-
$zip = $this->createZipFile([ 'a.txt' => 'AAAAA', 'b.txt' => 'BBBB' ]);
303+
$zip = $this->createZipFile(['a.txt' => 'AAAAA', 'b.txt' => 'BBBB']);
304304
$storage = new ZipFileStorage(new TmpFilesHelper($this->tmpDir), $zip);
305305
$fileC = $storage->fetch('c.txt');
306306
Assert::null($fileC);
@@ -312,7 +312,7 @@ class TestFileStorage extends Tester\TestCase
312312

313313
public function testZipFileStorageStoreFile()
314314
{
315-
$zip = $this->createZipFile([ 'a.txt' => 'AAAAA', 'b.txt' => 'BBBB' ]);
315+
$zip = $this->createZipFile(['a.txt' => 'AAAAA', 'b.txt' => 'BBBB']);
316316
$storage = new ZipFileStorage(new TmpFilesHelper($this->tmpDir), $zip);
317317

318318
$tmpX = $this->createTmpFile('XXX');
@@ -337,7 +337,7 @@ class TestFileStorage extends Tester\TestCase
337337

338338
public function testZipFileStorageStoreContents()
339339
{
340-
$zip = $this->createZipFile([ 'a.txt' => 'AAAAA', 'b.txt' => 'BBBB' ]);
340+
$zip = $this->createZipFile(['a.txt' => 'AAAAA', 'b.txt' => 'BBBB']);
341341
$storage = new ZipFileStorage(new TmpFilesHelper($this->tmpDir), $zip);
342342

343343
$storage->storeContents('XXX', 'x.txt', false);
@@ -356,7 +356,7 @@ class TestFileStorage extends Tester\TestCase
356356

357357
public function testZipFileStorageStoreStream()
358358
{
359-
$zip = $this->createZipFile([ 'a.txt' => 'AAAAA', 'b.txt' => 'BBBB' ]);
359+
$zip = $this->createZipFile(['a.txt' => 'AAAAA', 'b.txt' => 'BBBB']);
360360
$storage = new ZipFileStorage(new TmpFilesHelper($this->tmpDir), $zip);
361361

362362
$tmpX = $this->createTmpFile('XXX');
@@ -385,7 +385,7 @@ class TestFileStorage extends Tester\TestCase
385385

386386
public function testZipFileStorageStoreCopy()
387387
{
388-
$zip = $this->createZipFile([ 'a.txt' => 'AAAAA', 'b.txt' => 'BBBB' ]);
388+
$zip = $this->createZipFile(['a.txt' => 'AAAAA', 'b.txt' => 'BBBB']);
389389
$storage = new ZipFileStorage(new TmpFilesHelper($this->tmpDir), $zip);
390390

391391
$storage->copy('a.txt', 'c.txt');
@@ -403,7 +403,7 @@ class TestFileStorage extends Tester\TestCase
403403

404404
public function testZipFileStorageStoreMove()
405405
{
406-
$zip = $this->createZipFile([ 'a.txt' => 'AAAAA', 'b.txt' => 'BBBB' ]);
406+
$zip = $this->createZipFile(['a.txt' => 'AAAAA', 'b.txt' => 'BBBB']);
407407
$storage = new ZipFileStorage(new TmpFilesHelper($this->tmpDir), $zip);
408408

409409
$storage->move('a.txt', 'c.txt');
@@ -427,7 +427,7 @@ class TestFileStorage extends Tester\TestCase
427427

428428
public function testZipFileStorageStoreDelete()
429429
{
430-
$files = [ 'a.txt' => 'AAAAA', 'b.txt' => 'BBBB' ];
430+
$files = ['a.txt' => 'AAAAA', 'b.txt' => 'BBBB'];
431431
$zip = $this->createZipFile($files);
432432
$storage = new ZipFileStorage(new TmpFilesHelper($this->tmpDir), $zip);
433433

@@ -444,7 +444,7 @@ class TestFileStorage extends Tester\TestCase
444444
$storage = $this->prepareLocalStorage([
445445
'a.txt' => 'AAAAA',
446446
'b.txt' => 'BBB',
447-
'z/z.zip' => [ 'foo.md' => 'FOO', 'bar/bar' => 'BAR']
447+
'z/z.zip' => ['foo.md' => 'FOO', 'bar/bar' => 'BAR']
448448
]);
449449

450450
$fileA = $storage->fetch('a.txt');
@@ -481,7 +481,7 @@ class TestFileStorage extends Tester\TestCase
481481
{
482482
$storage = $this->prepareLocalStorage([
483483
'a.txt' => 'AAA',
484-
'z/z.zip' => [ 'foo.md' => 'FOO', 'bar/bar' => 'BAR']
484+
'z/z.zip' => ['foo.md' => 'FOO', 'bar/bar' => 'BAR']
485485
]);
486486

487487
// regular files
@@ -518,7 +518,7 @@ class TestFileStorage extends Tester\TestCase
518518
{
519519
$storage = $this->prepareLocalStorage([
520520
'a.txt' => 'AAA',
521-
'z/z.zip' => [ 'foo.md' => 'FOO', 'bar/bar' => 'BAR']
521+
'z/z.zip' => ['foo.md' => 'FOO', 'bar/bar' => 'BAR']
522522
]);
523523

524524
// regular files
@@ -548,7 +548,7 @@ class TestFileStorage extends Tester\TestCase
548548
{
549549
$storage = $this->prepareLocalStorage([
550550
'a.txt' => 'AAA',
551-
'z/z.zip' => [ 'foo.md' => 'FOO', 'bar/bar' => 'BAR']
551+
'z/z.zip' => ['foo.md' => 'FOO', 'bar/bar' => 'BAR']
552552
]);
553553

554554
// regular files
@@ -589,8 +589,8 @@ class TestFileStorage extends Tester\TestCase
589589
$storage = $this->prepareLocalStorage([
590590
'a.txt' => 'AAA',
591591
'b.txt' => 'BBB',
592-
'z/z.zip' => [ 'foo.md' => 'FOO', 'bar/bar' => 'BAR'],
593-
'z2.zip' => [ 'job.log' => 'failed' ]
592+
'z/z.zip' => ['foo.md' => 'FOO', 'bar/bar' => 'BAR'],
593+
'z2.zip' => ['job.log' => 'failed']
594594
]);
595595

596596
// regular files
@@ -662,8 +662,8 @@ class TestFileStorage extends Tester\TestCase
662662
'b.txt' => 'BBB',
663663
'c.txt' => 'CCC',
664664
'd.txt' => 'DDD',
665-
'z/z.zip' => [ 'foo.md' => 'FOO', 'boo' => 'BOO', 'loo' => 'LOO', 'zoo' => 'ZOO', 'bar/bar' => 'BAR'],
666-
'z2.zip' => [ 'job.log' => 'failed', 'config.yaml' => 'YAML' ]
665+
'z/z.zip' => ['foo.md' => 'FOO', 'boo' => 'BOO', 'loo' => 'LOO', 'zoo' => 'ZOO', 'bar/bar' => 'BAR'],
666+
'z2.zip' => ['job.log' => 'failed', 'config.yaml' => 'YAML']
667667
]);
668668

669669
// regular files
@@ -733,7 +733,7 @@ class TestFileStorage extends Tester\TestCase
733733
$storage = $this->prepareLocalStorage([
734734
'foo/bar/a.txt' => 'AAA',
735735
'foo/bar/b.txt' => 'BBB',
736-
'zip' => [ 'foo' => 'FOO', 'bar' => 'BAR', 'keeper' => 'placeholder' ],
736+
'zip' => ['foo' => 'FOO', 'bar' => 'BAR', 'keeper' => 'placeholder'],
737737
]);
738738
$root = $storage->getRootDirectory();
739739

@@ -782,8 +782,8 @@ class TestFileStorage extends Tester\TestCase
782782
'foo/bar/a.txt' => 'AAA',
783783
'foo/bar/b.txt' => 'BBB',
784784
'c.txt' => 'CCC',
785-
'zip' => [ 'foo' => 'FOO' ],
786-
'zip2' => [ 'job.log' => 'failed', 'config.yaml' => 'YAML' ]
785+
'zip' => ['foo' => 'FOO'],
786+
'zip2' => ['job.log' => 'failed', 'config.yaml' => 'YAML']
787787
]);
788788
$root = $storage->getRootDirectory();
789789

@@ -866,7 +866,7 @@ class TestFileStorage extends Tester\TestCase
866866
PresenterTestHelper::loginDefaultAdmin($this->container);
867867
$user = $this->users->getByEmail(PresenterTestHelper::ADMIN_LOGIN);
868868
$partialFile = new UploadedPartialFile("foo", 9, $user);
869-
$uploadedFile = new UploadedFile("foo", new DateTime(), 9, $user, true);
869+
$uploadedFile = new UploadedFile("foo", new DateTime(), 9, $user);
870870
self::setPropertyOfObject($partialFile, 'id', '123');
871871
self::setPropertyOfObject($uploadedFile, 'id', '123');
872872

@@ -902,7 +902,7 @@ class TestFileStorage extends Tester\TestCase
902902
$contents2 = '0123456789';
903903
$storage = $this->prepareLocalStorage([
904904
'foo.txt' => $contents1,
905-
'bar.zip' => [ 'foo.txt' => $contents2 ],
905+
'bar.zip' => ['foo.txt' => $contents2],
906906
]);
907907

908908
$file1 = $storage->fetch('foo.txt');
@@ -917,8 +917,8 @@ class TestFileStorage extends Tester\TestCase
917917
$storage = $this->prepareLocalStorage([
918918
'foo.txt' => 'abc',
919919
'foo.zip' => 'abc',
920-
'bar.zip' => [ 'foo.txt' => 'abcde' ],
921-
'inner.zip' => [ 'foo.txt' => 'abcdef' ],
920+
'bar.zip' => ['foo.txt' => 'abcde'],
921+
'inner.zip' => ['foo.txt' => 'abcdef'],
922922
]);
923923
$storage->move('inner.zip', 'bar.zip#inner.zip');
924924

@@ -933,14 +933,14 @@ class TestFileStorage extends Tester\TestCase
933933
{
934934
$storage = $this->prepareLocalStorage([
935935
'bad.zip' => 'abc',
936-
'foo.zip' => [ 'foo.txt' => 'abcde', 'bar.txt' => 'xyz' ],
937-
'inner.zip' => [ 'foo.txt' => 'abcde', 'bar.txt' => 'xyz' ],
936+
'foo.zip' => ['foo.txt' => 'abcde', 'bar.txt' => 'xyz'],
937+
'inner.zip' => ['foo.txt' => 'abcde', 'bar.txt' => 'xyz'],
938938
]);
939939
$storage->move('inner.zip', 'bar.zip#inner.zip');
940940

941941
Assert::equal([
942-
[ 'name' => 'foo.txt', 'size' => 5 ],
943-
[ 'name' => 'bar.txt', 'size' => 3 ],
942+
['name' => 'foo.txt', 'size' => 5],
943+
['name' => 'bar.txt', 'size' => 3],
944944
], $storage->fetch('foo.zip')->getZipEntries());
945945

946946
Assert::exception(

0 commit comments

Comments
 (0)