From b874da4a1bfef686f55ca0b89c2a5e5668cf72db Mon Sep 17 00:00:00 2001 From: Bilal Godil Date: Mon, 8 Jun 2026 13:08:18 -0700 Subject: [PATCH] fix(rde): validate the incoming token pair in updateAccessToken; route prefetch through runAsynchronously The session-ownership guard recomputed the key from the session's existing refresh token, so refresh-backed sessions accepted any access token. Validate the incoming token pair against this.sessionKey instead, so a foreign token can't be installed into either an access-only or a refresh-backed session. Also route the sign-in current-user prefetch through runAsynchronously instead of swallowing failures with .catch(() => {}), per the project's async-error handling guideline. --- packages/shared/src/sessions.test.ts | 33 ++++++++++++------- packages/shared/src/sessions.ts | 21 ++++++------ .../apps/implementations/client-app-impl.ts | 4 +-- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/packages/shared/src/sessions.test.ts b/packages/shared/src/sessions.test.ts index 708142c86..190eb691b 100644 --- a/packages/shared/src/sessions.test.ts +++ b/packages/shared/src/sessions.test.ts @@ -81,23 +81,23 @@ describe("InternalSession.calculateSessionKey", () => { }); describe("InternalSession#updateAccessToken", () => { - it("installs a fresh token for the same session in place", () => { + it("installs a fresh token for the same access-only session in place", () => { const initial = createAccessTokenString("rtid-1", { iatOffsetSeconds: 0 }); const refreshed = createAccessTokenString("rtid-1", { iatOffsetSeconds: 1 }); const session = createAccessOnlySession(initial); - session.updateAccessToken(refreshed); + session.updateAccessToken({ accessToken: refreshed, refreshToken: null }); expect(currentToken(session)).toBe(refreshed); // identity is unchanged — same session key, same object expect(session.sessionKey).toBe("access-session-rtid-1"); }); - it("rejects a token belonging to a different session", () => { + it("rejects a token pair belonging to a different access-only session", () => { const initial = createAccessTokenString("rtid-1"); const foreign = createAccessTokenString("rtid-2", { sub: "other-user" }); const session = createAccessOnlySession(initial); - session.updateAccessToken(foreign); + session.updateAccessToken({ accessToken: foreign, refreshToken: null }); expect(currentToken(session)).toBe(initial); }); @@ -105,9 +105,9 @@ describe("InternalSession#updateAccessToken", () => { const initial = createAccessTokenString("rtid-1"); const session = createAccessOnlySession(initial); - session.updateAccessToken(initial); - session.updateAccessToken(null); - session.updateAccessToken("not-a-jwt"); + session.updateAccessToken({ accessToken: initial, refreshToken: null }); + session.updateAccessToken({ accessToken: null, refreshToken: null }); + session.updateAccessToken({ accessToken: "not-a-jwt", refreshToken: null }); expect(currentToken(session)).toBe(initial); }); @@ -115,12 +115,12 @@ describe("InternalSession#updateAccessToken", () => { const session = createAccessOnlySession(createAccessTokenString("rtid-1")); session.markInvalid(); - session.updateAccessToken(createAccessTokenString("rtid-1", { iatOffsetSeconds: 1 })); + session.updateAccessToken({ accessToken: createAccessTokenString("rtid-1", { iatOffsetSeconds: 1 }), refreshToken: null }); expect(session.isKnownToBeInvalid()).toBe(true); expect(currentToken(session)).toBeUndefined(); }); - it("updates a refresh-token-backed session's access token in place", () => { + it("updates a refresh-token-backed session's access token in place when the refresh token matches", () => { const session = new InternalSession({ refreshAccessTokenCallback: async () => null, refreshToken: "rt-abc", @@ -128,9 +128,20 @@ describe("InternalSession#updateAccessToken", () => { }); const refreshed = createAccessTokenString("rtid-2", { iatOffsetSeconds: 1 }); - // a refresh-keyed session is identified by its refresh token, so any valid access token is accepted - session.updateAccessToken(refreshed); + session.updateAccessToken({ accessToken: refreshed, refreshToken: "rt-abc" }); expect(currentToken(session)).toBe(refreshed); expect(session.sessionKey).toBe("refresh-rt-abc"); }); + + it("rejects a token pair carrying a different refresh token for a refresh-backed session", () => { + const initial = createAccessTokenString("rtid-1"); + const session = new InternalSession({ + refreshAccessTokenCallback: async () => null, + refreshToken: "rt-abc", + accessToken: initial, + }); + + session.updateAccessToken({ accessToken: createAccessTokenString("rtid-2"), refreshToken: "rt-other" }); + expect(currentToken(session)).toBe(initial); + }); }); diff --git a/packages/shared/src/sessions.ts b/packages/shared/src/sessions.ts index 667d3a85a..d6744e2ed 100644 --- a/packages/shared/src/sessions.ts +++ b/packages/shared/src/sessions.ts @@ -219,20 +219,19 @@ export class InternalSession { } /** - * Installs a fresh access token into this session in place, keeping the session object (and therefore every - * session-scoped cache) stable instead of constructing a new InternalSession. No-op if the session is invalid, - * the token can't be decoded, it's unchanged, or it doesn't belong to this session (its session key differs); - * never clears an existing token. + * Installs a freshly obtained token pair's access token into this session in place, keeping the session object + * (and therefore every session-scoped cache) stable instead of constructing a new InternalSession. No-op if the + * session is invalid, the access token can't be decoded, it's unchanged, or the pair doesn't map to this session + * (so a foreign token can never be written into this object's cache); never clears an existing token. */ - updateAccessToken(accessToken: string | null) { + updateAccessToken(tokens: { accessToken: string | null, refreshToken: string | null }) { if (this._knownToBeInvalid.get()) return; - if (!accessToken) return; - const newAccessToken = AccessToken.createIfValid(accessToken); + if (!tokens.accessToken) return; + const newAccessToken = AccessToken.createIfValid(tokens.accessToken); if (!newAccessToken) return; - // Self-enforce the "a session never changes which session it belongs to" invariant: only install a token that - // maps to this same session key, so a foreign token can never be written into this object's cache. - const newSessionKey = InternalSession.calculateSessionKey({ refreshToken: this._refreshToken?.token ?? null, accessToken: newAccessToken.token }); - if (newSessionKey !== this.sessionKey) return; + // Self-enforce the "a session never changes which session it belongs to" invariant: only install a token pair + // that maps to this same session key (validated against the incoming pair, not this session's existing tokens). + if (InternalSession.calculateSessionKey(tokens) !== this.sessionKey) return; if (this._accessToken.get()?.token === newAccessToken.token) return; this._accessToken.set(newAccessToken); } diff --git a/packages/template/src/lib/hexclave-app/apps/implementations/client-app-impl.ts b/packages/template/src/lib/hexclave-app/apps/implementations/client-app-impl.ts index 90e7011db..b26d68011 100644 --- a/packages/template/src/lib/hexclave-app/apps/implementations/client-app-impl.ts +++ b/packages/template/src/lib/hexclave-app/apps/implementations/client-app-impl.ts @@ -1551,10 +1551,10 @@ export class _HexclaveClientAppImplIncomplete {}); + runAsynchronously(this._currentUserCache.getOrWait([session], "write-only")); } protected _getTokenStoreInitForFreshTokens(tokens: { accessToken: string | null, refreshToken: string }): TokenStoreInit | undefined {