Skip to content

Commit 8ba0796

Browse files
Subash-Mohanclaude
andauthored
feat(mobile): source selection in the actions menu + token refresh loop (#13825)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent f7b9d64 commit 8ba0796

91 files changed

Lines changed: 4660 additions & 359 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

mobile/src/api/auth/__tests__/sessionManager.test.ts

Lines changed: 107 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// Globals imported from `@jest/globals` so the app's TS config stays free of ambient test types.
21
import { beforeEach, describe, expect, it, jest } from "@jest/globals";
32
import type { Mock } from "jest-mock";
43

@@ -19,16 +18,19 @@ import { apiFetch, type ApiFetchInit } from "@/api/client";
1918
import { ApiError } from "@/api/errors";
2019
import { persister, queryClient } from "@/query/client";
2120

22-
// `jest.mock` is hoisted above the imports by babel-jest; mocking whole modules also keeps native deps (MMKV) out.
21+
// babel-jest hoists `jest.mock` above the imports, so these can sit below the import block.
2322
jest.mock("@/api/client");
2423
jest.mock("@/api/auth/tokenStore");
2524
jest.mock("@/api/auth/browserSso");
2625
jest.mock("@/query/client", () => ({
2726
queryClient: { clear: jest.fn() },
2827
persister: { removeClient: jest.fn() },
2928
}));
29+
// `mock`-prefixed and read lazily so the hoisted factory can close over it.
30+
let mockServerUrl: string | null = "https://a.test";
3031
jest.mock("@/state/session", () => ({
3132
useSession: { getState: () => ({ setStatus: jest.fn() }) },
33+
getStoredServerUrl: () => mockServerUrl,
3234
}));
3335

3436
// generic `apiFetch<T>` makes jest.mocked() infer `never`; cast to a concrete Mock signature.
@@ -61,8 +63,8 @@ const token = (access: string): BearerTokenResponse => ({
6163

6264
beforeEach(() => {
6365
jest.clearAllMocks();
64-
// Reset module-level session epoch + single-flight handle so state can't leak across tests.
6566
__resetSessionStateForTests();
67+
mockServerUrl = "https://a.test";
6668
mockSetToken.mockResolvedValue(undefined);
6769
mockGetToken.mockResolvedValue(null);
6870
mockRemoveClient.mockResolvedValue(undefined);
@@ -110,7 +112,6 @@ describe("login (browser SSO)", () => {
110112
expect(path).toBe("/auth/mobile/sso/exchange");
111113
expect(init?.method).toBe("POST");
112114
expect(init?.auth).toBe(false);
113-
// snake_case `code_verifier` for the backend.
114115
expect(init?.body).toEqual({
115116
code: "one-time-code",
116117
code_verifier: "the-verifier",
@@ -138,7 +139,6 @@ describe("login (browser SSO)", () => {
138139

139140
describe("register", () => {
140141
it("creates the account as JSON, then logs in to mint the token", async () => {
141-
// register returns no token; the subsequent login does.
142142
mockApiFetch.mockImplementation((path) =>
143143
Promise.resolve(path === "/auth/register" ? undefined : token("tok-new")),
144144
);
@@ -170,7 +170,6 @@ describe("register", () => {
170170
});
171171

172172
it("flags a post-register login failure distinctly (the account exists)", async () => {
173-
// register OK, auto-login fails (e.g. verification required): account created, not signed in.
174173
mockApiFetch.mockImplementation((path) =>
175174
path === "/auth/register"
176175
? Promise.resolve(undefined)
@@ -227,7 +226,6 @@ describe("refreshToken (single-flight)", () => {
227226
const p2 = refreshToken();
228227
const p3 = refreshToken();
229228

230-
// Only the first issued a request; the others shared the in-flight promise.
231229
expect(mockApiFetch).toHaveBeenCalledTimes(1);
232230

233231
resolveFetch(token("tok-refresh"));
@@ -240,6 +238,17 @@ describe("refreshToken (single-flight)", () => {
240238
expect(mockSetToken).toHaveBeenCalledWith("tok-refresh");
241239
});
242240

241+
it("sends the refresh with the stored token so it can't await its own promise", async () => {
242+
mockApiFetch.mockResolvedValueOnce(token("tok-a"));
243+
244+
await refreshToken();
245+
246+
expect(mockApiFetch).toHaveBeenCalledWith(
247+
"/auth/mobile/refresh",
248+
expect.objectContaining({ auth: "stored" }),
249+
);
250+
});
251+
243252
it("starts a fresh request after the previous refresh settles", async () => {
244253
mockApiFetch.mockResolvedValueOnce(token("tok-a"));
245254
await refreshToken();
@@ -261,12 +270,63 @@ describe("refreshToken (single-flight)", () => {
261270
});
262271

263272
it("re-throws a transient error without dropping the token", async () => {
273+
const warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {});
264274
mockApiFetch.mockRejectedValue(new ApiError({ status: 500 }));
265275

266276
await expect(refreshToken()).rejects.toBeInstanceOf(ApiError);
267277

268278
expect(mockSetToken).not.toHaveBeenCalledWith(null);
269279
expect(mockClear).not.toHaveBeenCalled();
280+
281+
warnSpy.mockRestore();
282+
});
283+
284+
it("still answers null when clearing the session fails after a rejected token", async () => {
285+
/*
286+
* A throw from inside the catch escapes it, so this used to reject instead — reaching the
287+
* fire-and-forget refresh loop as an unlogged rejection with the session half-cleared.
288+
*/
289+
const warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {});
290+
const clearFailure = new Error("keychain unavailable");
291+
mockApiFetch.mockRejectedValue(new ApiError({ status: 401 }));
292+
mockSetToken.mockRejectedValue(clearFailure);
293+
294+
await expect(refreshToken()).resolves.toBeNull();
295+
expect(warnSpy).toHaveBeenCalledWith(expect.any(String), clearFailure);
296+
297+
warnSpy.mockRestore();
298+
});
299+
300+
it("logs a transient failure once, however many callers were waiting on it", async () => {
301+
/*
302+
* Every caller swallows this rejection, so this line is the only trace a session that dies
303+
* from repeated failed refreshes ever leaves.
304+
*/
305+
const warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {});
306+
mockGetToken.mockResolvedValue("stored-tok");
307+
const failure = new ApiError({ status: 500 });
308+
let rejectFetch!: (reason: unknown) => void;
309+
mockApiFetch.mockReturnValue(
310+
new Promise<BearerTokenResponse>((_resolve, reject) => {
311+
rejectFetch = reject;
312+
}),
313+
);
314+
315+
const refreshP = refreshToken();
316+
const waiters = [getValidToken(), getValidToken(), getValidToken()];
317+
rejectFetch(failure);
318+
319+
await expect(refreshP).rejects.toBe(failure);
320+
await expect(Promise.all(waiters)).resolves.toEqual([
321+
"stored-tok",
322+
"stored-tok",
323+
"stored-tok",
324+
]);
325+
326+
expect(warnSpy).toHaveBeenCalledTimes(1);
327+
expect(warnSpy).toHaveBeenCalledWith(expect.any(String), failure);
328+
329+
warnSpy.mockRestore();
270330
});
271331

272332
it("does not resurrect the session when a logout completes mid-refresh", async () => {
@@ -283,13 +343,51 @@ describe("refreshToken (single-flight)", () => {
283343
const refreshP = refreshToken();
284344
await logout(); // bumps the session epoch
285345

286-
resolveRefresh(token("tok-late")); // resolves AFTER logout
346+
resolveRefresh(token("tok-late"));
287347
await expect(refreshP).resolves.toBeNull();
288348

289-
// Late token must NOT be written back over the logged-out session.
290349
expect(mockSetToken).not.toHaveBeenCalledWith("tok-late");
291350
expect(mockSetToken).toHaveBeenLastCalledWith(null);
292351
});
352+
353+
it("discards a refresh that lands after the user switched instances", async () => {
354+
let resolveRefresh!: (value: BearerTokenResponse) => void;
355+
mockApiFetch.mockReturnValue(
356+
new Promise<BearerTokenResponse>((resolve) => {
357+
resolveRefresh = resolve;
358+
}),
359+
);
360+
361+
const refreshP = refreshToken();
362+
// The connect screen swaps instances without touching the session epoch.
363+
mockServerUrl = "https://b.test";
364+
resolveRefresh(token("tok-instance-a"));
365+
366+
await expect(refreshP).resolves.toBeNull();
367+
/*
368+
* `setToken` keys off the *current* URL, so writing here would file instance A's bearer under
369+
* instance B's key and hand it to a different server on the next request.
370+
*/
371+
expect(mockSetToken).not.toHaveBeenCalledWith("tok-instance-a");
372+
});
373+
374+
it("leaves the new instance's session alone when the old one's refresh is rejected", async () => {
375+
let rejectRefresh!: (reason: unknown) => void;
376+
mockApiFetch.mockReturnValue(
377+
new Promise<BearerTokenResponse>((_resolve, reject) => {
378+
rejectRefresh = reject;
379+
}),
380+
);
381+
382+
const refreshP = refreshToken();
383+
mockServerUrl = "https://b.test";
384+
rejectRefresh(new ApiError({ status: 401 }));
385+
386+
await expect(refreshP).resolves.toBeNull();
387+
// A dead token on instance A says nothing about B; wiping would sign the user out of B.
388+
expect(mockClear).not.toHaveBeenCalled();
389+
expect(mockSetToken).not.toHaveBeenCalledWith(null);
390+
});
293391
});
294392

295393
describe("getValidToken", () => {
@@ -332,7 +430,6 @@ describe("getValidToken", () => {
332430

333431
rejectFetch(new ApiError({ status: 500 }));
334432

335-
// refresh propagates the transient error; getValidToken must not.
336433
await expect(refreshP).rejects.toBeInstanceOf(ApiError);
337434
await expect(validP).resolves.toBe("stored-tok");
338435
});
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Split out of `sessionManager` so `api/client` can await an in-flight refresh without importing it back — a cycle.
2+
import { getToken } from "@/api/auth/tokenStore";
3+
4+
let inFlightRefresh: Promise<string | null> | null = null;
5+
6+
export function getInFlightRefresh(): Promise<string | null> | null {
7+
return inFlightRefresh;
8+
}
9+
10+
export function setInFlightRefresh(
11+
refresh: Promise<string | null> | null,
12+
): void {
13+
inFlightRefresh = refresh;
14+
}
15+
16+
/*
17+
* The token is opaque (not a JWT), so there's no expiry to check: "valid" only means not about to be replaced.
18+
* A failed refresh falls back to the stored token — one flaky refresh must not fail every concurrent request.
19+
*/
20+
export async function getValidToken(): Promise<string | null> {
21+
const pending = inFlightRefresh;
22+
if (pending) {
23+
try {
24+
return await pending;
25+
} catch {
26+
return getToken();
27+
}
28+
}
29+
return getToken();
30+
}

0 commit comments

Comments
 (0)