Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/Http/Controllers/Api/V1/ExportController.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ public function export(Organization $organization, ExportService $exportService)

$filepath = $exportService->export($organization);
$downloadUrl = Storage::disk(config('filesystems.private'))
->temporaryUrl($filepath, Carbon::now()->addMinutes(10));
->temporaryUrl($filepath, Carbon::now()->addMinutes(10), [
'ResponseContentDisposition' => 'attachment; filename="'.basename($filepath).'"',
]);

return new JsonResponse([
'success' => true,
Expand Down
8 changes: 6 additions & 2 deletions app/Http/Controllers/Api/V1/TimeEntryController.php
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,9 @@ public function indexExport(Organization $organization, TimeEntryIndexExportRequ

return response()->json([
'download_url' => Storage::disk(config('filesystems.private'))
->temporaryUrl($path, now()->addMinutes(5)),
->temporaryUrl($path, now()->addMinutes(5), [
'ResponseContentDisposition' => 'attachment; filename="'.$filename.'"',
]),
]);
}

Expand Down Expand Up @@ -545,7 +547,9 @@ public function aggregateExport(Organization $organization, TimeEntryAggregateEx

return response()->json([
'download_url' => Storage::disk(config('filesystems.private'))
->temporaryUrl($path, now()->addMinutes(5)),
->temporaryUrl($path, now()->addMinutes(5), [
'ResponseContentDisposition' => 'attachment; filename="'.$filename.'"',
]),
]);
}

Expand Down
27 changes: 27 additions & 0 deletions app/Providers/AppServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use App\Service\IpLookup\IpLookupServiceContract;
use App\Service\IpLookup\NoIpLookupService;
use App\Service\PermissionStore;
use DateTimeInterface;
use Dedoc\Scramble\Scramble;
use Dedoc\Scramble\Support\Generator\OpenApi;
use Dedoc\Scramble\Support\Generator\SecurityScheme;
Expand All @@ -29,8 +30,13 @@
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\Relation;
use Illuminate\Foundation\Application;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Route;
use Illuminate\Support\Facades\Storage;
use Illuminate\Support\Facades\URL;
use Illuminate\Support\ServiceProvider;
use Illuminate\Support\Str;
use Symfony\Component\HttpFoundation\StreamedResponse;

class AppServiceProvider extends ServiceProvider
{
Expand Down Expand Up @@ -98,6 +104,27 @@ public function boot(): void
$this->app->bind(IpLookupServiceContract::class, NoIpLookupService::class);
$this->app->bind(BillingContract::class);

// Storage
// The local driver ignores the ResponseContentDisposition option of temporaryUrl,
// so mirror it through the signed query parameters of the storage route.
$privateDisk = config('filesystems.private');
if (config('filesystems.disks.'.$privateDisk.'.driver') === 'local') {
$disk = Storage::disk($privateDisk);
$disk->serveUsing(function (Request $request, string $path, array $headers) use ($disk): StreamedResponse {
return $disk->response($path, null, $headers, $request->query('disposition', 'inline'));
});
$disk->buildTemporaryUrlsUsing(function (string $path, DateTimeInterface $expiration, array $options) use ($privateDisk): string {
$parameters = array_filter([
'path' => $path,
'disposition' => isset($options['ResponseContentDisposition'])
? Str::before($options['ResponseContentDisposition'], ';')
: null,
]);

return url(URL::temporarySignedRoute('storage.'.$privateDisk, $expiration, $parameters, absolute: false));
});
}

// Routing
Route::model('member', Member::class);
Route::model('invitation', OrganizationInvitation::class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const showExportModal = defineModel('show', { default: false });

function downloadCurrentExport() {
if (props.exportUrl) {
window.open(props.exportUrl, '_blank')?.focus();
window.open(props.exportUrl, '_self');
}
}
</script>
Expand Down
25 changes: 25 additions & 0 deletions tests/Unit/Endpoint/Api/V1/ApiEndpointTestAbstract.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

namespace Tests\Unit\Endpoint\Api\V1;

use Closure;
use DateTimeInterface;
use Illuminate\Support\Facades\Storage;
use Illuminate\Testing\TestResponse;
use Tests\TestCaseWithDatabase;

Expand All @@ -16,4 +19,26 @@ protected function assertResponseCode(TestResponse $response, int $statusCode):
}
$response->assertStatus($statusCode);
}

/**
* Replaces the temporary URL builder of the private disk to capture the options
* passed to temporaryUrl. Returns a closure that yields the captured options.
*
* @return Closure(): (array<string, mixed>|null)
*/
protected function captureTemporaryUrlOptions(): Closure
{
$captured = null;
Storage::disk(config('filesystems.private'))->buildTemporaryUrlsUsing(
function (string $path, DateTimeInterface $expiration, array $options) use (&$captured): string {
$captured = $options;

return 'https://storage.fake/'.$path;
}
);

return function () use (&$captured): ?array {
return $captured;
};
}
}
31 changes: 31 additions & 0 deletions tests/Unit/Endpoint/Api/V1/ExportEndpointTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,35 @@ public function test_export_calls_export_service_if_user_has_permission(): void
$response->assertJsonPath('success', true);
$this->assertStringContainsString($filepath, $response->json('download_url'));
}

public function test_export_requests_the_download_url_as_an_attachment(): void
{
// Arrange
$user = $this->createUserWithPermission([
'export',
]);
$filepath = 'exports/export_test.zip';
$this->mock(ExportService::class, function (MockInterface $mock) use (&$user, $filepath): void {
$mock->shouldReceive('export')
->withArgs(function (Organization $organization) use (&$user): bool {
return $organization->is($user->organization);
})
->andReturn($filepath)
->once();
});
Passport::actingAs($user->user);
$capturedOptions = $this->captureTemporaryUrlOptions();

// Act
$response = $this->postJson(route('api.v1.export.export', [
'organization' => $user->organization->getKey(),
]));

// Assert
$response->assertStatus(200);
$options = $capturedOptions();
$this->assertIsArray($options);
$this->assertSame('attachment; filename="export_test.zip"', $options['ResponseContentDisposition'] ?? null);
$this->assertSame('https://storage.fake/'.$filepath, $response->json('download_url'));
}
}
61 changes: 61 additions & 0 deletions tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,35 @@ public function test_index_export_endpoint_can_create_a_detailed_time_entry_repo
$this->assertResponseCode($response, 200);
}

public function test_index_export_endpoint_requests_the_download_url_as_an_attachment(): void
{
// Arrange
$data = $this->createUserWithPermission([
'time-entries:view:all',
]);
TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->startWithDuration(Carbon::now(), 100)->create();
Passport::actingAs($data->user);
$capturedOptions = $this->captureTemporaryUrlOptions();

// Act
$response = $this->getJson(route('api.v1.time-entries.index-export', [
$data->organization->getKey(),
'format' => ExportFormat::CSV,
'start' => Carbon::now()->startOfYear()->toIso8601ZuluString(),
'end' => Carbon::now()->endOfYear()->toIso8601ZuluString(),
]));

// Assert
$this->assertResponseCode($response, 200);
$options = $capturedOptions();
$this->assertIsArray($options);
$this->assertMatchesRegularExpression(
'/^attachment; filename="time-entries-export-.+\.csv"$/',
$options['ResponseContentDisposition'] ?? ''
);
$this->assertStringStartsWith('https://storage.fake/exports/', $response->json('download_url'));
}

public function test_index_export_endpoint_can_create_a_detailed_time_entry_report_in_format_ods(): void
{
// Arrange
Expand Down Expand Up @@ -1319,6 +1348,38 @@ public function test_aggregate_export_endpoints_can_create_a_csv_report(): void
$this->assertResponseCode($response, 200);
}

public function test_aggregate_export_endpoint_requests_the_download_url_as_an_attachment(): void
{
// Arrange
$data = $this->createUserWithPermission([
'time-entries:view:all',
]);
TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->startWithDuration(Carbon::now(), 100)->create();
Passport::actingAs($data->user);
$capturedOptions = $this->captureTemporaryUrlOptions();

// Act
$response = $this->getJson(route('api.v1.time-entries.aggregate-export', [
$data->organization->getKey(),
'format' => ExportFormat::CSV,
'group' => TimeEntryAggregationType::Client,
'sub_group' => TimeEntryAggregationType::Project,
'history_group' => TimeEntryAggregationTypeInterval::Month,
'start' => Carbon::now()->startOfYear()->toIso8601ZuluString(),
'end' => Carbon::now()->endOfYear()->toIso8601ZuluString(),
]));

// Assert
$this->assertResponseCode($response, 200);
$options = $capturedOptions();
$this->assertIsArray($options);
$this->assertMatchesRegularExpression(
'/^attachment; filename="time-entries-report-.+\.csv"$/',
$options['ResponseContentDisposition'] ?? ''
);
$this->assertStringStartsWith('https://storage.fake/exports/', $response->json('download_url'));
}

public function test_aggregate_export_endpoints_can_create_a_csv_report_as_employee_role_with_show_billable_rate(): void
{
// Arrange
Expand Down
134 changes: 134 additions & 0 deletions tests/Unit/Endpoint/Web/FileDownloadEndpointTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
<?php

declare(strict_types=1);

namespace Tests\Unit\Endpoint\Web;

use Illuminate\Contracts\Filesystem\Filesystem;
use Illuminate\Support\Facades\Storage;

/**
* Tests the attachment downloads of the private disk via Laravel's storage.{disk} route,
* configured with serveUsing/buildTemporaryUrlsUsing in AppServiceProvider::boot.
* These tests use the real private disk, because Storage::fake would replace the disk
* instance and thereby remove that configuration.
*/
class FileDownloadEndpointTest extends EndpointTestAbstract
{
private function privateDisk(): Filesystem
{
return Storage::disk(config('filesystems.private'));
}

public function test_temporary_url_with_attachment_disposition_serves_file_as_attachment(): void
{
// Arrange
$disk = $this->privateDisk();
$disk->put('exports/test-attachment.csv', 'Description,Duration');
$url = $disk->temporaryUrl('exports/test-attachment.csv', now()->addMinutes(5), [
'ResponseContentDisposition' => 'attachment; filename="test-attachment.csv"',
]);

// Act
$response = $this->get($url);

// Assert
$response->assertOk();
$response->assertDownload('test-attachment.csv');
$disk->delete('exports/test-attachment.csv');
}

public function test_temporary_url_without_options_serves_file_inline(): void
{
// Arrange
$disk = $this->privateDisk();
$disk->put('exports/test-inline.csv', 'Description,Duration');
$url = $disk->temporaryUrl('exports/test-inline.csv', now()->addMinutes(5));

// Act
$response = $this->get($url);

// Assert
$response->assertOk();
$this->assertStringStartsWith('inline', (string) $response->headers->get('Content-Disposition'));
$disk->delete('exports/test-inline.csv');
}

public function test_download_fails_with_tampered_disposition(): void
{
// Arrange
$url = $this->privateDisk()->temporaryUrl('exports/test-tampered.csv', now()->addMinutes(5), [
'ResponseContentDisposition' => 'attachment; filename="test-tampered.csv"',
]);

// Act
$response = $this->get(str_replace('attachment', 'inline', $url));

// Assert
$response->assertForbidden();
}

public function test_download_fails_with_expired_signature(): void
{
// Arrange
$url = $this->privateDisk()->temporaryUrl('exports/test-expired.csv', now()->addMinutes(5), [
'ResponseContentDisposition' => 'attachment; filename="test-expired.csv"',
]);
$this->travel(6)->minutes();

// Act
$response = $this->get($url);

// Assert
$response->assertForbidden();
}

public function test_download_fails_if_file_does_not_exist(): void
{
// Arrange
$url = $this->privateDisk()->temporaryUrl('exports/test-missing.csv', now()->addMinutes(5), [
'ResponseContentDisposition' => 'attachment; filename="test-missing.csv"',
]);

// Act
$response = $this->get($url);

// Assert
$response->assertNotFound();
}

public function test_temporary_url_of_private_local_disk_points_to_storage_route_with_signed_disposition(): void
{
// Act
$url = $this->privateDisk()->temporaryUrl('exports/test-export.pdf', now()->addMinutes(5), [
'ResponseContentDisposition' => 'attachment; filename="test-export.pdf"',
]);

// Assert
$this->assertStringContainsString('/storage/exports/test-export.pdf', $url);
$this->assertStringContainsString('disposition=attachment', $url);
$this->assertStringContainsString('signature=', $url);
}

public function test_temporary_url_of_s3_disk_includes_content_disposition_from_options(): void
{
// Arrange
config([
'filesystems.disks.s3.key' => 'test-key',
'filesystems.disks.s3.secret' => 'test-secret',
'filesystems.disks.s3.region' => 'us-east-1',
'filesystems.disks.s3.bucket' => 'test-bucket',
]);

// Act
$url = Storage::disk('s3')->temporaryUrl('exports/test-export.pdf', now()->addMinutes(5), [
'ResponseContentDisposition' => 'attachment; filename="test-export.pdf"',
]);

// Assert
$query = [];
parse_str((string) parse_url($url, PHP_URL_QUERY), $query);
$this->assertSame('attachment; filename="test-export.pdf"', $query['response-content-disposition'] ?? null);
$this->assertArrayHasKey('X-Amz-Signature', $query);
}
}