diff --git a/public/index.php b/public/index.php index a2bcea4..37a9d30 100644 --- a/public/index.php +++ b/public/index.php @@ -106,6 +106,13 @@ $postUpload = static fn (Request $r, array $p): Response => (new PostImageUpload $getAssets = static function (Request $r, array $p) use ($placeholderDir, $uploadsRoot): Response { return (new GetAssets($placeholderDir, $uploadsRoot))->handle($r, $p); }; +$getUploadedAsset = static function (Request $r, array $p) use ($placeholderDir, $uploadsRoot): Response { + $handler = new GetAssets($placeholderDir, $uploadsRoot); + return $handler->handle( + $r, + ['kind' => 'uploads', 'userToken' => $p['userToken'] ?? '', 'filename' => $p['filename'] ?? ''], + ); +}; $router = new Router(); $router->add('GET', '/', $homePage); @@ -115,6 +122,7 @@ $router->add('GET', '/scenarios/{id}/edit/battlefield', $getBattlefield); $router->add('POST', '/scenarios/{id}/edit/battlefield', $postBattlefield); $router->add('POST', '/scenarios/{id}/start', $postStart); $router->add('POST', '/assets/upload', $postUpload); +$router->add('GET', '/assets/uploads/{userToken}/{filename}', $getUploadedAsset); $router->add('GET', '/assets/{kind}/{filename}', $getAssets); // 8. Dispatch and emit. diff --git a/src/Application/ImageUploadService.php b/src/Application/ImageUploadService.php index c375eee..797f7a4 100644 --- a/src/Application/ImageUploadService.php +++ b/src/Application/ImageUploadService.php @@ -33,6 +33,6 @@ final class ImageUploadService // file itself to 0600 in case the server's umask is permissive. chmod($destination, 0600); - return '/assets/' . $this->userToken . '/' . $filename; + return '/assets/uploads/' . $this->userToken . '/' . $filename; } } diff --git a/src/Http/Handlers/GetAssets.php b/src/Http/Handlers/GetAssets.php index b495284..1696085 100644 --- a/src/Http/Handlers/GetAssets.php +++ b/src/Http/Handlers/GetAssets.php @@ -31,7 +31,7 @@ final class GetAssets return Response::html(404, '

Not found

'); } - $requestToken = $request->cookies['__uploads_token'] ?? ''; + $requestToken = (string) ($request->server['__uploads_token'] ?? ''); if ($userToken !== $requestToken) { return Response::html(404, '

Not found

'); diff --git a/tests/Integration/FullFlowTest.php b/tests/Integration/FullFlowTest.php index 133ea2a..f07ffcd 100644 --- a/tests/Integration/FullFlowTest.php +++ b/tests/Integration/FullFlowTest.php @@ -24,8 +24,8 @@ final class FullFlowTest extends TestCase public function testTheFullCreateAndSaveFlow(): void { [$token, $cookie] = CsrfToken::issue(self::SECRET); - $uploadsRoot = sys_get_temp_dir() . '/bf-fullflow-' . bin2hex(random_bytes(4)); - mkdir($uploadsRoot, 0700, true); + // Use a deterministic secret so the upload token HMAC matches between upload and fetch. + putenv('BATTLEFORGE_SECRET=' . self::SECRET); try { // Step 1: GET home page. @@ -71,19 +71,82 @@ final class FullFlowTest extends TestCase self::assertSame('alpha', $startJson['match']['activeTeamId'] ?? null); self::assertSame(1, $startJson['match']['round'] ?? null); self::assertCount(6, $startJson['match']['units'] ?? []); + + // Step 5: POST image upload, then GET the returned URL. + $this->uploadAndFetchAsset(); } finally { + // Always purge the per-install uploads dir after the test. + $uploadsRoot = __DIR__ . '/../../var/uploads'; if (is_dir($uploadsRoot)) { foreach (glob($uploadsRoot . '/*/*') as $file) { - unlink($file); + @unlink($file); } foreach (glob($uploadsRoot . '/*') as $dir) { - rmdir($dir); + @rmdir($dir); } - rmdir($uploadsRoot); } } } + private function uploadAndFetchAsset(): void + { + $expectedToken = hash_hmac('sha256', self::SECRET, 'bf-uploads'); + $expectedDir = realpath(__DIR__ . '/../../var/uploads') ?: (__DIR__ . '/../../var/uploads'); + + // Create the multipart body manually so we can pre-compute the file we expect to find. + $png = base64_decode( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGNgYGBgAAAABQABh6FO1AAAAABJRU5ErkJggg==', + true, + ); + $tmp = tempnam(sys_get_temp_dir(), 'bf-upload-'); + file_put_contents($tmp, $png); + + try { + [$token, $cookie] = CsrfToken::issue(self::SECRET); + $_COOKIE['__csrf'] = $cookie; + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_SERVER['REQUEST_URI'] = '/assets/upload'; + $_SERVER['CONTENT_TYPE'] = 'multipart/form-data; boundary=----test'; + $_SERVER['__csrf_secret'] = self::SECRET; + $_POST = ['_csrf' => $token]; + $_FILES = [ + 'image' => [ + 'name' => 'test.png', + 'type' => 'image/png', + 'tmp_name' => $tmp, + 'error' => 0, + 'size' => strlen($png), + ], + ]; + $this->rawBody = ''; + $_SERVER['__raw_body'] = ''; + $body = $this->runFrontController(); + self::assertSame(200, $this->lastStatus, 'upload response: ' . $body); + $payload = json_decode($body, true); + $url = is_array($payload) ? (string) ($payload['url'] ?? '') : ''; + self::assertNotSame('', $url); + self::assertStringStartsWith('/assets/uploads/' . $expectedToken . '/', $url); + + // Now fetch the asset back through the front controller. + $_COOKIE = []; + $_POST = []; + $_FILES = []; + $_SERVER['REQUEST_METHOD'] = 'GET'; + $_SERVER['REQUEST_URI'] = $url; + unset($_SERVER['CONTENT_TYPE'], $_SERVER['HTTP_X_CSRF_TOKEN'], $_SERVER['__raw_body']); + $this->rawBody = ''; + $fetchBody = $this->runFrontController(); + self::assertSame(200, $this->lastStatus, 'fetch response: ' . $fetchBody); + self::assertSame($png, $fetchBody); + + // Clean up the uploaded file so the outer finally does not loop on it. + $filename = substr($url, strlen('/assets/uploads/' . $expectedToken . '/')); + @unlink($expectedDir . '/' . $expectedToken . '/' . $filename); + } finally { + @unlink($tmp); + } + } + private string $rawBody = ''; private int $lastStatus = 0; diff --git a/tests/Integration/GetAssetsTest.php b/tests/Integration/GetAssetsTest.php index 31f6bf6..4caa14b 100644 --- a/tests/Integration/GetAssetsTest.php +++ b/tests/Integration/GetAssetsTest.php @@ -67,4 +67,61 @@ final class GetAssetsTest extends TestCase self::assertSame(404, $response->status); } + + public function testItServesAnUploadedAssetWhenTheServerTokenMatches(): void + { + $userToken = str_repeat('a', 64); + $namespace = $this->uploadsDir . '/' . $userToken; + mkdir($namespace, 0700, true); + $payload = base64_decode( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGNgYGBgAAAABQABh6FO1AAAAABJRU5ErkJggg==', + true, + ); + file_put_contents($namespace . '/demo.png', $payload); + + $handler = new GetAssets($this->placeholderDir, $this->uploadsDir); + $request = new Request( + [], + [], + [], + [], + ['__uploads_token' => $userToken], + '', + 'GET', + '/assets/uploads/' . $userToken . '/demo.png', + null, + '', + ); + $response = $handler->handle( + $request, + ['kind' => 'uploads', 'userToken' => $userToken, 'filename' => 'demo.png'], + ); + + self::assertSame(200, $response->status); + self::assertSame('image/png', $response->headers['Content-Type'] ?? ''); + self::assertSame($payload, $response->body); + } + + public function testItRefusesAnUploadedAssetWhenTheServerTokenDiffers(): void + { + $handler = new GetAssets($this->placeholderDir, $this->uploadsDir); + $request = new Request( + [], + [], + [], + [], + ['__uploads_token' => str_repeat('a', 64)], + '', + 'GET', + '/assets/uploads/abc/def.png', + null, + '', + ); + $response = $handler->handle( + $request, + ['kind' => 'uploads', 'userToken' => str_repeat('b', 64), 'filename' => 'def.png'], + ); + + self::assertSame(404, $response->status); + } } diff --git a/tests/Unit/Application/ImageUploadServiceTest.php b/tests/Unit/Application/ImageUploadServiceTest.php index a80cfdd..4a0c2b2 100644 --- a/tests/Unit/Application/ImageUploadServiceTest.php +++ b/tests/Unit/Application/ImageUploadServiceTest.php @@ -38,7 +38,7 @@ final class ImageUploadServiceTest extends TestCase $url = $service->store($tmp, 'image/png'); - self::assertStringStartsWith('/assets/user-token-1/', $url); + self::assertStringStartsWith('/assets/uploads/user-token-1/', $url); self::assertStringEndsWith('.png', $url); $stored = $this->tmpDir . '/user-token-1/' . basename($url);