From 916b204688af407b5939934de162e9eaf07b9d91 Mon Sep 17 00:00:00 2001 From: Codex Agent Date: Fri, 30 Jan 2026 14:53:51 +0100 Subject: [PATCH] Harden tenant admin auth and photo moderation --- .../Api/Tenant/PhotoController.php | 25 ++++- .../Auth/AuthenticatedSessionController.php | 17 ++- .../TenantAdminFacebookController.php | 19 +++- .../TenantAdminGoogleController.php | 19 +++- routes/api.php | 24 +++- tests/Feature/Auth/LoginTest.php | 24 ++++ .../Tenant/PhotoUploadSecurityTest.php | 103 ++++++++++++++++++ 7 files changed, 220 insertions(+), 11 deletions(-) create mode 100644 tests/Feature/Tenant/PhotoUploadSecurityTest.php diff --git a/app/Http/Controllers/Api/Tenant/PhotoController.php b/app/Http/Controllers/Api/Tenant/PhotoController.php index 3f575a2..fc4221e 100644 --- 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; + } } diff --git a/app/Http/Controllers/Auth/AuthenticatedSessionController.php b/app/Http/Controllers/Auth/AuthenticatedSessionController.php index 24a6951..ddff89a 100644 --- 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(); diff --git a/app/Http/Controllers/TenantAdminFacebookController.php b/app/Http/Controllers/TenantAdminFacebookController.php index a3bd7d2..463fbce 100644 --- 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); + } } diff --git a/app/Http/Controllers/TenantAdminGoogleController.php b/app/Http/Controllers/TenantAdminGoogleController.php index ba1d665..cb9738a 100644 --- 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); + } } diff --git a/routes/api.php b/routes/api.php index 72adb72..e6978d3 100644 --- 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'); diff --git a/tests/Feature/Auth/LoginTest.php b/tests/Feature/Auth/LoginTest.php index 8453867..d7df048 100644 --- 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([ diff --git a/tests/Feature/Tenant/PhotoUploadSecurityTest.php b/tests/Feature/Tenant/PhotoUploadSecurityTest.php new file mode 100644 index 0000000..ba7bca1 --- /dev/null +++ b/tests/Feature/Tenant/PhotoUploadSecurityTest.php @@ -0,0 +1,103 @@ +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'); + } +}