diff --git a/server/src/Http/Controllers/Internal/v1/ProofController.php b/server/src/Http/Controllers/Internal/v1/ProofController.php index eab7bd779..186d73495 100644 --- a/server/src/Http/Controllers/Internal/v1/ProofController.php +++ b/server/src/Http/Controllers/Internal/v1/ProofController.php @@ -83,8 +83,8 @@ public function captureSignature(string $publicId, Request $request) // set the signature storage path $path = $this->signatureStoragePath($proof); - // upload signature - $this->storeSignature($path, base64_decode($signature), 'public'); + // upload signature (private object; File::url serves it through a signed URL) + $this->storeSignature($path, base64_decode($signature)); // create file record for upload $file = $this->createSignatureFile($path, $signature, $proof); @@ -121,9 +121,13 @@ protected function createProof(array $attributes): Proof return Proof::create($attributes); } - protected function storeSignature(string $path, string|false $contents, string $visibility): void + /** + * Writes without an ACL: the media bucket enforces bucket-owner object ownership, so a + * 'public' visibility (public-read ACL) makes S3 reject the PUT and put() return false. + */ + protected function storeSignature(string $path, string|false $contents): void { - Storage::disk('s3')->put($path, $contents, $visibility); + Storage::disk('s3')->put($path, $contents); } /** diff --git a/server/src/Models/Driver.php b/server/src/Models/Driver.php index 7eef1ac1c..c796257fa 100644 --- a/server/src/Models/Driver.php +++ b/server/src/Models/Driver.php @@ -431,7 +431,8 @@ public function getAvatarUrlAttribute($value): ?string return static::getAvatar($value); } - return $value; + // legacy rows hold an absolute URL; re-sign it if it points into the private media bucket + return Utils::signStoredFileUrl($value); } /** diff --git a/server/src/Models/Place.php b/server/src/Models/Place.php index 4b3a94c97..898259e5f 100644 --- a/server/src/Models/Place.php +++ b/server/src/Models/Place.php @@ -187,7 +187,8 @@ public function getAvatarUrlAttribute($value) return static::getAvatar($value); } - return $value; + // legacy rows hold an absolute URL; re-sign it if it points into the private media bucket + return Utils::signStoredFileUrl($value); } /** diff --git a/server/src/Models/Vehicle.php b/server/src/Models/Vehicle.php index 0fb69577a..250b440d9 100644 --- a/server/src/Models/Vehicle.php +++ b/server/src/Models/Vehicle.php @@ -525,7 +525,8 @@ public function getAvatarUrlAttribute($value) return static::getAvatar($value); } - return $value; + // legacy rows hold an absolute URL; re-sign it if it points into the private media bucket + return Utils::signStoredFileUrl($value); } /** diff --git a/server/src/Support/Utils.php b/server/src/Support/Utils.php index 8671742e1..b7037d3ab 100644 --- a/server/src/Support/Utils.php +++ b/server/src/Support/Utils.php @@ -1603,4 +1603,15 @@ public static function fixPhone(?string $phone): ?string return $phone; } + + /** + * Re-sign a URL stored as a string (e.g. a legacy `avatar_url` value) when it points into the + * private S3 media bucket, so it keeps working once the bucket stops allowing public reads. + * Other values pass through unchanged. Falls back to the raw value on core-api releases that + * predate File::signStoredUrl(). + */ + public static function signStoredFileUrl(?string $url): ?string + { + return method_exists(\Fleetbase\Models\File::class, 'signStoredUrl') ? \Fleetbase\Models\File::signStoredUrl($url) : $url; + } } diff --git a/server/tests/Feature/Http/Internal/ProofControllerContractsTest.php b/server/tests/Feature/Http/Internal/ProofControllerContractsTest.php index 5cdc1cf5d..dac559930 100644 --- a/server/tests/Feature/Http/Internal/ProofControllerContractsTest.php +++ b/server/tests/Feature/Http/Internal/ProofControllerContractsTest.php @@ -43,9 +43,9 @@ protected function createProof(array $attributes): Proof return $proof; } - protected function storeSignature(string $path, string|false $contents, string $visibility): void + protected function storeSignature(string $path, string|false $contents): void { - $this->storedSignatures[] = [$path, $contents, $visibility]; + $this->storedSignatures[] = [$path, $contents]; } protected function createSignatureFile(string $path, string $signature, Proof $proof): Fleetbase\Models\File @@ -206,7 +206,7 @@ public function setKey($model, $type = null): Fleetbase\Models\File 'raw_data' => $signature, ]) ->and($controller->storedSignatures)->toBe([ - ['uploads/company-uuid/signatures/proof-public-1.png', 'signature-bytes', 'public'], + ['uploads/company-uuid/signatures/proof-public-1.png', 'signature-bytes'], ]) ->and($controller->createdFiles)->toBe([ ['uploads/company-uuid/signatures/proof-public-1.png', $signature, 'proof-public-1'], diff --git a/server/tests/Unit/Support/OsrmProofAndIssueFilterTest.php b/server/tests/Unit/Support/OsrmProofAndIssueFilterTest.php index 3c311fa00..20304b1f8 100644 --- a/server/tests/Unit/Support/OsrmProofAndIssueFilterTest.php +++ b/server/tests/Unit/Support/OsrmProofAndIssueFilterTest.php @@ -119,7 +119,7 @@ public function disk($disk = null) public function put($path, $contents, $options = []) { - $this->writes[] = [$path]; + $this->writes[] = [$path, $options]; return true; } @@ -200,8 +200,9 @@ public function put($path, $contents, $options = []) expect($proof)->toBeInstanceOf(Proof::class) ->and($connection->table('proofs')->count())->toBe(1); - $probe->callHelper('storeSignature', 'signatures/proof.png', 'binary', 'public'); - expect($GLOBALS['fleetopsProofStorageFake']->writes)->toHaveCount(1); + $probe->callHelper('storeSignature', 'signatures/proof.png', 'binary'); + // Signatures are written without a visibility/ACL option; the media bucket rejects ACLs. + expect($GLOBALS['fleetopsProofStorageFake']->writes)->toBe([['signatures/proof.png', []]]); expect($probe->callHelper('jsonResponse', ['status' => 'ok'])->getData(true))->toBe(['status' => 'ok']) ->and($probe->callHelper('errorResponse', 'nope')->getData(true)['error'])->toBe('nope') diff --git a/server/tests/Unit/Support/UtilsAdditionalTest.php b/server/tests/Unit/Support/UtilsAdditionalTest.php index ca8931829..8f5e10f35 100644 --- a/server/tests/Unit/Support/UtilsAdditionalTest.php +++ b/server/tests/Unit/Support/UtilsAdditionalTest.php @@ -70,6 +70,15 @@ function fleetopsUtilsAdditionalPolygon(): Polygon ]); } +test('stored file url signing passes through values that are not absolute urls', function () { + // Absolute URLs into the media bucket are re-signed by core-api File::signStoredUrl (covered there); + // everything else, including avatar option keys and empty values, must come back untouched. + expect(Utils::signStoredFileUrl(null))->toBeNull() + ->and(Utils::signStoredFileUrl(''))->toBe('') + ->and(Utils::signStoredFileUrl('mini_bus'))->toBe('mini_bus') + ->and(Utils::signStoredFileUrl('custom-avatars/vehicles/c/van.png'))->toBe('custom-avatars/vehicles/c/van.png'); +}); + test('company transaction currency prefers organization currency and falls back to usd', function () { $company = new Company(); $company->setRawAttributes([