noNameWEB/SealShare
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 4 Wiki Activity

Count a share's download limit per recipient, not per file

Browse Source 
With a download limit of 1, downloading one file of a share with
several files deleted the share and the files not yet downloaded.
ShareService::recordDownload() ran at the end of every download
request, one file or the ZIP alike, and deleted the share as soon as
download_count reached max_downloads. It has worked that way since
the first commit.

- One recipient's visit is one download. The first file or ZIP a
  session downloads is counted when it starts, in one conditional
  UPDATE that also checks the limit, so two recipients starting at
  once can't both take the last download. The session remembers the
  time, and for ShareService::DOWNLOAD_WINDOW_MINUTES (60) it may
  start more downloads of the share without counting them, even once
  the limit is reached. The claim happens in the controller before
  streaming, because the session is saved before the body is sent,
  and after the share key is resolved, so a request without the key
  uses nothing.
- A share at its limit is closed to everyone else at once. The hourly
  cleanup deletes it 24 hours after shares.last_downloaded_at (new
  column), since a ZIP opens each file only when it reaches it and a
  large download can outlast the hour.
- The download page of a limited share says how many downloads are
  left, switches to "You have 1 hour" on the first press (Alpine, as
  a download link does not render the page again), and shows the time
  left on the next visit.
- The admin dashboard shows "2 of 3 downloads", marks shares at their
  limit "Download limit reached" and leaves them out of Active Shares.
- Tests: the regression (3 files, limit 1: every file and the ZIP
  download, counted once), another recipient, the end of the hour,
  the last download going to one of two recipients, requests refused
  before streaming, unlimited shares, the page notes in PHP and in
  Chromium, the dashboard, and the cleanup at 23 and 25 hours. The
  tests of recordDownload() and of the instant deletion are gone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Andreas Reinhold / reini
2026-09-17 15:35:13 +02:00
co-authored by Claude Opus 5
parent c1906d2009
commit e057cada3d
15 changed files with 344 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Feature/Admin/AdminDashboardTest.php
+14
View File
@@ -73,6 +73,20 @@ test('admin dashboard lists the shares', function () {
$response->assertSee('href="'.route('share.download', $share).'"', false);
});
test('the shares list shows downloads against the limit, and a share at its limit as closed and not active', function () {
$admin = User::query()->where('is_admin', true)->first();
Share::factory()->withMaxDownloads(3)->create(['token' => 'limitedshare0000', 'download_count' => 2, 'expires_at' => null]);
Share::factory()->withMaxDownloads(1)->create(['token' => 'limitreached0000', 'download_count' => 1, 'expires_at' => null]);
Share::factory()->create(['token' => 'unlimitedshare00', 'download_count' => 1, 'expires_at' => null]);
$dashboard = Livewire::actingAs($admin)->test(AdminDashboard::class);
$dashboard->assertSeeInOrder(['limitedshare0000', '2 of 3 downloads', 'Never expires'])
->assertSeeInOrder(['limitreached0000', '1 of 1 download', 'Download limit reached'])
->assertSeeInOrder(['unlimitedshare00', '1 download', 'Never expires'])
->assertViewHas('activeShares', 2);
});
test('the shares list sorts only by its own orders', function () {
$admin = User::query()->where('is_admin', true)->first();
tests/Feature/CleanupExpiredSharesTest.php
+13
View File
@@ -34,6 +34,19 @@ test('cleanup removes shares that reached download limit', function () {
expect(Share::query()->find($underLimit->id))->not->toBeNull();
});
test('cleanup keeps a share at its download limit for a day after its last download', function () {
Storage::fake('shares');
$recentlyDownloaded = Share::factory()->withMaxDownloads(1)->create(['download_count' => 1, 'last_downloaded_at' => now()->subHours(23)]);
$downloadedYesterday = Share::factory()->withMaxDownloads(1)->create(['download_count' => 1, 'last_downloaded_at' => now()->subHours(25)]);
$this->artisan('shares:cleanup')
->expectsOutputToContain('Cleaned up 1 expired share(s)')
->assertExitCode(0);
$this->assertModelExists($recentlyDownloaded);
$this->assertModelMissing($downloadedYesterday);
});
test('cleanup handles no expired shares', function () {
Storage::fake('shares');
tests/Feature/ShareDownloadTest.php
+151 -19
View File
@@ -112,15 +112,149 @@ test('zip download streams a valid archive with every file\'s original content',
unlink($zipPath);
});
test('last download streams successfully before auto-delete', function () {
test('the last download streams and the share stays for the cleanup, with the time of that download', function () {
Storage::fake('shares');
$this->freezeSecond();
$share = createShareWithFile();
$share->update(['max_downloads' => 1]);
$content = $this->get(route('share.download.all', $share))->streamedContent();
expect($content)->toStartWith("PK\x03\x04");
$this->assertModelMissing($share);
expect($share->fresh()->last_downloaded_at)->toEqual(now());
});
test('a recipient downloads every file and the zip of a share limited to one download, which counts once', function () {
Storage::fake('shares');
$share = createShareWithFiles(maxDownloads: 1);
[$first, $second, $third] = $share->files->all();
$files = [
$this->get(route('share.download.file', [$share, $first]))->streamedContent(),
$this->get(route('share.download.file', [$share, $second]))->streamedContent(),
$this->get(route('share.download.file', [$share, $third]))->streamedContent(),
];
$zip = $this->get(route('share.download.all', $share))->streamedContent();
expect($files)->toBe(['first file', 'second file', 'third file']);
expect($zip)->toStartWith("PK\x03\x04");
expect($share->fresh()->download_count)->toBe(1);
});
test('another recipient cannot open a share whose last download was taken', function (string $route) {
Storage::fake('shares');
$share = createShareWithFiles(maxDownloads: 1);
$this->get(route('share.download.file', [$share, $share->files->first()]));
$this->flushSession();
$response = $this->get(route($route, ['share' => $share, 'shareFile' => $share->files->last()]));
$response->assertNotFound();
})->with([
'download page' => 'share.download',
'download all' => 'share.download.all',
'download one file' => 'share.download.file',
]);
test('the recipient who took the last download can no longer open the share an hour later', function (string $route) {
Storage::fake('shares');
$share = createShareWithFiles(maxDownloads: 1);
$this->get(route('share.download.file', [$share, $share->files->first()]));
$this->travel(61)->minutes();
$response = $this->get(route($route, ['share' => $share, 'shareFile' => $share->files->last()]));
$response->assertNotFound();
})->with([
'download page' => 'share.download',
'download all' => 'share.download.all',
'download one file' => 'share.download.file',
]);
test('a recipient whose window has ended uses another download when one is left', function () {
Storage::fake('shares');
$share = createShareWithFiles(maxDownloads: 2);
$this->get(route('share.download.file', [$share, $share->files->first()]));
$this->travel(61)->minutes();
$response = $this->get(route('share.download.file', [$share, $share->files->last()]));
$response->assertOk();
expect($share->fresh()->download_count)->toBe(2);
});
test('a recipient is refused the last download once another recipient has taken it', function () {
Storage::fake('shares');
$share = createShareWithFiles(maxDownloads: 2);
$share->update(['download_count' => 1]);
$this->get(route('share.download.file', [$share, $share->files->first()]));
$this->flushSession();
$response = $this->get(route('share.download.file', [$share, $share->files->first()]));
$response->assertNotFound();
expect($share->fresh()->download_count)->toBe(2);
});
test('a download refused before it starts uses no download', function (?string $password, Closure $file, string $assertion) {
Storage::fake('shares');
$share = createShareWithFiles(maxDownloads: 1, password: $password);
$otherShare = createShareWithFiles();
$response = $this->get(route('share.download.file', [$share, $file($share, $otherShare)]));
$response->{$assertion}();
expect($share->fresh()->download_count)->toBe(0);
})->with([
'password share without its key' => ['secret-pass', fn (Share $share) => $share->files->first(), 'assertForbidden'],
'file of another share' => [null, fn (Share $share, Share $otherShare) => $otherShare->files->first(), 'assertNotFound'],
]);
test('a share without a limit counts each recipient once', function () {
Storage::fake('shares');
$share = createShareWithFiles();
$this->get(route('share.download.file', [$share, $share->files->first()]));
$this->get(route('share.download.file', [$share, $share->files->last()]));
$this->flushSession();
$this->get(route('share.download.file', [$share, $share->files->first()]));
expect($share->fresh()->download_count)->toBe(2);
});
test('the download page tells a recipient how many downloads are left', function (int $maxDownloads, string $note) {
Storage::fake('shares');
$share = createShareWithFiles(maxDownloads: $maxDownloads);
$response = $this->get(route('share.download', $share));
$response->assertSee($note);
})->with([
'several left' => [3, 'Downloading uses 1 of 3 remaining downloads. You then have 1 hour to download the files.'],
'one left' => [1, 'Downloading uses the last remaining download. You then have 1 hour to download the files.'],
]);
test('the download page tells the recipient who took the last download how long they have left', function () {
Storage::fake('shares');
$this->freezeSecond();
$share = createShareWithFiles(maxDownloads: 1);
$this->get(route('share.download.file', [$share, $share->files->first()]));
$this->travel(2)->minutes();
$response = $this->get(route('share.download', $share));
$response->assertSee('You can download these files for another 58 minutes.');
$response->assertDontSee('remaining download');
});
test('the download page says nothing about downloads on a share without a limit', function () {
Storage::fake('shares');
$share = createShareWithFiles();
$response = $this->get(route('share.download', $share));
$response->assertDontSee('to download the files');
$response->assertDontSee('You can download these files');
});
test('a share whose files are still uploading is not found anywhere a recipient or uploader could open it', function (string $route) {
@@ -157,23 +291,6 @@ test('a password share created before key wrapping still unlocks and downloads',
expect($this->get(route('share.download.file', [$share, $file]))->streamedContent())->toBe('old content');
});
test('share auto-deletes after reaching download limit', function () {
Storage::fake('shares');
$service = app(ShareService::class);
$file = UploadedFile::fake()->create('file.txt', 100);
$share = $service->createShare([
['file' => $file, 'relativePath' => null],
], [
'max_downloads' => 1,
]);
$service->recordDownload($share);
expect(Share::query()->find($share->id))->toBeNull();
});
/**
* Helper to create a share with an actual encrypted file.
*/
@@ -185,3 +302,18 @@ function createShareWithFile(?string $password = null, string $content = 'test c
'password' => $password,
]);
}
/**
* Helper to create a share with three encrypted files: "first file", "second file" and "third file".
*/
function createShareWithFiles(?int $maxDownloads = null, ?string $password = null): Share
{
return app(ShareService::class)->createShare([
['file' => UploadedFile::fake()->createWithContent('first.txt', 'first file'), 'relativePath' => null],
['file' => UploadedFile::fake()->createWithContent('second.txt', 'second file'), 'relativePath' => null],
['file' => UploadedFile::fake()->createWithContent('third.txt', 'third file'), 'relativePath' => null],
], [
'password' => $password,
'max_downloads' => $maxDownloads,
]);
}