From add5cbbe55cfc83c7f9e2b4f88d62d8d2b6f2778 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Tue, 29 Sep 2026 18:37:49 +0800 Subject: [PATCH] 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());