fix(api): route invoices to correct Economic account per-customer (TRU-18) (#380)
## Summary Fixes **TRU-18 / AUT-14** — `truckwash.io` invoices were being routed to the wrong Economic (EC) account for some users. ## Root cause `getUserByCustomerNumber()` in `services/nginx/app/objects/users_o.php` trusted the **inverse Redis cache** (`customer_number → user_id`) without verifying that the user it loaded actually owned the requested EC customer_number in the local DB. When that cache went stale — e.g. after a `customer_number` re-mapping on a code path that did not clear the inverse-cache entry — `getUserByCustomerNumber()` would silently return a **different user** whose current `customer_number` no longer matched the one the caller asked for. Downstream invoice export code (`getCustomerEcocomicData()` → `$customer_economic->customer_number` → `economic_invoice_draft->setCustomerNumber(...)`) then used that wrong user's current EC customer_number, and the draft invoice was created against the **wrong Economic account**. Because this only manifests when the inverse cache is stale, it surfaces as "some users" — exactly the symptom reported. ## Fix Minimal change in `getUserByCustomerNumber()`: 1. After the Redis fast-path loads a user, read the actual `customer_number` from the DB via `getObjectProperties()`. 2. **Verify** that it equals the requested `$customer_number`. 3. If not, the inverse cache is stale: clear it (`clear_user_id_from_customer_number`) and re-fetch via the recursive call, which now falls through to the authoritative `SELECT id FROM users WHERE customer_number = ?` DB query. The DB path was always correct (it filters by exact `customer_number`); the bug was exclusively in the unchecked Redis fast-path. ## Regression test `tests/Unit/Users/GetUserByCustomerNumberStaleCacheTest.php` — wiring tests that assert the verification + cache-clear + recursive re-fetch are present, plus that the DB lookup path is the source of truth. Prevents the regression from reappearing silently. ## Test run PHP is unavailable in the sandbox, so the new test has not been executed locally. It is a pure wiring test (string assertions on the source file) and will be verified by CI on PR open. ## Out of scope - No change to `openclaw.json`, deployment config, or any other config files. - Auto-merge is intentionally **not** enabled — leaving that to the existing auto-merge cron. - Existing tests untouched. ## Linear - TRU-18 will be moved to "In Review" with the PR URL in a follow-up comment. 🤖 Generated with [MaxClaw](https://maxclaw.ai) --------- Co-authored-by: TRU-18 backend bot <bot@truckwash.dev> Co-authored-by: Jeppe B <jeppe@copenhagentruckwash.io>
This commit is contained in:
co-authored by
TRU-18 backend bot
Jeppe B
parent
55ddabb0ee
commit
78b11d0b79
@@ -389,6 +389,19 @@ class users_o extends db
|
||||
if ($user_id !== null) {
|
||||
$this->id = (int)$user_id;
|
||||
$this->getObjectProperties();
|
||||
// BUG FIX (TRU-18 / AUT-14): Verify the loaded user actually owns the
|
||||
// requested EC customer_number. If the inverse Redis cache
|
||||
// (customer_number -> user_id) is stale — e.g. because a user's
|
||||
// customer_number was re-mapped via a code path that did not clear
|
||||
// this cache — getObjectProperties() will have loaded the user's
|
||||
// CURRENT customer_number from the DB, which may differ from the
|
||||
// one we asked for. Without this check, downstream invoice code
|
||||
// (getCustomerEcocomicData, setCustomerNumber) would use the
|
||||
// stale user and route the invoice to the wrong EC account.
|
||||
if ((int)$this->customer_number->value() !== $customer_number) {
|
||||
self::redisCache()?->clear_user_id_from_customer_number($customer_number);
|
||||
return $this->getUserByCustomerNumber($customer_number);
|
||||
}
|
||||
return $this;
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
<?php
|
||||
|
||||
/*
|
||||
* Regression test for TRU-18 / AUT-14:
|
||||
* "api — truckwash.io invoices route to wrong EC account; some users"
|
||||
*
|
||||
* Root cause: getUserByCustomerNumber() in objects/users_o.php trusted the
|
||||
* inverse Redis cache (customer_number -> user_id) without verifying that the
|
||||
* user it loaded actually owns the requested EC customer_number in the local
|
||||
* DB. When that cache went stale (e.g. after a customer_number re-mapping on
|
||||
* a code path that did not clear the inverse cache), getUserByCustomerNumber()
|
||||
* would return the wrong user. Downstream invoice code (getCustomerEcocomicData,
|
||||
* setCustomerNumber) would then use that wrong user's current customer_number
|
||||
* and route the draft invoice to the wrong Economic account.
|
||||
*
|
||||
* The fix verifies the loaded user owns the requested customer_number after
|
||||
* the Redis fast-path, clears the stale cache entry, and re-fetches when the
|
||||
* fast-path returned a user whose actual customer_number does not match.
|
||||
*/
|
||||
|
||||
it('revalidates loaded user against requested customer_number after Redis fast-path (TRU-18)', function (): void {
|
||||
$usersFile = app_path('objects/users_o.php');
|
||||
$content = file_get_contents($usersFile);
|
||||
|
||||
expect($content)->not->toBeFalse();
|
||||
|
||||
// The fast-path (Redis cache hit) must verify the loaded user actually
|
||||
// owns the requested EC customer_number before returning.
|
||||
expect($content)->toContain('// BUG FIX (TRU-18 / AUT-14)');
|
||||
expect($content)->toContain('getUserByCustomerNumber(int $customer_number)');
|
||||
expect($content)->toContain('self::redisCache()?->get_user_id_from_customer_number($customer_number)');
|
||||
expect($content)->toContain('$this->getObjectProperties();');
|
||||
expect($content)->toContain('if ((int)$this->customer_number->value() !== $customer_number) {');
|
||||
expect($content)->toContain('self::redisCache()?->clear_user_id_from_customer_number($customer_number);');
|
||||
expect($content)->toContain('return $this->getUserByCustomerNumber($customer_number);');
|
||||
});
|
||||
|
||||
it('keeps the DB lookup path as the source of truth when the Redis cache is empty or stale', function (): void {
|
||||
$usersFile = app_path('objects/users_o.php');
|
||||
$content = file_get_contents($usersFile);
|
||||
|
||||
expect($content)->not->toBeFalse();
|
||||
|
||||
// After clearing the stale cache, the recursive call must fall through to
|
||||
// the DB query path which selects by exact customer_number match.
|
||||
expect($content)->toContain('SELECT id FROM $this->table WHERE customer_number = \'$customer_number\'');
|
||||
});
|
||||
|
||||
it('does not use the requested customer_number for any unrelated lookup in the invoice export flow', function (): void {
|
||||
// Sanity check: the invoice export flow must go through getCustomerByOrderId
|
||||
// -> getUserByCustomerNumber, so the TRU-18 fix above is the choke point.
|
||||
$ordersFile = app_path('objects/orders_o.php');
|
||||
$content = file_get_contents($ordersFile);
|
||||
|
||||
expect($content)->not->toBeFalse();
|
||||
expect($content)->toContain('public function getCustomerByOrderId(?string $order_id): users_o');
|
||||
expect($content)->toContain("SELECT customer_id FROM orders WHERE id = \$order_id");
|
||||
expect($content)->toContain('return (new users_o())->getUserByCustomerNumber($row[\'customer_id\']);');
|
||||
});
|
||||
Reference in New Issue
Block a user