refactor(auth): make the token cipher async#3444
Merged
Merged
Conversation
IAuthTokenCipher.encrypt/decrypt become Promise-returning so a host can back them with an async primitive (e.g. the browser's Web Crypto, which is async). Thread await through the AuthService call sites; the desktop adapter wraps its existing synchronous node:crypto cipher in Promise.resolve, so desktop behavior is unchanged. Enabling change for the browser web host, but self-contained and useful on its own. Generated-By: PostHog Code Task-Id: 77d13a30-444d-4045-9d80-5e2a9f2e68ae
|
😎 Merged successfully - details. |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
Contributor
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This was referenced Jul 14, 2026
commitSessionState mutated this.session and saved the project preference before awaiting persistSession. Now that encryption is async and can reject, a failure left the service on the new project in memory (and in the saved preference) while the stored session and published state stayed on the old one — a partial, inconsistent commit the caller saw only as a rejected promise. Build the next session locally, await the persist first (the only fallible step), then assign this.session, save the preference, and publish state — so a rejection leaves every layer on the prior session. Add a regression test that fails encryption mid-selectProject and asserts nothing moved. Generated-By: PostHog Code Task-Id: 77d13a30-444d-4045-9d80-5e2a9f2e68ae
Making getSessionInputForRefresh async put an await (stored-token decryption) between ensureValidSession's `if (this.refreshPromise)` guard and the `this.refreshPromise = ...` assignment. Two concurrent callers could both observe a null refreshPromise, both decrypt, then both start a refresh — spending the rotating refresh token twice, so the second request fails with auth_error and clears the session the first one established, logging the user out. Move the decryption inside refreshAndSync so refreshPromise is assigned synchronously right after the guard, restoring single-flight dedup. Add a regression test firing two concurrent refreshAccessToken() calls and asserting one underlying token request. Generated-By: PostHog Code Task-Id: 77d13a30-444d-4045-9d80-5e2a9f2e68ae
With async encryption, two overlapping project selections could interleave across the await in commitSessionState: an earlier selection completing last would overwrite the newer stored session and published state (and persistSession's own read of the prior selection could race), leaving the committed selection stale — not the one the user picked last. Serialize commits onto a promise chain so each commit's persist-then-publish runs to completion before the next begins; the latest selection now wins consistently across the in-memory session, storage, and subscribers. The stored chain swallows rejections so a failed commit can't wedge later ones, while the returned promise still rejects for the caller. Add a regression test that overlaps two selections, drains encryption newest-first, and asserts the latest selection wins everywhere. Generated-By: PostHog Code Task-Id: 77d13a30-444d-4045-9d80-5e2a9f2e68ae
jonathanlab
approved these changes
Jul 15, 2026
Generated-By: PostHog Code Task-Id: f67230a6-4c1c-4433-91cc-ee487135fead
gantoine
force-pushed
the
posthog-code/async-token-cipher
branch
from
39e1fea to
4a90dec
Compare
Contributor
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
Member
Author
|
/trunk merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes
IAuthTokenCipher.encrypt/decryptasync (Promise-returning) so a host can back the refresh-token-at-rest cipher with an asynchronous primitive — specifically the browser's Web Crypto API, which is async.awaitthrough theAuthServicecall sites (persistSession,commitSessionState,resolveStoredSession,getSessionInputForRefresh, plus an extractedrecoverSessionthat keeps the synchronous re-entrancy guard).TokenCipherPortAdapter) wraps its existing synchronousnode:cryptocipher inPromise.resolve, so desktop behavior is unchanged.Why
This is the first of a few small enabling changes peeled out of the
apps/webbrowser-host work (previously #3353). It stands on its own: the interface is genuinely async-capable now, and desktop keeps working exactly as before.Testing
@posthog/coretypecheck + full auth test suite (2250 tests) pass.apps/codetypecheck passes.Created with PostHog Code