Harden tenant admin auth and photo moderation

2026-01-30 14:53:51 +01:00
parent d45cb6a087
commit 916b204688
7 changed files with 220 additions and 11 deletions
--- a/app/Http/Controllers/Api/Tenant/PhotoController.php
+++ b/app/Http/Controllers/Api/Tenant/PhotoController.php
@@ -20,6 +20,7 @@ use App\Support\WatermarkConfigResolver;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Request;
 use Illuminate\Http\Resources\Json\AnonymousResourceCollection;
+use Illuminate\Http\UploadedFile;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Log;
@@ -115,6 +116,7 @@ class PhotoController extends Controller
        $event = Event::where('slug', $eventSlug)
            ->where('tenant_id', $tenantId)
            ->firstOrFail();
+        TenantMemberPermissions::ensureEventPermission($request, $event, 'photos:moderate');

        if ($photo->event_id !== $event->id) {
            return ApiError::response(
@@ -321,7 +323,7 @@ class PhotoController extends Controller
        $disk = $this->eventStorageManager->getHotDiskForEvent($event);

        // Generate unique filename
-        $extension = $file->getClientOriginalExtension();
+        $extension = $this->resolvePhotoExtension($file);
        $filename = Str::uuid().'.'.$extension;
        $path = "events/{$eventSlug}/photos/{$filename}";

@@ -563,6 +565,7 @@ class PhotoController extends Controller
        $event = Event::where('slug', $eventSlug)
            ->where('tenant_id', $tenantId)
            ->firstOrFail();
+        TenantMemberPermissions::ensureEventPermission($request, $event, 'photos:moderate');

        if ($photo->event_id !== $event->id) {
            return ApiError::response(
@@ -779,6 +782,7 @@ class PhotoController extends Controller
        $event = Event::where('slug', $eventSlug)
            ->where('tenant_id', $tenantId)
            ->firstOrFail();
+        TenantMemberPermissions::ensureEventPermission($request, $event, 'photos:moderate');

        $photos = Photo::where('event_id', $event->id)
            ->where('status', 'pending')
@@ -1043,4 +1047,23 @@ class PhotoController extends Controller

        return array_values(array_unique(array_filter($candidates)));
    }
+
+    private function resolvePhotoExtension(UploadedFile $file): string
+    {
+        $extension = strtolower((string) $file->extension());
+
+        if (! in_array($extension, ['jpg', 'jpeg', 'png', 'webp'], true)) {
+            $extension = strtolower((string) $file->getClientOriginalExtension());
+        }
+
+        if (! in_array($extension, ['jpg', 'jpeg', 'png', 'webp'], true)) {
+            $extension = match ($file->getMimeType()) {
+                'image/png' => 'png',
+                'image/webp' => 'webp',
+                default => 'jpg',
+            };
+        }
+
+        return $extension === 'jpeg' ? 'jpg' : $extension;
+    }
 }
--- a/app/Http/Controllers/Auth/AuthenticatedSessionController.php
+++ b/app/Http/Controllers/Auth/AuthenticatedSessionController.php
@@ -157,6 +157,10 @@ class AuthenticatedSessionController extends Controller
            return null;
        }

+        if (str_starts_with($candidate, '//')) {
+            return null;
+        }
+
        if (str_starts_with($candidate, '/')) {
            return $candidate;
        }
@@ -170,7 +174,7 @@ class AuthenticatedSessionController extends Controller

        $appHost = parse_url($request->getSchemeAndHttpHost(), PHP_URL_HOST);

-        if ($appHost && ! Str::endsWith($targetHost, $appHost)) {
+        if (! $appHost || ! $this->isAllowedReturnHost($targetHost, $appHost)) {
            return null;
        }

@@ -222,7 +226,7 @@ class AuthenticatedSessionController extends Controller
            $scheme = $parsed['scheme'] ?? null;
            $requestHost = parse_url($request->getSchemeAndHttpHost(), PHP_URL_HOST);

-            if ($scheme && $host && $requestHost && ! Str::endsWith($host, $requestHost)) {
+            if ($scheme && $host && $requestHost && ! $this->isAllowedReturnHost($host, $requestHost)) {
                return '/event-admin/dashboard';
            }

@@ -265,6 +269,15 @@ class AuthenticatedSessionController extends Controller
        return $decoded;
    }

+    private function isAllowedReturnHost(string $targetHost, string $appHost): bool
+    {
+        if ($targetHost === $appHost) {
+            return true;
+        }
+
+        return Str::endsWith($targetHost, '.'.$appHost);
+    }
+
    private function rememberTenantAdminTarget(Request $request, ?string $target): void
    {
        $user = Auth::user();
--- a/app/Http/Controllers/TenantAdminFacebookController.php
+++ b/app/Http/Controllers/TenantAdminFacebookController.php
@@ -100,13 +100,30 @@ class TenantAdminFacebookController extends Controller
            return null;
        }

+        if (str_starts_with($decoded, '//')) {
+            return null;
+        }
+
+        if (str_starts_with($decoded, '/')) {
+            return $decoded;
+        }
+
        $targetHost = parse_url($decoded, PHP_URL_HOST);
        $appHost = parse_url($request->getSchemeAndHttpHost(), PHP_URL_HOST);

-        if ($targetHost && $appHost && ! Str::endsWith($targetHost, $appHost)) {
+        if (! $targetHost || ! $appHost || ! $this->isAllowedReturnHost($targetHost, $appHost)) {
            return null;
        }

        return $decoded;
    }
+
+    private function isAllowedReturnHost(string $targetHost, string $appHost): bool
+    {
+        if ($targetHost === $appHost) {
+            return true;
+        }
+
+        return Str::endsWith($targetHost, '.'.$appHost);
+    }
 }
--- a/app/Http/Controllers/TenantAdminGoogleController.php
+++ b/app/Http/Controllers/TenantAdminGoogleController.php
@@ -100,13 +100,30 @@ class TenantAdminGoogleController extends Controller
            return null;
        }

+        if (str_starts_with($decoded, '//')) {
+            return null;
+        }
+
+        if (str_starts_with($decoded, '/')) {
+            return $decoded;
+        }
+
        $targetHost = parse_url($decoded, PHP_URL_HOST);
        $appHost = parse_url($request->getSchemeAndHttpHost(), PHP_URL_HOST);

-        if ($targetHost && $appHost && ! Str::endsWith($targetHost, $appHost)) {
+        if (! $targetHost || ! $appHost || ! $this->isAllowedReturnHost($targetHost, $appHost)) {
            return null;
        }

        return $decoded;
    }
+
+    private function isAllowedReturnHost(string $targetHost, string $appHost): bool
+    {
+        if ($targetHost === $appHost) {
+            return true;
+        }
+
+        return Str::endsWith($targetHost, '.'.$appHost);
+    }
 }
--- a/routes/api.php
+++ b/routes/api.php
@@ -322,12 +322,24 @@ Route::prefix('v1')->name('api.v1.')->group(function () {
            Route::prefix('photos')->group(function () {
                Route::get('/', [PhotoController::class, 'index'])->name('tenant.events.photos.index');
                Route::post('/', [PhotoController::class, 'store'])->name('tenant.events.photos.store');
-                Route::get('{photo}', [PhotoController::class, 'show'])->name('tenant.events.photos.show');
-                Route::patch('{photo}', [PhotoController::class, 'update'])->name('tenant.events.photos.update');
-                Route::delete('{photo}', [PhotoController::class, 'destroy'])->name('tenant.events.photos.destroy');
-                Route::post('{photo}/feature', [PhotoController::class, 'feature'])->name('tenant.events.photos.feature');
-                Route::post('{photo}/unfeature', [PhotoController::class, 'unfeature'])->name('tenant.events.photos.unfeature');
-                Route::post('{photo}/visibility', [PhotoController::class, 'visibility'])->name('tenant.events.photos.visibility');
+                Route::get('{photo}', [PhotoController::class, 'show'])
+                    ->whereNumber('photo')
+                    ->name('tenant.events.photos.show');
+                Route::patch('{photo}', [PhotoController::class, 'update'])
+                    ->whereNumber('photo')
+                    ->name('tenant.events.photos.update');
+                Route::delete('{photo}', [PhotoController::class, 'destroy'])
+                    ->whereNumber('photo')
+                    ->name('tenant.events.photos.destroy');
+                Route::post('{photo}/feature', [PhotoController::class, 'feature'])
+                    ->whereNumber('photo')
+                    ->name('tenant.events.photos.feature');
+                Route::post('{photo}/unfeature', [PhotoController::class, 'unfeature'])
+                    ->whereNumber('photo')
+                    ->name('tenant.events.photos.unfeature');
+                Route::post('{photo}/visibility', [PhotoController::class, 'visibility'])
+                    ->whereNumber('photo')
+                    ->name('tenant.events.photos.visibility');
                Route::post('bulk-approve', [PhotoController::class, 'bulkApprove'])->name('tenant.events.photos.bulk-approve');
                Route::post('bulk-reject', [PhotoController::class, 'bulkReject'])->name('tenant.events.photos.bulk-reject');
                Route::get('moderation', [PhotoController::class, 'forModeration'])->name('tenant.events.photos.for-moderation');
--- a/tests/Feature/Auth/LoginTest.php
+++ b/tests/Feature/Auth/LoginTest.php
@@ -184,6 +184,30 @@ class LoginTest extends TestCase
        $response->assertRedirect('/event-admin/dashboard');
    }

+    public function test_tenant_admin_login_rejects_lookalike_return_host(): void
+    {
+        $user = User::factory()->create([
+            'email' => 'hostcheck@example.com',
+            'role' => 'tenant_admin',
+            'password' => bcrypt('password'),
+            'email_verified_at' => now(),
+        ]);
+
+        $appHost = parse_url(config('app.url'), PHP_URL_HOST) ?? 'localhost';
+        $targetHost = 'evil-'.$appHost;
+        $target = "https://{$targetHost}/event-admin/dashboard";
+        $encodedReturn = rtrim(strtr(base64_encode($target), '+/', '-_'), '=');
+
+        $response = $this->post(route('login.store'), [
+            'login' => 'hostcheck@example.com',
+            'password' => 'password',
+            'return_to' => $encodedReturn,
+        ]);
+
+        $this->assertAuthenticatedAs($user);
+        $response->assertRedirect('/event-admin/dashboard');
+    }
+
    public function test_tenant_admin_can_access_login_with_admin_return_path_when_authenticated()
    {
        $user = User::factory()->create([
--- a/tests/Feature/Tenant/PhotoUploadSecurityTest.php
+++ b/tests/Feature/Tenant/PhotoUploadSecurityTest.php
@@ -0,0 +1,103 @@
+<?php
+
+namespace Tests\Feature\Tenant;
+
+use App\Models\Event;
+use App\Models\EventMember;
+use App\Models\EventPackage;
+use App\Models\MediaStorageTarget;
+use App\Models\Package;
+use App\Models\Photo;
+use App\Models\User;
+use Illuminate\Http\UploadedFile;
+use Illuminate\Support\Facades\Hash;
+
+class PhotoUploadSecurityTest extends TenantTestCase
+{
+    public function test_photo_upload_uses_safe_extension_from_mime(): void
+    {
+        $event = Event::factory()->for($this->tenant)->create([
+            'slug' => 'secure-photo-upload',
+        ]);
+        MediaStorageTarget::query()->create([
+            'key' => 'local',
+            'name' => 'Local Storage',
+            'driver' => 'local',
+            'is_hot' => true,
+            'is_default' => true,
+            'is_active' => true,
+            'priority' => 1,
+        ]);
+        $package = Package::factory()->endcustomer()->create([
+            'max_photos' => 500,
+            'max_guests' => 100,
+            'gallery_days' => 30,
+        ]);
+        EventPackage::query()->create([
+            'event_id' => $event->id,
+            'package_id' => $package->id,
+            'purchased_price' => 0,
+            'used_photos' => 0,
+            'used_guests' => 0,
+        ]);
+
+        $file = UploadedFile::fake()->image('photo.jpeg', 800, 600);
+
+        $response = $this->withHeaders([
+            'Authorization' => 'Bearer '.$this->token,
+            'Accept' => 'application/json',
+        ])->post("/api/v1/tenant/events/{$event->slug}/photos", [
+            'photo' => $file,
+        ]);
+
+        $response->assertCreated();
+
+        $photo = Photo::query()->first();
+        $this->assertNotNull($photo);
+        $this->assertSame('jpg', strtolower(pathinfo((string) $photo->file_path, PATHINFO_EXTENSION)));
+
+        if (Photo::supportsFilenameColumn()) {
+            $this->assertStringEndsWith('.jpg', (string) $photo->filename);
+        }
+    }
+
+    public function test_member_without_moderation_permission_cannot_access_moderation_queue(): void
+    {
+        $event = Event::factory()->for($this->tenant)->create([
+            'slug' => 'no-moderation-permission',
+        ]);
+
+        $memberUser = User::factory()->create([
+            'email' => 'limited.member@example.com',
+            'tenant_id' => $this->tenant->id,
+            'role' => 'member',
+            'password' => Hash::make('secret123'),
+        ]);
+
+        EventMember::factory()->create([
+            'tenant_id' => $this->tenant->id,
+            'event_id' => $event->id,
+            'user_id' => $memberUser->id,
+            'email' => $memberUser->email,
+            'role' => 'member',
+            'status' => 'active',
+            'permissions' => [],
+        ]);
+
+        $login = $this->postJson('/api/v1/tenant-auth/login', [
+            'login' => $memberUser->email,
+            'password' => 'secret123',
+        ]);
+
+        $login->assertOk();
+        $token = $login->json('token');
+
+        $response = $this->withHeaders([
+            'Authorization' => 'Bearer '.$token,
+            'Accept' => 'application/json',
+        ])->getJson("/api/v1/tenant/events/{$event->slug}/photos/moderation");
+
+        $response->assertForbidden();
+        $response->assertJsonPath('error.code', 'insufficient_permission');
+    }
+}