Skip to content

fix(security): generate API keys from the CSPRNG instead of the creation time - #283

Merged
roncodes merged 1 commit into
release/v1.6.67from
fix/unique-api-keys
Sep 29, 2026
Merged

roncodes merged 1 commit into
release/v1.6.67from
fix/unique-api-keys

Conversation

@roncodes

Copy link
Copy Markdown
Member

Problem

API keys were sqids(digits of the creation timestamp + row id). ApiCredential uses the uuid as its primary key with $incrementing = false, so the auto-increment id is never read back on insert and ApiCredentialObserver always saw null. Every API key created in the same second, on any organization, was identical. AuthenticateOnceWithBasicAuth resolves a key with first(), so one organization's key could authenticate as another's credential. Sandbox (flb_test_) keys have the same flaw. Even with the id, a key derived from a timestamp and a sequential id is guessable.

Reproduced on the dev stack: two credentials created in the same second got the same key, and the observer saw id = NULL. Found when the k6 release benchmark minted three keys within a second and two came out identical (fleetbase/fleetbase performance run for v0.7.66).

Fix

  • ApiCredential::generateKeys() returns 32 random alphanumeric characters (Str::random, backed by random_bytes, about 190 bits). Its argument is now optional and ignored, so existing callers don't break.
  • ApiCredentialObserver::created() and ApiCredentialController::roll() no longer build a timestamp seed.
  • The key format (flb_live_… / flb_test_…) and the hashed secret are unchanged.

Tests

  • New: the same input no longer yields the same key; the key matches ^flb_live_[A-Za-z0-9]{32}$; the observer gives credentials created in the same second, with a null id, different keys.
  • Full suite: 1922 passed. test:lint and test:date-drift are clean.
  • On the dev stack after the fix: same-second credentials get different keys.

After merging

Existing duplicates stay until they are rolled. Check both the live and sandbox databases:

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;

…ion 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.
@roncodes roncodes mentioned this pull request Sep 29, 2026
@roncodes
roncodes merged commit 3eaf512 into release/v1.6.67 Sep 29, 2026
3 checks passed
@roncodes
roncodes deleted the fix/unique-api-keys branch September 29, 2026 10:57
@codecov

codecov Bot commented Sep 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (74cc07c) to head (add5cbb).
⚠️ Report is 3 commits behind head on release/v1.6.67.

Additional details and impacted files
@@                 Coverage Diff                 @@
##             release/v1.6.67      #283   +/-   ##
===================================================
  Coverage             100.00%   100.00%           
  Complexity              7772      7772           
===================================================
  Files                    436       436           
  Lines                  25337     25334    -3     
===================================================
- Hits                   25337     25334    -3     
Flag Coverage Δ
backend 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant