fix(auth): no-issue: fall back to cleartext token storage when Web Crypto unavailable (backport 2.59)#10153
fix(auth): no-issue: fall back to cleartext token storage when Web Crypto unavailable (backport 2.59)#10153passcod wants to merge 1 commit into
Conversation
…ypto unavailable Web Crypto (crypto.subtle) is only available in secure contexts (HTTPS or localhost). On plain HTTP local-network connections to ITIs, writePersistedAuthToken was throwing, which caused setFacilityId to silently swallow the error and leave facilityId unset — surfacing the facility selector even with a single facility, then showing the crypto error on submit. Fall back to cleartext localStorage when crypto.subtle is absent. The session is already transmitted unencrypted over HTTP so cleartext storage is no worse; a console.warn is emitted to make the condition visible. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
🦸 Review Hero (could not post inline comments — showing here instead)
[Security] The cleartext fallback stores the raw bearer token indefinitely in localStorage with no expiry marker. On HTTP, a stolen device or shared browser profile exposes a valid token for its entire server-side lifetime. The comment argues this is 'no worse' than HTTP network transmission, but localStorage persists across sessions and is accessible to any same-origin script (e.g. a browser extension or injected third-party script), whereas network interception requires active monitoring during the session. Consider adding a sessionStorage fallback instead of localStorage when Web Crypto is unavailable — tokens would still survive page reloads but not be left behind after the browser closes, reducing the persistence window.
[Security] The legacy-to-encrypted migration in readPersistedAuthToken (lines 230-236) fires on every read of any non-encrypted token. With the new cleartext fallback, when Web Crypto is unavailable writePersistedAuthToken no longer throws — it silently succeeds by writing cleartext again. Because a cleartext token has no tamanuenc prefix, isEncryptedStoredValue(raw) is always false for it, so every single call to readPersistedAuthToken will call writePersistedAuthToken and emit another console.warn. The fix is to guard the migration attempt with the same !globalThis.crypto?.subtle check used in writePersistedAuthToken, or skip the upgrade attempt entirely when crypto is unavailable, so the warn fires once on write rather than on every read. |
|
🦸 Review Hero Summary Local fix prompt (copy to your coding agent)Fix these issues identified on the pull request. One commit per issue fixed.
|
Changes
🤖 Backport of #9942 to
release/2.59.On plain HTTP connections (e.g. local-network access to an ITI),
window.crypto.subtleis unavailable because browsers only expose Web Crypto in secure contexts.writePersistedAuthTokenwas throwing, which causedsetFacilityIdto silently swallow the error and leavefacilityIdunset — making the facility selector appear even with a single facility, then showing the crypto error on submit.Fix: fall back to storing the token as cleartext when
crypto.subtleis absent. The session is already transmitted unencrypted over HTTP, so cleartext localStorage is no worse; aconsole.warnmakes the condition visible. Encrypted storage continues to work as before on HTTPS.This is needed for deployments where secure origins are not possible and security is ensured through other means.
Auto-Deploy
Options
Tests
Review Hero
.github/review-hero/suppressions.yml. Also runs automatically at the end of any auto-fix run.Remember to...