Skip to content

Commit b70601c

Browse files
committed
fix(auth): adds localStorage resilience and 401 retry
1 parent f8a0e8c commit b70601c

File tree

6 files changed

+219
-34
lines changed

6 files changed

+219
-34
lines changed

src/app/components/dashboard/DashboardPage.tsx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ import {
1919
fetchAllData,
2020
type DashboardData,
2121
} from "../../services/poll";
22-
import { clearAuth, user, onAuthCleared, DASHBOARD_STORAGE_KEY } from "../../stores/auth";
22+
import { expireToken, user, onAuthCleared, DASHBOARD_STORAGE_KEY } from "../../stores/auth";
23+
import { pushNotification } from "../../lib/errors";
2324
import { getClient, getGraphqlRateLimit } from "../../services/github";
2425
import { formatCount } from "../../lib/format";
2526
import { setsEqual } from "../../lib/collections";
@@ -191,7 +192,7 @@ async function pollFetch(): Promise<DashboardData> {
191192
try {
192193
localStorage.setItem(DASHBOARD_STORAGE_KEY, JSON.stringify(cachePayload));
193194
} catch {
194-
// localStorage full or unavailable — non-fatal
195+
pushNotification("localStorage:dashboard", "Dashboard cache write failed — storage may be full", "warning");
195196
}
196197
}, 0);
197198
} else {
@@ -208,9 +209,9 @@ async function pollFetch(): Promise<DashboardData> {
208209
: null;
209210

210211
if (status === 401) {
211-
// Hard redirect (not navigate()) forces a full page reload, which clears
212-
// module-level state like _coordinator and dashboardData for the next user.
213-
clearAuth();
212+
// Token invalid — clear token only, preserve user config/view/dashboard.
213+
// Hard redirect forces a full page reload, clearing module-level state.
214+
expireToken();
214215
window.location.replace("/login");
215216
}
216217
setDashboardData("loading", false);

src/app/components/onboarding/OnboardingWizard.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
Match,
88
} from "solid-js";
99
import { config, updateConfig, CONFIG_STORAGE_KEY } from "../../stores/config";
10+
import { pushNotification } from "../../lib/errors";
1011
import { fetchOrgs, type OrgEntry, type RepoRef } from "../../services/api";
1112
import { getClient } from "../../services/github";
1213
import RepoSelector from "./RepoSelector";
@@ -67,7 +68,11 @@ export default function OnboardingWizard() {
6768
onboardingComplete: true,
6869
});
6970
// Flush synchronously — the debounced persistence effect won't fire before page unload
70-
localStorage.setItem(CONFIG_STORAGE_KEY, JSON.stringify(config));
71+
try {
72+
localStorage.setItem(CONFIG_STORAGE_KEY, JSON.stringify(config));
73+
} catch {
74+
pushNotification("localStorage:config", "Config write failed — storage may be full", "warning");
75+
}
7176
window.location.replace("/dashboard");
7277
}
7378

src/app/stores/auth.ts

Lines changed: 66 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { createSignal } from "solid-js";
22
import { clearCache } from "./cache";
33
import { CONFIG_STORAGE_KEY, resetConfig, updateConfig, config } from "./config";
44
import { VIEW_STORAGE_KEY, resetViewState } from "./view";
5+
import { pushNotification } from "../lib/errors";
56

67
export const AUTH_STORAGE_KEY = "github-tracker:auth-token";
78
export const DASHBOARD_STORAGE_KEY = "github-tracker:dashboard";
@@ -40,9 +41,13 @@ export { user };
4041
// ── Actions ─────────────────────────────────────────────────────────────────
4142

4243
export function setAuth(response: TokenExchangeResponse): void {
43-
localStorage.setItem(AUTH_STORAGE_KEY, response.access_token);
44+
try {
45+
localStorage.setItem(AUTH_STORAGE_KEY, response.access_token);
46+
} catch {
47+
pushNotification("localStorage:auth", "Auth token write failed — storage may be full. Token exists in memory only this session.", "warning");
48+
}
4449
_setToken(response.access_token);
45-
console.info("[auth] access token set (localStorage)");
50+
console.info("[auth] access token set");
4651
}
4752

4853
export function setAuthFromPat(token: string, userData: GitHubUser): void {
@@ -89,37 +94,77 @@ export function clearAuth(): void {
8994
}
9095
}
9196

97+
/** Clear only the auth token. Preserves all user data (config, view state, dashboard
98+
* cache) so the same user's preferences and cached data survive re-authentication.
99+
* Used when a token becomes invalid (expired PAT, revoked OAuth) — NOT for explicit
100+
* logout. Full data wipe (cross-user data isolation) is handled by clearAuth()
101+
* which is reserved for explicit user actions (Sign out, Reset all).
102+
*
103+
* Callers MUST navigate away after calling this (e.g., window.location.replace or
104+
* router navigate to /login). The poll coordinator is not stopped here — page
105+
* navigation handles teardown. Use clearAuth() if not navigating. */
106+
export function expireToken(): void {
107+
localStorage.removeItem(AUTH_STORAGE_KEY);
108+
_setToken(null);
109+
setUser(null);
110+
console.info("[auth] token expired (user data preserved)");
111+
}
112+
113+
const VALIDATE_HEADERS = {
114+
Accept: "application/vnd.github+json",
115+
"X-GitHub-Api-Version": "2022-11-28",
116+
} as const;
117+
92118
export async function validateToken(): Promise<boolean> {
93119
const currentToken = _token();
94120
if (!currentToken) return false;
95121

122+
const headers = { ...VALIDATE_HEADERS, Authorization: `Bearer ${currentToken}` };
123+
124+
function handleSuccess(userData: GitHubUser): true {
125+
setUser({ login: userData.login, avatar_url: userData.avatar_url, name: userData.name });
126+
navigator.storage?.persist?.()?.catch(() => {});
127+
return true;
128+
}
129+
96130
try {
97-
const resp = await fetch("https://api.github.com/user", {
98-
headers: {
99-
Authorization: `Bearer ${currentToken}`,
100-
Accept: "application/vnd.github+json",
101-
"X-GitHub-Api-Version": "2022-11-28",
102-
},
103-
});
131+
const resp = await fetch("https://api.github.com/user", { headers });
104132

105133
if (resp.ok) {
106-
const userData = (await resp.json()) as GitHubUser;
107-
setUser({
108-
login: userData.login,
109-
avatar_url: userData.avatar_url,
110-
name: userData.name,
111-
});
112-
return true;
134+
return handleSuccess((await resp.json()) as GitHubUser);
113135
}
114136

115137
if (resp.status === 401) {
116-
const method = config.authMethod;
138+
// GitHub API can return transient 401s due to database replication lag.
139+
// Retry once after a delay before invalidating the token.
140+
await new Promise((r) => setTimeout(r, 1000));
141+
try {
142+
const retry = await fetch("https://api.github.com/user", { headers });
143+
if (retry.ok) {
144+
return handleSuccess((await retry.json()) as GitHubUser);
145+
}
146+
if (retry.status !== 401) {
147+
// Non-auth error on retry (e.g. 500) — preserve token, try next load
148+
return false;
149+
}
150+
} catch {
151+
// Network error on retry — preserve token, try next load
152+
return false;
153+
}
154+
155+
// Guard: if the token was replaced during the retry window (e.g., user
156+
// re-authenticated via OAuth callback), don't invalidate the new token.
157+
if (_token() !== currentToken) {
158+
return false;
159+
}
160+
161+
// Both attempts returned 401 — token is genuinely invalid
117162
console.info(
118-
method === "pat"
119-
? "[auth] PAT invalid or expired — clearing auth"
120-
: "[auth] access token invalid — clearing auth"
163+
config.authMethod === "pat"
164+
? "[auth] PAT invalid or expired — clearing token"
165+
: "[auth] access token invalid — clearing token"
121166
);
122-
clearAuth();
167+
expireToken();
123168
return false;
124169
}
125170

src/app/stores/view.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { z } from "zod";
22
import { createStore, produce } from "solid-js/store";
33
import { createEffect, onCleanup, untrack } from "solid-js";
4+
import { pushNotification } from "../lib/errors";
45

56
export const VIEW_STORAGE_KEY = "github-tracker:view";
67

@@ -332,7 +333,7 @@ export function initViewPersistence(): void {
332333
try {
333334
localStorage.setItem(VIEW_STORAGE_KEY, json);
334335
} catch {
335-
// QuotaExceededError — silently fail rather than kill the reactive graph
336+
pushNotification("localStorage:view", "View state write failed — storage may be full", "warning");
336337
}
337338
}, 200);
338339
onCleanup(() => {

tests/components/DashboardPage.test.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ vi.mock("@solidjs/router", () => ({
2626
const authClearCallbacks: (() => void)[] = [];
2727
vi.mock("../../src/app/stores/auth", () => ({
2828
clearAuth: vi.fn(),
29+
expireToken: vi.fn(),
2930
token: () => "fake-token",
3031
user: () => ({ login: "testuser", avatar_url: "", name: "Test User" }),
3132
isAuthenticated: () => true,
@@ -127,6 +128,7 @@ beforeEach(async () => {
127128
capturedFetchAll = null;
128129
capturedOnHotData = null;
129130
vi.mocked(authStore.clearAuth).mockClear();
131+
vi.mocked(authStore.expireToken).mockClear();
130132
vi.mocked(pollService.fetchAllData).mockResolvedValue({
131133
issues: [],
132134
pullRequests: [],
@@ -296,25 +298,28 @@ describe("DashboardPage — auth error handling", () => {
296298
consoleErrorSpy.mockRestore();
297299
});
298300

299-
it("calls clearAuth and redirects to /login on 401 error (permanent token revoked)", async () => {
301+
it("calls expireToken (not clearAuth) and redirects to /login on 401 error", async () => {
300302
const err401 = Object.assign(new Error("Unauthorized"), { status: 401 });
301303
vi.mocked(pollService.fetchAllData).mockRejectedValue(err401);
302304

303305
render(() => <DashboardPage />);
304306
await waitFor(() => {
305-
expect(authStore.clearAuth).toHaveBeenCalledOnce();
307+
expect(authStore.expireToken).toHaveBeenCalledOnce();
306308
expect(mockLocationReplace).toHaveBeenCalledWith("/login");
307309
});
310+
// clearAuth should NOT be called — user config/view preserved on token failure
311+
expect(authStore.clearAuth).not.toHaveBeenCalled();
308312
});
309313

310-
it("does not call clearAuth for non-401 errors", async () => {
314+
it("does not call expireToken or clearAuth for non-401 errors", async () => {
311315
const err500 = Object.assign(new Error("Server Error"), { status: 500 });
312316
vi.mocked(pollService.fetchAllData).mockRejectedValue(err500);
313317

314318
render(() => <DashboardPage />);
315319
// Flush all pending microtasks so the rejected promise settles
316320
await Promise.resolve();
317321
await Promise.resolve();
322+
expect(authStore.expireToken).not.toHaveBeenCalled();
318323
expect(authStore.clearAuth).not.toHaveBeenCalled();
319324
expect(mockLocationReplace).not.toHaveBeenCalledWith("/login");
320325
});

0 commit comments

Comments
 (0)