Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 51 additions & 4 deletions server/src/Http/Controllers/Api/v1/ManifestController.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Fleetbase\FleetOps\Models\ManifestStop;
use Fleetbase\FleetOps\Support\Utils;
use Fleetbase\Http\Controllers\Controller;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Http\Request;

/**
Expand Down Expand Up @@ -189,24 +190,70 @@ public function optimize(Request $request, string $id)
/** The driver's manifests, newest first. Separated so it can be stubbed. */
protected static function manifestsFor(Driver $driver)
{
return Manifest::where('driver_uuid', $driver->uuid)
return static::scopedToCompany(Manifest::where('driver_uuid', $driver->uuid))
->with(['driver', 'vehicle'])
->orderBy('scheduled_date', 'desc');
}

protected static function findDriverRecord(string $id): ?Driver
{
return Driver::where('public_id', $id)->orWhere('uuid', $id)->first();
/** @var Driver|null $driver */
$driver = static::scopedToCompany(static::matchingIdentifier(Driver::query(), $id))->first();

return $driver;
}

protected static function findManifest(string $id): ?Manifest
{
return Manifest::where('public_id', $id)->orWhere('uuid', $id)->first();
/** @var Manifest|null $manifest */
$manifest = static::scopedToCompany(static::matchingIdentifier(Manifest::query(), $id))->first();

return $manifest;
}

/**
* A stop carries no company of its own, so it belongs to the caller only
* through its manifest.
*/
protected static function findStop(string $id): ?ManifestStop
{
return ManifestStop::where('public_id', $id)->orWhere('uuid', $id)->first();
/** @var ManifestStop|null $stop */
$stop = static::matchingIdentifier(ManifestStop::query(), $id)
->whereHas('manifest', fn (Builder $query) => static::scopedToCompany($query))
->first();

return $stop;
}

/**
* Match a public_id or uuid, grouped so a constraint added afterwards
* applies to both arms — ungrouped it would read as `public_id = ? OR
* (uuid = ? AND company_uuid = ?)` and still resolve another
* organization's records.
*/
protected static function matchingIdentifier(Builder $query, string $id): Builder
{
return $query->where(fn (Builder $query) => $query->where('public_id', $id)->orWhere('uuid', $id));
}

/**
* Constrain a query to the caller's company.
*
* Nothing upstream does this: there is no global company scope, and the API
* middleware authenticates the caller without narrowing which records an
* identifier may resolve to. A missing company session fails closed instead
* of letting the query run across every tenant.
*/
protected static function scopedToCompany(Builder $query): Builder
{
$companyUuid = session('company');
if (!$companyUuid) {
$query->whereRaw('1 = 0');

return $query;
}

return $query->where($query->getModel()->qualifyColumn('company_uuid'), $companyUuid);
}

/**
Expand Down
31 changes: 29 additions & 2 deletions server/src/Http/Controllers/Internal/v1/ManifestController.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Fleetbase\FleetOps\Models\ManifestStop;
use Fleetbase\FleetOps\Traits\AuthorizesMethods;
use Fleetbase\Http\Controllers\Controller;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;

Expand Down Expand Up @@ -158,12 +159,38 @@ protected function manifestQueryForCompany(?string $companyUuid)

protected function manifestQueryByPublicId(string $id)
{
return Manifest::where('public_id', $id);
return $this->scopedToCompany(Manifest::where('public_id', $id));
}

/**
* A stop carries no company of its own, so it belongs to the caller only
* through its manifest.
*/
protected function manifestStopQueryByPublicId(string $id)
{
return ManifestStop::where('public_id', $id);
return ManifestStop::where('public_id', $id)
->whereHas('manifest', fn (Builder $query) => $this->scopedToCompany($query));
}

/**
* Constrain a query to the caller's company.
*
* The record arrives as a route identifier, and nothing upstream narrows
* which tenant it may resolve to: there is no global company scope, and
* `fleetbase.protected` only checks that the caller holds the named
* capability. A missing company session fails closed instead of letting
* the query run across every tenant.
*/
protected function scopedToCompany(Builder $query): Builder
{
$companyUuid = session('company');
if (!$companyUuid) {
$query->whereRaw('1 = 0');

return $query;
}

return $query->where($query->getModel()->qualifyColumn('company_uuid'), $companyUuid);
}

protected function manifestShowRelations(): array
Expand Down
113 changes: 91 additions & 22 deletions server/tests/ApiManifestControllerContractsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -570,30 +570,99 @@ public static function callManifestsFor(Driver $driver)
}
}

test('manifest controller lookups go to the database', function () {
/**
* An in-memory database holding two companies' manifests, stops and drivers,
* so the lookups are asserted against real queries rather than a stub.
*/
function fleetopsManifestTenantDatabase(): Illuminate\Database\SQLiteConnection
{
$connection = new Illuminate\Database\SQLiteConnection(new PDO('sqlite::memory:'), ':memory:', '', ['driver' => 'sqlite']);
$resolver = new Illuminate\Database\ConnectionResolver(['default' => $connection, 'mysql' => $connection]);
$resolver->setDefaultConnection('mysql');
Illuminate\Database\Eloquent\Model::setConnectionResolver($resolver);

$schema = $connection->getSchemaBuilder();
$tables = [
'manifests' => ['uuid', 'public_id', 'company_uuid', 'driver_uuid', 'vehicle_uuid', 'status', 'scheduled_date'],
'manifest_stops' => ['uuid', 'public_id', 'manifest_uuid', 'status', 'sequence'],
'drivers' => ['uuid', 'public_id', 'company_uuid', 'user_uuid'],
'users' => ['uuid', 'public_id', 'company_uuid', 'name'],
];
foreach ($tables as $table => $columns) {
$schema->create($table, function ($blueprint) use ($columns) {
$blueprint->increments('id');
foreach ($columns as $column) {
$blueprint->string($column)->nullable();
}
$blueprint->timestamps();
$blueprint->timestamp('deleted_at')->nullable();
});
}

foreach (['a', 'b'] as $tenant) {
$connection->table('users')->insert(['uuid' => "user-{$tenant}", 'company_uuid' => "company-{$tenant}", 'name' => "Driver {$tenant}"]);
$connection->table('drivers')->insert(['uuid' => "driver-{$tenant}-uuid", 'public_id' => "driver_{$tenant}", 'company_uuid' => "company-{$tenant}", 'user_uuid' => "user-{$tenant}"]);
$connection->table('manifests')->insert(['uuid' => "manifest-{$tenant}-uuid", 'public_id' => "manifest_{$tenant}", 'company_uuid' => "company-{$tenant}", 'driver_uuid' => "driver-{$tenant}-uuid", 'status' => 'pending']);
$connection->table('manifest_stops')->insert(['uuid' => "stop-{$tenant}-uuid", 'public_id' => "stop_{$tenant}", 'manifest_uuid' => "manifest-{$tenant}-uuid", 'status' => 'pending', 'sequence' => '1']);
}

return $connection;
}

test('manifest controller lookups resolve the caller\'s own records by public id or uuid', function () {
fleetopsManifestTenantDatabase();
session(['company' => 'company-a']);

$driver = FleetOpsManifestRealLookupProbe::callFindDriverRecord('driver_a');

expect(FleetOpsManifestRealLookupProbe::callFindManifest('manifest_a')?->uuid)->toBe('manifest-a-uuid')
->and(FleetOpsManifestRealLookupProbe::callFindManifest('manifest-a-uuid')?->public_id)->toBe('manifest_a')
->and(FleetOpsManifestRealLookupProbe::callFindStop('stop_a')?->uuid)->toBe('stop-a-uuid')
->and(FleetOpsManifestRealLookupProbe::callFindStop('stop-a-uuid')?->public_id)->toBe('stop_a')
->and($driver?->uuid)->toBe('driver-a-uuid')
->and(FleetOpsManifestRealLookupProbe::callFindDriverRecord('driver-a-uuid')?->public_id)->toBe('driver_a')
->and(FleetOpsManifestRealLookupProbe::callManifestsFor($driver)->pluck('uuid')->all())->toBe(['manifest-a-uuid']);
});

test('manifest controller lookups never resolve another organization\'s records', function () {
/*
* Exercises the real lookups, which the probe above replaces. Whether the
* environment has a database or not, the call must reach it: a lookup that
* quietly returned null on a connection failure would make a broken
* database indistinguishable from a missing record, and both would answer
* 404. Either outcome is accepted here — what is asserted is that the
* lookup is a query and not a stub.
* GHSA-m8m5-5qhf-94rv: every lookup used to be a bare `public_id OR uuid`
* match, so company A could read company B's manifests and write to its
* stops. Both identifier forms are tried, because an ungrouped OR would
* let one of them slip past the company constraint.
*/
$driver = new Driver();
$driver->setRawAttributes(['uuid' => 'driver-uuid', 'public_id' => 'driver_a'], true);

$reached = function (callable $lookup): bool {
try {
$lookup();
} catch (Throwable $e) {
return true;
}
fleetopsManifestTenantDatabase();
session(['company' => 'company-a']);

$foreignDriver = new Driver();
$foreignDriver->setRawAttributes(['uuid' => 'driver-b-uuid', 'public_id' => 'driver_b'], true);

expect(FleetOpsManifestRealLookupProbe::callFindManifest('manifest_b'))->toBeNull()
->and(FleetOpsManifestRealLookupProbe::callFindManifest('manifest-b-uuid'))->toBeNull()
->and(FleetOpsManifestRealLookupProbe::callFindStop('stop_b'))->toBeNull()
->and(FleetOpsManifestRealLookupProbe::callFindStop('stop-b-uuid'))->toBeNull()
->and(FleetOpsManifestRealLookupProbe::callFindDriverRecord('driver_b'))->toBeNull()
->and(FleetOpsManifestRealLookupProbe::callFindDriverRecord('driver-b-uuid'))->toBeNull()
->and(FleetOpsManifestRealLookupProbe::callManifestsFor($foreignDriver)->get())->toBeEmpty();
});

return true;
};
test('manifest controller refuses another organization\'s manifest and stop at the endpoints', function () {
fleetopsManifestTenantDatabase();
session(['company' => 'company-a']);

$controller = new ManifestController();

expect($controller->show('manifest_b')->getStatusCode())->toBe(404)
->and($controller->optimize(new Request(), 'manifest_b')->getStatusCode())->toBe(404)
->and($controller->forDriver(new Request(), 'driver_b')->getStatusCode())->toBe(404)
->and($controller->updateStop(new Request(['status' => 'completed', 'meta' => ['tampered' => true]]), 'stop_b')->getStatusCode())->toBe(404);
});

test('manifest controller lookups resolve nothing without a company session', function () {
fleetopsManifestTenantDatabase();
session(['company' => null]);

expect($reached(fn () => FleetOpsManifestRealLookupProbe::callFindManifest('manifest_a')))->toBeTrue()
->and($reached(fn () => FleetOpsManifestRealLookupProbe::callFindStop('stop_a')))->toBeTrue()
->and($reached(fn () => FleetOpsManifestRealLookupProbe::callFindDriverRecord('driver_a')))->toBeTrue()
->and($reached(fn () => FleetOpsManifestRealLookupProbe::callManifestsFor($driver)))->toBeTrue();
expect(FleetOpsManifestRealLookupProbe::callFindManifest('manifest_a'))->toBeNull()
->and(FleetOpsManifestRealLookupProbe::callFindStop('stop_a'))->toBeNull()
->and(FleetOpsManifestRealLookupProbe::callFindDriverRecord('driver_a'))->toBeNull();
});
71 changes: 66 additions & 5 deletions server/tests/ManifestControllerContractsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,13 @@ function fleetopsManifestStopFake(): FleetOpsManifestStopFake
->and($controller->stopQuery->record->freshes[0])->toBe(['place', 'order.trackingNumber']);
});

test('manifest real query helpers and stop endpoints hit the database', function () {
/**
* An in-memory database for the controller's real queries. Rows for a second
* company are seeded alongside the caller's so every lookup is also asserted
* not to cross between them.
*/
function fleetopsManifestDatabase(): Illuminate\Database\SQLiteConnection
{
$connection = new Illuminate\Database\SQLiteConnection(new PDO('sqlite::memory:'));
$resolver = new Illuminate\Database\ConnectionResolver(['default' => $connection, 'mysql' => $connection]);
$resolver->setDefaultConnection('mysql');
Expand All @@ -252,7 +258,7 @@ function fleetopsManifestStopFake(): FleetOpsManifestStopFake
$schema = $connection->getSchemaBuilder();
$tables = [
'manifests' => ['uuid', 'public_id', 'company_uuid', 'driver_uuid', 'vehicle_uuid', 'status', 'name', 'date', 'meta', '_key'],
'manifest_stops' => ['uuid', 'public_id', 'company_uuid', 'manifest_uuid', 'place_uuid', 'order_uuid', 'waypoint_uuid', 'status', 'sequence', 'notes', 'actual_arrival', 'meta', '_key'],
'manifest_stops' => ['uuid', 'public_id', 'manifest_uuid', 'place_uuid', 'order_uuid', 'waypoint_uuid', 'status', 'sequence', 'notes', 'actual_arrival', 'meta', '_key'],
'places' => ['uuid', 'public_id', 'company_uuid', 'name', 'location', '_key'],
'orders' => ['uuid', 'public_id', 'company_uuid', 'payload_uuid', 'tracking_number_uuid', 'status', 'meta', '_key'],
'payloads' => ['uuid', 'public_id', 'company_uuid', 'pickup_uuid', 'dropoff_uuid', 'meta', '_key'],
Expand All @@ -273,7 +279,6 @@ function fleetopsManifestStopFake(): FleetOpsManifestStopFake
});
}

session(['company' => 'company-mfr-1']);
app()->instance('responsecache', new class {
public function __call($method, $arguments)
{
Expand All @@ -284,9 +289,19 @@ public function __call($method, $arguments)
config()->set('activitylog.default_auth_driver', 'web');
app()->bind(Illuminate\Contracts\Config\Repository::class, fn () => config());
$connection->table('manifests')->insert(['uuid' => 'manifest-real-1', 'public_id' => 'manifest_realone', 'company_uuid' => 'company-mfr-1', 'status' => 'active']);
$connection->table('manifest_stops')->insert(['uuid' => 'stop-real-1', 'public_id' => 'manifest_stop_realone', 'company_uuid' => 'company-mfr-1', 'manifest_uuid' => 'manifest-real-1', 'place_uuid' => 'place-mfr-1', 'status' => 'pending', 'sequence' => '1', 'meta' => json_encode([])]);
$connection->table('manifest_stops')->insert(['uuid' => 'stop-real-1', 'public_id' => 'manifest_stop_realone', 'manifest_uuid' => 'manifest-real-1', 'place_uuid' => 'place-mfr-1', 'status' => 'pending', 'sequence' => '1', 'meta' => json_encode([])]);
$connection->table('places')->insert(['uuid' => 'place-mfr-1', 'company_uuid' => 'company-mfr-1', 'name' => 'Manifest Stop Place']);

$connection->table('manifests')->insert(['uuid' => 'manifest-foreign-1', 'public_id' => 'manifest_foreign', 'company_uuid' => 'company-mfr-2', 'status' => 'active']);
$connection->table('manifest_stops')->insert(['uuid' => 'stop-foreign-1', 'public_id' => 'manifest_stop_foreign', 'manifest_uuid' => 'manifest-foreign-1', 'status' => 'pending', 'sequence' => '1']);

return $connection;
}

test('manifest real query helpers and stop endpoints hit the database', function () {
$connection = fleetopsManifestDatabase();
session(['company' => 'company-mfr-1']);

$controller = new ManifestController();
$helper = function (string $method, ...$arguments) use ($controller) {
$reflection = new ReflectionMethod(ManifestController::class, $method);
Expand All @@ -306,5 +321,51 @@ public function __call($method, $arguments)

// Sequence-only updates persist without status transitions
$updated = $controller->updateStop(new Request(['sequence' => 7]), 'manifest_stop_realone')->getData(true);
expect((string) $connection->table('manifest_stops')->value('sequence'))->toBe('7');
expect((string) $connection->table('manifest_stops')->where('uuid', 'stop-real-1')->value('sequence'))->toBe('7');
});

test('manifest endpoints never resolve another organization\'s manifest or stop', function () {
/*
* GHSA-q479-qp9q-mg27: the single-record handlers matched on public_id
* alone, so any organization could read, cancel, delete and rewrite
* another's manifests. Stops have no company of their own and are owned
* through their manifest.
*/
$connection = fleetopsManifestDatabase();
session(['company' => 'company-mfr-1']);

$controller = new ManifestController();
$refused = function (callable $call): bool {
try {
$call();
} catch (Illuminate\Database\Eloquent\ModelNotFoundException $e) {
return true;
}

return false;
};

expect($refused(fn () => $controller->show('manifest_foreign')))->toBeTrue()
->and($refused(fn () => $controller->cancel('manifest_foreign')))->toBeTrue()
->and($refused(fn () => $controller->destroy('manifest_foreign')))->toBeTrue()
->and($refused(fn () => $controller->showStop('manifest_stop_foreign')))->toBeTrue()
->and($refused(fn () => $controller->updateStop(new Request(['status' => 'completed', 'sequence' => 9]), 'manifest_stop_foreign')))->toBeTrue()
->and($connection->table('manifests')->where('uuid', 'manifest-foreign-1')->first())->toMatchArray(['status' => 'active', 'deleted_at' => null])
->and($connection->table('manifest_stops')->where('uuid', 'stop-foreign-1')->first())->toMatchArray(['status' => 'pending', 'sequence' => '1']);
});

test('manifest lookups resolve nothing without a company session', function () {
fleetopsManifestDatabase();
session(['company' => null]);

$controller = new ManifestController();
$helper = function (string $method, ...$arguments) use ($controller) {
$reflection = new ReflectionMethod(ManifestController::class, $method);
$reflection->setAccessible(true);

return $reflection->invoke($controller, ...$arguments);
};

expect($helper('manifestQueryByPublicId', 'manifest_realone')->count())->toBe(0)
->and($helper('manifestStopQueryByPublicId', 'manifest_stop_realone')->count())->toBe(0);
});
Loading