From add5cbbe55cfc83c7f9e2b4f88d62d8d2b6f2778 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Tue, 29 Sep 2026 18:37:49 +0800 Subject: [PATCH 1/2] fix(security): generate API keys from the CSPRNG instead of the creation time API keys were sqids(digits of the creation timestamp + row id). ApiCredential uses the uuid as its primary key with incrementing disabled, so the auto-increment id is never read back on insert and the observer always saw null: every key created in the same second, on any organization, was identical. AuthenticateOnceWithBasicAuth resolves a key with first(), so a holder of one organization's key could authenticate as another's. Even with the id, a key built from a timestamp and a sequential id is guessable. generateKeys() now returns 32 random alphanumeric characters (Str::random, backed by random_bytes). Its argument is ignored and optional; the observer and the roll endpoint no longer build a seed. Found when the k6 release benchmark minted three keys within a second and two came out identical. --- .../Internal/v1/ApiCredentialController.php | 7 ++--- src/Models/ApiCredential.php | 17 ++++++++--- src/Observers/ApiCredentialObserver.php | 5 ++-- tests/Unit/Models/ApiAndWebhookModelsTest.php | 29 +++++++++++++++++++ 4 files changed, 46 insertions(+), 12 deletions(-) diff --git a/src/Http/Controllers/Internal/v1/ApiCredentialController.php b/src/Http/Controllers/Internal/v1/ApiCredentialController.php index 9e9c64b4..4153c412 100644 --- a/src/Http/Controllers/Internal/v1/ApiCredentialController.php +++ b/src/Http/Controllers/Internal/v1/ApiCredentialController.php @@ -196,11 +196,8 @@ public static function roll($id, Request $request) return response()->error('API credential attempted to roll could not be found.'); } - // create api credentials seed - $seed = array_map('intval', str_split(time() . $apiCredential->id)); - - // regenerate api key - $newCredentials = ApiCredential::generateKeys($seed, $apiCredential->test_mode); + // regenerate api key (random; see ApiCredential::generateKeys) + $newCredentials = ApiCredential::generateKeys(null, $apiCredential->test_mode); // store the previous key $previousApiKey = $apiCredential->key; diff --git a/src/Models/ApiCredential.php b/src/Models/ApiCredential.php index e717f043..03e0b67e 100644 --- a/src/Models/ApiCredential.php +++ b/src/Models/ApiCredential.php @@ -12,6 +12,7 @@ use Fleetbase\Traits\Searchable; use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Hash; +use Illuminate\Support\Str; use Spatie\Activitylog\LogOptions; use Spatie\Activitylog\Traits\LogsActivity; use Spatie\Permission\Traits\HasPermissions; @@ -159,13 +160,21 @@ public function setExpiresAtAttribute($expiresAt) /** * Generate an API Key. * + * The key is 32 random alphanumeric characters from the CSPRNG (~190 bits). It used to be + * sqids($encode), where callers passed the digits of the creation time and row id. The + * model's primary key is the uuid and `id` is never read back on insert, so the observer + * saw a null id and every key created in the same second was identical: the lookup in + * AuthenticateOnceWithBasicAuth then resolved one organization's key to another's + * credential. Keys derived from a timestamp and a sequential id are guessable anyway. + * + * @param mixed $encode ignored; kept so existing callers do not break + * * @return array */ - public static function generateKeys($encode, $testKey = false) + public static function generateKeys($encode = null, $testKey = false) { - $sqids = new \Sqids\Sqids(); - $key = $sqids->encode($encode); - $hash = Hash::make($key); + $key = Str::random(32); + $hash = Hash::make($key); return [ 'key' => ($testKey ? 'flb_test_' : 'flb_live_') . $key, diff --git a/src/Observers/ApiCredentialObserver.php b/src/Observers/ApiCredentialObserver.php index 2449ce3e..ffc904d2 100644 --- a/src/Observers/ApiCredentialObserver.php +++ b/src/Observers/ApiCredentialObserver.php @@ -13,9 +13,8 @@ class ApiCredentialObserver */ public function created(ApiCredential $apiCredential) { - // generate the api credentials - $seed = array_map('intval', str_split(strtotime($apiCredential->created_at) . $apiCredential->id)); - $credentials = ApiCredential::generateKeys($seed, $apiCredential->test_mode); + // generate the api credentials (random; see ApiCredential::generateKeys) + $credentials = ApiCredential::generateKeys(null, $apiCredential->test_mode); // set the credentials $apiCredential->key = data_get($credentials, 'key'); diff --git a/tests/Unit/Models/ApiAndWebhookModelsTest.php b/tests/Unit/Models/ApiAndWebhookModelsTest.php index da262f51..a2df3d25 100644 --- a/tests/Unit/Models/ApiAndWebhookModelsTest.php +++ b/tests/Unit/Models/ApiAndWebhookModelsTest.php @@ -110,6 +110,35 @@ public function save(array $options = []): bool ->and($testKeys['secret'])->toBe('hashed:' . substr($testKeys['key'], strlen('flb_test_'))); }); +it('api credential keys are random rather than derived from their input', function () { + bind_test_container()->instance('hash', new ApiAndWebhookModelsHashFake()); + + // The old keys were sqids(creation second + id); the same input gave the same key. + $first = ApiCredential::generateKeys([1, 7, 9, 0, 0, 0, 0, 0, 0, 0], false)['key']; + $second = ApiCredential::generateKeys([1, 7, 9, 0, 0, 0, 0, 0, 0, 0], false)['key']; + + expect($first)->not->toBe($second) + ->and($first)->toMatch('/^flb_live_[A-Za-z0-9]{32}$/') + ->and(ApiCredential::generateKeys()['key'])->toMatch('/^flb_live_[A-Za-z0-9]{32}$/'); +}); + +it('api credential observer gives credentials created in the same second different keys', function () { + $container = bind_test_container(); + $container->instance('hash', new ApiAndWebhookModelsHashFake()); + + // The uuid primary key means `id` is never loaded on insert, so the observer sees null. + $keys = []; + foreach ([1, 2] as $n) { + $credential = new ApiCredentialObserverSaveSpy(); + $credential->setDateFormat('Y-m-d H:i:s'); + $credential->setRawAttributes(['id' => null, 'created_at' => '2026-07-17 12:34:56', 'test_mode' => false], true); + (new ApiCredentialObserver())->created($credential); + $keys[] = $credential->key; + } + + expect($keys[0])->not->toBe($keys[1]); +}); + it('api credential observer writes generated keys and persists live and test credentials', function (bool $testMode, string $expectedPrefix) { $container = bind_test_container(); $container->instance('hash', new ApiAndWebhookModelsHashFake()); From faa0982c395bc423ba83bef58de45e2ea18cb5ed Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Tue, 29 Sep 2026 18:51:14 +0800 Subject: [PATCH 2/2] release: v1.6.67 --- RELEASE.md | 17 ++++++++++------- composer.json | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 87bbbdc8..79a30cec 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -1,11 +1,14 @@ -# v1.6.66 — Per-consumer API rate limiting, admin rate-limit controls and API consumer metrics +# v1.6.67 — API keys are generated randomly -## Fixes +## Security -- One API consumer can no longer rate-limit the whole platform. `ThrottleRequests` ran before API authentication, so Laravel keyed every bucket on the client IP. Behind a load balancer that is the balancer's IP, so every tenant, API key and console visitor shared a single 120/min bucket, and one busy integration returned 429 to everyone. The limiter now keys on the presented credential (API key, Sanctum token or basic auth), falling back to the user and then the IP only when none is sent. The first path segment keeps `/v1` and `/int` in separate buckets. (#280) -- 429 responses keep `Retry-After` and `X-RateLimit-*`. The exception handler used to drop them, so throttled clients could not back off correctly. (#280) +- **API keys created in the same second were identical, across organizations.** A key was derived from its creation time and row id, but the id is never loaded after insert (the primary key is the uuid), so every key created in the same second got the same value. API authentication resolves a key to the first matching credential, so a key issued to one organization could authenticate as another's. Keys are now 32 random characters from the CSPRNG, for new keys and for rolled keys. (#283) -## Improvements +## Upgrade Steps -- System admins can manage API rate limits at runtime. The limits (`THROTTLE_*` environment defaults) can be overridden from the console and stored as the `system.rate-limits` setting: enable/disable, requests per window, and window length. **Per-organization overrides** give an organization a custom limit or none. New admin-only endpoints `GET/POST/DELETE int/v1/rate-limits/settings`. (#281) -- API consumer metrics. The throttle middleware counts every request and every 429 per consumer in Redis: minute buckets are kept for 2 hours, hour buckets for 8 days, at one pipelined round trip per request. `GET int/v1/rate-limits/consumers?window=&sort=&limit=` lists the busiest or most throttled consumers with their organization, masked key, scope, IP, avg and peak per minute, and share. `POST int/v1/rate-limits/consumers/{signature}/reset` clears one consumer's window. Tracking can be disabled with `THROTTLE_TRACK_CONSUMERS=false`; `THROTTLE_METRICS_REDIS_CONNECTION` picks the Redis connection (default `cache`). (#281) +- Check for existing duplicate keys and roll every credential that shares one, in both the live and sandbox databases: + ```sql + SELECT `key`, COUNT(*) AS credentials, COUNT(DISTINCT company_uuid) AS orgs + FROM api_credentials WHERE deleted_at IS NULL + GROUP BY `key` HAVING COUNT(*) > 1; + ``` diff --git a/composer.json b/composer.json index 2cf39cdb..af7478ac 100644 --- a/composer.json +++ b/composer.json @@ -1,6 +1,6 @@ { "name": "fleetbase/core-api", - "version": "1.6.66", + "version": "1.6.67", "description": "Core Framework and Resources for Fleetbase API", "keywords": [ "fleetbase",