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
41 changes: 30 additions & 11 deletions RELEASE.md
Original file line number Diff line number Diff line change
@@ -1,29 +1,48 @@
> v1.6.59 ~ "Verification and credentials email can render again"
> v1.6.60 ~ "Revoked keys stay revoked, transactions stop colliding, webhooks stop signing with the wrong secret"

---
## Highlights
Both mail templates failed to compile, so **every verification and credentials email threw instead of sending**. Any flow that delivers a code — customer signup, SMS login with email fallback, password reset, account closure, driver login — returned a 400 carrying a Blade parse error.
Three independent defects, each of which let the platform keep doing something an operator had already told it to stop.

Shipped in v1.6.56 and present in v1.6.57 and v1.6.58. Upgrade if you are on any of those.
**Deleting an API credential did not revoke it.** The console hid the row and the key kept authenticating — indefinitely, on every endpoint. Combined with an "expire immediately" option that also did not take effect, a stock Fleetbase console had **no working way to revoke an API credential**. Anyone who has ever deleted a key in the console should read the upgrade steps.

**Persistent PDO handles shared one MySQL transaction.** Writes landed and the API still answered `422 There is no active transaction`, so anyone who retried after the error applied the write twice.

**A queue worker signed every lifecycle webhook with the first event's secret.** Every outbound webhook after the first carried the wrong HMAC key, along with the wrong credential, environment and company.

---
## Security Fixes
- **Soft-deleted API credentials still authenticated.** `AuthenticateOnceWithBasicAuth` looks the credential up with `withoutGlobalScopes()`, which strips `SoftDeletingScope` along with `ExpiryScope`. Expiry was re-applied in PHP; soft-deletion never was. A credential the console reports as **Deleted** kept authenticating indefinitely — and Delete is the only revocation most operators ever perform. Now rejected with a 401, before the `OPTIONS` shortcut so a revoked key cannot seed api key session context on a preflight either.

- **Authentication was fail-open when the credential's creator was gone.** An API credential carries no identity of its own; it acts as the user that created it. When that user no longer resolved — deleted, or soft-deleted on off-boarding — the `is_admin` guard was skipped but `Auth::setSession()` still returned `true`. Authorization degraded safely, since a null user fails every group and admin check, but authentication did not: the key kept working on every read endpoint and every ungated write. Off-boarding a person did not revoke the keys they had created. `setSession()` now returns `false` in that case and the middleware answers a clean 401.

---
## Bug Fixes
- **`verification.blade.php` and `user-credentials.blade.php` did not parse.** The greeting read `Good Morning@if($user->name), ...@endif`, and Blade only treats `@` as a directive when the preceding character is **not** a word character — the rule that keeps `foo@bar.com` from compiling. So the `@if` was left as literal text while its `@endif` compiled anyway, leaving an unmatched `endif` that broke the enclosing `if/elseif/else`:
- **"Expire immediately" did not expire the credential.** `ApiCredential::setExpiresAtAttribute()` maps the console's `immediately` option to `Carbon::now()`, but `Expirable::hasExpired()` used a strict `<`, so `now() < now()` was false. `ExpiryScope` already disagreed with it — it keeps a row only while `expires_at > now()`, i.e. it treats an exactly-now expiry as expired. `hasExpired()` is now inclusive, so the trait and the scope agree.

```
syntax error, unexpected token "else", expecting end of file
(View: .../core-api/views/mail/verification.blade.php)
```
- **Persistent PDO handles shared one MySQL transaction.** `Connection::commit()` decides whether to issue a COMMIT from its own counter; PDO decides whether one is legal from the server's `SERVER_STATUS_IN_TRANS` flag, and nothing reconciled the two. With `PDO::ATTR_PERSISTENT => true`, PHP hands the same MySQL session to a second handle built from the same DSN while the first is still using it, so one handle could commit — and thereby invalidate — the other's transaction. Observed on onboarding account creation, ledger invoice creation and inventory stock adjustments, none of which share a code path. Persistence is now `env('DB_PERSISTENT', false)` on the `mysql` and `sandbox` connections.

The greeting is now built in one expression, so no directive sits against a word.
- **A queue worker signed lifecycle webhooks with a stale secret.** `SendResourceLifecycleWebhook::setSessionFromEvent()` only wrote a session key when it was absent, and `handle()` preferred the session value over the event's. A `queue:work` process is long-running and its session store is a container singleton, so once the worker handled one lifecycle event, every later event reused that first event's `api_secret` — plus its `api_credential`, `api_key`, `api_environment`, `is_sandbox`, `company` and `user`. The context serialized onto the event is now authoritative for the job that carries it, and a restorer running in a `finally` hands the session back as it was found. Reported in #244.

---
## Testing
- Added a test that compiles **every** Blade view in the package and runs `php -l` over the result. Nothing caught the original break because no test ever compiled a view — the templates were only exercised through mocked mailers, which never render them.
- New coverage for revoked credentials on both normal and `OPTIONS` requests, for a credential whose creator has been soft-deleted, for `Auth::setSession()` returning `false`, and for the exactly-now expiry boundary.
- Four tests for the webhook secret bleed, each verified to fail against the unpatched listener, covering per-event signing, sandbox/live isolation, session restoration, and event-over-session credential attribution.
- The middleware fixture previously seeded the sandbox user only on the sandbox connection. `User` is pinned to the `mysql` connection and `sandbox:sync` mirrors `mysql → sandbox`, so the fixture now matches the real system.

---
## Upgrade Steps
No migration and no configuration change. If verification emails were failing, they will work again once this is deployed; no codes need reissuing.
**Audit your API credentials.** Any credential deleted from the console before this release was never actually revoked and has been live the whole time. After upgrading, those keys stop working — which is the point, but it means an integration quietly running on a key someone believed they had deleted will break at upgrade rather than at deletion. List your credentials including soft-deleted rows before deploying if you want to know what will change.

**Keys whose creator has been off-boarded will start returning 401.** That is the intent of the fix, but it is a behaviour change for anyone whose integrations run on a departed employee's credential. Reassign those to a dedicated service user before upgrading.

**Deployments not running Octane may see per-request connect cost.** `PDO::ATTR_PERSISTENT` now defaults to off. Under Octane connection counts are unchanged (measured at 17 on a 24-thread FrankenPHP container). Short-lived PHP-FPM workers that relied on the pool will pay roughly 1–3 ms per request; `DB_PERSISTENT=true` restores the old behaviour, at the cost of re-arming the transaction bug for any request that opens a transaction. Note that persistent connections also silently defeated Octane's `DisconnectFromDatabases` listener, which now works as intended.

No migration and no configuration change is required.

---
## Still Open
API credentials still carry no scope of their own — `Auth::setSession()` derives `is_admin` from whoever created the key, so every key an admin creates is a full-admin key regardless of what it is named. Least privilege is currently only reachable indirectly, by scoping the *creator* into a dedicated service user. Per-key roles, or an explicit key assignee decoupled from the creator, is a feature rather than a fix and is tracked separately.

---
## Need help?
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "fleetbase/core-api",
"version": "1.6.59",
"version": "1.6.60",
"description": "Core Framework and Resources for Fleetbase API",
"keywords": [
"fleetbase",
Expand Down
14 changes: 13 additions & 1 deletion config/database.connections.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,19 @@
}

$mysql_options = [
PDO::ATTR_PERSISTENT => true,
// Persistent connections keep the MySQL session alive in PHP's persistent pool
// after the PDO object is gone, and hand that same session to the next PDO
// built from the same DSN/user/password - including one built while another
// handle is still using it. Two handles then share one transaction: a COMMIT
// through either ends it for both, so the other's commit() raises
// "There is no active transaction" for a write that already committed, and the
// caller reports failure for data that landed. Laravel cannot detect this,
// because commit() gates on its own $transactions counter rather than on
// PDO::inTransaction() (Illuminate\Database\Concerns\ManagesTransactions).
//
// Off by default. Set DB_PERSISTENT=true only where the reconnect cost is
// measured and no request opens a transaction.
PDO::ATTR_PERSISTENT => env('DB_PERSISTENT', false),
PDO::ATTR_TIMEOUT => 5,
];

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration {
/**
* Add the running-statistics columns Report::updateExecutionStats writes.
*
* The report builder's execution statistics were written but never migrated.
* `Report::updateExecutionStats()` sets `execution_count`, `average_execution_time`
* and `last_result_count` on every run, `getPerformanceMetrics()` returns all three,
* and `cloneWithConfig()` resets them — but the reports table has none of them, so
* the save at the end of updateExecutionStats fails:
*
* SQLSTATE[42S22]: Column not found: 1054
* Unknown column 'execution_count' in 'field list'
*
* The effect is that **every saved report fails to execute**, in every extension.
* The query itself succeeds — the failure happens afterwards, while recording that
* it ran — so the caller sees an error for a report that actually worked, and the
* report builder's preview shows an empty result with no explanation.
*
* These are distinct from `execution_time` and `row_count`, which
* 2025_09_25_084135_report_enhancements added and the API resource exposes: those
* describe the last run, while these accumulate across runs.
*
* @return void
*/
public function up()
{
Schema::table('reports', function (Blueprint $table) {
$table->unsignedInteger('execution_count')->default(0)->after('row_count');

// A mean of integer millisecond timings, so it needs somewhere for the
// fraction to go — `execution_time` is an integer because it holds one
// measurement rather than an average of several.
$table->float('average_execution_time')->nullable()->comment('Mean execution time in milliseconds across all runs')->after('execution_count');

$table->integer('last_result_count')->nullable()->after('average_execution_time');
});
}

/**
* Reverse the migrations.
*
* @return void
*/
public function down()
{
Schema::table('reports', function (Blueprint $table) {
$table->dropColumn([
'execution_count',
'average_execution_time',
'last_result_count',
]);
});
}
};
20 changes: 19 additions & 1 deletion src/Http/Middleware/AuthenticateOnceWithBasicAuth.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,18 @@ public function authenticatedWithBasic(Request $request, $connection = null)
return response()->error('Oops! The api credentials provided were not valid', 401);
}

// Credentials have been revoked.
//
// withoutGlobalScopes() above strips SoftDeletingScope along with ExpiryScope, so
// the lookup deliberately sees deleted rows. Expiry is re-applied in PHP below, but
// soft-deletion never was — a credential the console reports as "Deleted" kept
// authenticating indefinitely, and Delete was the only revocation most operators
// ever performed. Treated as "not valid" rather than a distinct message so a caller
// cannot distinguish a revoked key from one that never existed.
if ($apiCredential->trashed()) {
return response()->error('Oops! The api credentials provided were not valid', 401);
}

// If OPTIONS set api key and continue
if ($request->isMethod('OPTIONS')) {
// Set api credential session
Expand All @@ -96,7 +108,13 @@ public function authenticatedWithBasic(Request $request, $connection = null)
}

// Login user
Auth::setSession($apiCredential);
//
// Fails when the credential's creating user no longer resolves — the credential has
// no identity to act as, so the request is rejected rather than continuing with a
// half-populated session.
if (Auth::setSession($apiCredential) !== true) {
return response()->error('Oops! The api credentials provided were not valid', 401);
}

// Bind the user resolver so $request->user() answers on the public API.
//
Expand Down
Loading
Loading