diff --git a/.gitignore b/.gitignore index 0952a15..887dc1c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ .DS_Store .idea +.phpunit.cache .phpunit.result.cache .vscode build diff --git a/README.md b/README.md index 5322bb2..d14903e 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ Optionally, you can publish the views using php artisan vendor:publish --tag="mails-views" ``` -Add the routes to the PanelProvider using the `routes()` method, like this: +Add the routes to the PanelProvider using the `authenticatedRoutes()` method, like this: ```php use Backstage\Mails\Facades\Mails; @@ -68,10 +68,17 @@ use Backstage\Mails\Facades\Mails; public function panel(Panel $panel): Panel { return $panel - ->routes(fn () => Mails::routes()); + ->authenticatedRoutes(fn () => Mails::routes()); } ``` +> [!NOTE] +> `authenticatedRoutes()` registers the routes inside the panel's authentication +> middleware. The preview and attachment routes also enforce authentication and +> the `canManageMails()` check themselves, so they stay protected even if you +> register them with `routes()` — but `authenticatedRoutes()` is the correct +> place for them. + Then add the plugin to your `PanelProvider` ```php @@ -116,9 +123,14 @@ $panel This example demonstrates how to combine role-based and permission-based access control, providing a more robust and flexible approach to managing access to mail resources. +The `canManageMails()` check also guards the mail preview and attachment download +routes. Unauthenticated visitors are redirected to the panel login, authenticated +users without permission receive a 403, and attachments are only served through +the mail record they belong to. + ### Tenant middleware and route protection -If you want to protect the mail routes with your (tenant) middleware, you can do so by adding the routes to the `tenantRoutes`: +If you want to protect the mail routes with your (tenant) middleware, you can do so by adding the routes to the `authenticatedTenantRoutes`: ```php use Backstage\Mails\MailsPlugin; @@ -128,7 +140,7 @@ public function panel(Panel $panel): Panel { return $panel ->plugin(MailsPlugin::make()) - ->tenantRoutes(fn() => Mails::routes()); + ->authenticatedTenantRoutes(fn () => Mails::routes()); } ``` diff --git a/src/Controllers/MailDownloadController.php b/src/Controllers/MailDownloadController.php index 95fe4b7..b091894 100644 --- a/src/Controllers/MailDownloadController.php +++ b/src/Controllers/MailDownloadController.php @@ -2,24 +2,19 @@ namespace Backstage\Mails\Controllers; -use Backstage\Mails\Laravel\Models\MailAttachment; +use Illuminate\Http\Request; use Illuminate\Routing\Controller; use Illuminate\Support\Facades\Config; class MailDownloadController extends Controller { - public function __invoke(...$arguments) + public function __invoke(Request $request) { - if (count($arguments) === 4) { - [$tenant, $mail, $attachment, $filename] = $arguments; - } else { - [$mail, $attachment, $filename] = $arguments; - $tenant = null; - } + $mailModel = Config::get('mails.models.mail'); - $attachmentModel = Config::get('mails.models.attachment'); - /** @var MailAttachment $attachment */ - $attachment = $attachmentModel::find($attachment); + $mail = $mailModel::findOrFail($request->route('mail')); + + $attachment = $mail->attachments()->findOrFail($request->route('attachment')); return $attachment->downloadFileFromStorage(); } diff --git a/src/Controllers/MailPreviewController.php b/src/Controllers/MailPreviewController.php index d017789..975096e 100644 --- a/src/Controllers/MailPreviewController.php +++ b/src/Controllers/MailPreviewController.php @@ -2,17 +2,22 @@ namespace Backstage\Mails\Controllers; -use Backstage\Mails\Laravel\Models\Mail; use Illuminate\Http\Request; use Illuminate\Routing\Controller; +use Illuminate\Support\Facades\Config; class MailPreviewController extends Controller { public function __invoke(Request $request) { - /** @var Mail $mail */ - $mail = Mail::find($request->mail); + $mailModel = Config::get('mails.models.mail'); - return response($mail->html); + $mail = $mailModel::findOrFail($request->route('mail')); + + return response($mail->html, 200, [ + 'Content-Type' => 'text/html; charset=UTF-8', + 'X-Content-Type-Options' => 'nosniff', + 'Content-Security-Policy' => "frame-ancestors 'self'", + ]); } } diff --git a/src/Http/Middleware/EnsureUserCanManageMails.php b/src/Http/Middleware/EnsureUserCanManageMails.php new file mode 100644 index 0000000..c0d3a6d --- /dev/null +++ b/src/Http/Middleware/EnsureUserCanManageMails.php @@ -0,0 +1,23 @@ +hasPlugin('mails'), 403); + + abort_unless(MailsPlugin::get()->userCanManageMails(), 403); + + return $next($request); + } +} diff --git a/src/Mails.php b/src/Mails.php index 08e8e3f..3029c19 100644 --- a/src/Mails.php +++ b/src/Mails.php @@ -4,13 +4,23 @@ use Backstage\Mails\Controllers\MailDownloadController; use Backstage\Mails\Controllers\MailPreviewController; +use Backstage\Mails\Http\Middleware\EnsureUserCanManageMails; +use Filament\Http\Middleware\Authenticate; use Illuminate\Support\Facades\Route; class Mails { public static function routes(): void { - Route::get('mails/{mail}/preview', MailPreviewController::class)->name('mails.preview'); - Route::get('mails/{mail}/attachment/{attachment}/{filename}', MailDownloadController::class)->name('mails.attachment.download'); + Route::middleware([ + Authenticate::class, + EnsureUserCanManageMails::class, + ])->group(function (): void { + Route::get('mails/{mail}/preview', MailPreviewController::class) + ->name('mails.preview'); + + Route::get('mails/{mail}/attachment/{attachment}/{filename}', MailDownloadController::class) + ->name('mails.attachment.download'); + }); } } diff --git a/tests/Fixtures/TestPanelProvider.php b/tests/Fixtures/TestPanelProvider.php index d2f2ad1..193b070 100644 --- a/tests/Fixtures/TestPanelProvider.php +++ b/tests/Fixtures/TestPanelProvider.php @@ -2,6 +2,7 @@ namespace Backstage\Mails\Tests\Fixtures; +use Backstage\Mails\Mails; use Backstage\Mails\MailsPlugin; use Filament\Panel; use Filament\PanelProvider; @@ -14,7 +15,10 @@ public function panel(Panel $panel): Panel ->default() ->id('admin') ->path('admin') + ->login() + ->authGuard('web') ->topNavigation() + ->routes(fn () => Mails::routes()) ->plugin(MailsPlugin::make()); } } diff --git a/tests/Fixtures/User.php b/tests/Fixtures/User.php new file mode 100644 index 0000000..279a8c2 --- /dev/null +++ b/tests/Fixtures/User.php @@ -0,0 +1,19 @@ + 'Test User', + 'email' => 'test@example.com', + 'password' => bcrypt('password'), + ]); +} + +function attachmentFor(Mail $mail, string $filename = 'invoice.pdf', string $contents = 'CONFIDENTIAL') +{ + $attachment = $mail->attachments()->create([ + 'uuid' => (string) Str::uuid(), + 'disk' => 'local', + 'filename' => $filename, + 'mime' => 'application/pdf', + 'inline' => false, + 'size' => strlen($contents), + ]); + + Storage::disk('local')->put($attachment->storagePath, $contents); + + return $attachment; +} + +function previewUrl(Mail $mail): string +{ + return route('filament.admin.mails.preview', ['mail' => $mail->id]); +} + +function downloadUrl(Mail $mail, $attachment): string +{ + return route('filament.admin.mails.attachment.download', [ + 'mail' => $mail->id, + 'attachment' => $attachment->id, + 'filename' => $attachment->filename, + ]); +} + +it('redirects a guest away from the mail preview', function () { + $mail = Mail::factory()->create(['html' => '
secret
']); + + $this->get(previewUrl($mail)) + ->assertRedirect(route('filament.admin.auth.login')); +}); + +it('forbids an authenticated user without mail permissions', function () { + $mail = Mail::factory()->create(['html' => 'secret
']); + + MailsPlugin::get()->canManageMails(false); + + $this->actingAs(mailUser()) + ->get(previewUrl($mail)) + ->assertForbidden(); +}); + +it('allows a permitted user to preview a mail', function () { + $mail = Mail::factory()->create(['html' => 'secret
']); + + MailsPlugin::get()->canManageMails(true); + + $this->actingAs(mailUser()) + ->get(previewUrl($mail)) + ->assertOk() + ->assertSee('secret'); +}); + +it('redirects a guest away from an attachment download', function () { + Storage::fake('local'); + + $mail = Mail::factory()->create(); + $attachment = attachmentFor($mail); + + $this->get(downloadUrl($mail, $attachment)) + ->assertRedirect(route('filament.admin.auth.login')); +}); + +it('does not serve an attachment belonging to a different mail', function () { + Storage::fake('local'); + + $mail = Mail::factory()->create(); + $otherMail = Mail::factory()->create(); + $attachmentOfOtherMail = attachmentFor($otherMail, 'secret.pdf'); + + MailsPlugin::get()->canManageMails(true); + + $this->actingAs(mailUser()) + ->get(downloadUrl($mail, $attachmentOfOtherMail)) + ->assertNotFound(); +}); + +it('serves an attachment that belongs to the mail', function () { + Storage::fake('local'); + + $mail = Mail::factory()->create(); + $attachment = attachmentFor($mail, 'invoice.pdf', 'INVOICE BODY'); + + MailsPlugin::get()->canManageMails(true); + + $response = $this->actingAs(mailUser()) + ->get(downloadUrl($mail, $attachment)) + ->assertOk(); + + expect($response->streamedContent())->toBe('INVOICE BODY'); +}); + +it('returns 404 for an unknown attachment', function () { + Storage::fake('local'); + + $mail = Mail::factory()->create(); + + MailsPlugin::get()->canManageMails(true); + + $url = route('filament.admin.mails.attachment.download', [ + 'mail' => $mail->id, + 'attachment' => 99999, + 'filename' => 'nope.pdf', + ]); + + $this->actingAs(mailUser())->get($url)->assertNotFound(); +}); + +it('returns 404 for an unknown mail preview', function () { + MailsPlugin::get()->canManageMails(true); + + $url = route('filament.admin.mails.preview', ['mail' => 99999]); + + $this->actingAs(mailUser())->get($url)->assertNotFound(); +}); + +it('sends hardening headers with the preview', function () { + $mail = Mail::factory()->create(['html' => 'secret
']); + + MailsPlugin::get()->canManageMails(true); + + $this->actingAs(mailUser()) + ->get(previewUrl($mail)) + ->assertOk() + ->assertHeader('X-Content-Type-Options', 'nosniff') + ->assertHeader('Content-Security-Policy', "frame-ancestors 'self'"); +}); diff --git a/tests/TestCase.php b/tests/TestCase.php index 5f7b71b..65f290c 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -2,8 +2,10 @@ namespace Backstage\Mails\Tests; +use Backstage\Mails\Laravel\MailsServiceProvider as LaravelMailsServiceProvider; use Backstage\Mails\MailsServiceProvider; use Backstage\Mails\Tests\Fixtures\TestPanelProvider; +use Backstage\Mails\Tests\Fixtures\User; use BladeUI\Heroicons\BladeHeroiconsServiceProvider; use BladeUI\Icons\BladeIconsServiceProvider; use Filament\Actions\ActionsServiceProvider; @@ -15,6 +17,9 @@ use Filament\Tables\TablesServiceProvider; use Filament\Widgets\WidgetsServiceProvider; use Illuminate\Database\Eloquent\Factories\Factory; +use Illuminate\Database\Schema\Blueprint; +use Illuminate\Support\Facades\File; +use Illuminate\Support\Facades\Schema; use Livewire\LivewireServiceProvider; use Orchestra\Testbench\TestCase as Orchestra; use RyanChandler\BladeCaptureDirective\BladeCaptureDirectiveServiceProvider; @@ -45,6 +50,7 @@ protected function getPackageProviders($app) SupportServiceProvider::class, TablesServiceProvider::class, WidgetsServiceProvider::class, + LaravelMailsServiceProvider::class, MailsServiceProvider::class, TestPanelProvider::class, ]; @@ -53,5 +59,24 @@ protected function getPackageProviders($app) public function getEnvironmentSetUp($app) { config()->set('database.default', 'testing'); + config()->set('auth.providers.users.model', User::class); + } + + protected function defineDatabaseMigrations(): void + { + Schema::create('users', function (Blueprint $table): void { + $table->id(); + $table->string('name'); + $table->string('email')->unique(); + $table->string('password'); + $table->rememberToken(); + $table->timestamps(); + }); + + $directory = __DIR__ . '/../vendor/backstage/laravel-mails/database/migrations'; + + foreach (File::files($directory) as $file) { + (include $file->getPathname())->up(); + } } }