diff --git a/RELEASE.md b/RELEASE.md index 2adbbd38..a88a6340 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -1,27 +1,23 @@ -> v1.6.56 ~ "The public API answers callers, and answers them in JSON" +> v1.6.57 ~ "Settings changes take effect without clearing the cache by hand" --- ## Highlights -A release of platform-level fixes, most of them found by running the official Postman collections against a live stack. Several affected every Fleetbase install, not just CI: public API requests could not identify their own user, error responses arrived as HTML stack traces, and a verification email could not be sent at all to a recipient without a name. +Writing a system setting did not invalidate the cache entry the reader actually uses, so the old value kept being served until the cache was cleared manually. On a default install — `api/.env.example` ships `CACHE_DRIVER=file` — nothing invalidated it at all. + +This surfaced as a rotated platform API token being rejected as invalid: the new hash was written to the database, and every request kept validating against the previously cached one. --- ## Bug Fixes -- **`$request->user()` was null on every public API request.** The `fleetbase.api` middleware authenticates with `Auth::setSession()`, which writes the session keys but never binds a user resolver. Anything downstream that asked the request who was calling got nothing. -- **API clients received HTML stack traces.** The exception handler rendered Laravel's debug page to callers that had asked for JSON, so a client parsing the response found markup where an error body belonged. -- **A user without a name broke verification email.** `Utils::delinkify()` required a string and the mail view passed it a null name, so the send threw instead of delivering. This blocked customer-creation verification for any recipient the code had no name for. -- **A multi-table `findModel()` miss queried a table named `Array`.** When no model matched, the table list itself was stringified into the query, producing `SQLSTATE[42S02]` rather than a clean null. -- **The public download endpoint would not take a `public_id`**, only the internal identifier. +- **A saved setting invalidated the wrong cache key.** `Setting::system('platform_api.token_hash')` caches under `system_settings.platform_api.token_hash`, using the key exactly as the caller passed it, while the row is stored as `system.platform_api.token_hash`. The model's `saved`/`deleted` events built the cache key from the row, producing `system_settings.system.platform_api.token_hash` — a key nothing ever writes. Both spellings are now forgotten. +- **Saving a setting threw when Redis was not configured.** `Utils::clearCacheByPattern()` resolved a Redis connection unguarded, and the `saved` event calls through it — so on an install without Redis, an ordinary `Setting::configureSystem()` write raised a binding exception. Pattern clearing is inherently Redis-only; without Redis there is nothing to enumerate, so it now skips instead of failing. --- -## Improvements -- Added a safe user deletion console command. -- Fixed the Sentry configuration probe's validation, and restored its coverage gate. +## Known limitations +`Utils::clearCacheByPattern()` still only clears entries on Redis. That no longer matters for settings, which now invalidate their own keys directly, but a caller relying on pattern clearing for something else will not see it work on another driver. --- -## Continuous Integration -- PHP CI and the Postman contract now run on `dev-v*` release branches, so work merged into a release branch is tested before the release PR. -- The contract run tests this branch's API code rather than the published package. -- Removed a second-boundary race in the API credential expiry test. +## Upgrade Steps +No migration and no configuration change. If a platform API token was rotated and rejected on an earlier version, rotate it once more after upgrading — or clear the cache — so the stale hash is dropped. --- ## Need help? diff --git a/composer.json b/composer.json index d3629dbd..38999037 100644 --- a/composer.json +++ b/composer.json @@ -1,6 +1,6 @@ { "name": "fleetbase/core-api", - "version": "1.6.56", + "version": "1.6.57", "description": "Core Framework and Resources for Fleetbase API", "keywords": [ "fleetbase", diff --git a/src/Models/Setting.php b/src/Models/Setting.php index df1a299f..7c29cacb 100644 --- a/src/Models/Setting.php +++ b/src/Models/Setting.php @@ -103,17 +103,46 @@ protected static function boot() // Using saved event to cover both creating and updating scenarios static::saved(function ($setting) { - $cacheKey = 'system_settings.' . $setting->key; - cache()->forget($cacheKey); + static::forgetCachedSetting($setting->key); }); // Handle the setting deletion scenario static::deleted(function ($setting) { - $cacheKey = 'system_settings.' . $setting->key; - cache()->forget($cacheKey); + static::forgetCachedSetting($setting->key); }); } + /** + * Forgets every cache entry system() could be holding for a stored setting key. + * + * system() caches under 'system_settings.' . $key using the key EXACTLY as the caller + * passed it, and callers pass it without the 'system.' prefix that is stored on the + * row — configureSystem('platform_api.token_hash') writes the row + * 'system.platform_api.token_hash'. Building the cache key from the row therefore + * produced 'system_settings.system.platform_api.token_hash', which nothing ever + * writes, so the entry the reader actually uses was never invalidated. + * + * Nothing surfaced it because clearSystemCache() appeared to cover the gap — but that + * pattern-clear is Redis-only, and api/.env.example ships CACHE_DRIVER=file, so on a + * default install neither path invalidated anything. A rotated platform API token kept + * failing against the previously cached hash until the cache was cleared by hand. + */ + protected static function forgetCachedSetting(?string $key): void + { + // Cast rather than guard-and-return: a keyless row is not reachable through the + // events that call this, so a guard would be an untestable branch, and forgetting + // 'system_settings.' costs nothing on the off chance. + $key = (string) $key; + + // The key as stored, for callers that pass the fully-qualified form. + cache()->forget('system_settings.' . $key); + + // And the unprefixed form, which is what configureSystem() callers actually read. + if (Str::startsWith($key, 'system.')) { + cache()->forget('system_settings.' . Str::after($key, 'system.')); + } + } + /** * Retrieves a system setting by key, with optional default value. The settings are cached indefinitely * to optimize performance by reducing database access. If the setting involves nested keys, it uses a dot notation diff --git a/src/Support/Utils.php b/src/Support/Utils.php index 3e0c9576..58ec2700 100644 --- a/src/Support/Utils.php +++ b/src/Support/Utils.php @@ -2704,9 +2704,18 @@ public static function slugify($string) */ public static function clearCacheByPattern(string $pattern): void { - $redis = Redis::connection(); - $prefix = Cache::getPrefix(); - $keys = $redis->keys($prefix . $pattern); + // Only Redis can be searched by pattern, and it is not always there — the default + // api/.env.example ships CACHE_DRIVER=file. Resolving the connection unguarded made + // this throw on an install with no Redis configured, which took a plain + // Setting::configureSystem() write down with it, since the model's saved event + // calls through here. Nothing to enumerate without Redis, so skip rather than fail. + try { + $redis = Redis::connection(); + $prefix = Cache::getPrefix(); + $keys = $redis->keys($prefix . $pattern); + } catch (\Throwable $e) { + return; + } if (is_array($keys)) { $keys = array_map(function ($key) { diff --git a/tests/Unit/Models/SettingModelTest.php b/tests/Unit/Models/SettingModelTest.php index 95aee543..e6df5fab 100644 --- a/tests/Unit/Models/SettingModelTest.php +++ b/tests/Unit/Models/SettingModelTest.php @@ -290,6 +290,26 @@ function setting_model_database(): array ->and($cache->forgotten)->toContain('system_settings.system.timezone'); }); +it('invalidates the cache entry system() actually reads when a setting is written', function () { + // system() caches under the key AS PASSED — 'system_settings.platform_api.token_hash' — + // while the row is stored as 'system.platform_api.token_hash'. Building the cache key + // from the row produced 'system_settings.system.platform_api.token_hash', which nothing + // ever writes, so the entry the reader uses survived every write. + // + // clearSystemCache() looked like it covered the gap, but it pattern-clears through + // Redis and api/.env.example ships CACHE_DRIVER=file — so on a default install neither + // path invalidated anything, and a rotated platform API token kept validating against + // the previously cached hash. + setting_model_database(); + + Setting::configureSystem('platform_api.token_hash', 'OLD_HASH'); + expect(Setting::system('platform_api.token_hash'))->toBe('OLD_HASH'); + + Setting::configureSystem('platform_api.token_hash', 'NEW_HASH'); + + expect(Setting::system('platform_api.token_hash'))->toBe('NEW_HASH'); +}); + it('configures and looks up company settings from session context', function () { setting_model_database(); session()->flush();