diff --git a/apps/code/src/main/services/auth/port-adapters.ts b/apps/code/src/main/services/auth/port-adapters.ts index c354a9e5d5..784d379df1 100644 --- a/apps/code/src/main/services/auth/port-adapters.ts +++ b/apps/code/src/main/services/auth/port-adapters.ts @@ -37,12 +37,12 @@ import { @injectable() export class TokenCipherPortAdapter implements IAuthTokenCipher { - encrypt(plaintext: string): string { - return encrypt(plaintext); + encrypt(plaintext: string): Promise { + return Promise.resolve(encrypt(plaintext)); } - decrypt(encrypted: string): string | null { - return decrypt(encrypted); + decrypt(encrypted: string): Promise { + return Promise.resolve(decrypt(encrypted)); } } diff --git a/packages/core/src/auth/auth.test.ts b/packages/core/src/auth/auth.test.ts index 00bd2ee104..38be8ae05e 100644 --- a/packages/core/src/auth/auth.test.ts +++ b/packages/core/src/auth/auth.test.ts @@ -64,8 +64,8 @@ function createPreferencePort(): IAuthPreferenceStore { } const identityCipher: IAuthTokenCipher = { - encrypt: (plaintext) => plaintext, - decrypt: (encrypted) => encrypted, + encrypt: (plaintext) => Promise.resolve(plaintext), + decrypt: (encrypted) => Promise.resolve(encrypted), }; const mockLogger: RootLogger = { @@ -413,6 +413,52 @@ describe("AuthService", () => { } }); + it("dedupes concurrent refreshes into a single token request", async () => { + oauthFlow.startFlow.mockResolvedValue( + mockTokenResponse({ + accessToken: "initial-access-token", + refreshToken: "initial-refresh-token", + }), + ); + stubAuthFetch(); + + // Keep the refresh in flight so both callers overlap; if the refresh + // isn't deduped, the rotating refresh token is spent twice. + let resolveRefresh!: (value: unknown) => void; + oauthFlow.refreshToken.mockReturnValue( + new Promise((resolve) => { + resolveRefresh = resolve; + }), + ); + + await service.initialize(); + await service.login("us"); + + // Two forced refreshes fired in the same tick. The dedup guard must close + // synchronously — resolving the stored session now awaits decryption, so + // if that await sat between the guard and the refreshPromise assignment, + // both callers would slip through and fire their own request. + oauthFlow.refreshToken.mockClear(); + const both = Promise.all([ + service.refreshAccessToken(), + service.refreshAccessToken(), + ]); + + await new Promise((r) => setTimeout(r, 0)); + expect(oauthFlow.refreshToken).toHaveBeenCalledTimes(1); + + resolveRefresh( + mockTokenResponse({ + accessToken: "refreshed-access-token", + refreshToken: "refreshed-refresh-token", + }), + ); + const [a, b] = await both; + expect(a.accessToken).toBe("refreshed-access-token"); + expect(b.accessToken).toBe("refreshed-access-token"); + expect(oauthFlow.refreshToken).toHaveBeenCalledTimes(1); + }); + it("forces a token refresh when explicitly requested", async () => { oauthFlow.startFlow.mockResolvedValue( mockTokenResponse({ @@ -500,6 +546,141 @@ describe("AuthService", () => { }); }); + it("keeps the prior selection committed when persisting a new selection fails", async () => { + const orgs = { + "org-1": { + name: "Org 1", + projects: [ + { id: 42, name: "Project 42" }, + { id: 84, name: "Project 84" }, + ], + }, + }; + oauthFlow.startFlow.mockResolvedValue( + mockTokenResponse({ + accessToken: "initial-access-token", + refreshToken: "initial-refresh-token", + }), + ); + stubAuthFetch({ orgs }); + + // Encrypts fine during login, then rejects so the selectProject persist + // fails mid-flow (mirrors the browser cipher's Web Crypto rejecting). + let failEncrypt = false; + const flakyCipher: IAuthTokenCipher = { + encrypt: (plaintext) => + failEncrypt + ? Promise.reject(new Error("encryption unavailable")) + : Promise.resolve(plaintext), + decrypt: (encrypted) => Promise.resolve(encrypted), + }; + service = new AuthService( + preferencePort, + sessionPort, + oauthFlow as unknown as IAuthOAuthFlowService, + connectivity, + flakyCipher, + mockPowerManager as unknown as IPowerManager, + mockLogger, + null, + ); + + // Initialize once while the session store is empty so login/selectProject + // don't later trigger the stored-session restore path. + service.init(); + await service.initialize(); + + await service.login("us"); + const priorProjectId = service.getState().currentProjectId; + expect(priorProjectId).toBe(42); + + failEncrypt = true; + await expect(service.selectProject(84)).rejects.toThrow( + "encryption unavailable", + ); + + // A failed persist must not commit the new project anywhere: published + // state, the stored session, and the saved preference all stay on the + // prior selection (the preference is the tell — the buggy order saved it + // to 84 before the encrypt rejected). + expect(service.getState().currentProjectId).toBe(priorProjectId); + expect(sessionPort.getCurrent()?.selectedProjectId).toBe(priorProjectId); + expect(preferencePort.get("user-1", "us")?.lastSelectedProjectId).toBe( + priorProjectId, + ); + }); + + it("keeps overlapping selections consistent so the latest one wins", async () => { + const orgs = { + "org-1": { + name: "Org 1", + projects: [ + { id: 42, name: "Project 42" }, + { id: 84, name: "Project 84" }, + { id: 99, name: "Project 99" }, + ], + }, + }; + oauthFlow.startFlow.mockResolvedValue( + mockTokenResponse({ + accessToken: "initial-access-token", + refreshToken: "initial-refresh-token", + }), + ); + stubAuthFetch({ orgs }); + + // Encryption is gated so overlapping selection commits stay in flight and + // can be resolved out of order below. + let deferEncrypt = false; + const pending: Array<() => void> = []; + const gatedCipher: IAuthTokenCipher = { + encrypt: (plaintext) => + deferEncrypt + ? new Promise((resolve) => { + pending.push(() => resolve(plaintext)); + }) + : Promise.resolve(plaintext), + decrypt: (encrypted) => Promise.resolve(encrypted), + }; + service = new AuthService( + preferencePort, + sessionPort, + oauthFlow as unknown as IAuthOAuthFlowService, + connectivity, + gatedCipher, + mockPowerManager as unknown as IPowerManager, + mockLogger, + null, + ); + service.init(); + await service.initialize(); + await service.login("us"); + expect(service.getState().currentProjectId).toBe(42); + + deferEncrypt = true; + // Two overlapping selections: 84 first, then 99 (the latest intent). + const first = service.selectProject(84); + const second = service.selectProject(99); + + // Drain pending encryptions newest-first each round to surface any + // out-of-order completion, until both commits settle. Serialized commits + // only expose one pending encryption at a time; unserialized ones would + // let the stale 84 commit land last. + const flush = () => new Promise((r) => setTimeout(r, 0)); + await flush(); + while (pending.length > 0) { + for (const resolve of pending.splice(0).reverse()) resolve(); + await flush(); + } + await Promise.all([first, second]); + + // The latest selection wins everywhere — no stale overwrite and no split + // between the in-memory session (getState), stored session, and preference. + expect(service.getState().currentProjectId).toBe(99); + expect(sessionPort.getCurrent()?.selectedProjectId).toBe(99); + expect(preferencePort.get("user-1", "us")?.lastSelectedProjectId).toBe(99); + }); + it("restores the selected project after app restart while logged out", async () => { const orgs = { "org-1": { diff --git a/packages/core/src/auth/auth.ts b/packages/core/src/auth/auth.ts index 18e3882e88..9f9452ec04 100644 --- a/packages/core/src/auth/auth.ts +++ b/packages/core/src/auth/auth.ts @@ -86,6 +86,9 @@ export class AuthService extends TypedEventEmitter { private session: InMemorySession | null = null; private initializePromise: Promise | null = null; private refreshPromise: Promise | null = null; + // Serializes session-state commits so overlapping selections can't + // interleave across async encryption (see commitSessionState). + private commitChain: Promise = Promise.resolve(); constructor( @inject(AUTH_PREFERENCE_STORE) private readonly authPreference: IAuthPreferenceStore, @@ -254,7 +257,7 @@ export class AuthService extends TypedEventEmitter { ? await this.applyOrgChange(session, newOrgId) : session.orgProjectsMap; - this.commitSessionState(session, { + await this.commitSessionState(session, { orgProjectsMap, currentOrgId: newOrgId, currentProjectId: projectId, @@ -277,7 +280,7 @@ export class AuthService extends TypedEventEmitter { orgId, ); - this.commitSessionState(session, { + await this.commitSessionState(session, { orgProjectsMap, currentOrgId: orgId, currentProjectId, @@ -333,8 +336,26 @@ export class AuthService extends TypedEventEmitter { currentOrgId: string | null; currentProjectId: number | null; }, - ): void { - this.session = { + ): Promise { + // Serialize commits onto a chain so overlapping selections can't + // interleave across async encryption and clobber a newer one. The chain + // swallows rejections so one failure doesn't wedge later commits; the + // returned promise still rejects for the caller. + const run = this.commitChain.then(() => + this.applyCommittedSession(prevSession, next), + ); + this.commitChain = run.catch(() => {}); + return run; + } + private async applyCommittedSession( + prevSession: InMemorySession, + next: { + orgProjectsMap: OrgProjectsMap; + currentOrgId: string | null; + currentProjectId: number | null; + }, + ): Promise { + const nextSession: InMemorySession = { ...prevSession, orgProjectsMap: next.orgProjectsMap, currentOrgId: next.currentOrgId, @@ -342,13 +363,18 @@ export class AuthService extends TypedEventEmitter { orgProjectsIncomplete: false, }; - this.persistProjectPreference(this.session); - this.persistSession({ - refreshToken: this.session.refreshToken, - cloudRegion: this.session.cloudRegion, + // Persist the durable session first — the only step that can fail (async + // encryption may reject). Mutate this.session, the preference, and + // published state only after it resolves, so a rejection leaves every + // layer on the prior session. + await this.persistSession({ + refreshToken: nextSession.refreshToken, + cloudRegion: nextSession.cloudRegion, selectedProjectId: next.currentProjectId, }); + this.session = nextSession; + this.persistProjectPreference(nextSession); this.updateState({ orgProjectsMap: next.orgProjectsMap, currentOrgId: next.currentOrgId, @@ -413,7 +439,7 @@ export class AuthService extends TypedEventEmitter { return; } - const storedSession = this.resolveStoredSession(); + const storedSession = await this.resolveStoredSession(); if (!storedSession) { this.logger.warn("Stored auth session could not be decrypted"); this.authSession.clearCurrent(); @@ -500,9 +526,12 @@ export class AuthService extends TypedEventEmitter { return this.refreshPromise; } - const sessionInput = this.getSessionInputForRefresh(); - + // Assign refreshPromise synchronously — no await before this — so + // concurrent callers dedupe onto one refresh. Resolving the stored session + // (now async) must happen INSIDE refreshAndSync, else two callers both + // refresh and burn the rotating token twice. const refreshAndSync = async (): Promise => { + const sessionInput = await this.getSessionInputForRefresh(); let session: InMemorySession; try { session = await this.refreshSession(sessionInput); @@ -532,7 +561,7 @@ export class AuthService extends TypedEventEmitter { return this.refreshPromise; } - private getSessionInputForRefresh(): StoredSessionInput { + private async getSessionInputForRefresh(): Promise { if (this.session) { return { refreshToken: this.session.refreshToken, @@ -541,7 +570,7 @@ export class AuthService extends TypedEventEmitter { }; } - const storedSession = this.resolveStoredSession(); + const storedSession = await this.resolveStoredSession(); if (!storedSession) { throw new NotAuthenticatedError(); } @@ -794,7 +823,7 @@ export class AuthService extends TypedEventEmitter { session: InMemorySession, ): Promise { this.persistProjectPreference(session); - this.persistSession({ + await this.persistSession({ refreshToken: session.refreshToken, cloudRegion: session.cloudRegion, selectedProjectId: session.currentProjectId, @@ -816,15 +845,15 @@ export class AuthService extends TypedEventEmitter { void this.refreshOrgProjects(); } } - private persistSession(input: { + private async persistSession(input: { refreshToken: string; cloudRegion: CloudRegion; selectedProjectId: number | null; - }): void { + }): Promise { const priorSelected = this.authSession.getCurrent()?.selectedProjectId ?? null; this.authSession.saveCurrent({ - refreshTokenEncrypted: this.cipher.encrypt(input.refreshToken), + refreshTokenEncrypted: await this.cipher.encrypt(input.refreshToken), cloudRegion: input.cloudRegion, selectedProjectId: input.selectedProjectId ?? priorSelected, scopeVersion: OAUTH_SCOPE_VERSION, @@ -1051,11 +1080,13 @@ export class AuthService extends TypedEventEmitter { private handleResume = (): void => { this.attemptSessionRecovery(); }; - private resolveStoredSession(): StoredSessionInput | null { + private async resolveStoredSession(): Promise { const stored = this.authSession.getCurrent(); if (!stored) return null; - const refreshToken = this.cipher.decrypt(stored.refreshTokenEncrypted); + const refreshToken = await this.cipher.decrypt( + stored.refreshTokenEncrypted, + ); if (!refreshToken) return null; return { @@ -1077,14 +1108,10 @@ export class AuthService extends TypedEventEmitter { if (!stored) return; if (stored.scopeVersion < OAUTH_SCOPE_VERSION) return; - if (!this.resolveStoredSession()) return; - - // Route through ensureValidSession so a refresh already in flight (e.g. the - // background bootstrap restore past its deadline) is shared instead of - // kicking a second concurrent token refresh that would burn the same - // rotating refresh token twice. - this.recoveryPromise = this.ensureValidSession() - .then(() => undefined) + // Claim the recovery slot synchronously so concurrent triggers don't both + // kick a token refresh; decryptability is now async (Web Crypto), so it's + // validated inside recoverSession. + this.recoveryPromise = this.recoverSession() .catch((error) => { this.logger.warn("Session recovery failed", { error }); }) @@ -1092,6 +1119,16 @@ export class AuthService extends TypedEventEmitter { this.recoveryPromise = null; }); } + private async recoverSession(): Promise { + // Bail before touching the network if the stored token can't be decrypted. + if (!(await this.resolveStoredSession())) return; + + // Route through ensureValidSession so a refresh already in flight (e.g. the + // background bootstrap restore past its deadline) is shared instead of + // kicking a second concurrent token refresh that would burn the same + // rotating refresh token twice. + await this.ensureValidSession(); + } private refreshOrgProjects(): Promise { if (this.orgProjectsRefreshPromise) { @@ -1157,7 +1194,7 @@ export class AuthService extends TypedEventEmitter { null, lastSelectedOrgId: lastPrefs?.lastSelectedOrgId ?? null, }); - this.commitSessionState(session, { + await this.commitSessionState(session, { orgProjectsMap: map, currentOrgId: session.currentOrgId, currentProjectId, diff --git a/packages/core/src/auth/identifiers.ts b/packages/core/src/auth/identifiers.ts index 300efbd74a..b0cb002595 100644 --- a/packages/core/src/auth/identifiers.ts +++ b/packages/core/src/auth/identifiers.ts @@ -89,11 +89,12 @@ export const AUTH_OAUTH_FLOW_SERVICE = Symbol.for( /** * Machine-bound symmetric cipher for the refresh token at rest. Desktop adapter - * wraps the existing encryption util (node:crypto + machine id). + * wraps the existing encryption util (node:crypto + machine id); the web adapter + * uses a non-extractable Web Crypto key (async), so the contract is async. */ export interface IAuthTokenCipher { - encrypt(plaintext: string): string; - decrypt(encrypted: string): string | null; + encrypt(plaintext: string): Promise; + decrypt(encrypted: string): Promise; } export const AUTH_TOKEN_CIPHER = Symbol.for("posthog.core.auth.tokenCipher");