From e3576b4b3cb0428a6cc738289a66b7951a133320 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Mon, 27 Apr 2026 07:10:41 +0900 Subject: fix(auth): redirect to login page on session expiry Previously when the session expired, the API client cleared tokens but the UI displayed "Invalid or expired token" instead of redirecting to the login page. The root cause was that isAuthenticatedAtom was derived from userAtom only as a re-evaluation trigger, while the actual value came from apiClient.isAuthenticated(). On page reload userAtom is null, so setting it to null on session expiry did not trigger a re-render and ProtectedRoute never redirected. Make userAtom (persisted via atomWithStorage) the single source of truth for auth state, derive isAuthenticatedAtom from it, drop the redundant apiClient.isAuthenticated(), and explicitly navigate to /login on session expiry. Also trigger session expiry when a 401 comes back with no refresh token available. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/client/App.test.tsx | 17 +++++++++------- src/client/api/client.test.ts | 21 -------------------- src/client/api/client.ts | 28 ++++++++++++++------------- src/client/atoms/auth.ts | 27 ++++++++++++-------------- src/client/components/ProtectedRoute.test.tsx | 14 +++++--------- src/client/pages/DeckCardsPage.test.tsx | 2 -- src/client/pages/DeckDetailPage.test.tsx | 2 -- src/client/pages/HomePage.test.tsx | 2 -- src/client/pages/LoginPage.test.tsx | 6 ++---- src/client/pages/NoteTypesPage.test.tsx | 2 -- src/client/pages/StudyPage.test.tsx | 2 -- 11 files changed, 44 insertions(+), 79 deletions(-) (limited to 'src') diff --git a/src/client/App.test.tsx b/src/client/App.test.tsx index 189a8e1..5b1607c 100644 --- a/src/client/App.test.tsx +++ b/src/client/App.test.tsx @@ -8,13 +8,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { Router } from "wouter"; import { memoryLocation } from "wouter/memory-location"; import { App } from "./App"; -import { authLoadingAtom } from "./atoms"; +import { authLoadingAtom, userAtom } from "./atoms"; vi.mock("./api/client", () => ({ apiClient: { login: vi.fn(), logout: vi.fn(), - isAuthenticated: vi.fn(), getTokens: vi.fn(), getAuthHeader: vi.fn(), onSessionExpired: vi.fn(() => vi.fn()), @@ -52,10 +51,17 @@ function mockResponse(data: { >; } -function renderWithRouter(path: string) { +function renderWithRouter( + path: string, + { isAuthenticated = false }: { isAuthenticated?: boolean } = {}, +) { const { hook } = memoryLocation({ path, static: true }); const store = createStore(); store.set(authLoadingAtom, false); + store.set( + userAtom, + isAuthenticated ? { id: "u1", username: "tester" } : null, + ); return render( @@ -68,7 +74,6 @@ function renderWithRouter(path: string) { beforeEach(() => { vi.clearAllMocks(); vi.mocked(apiClient.getTokens).mockReturnValue(null); - vi.mocked(apiClient.isAuthenticated).mockReturnValue(false); }); afterEach(() => { @@ -83,7 +88,6 @@ describe("App routing", () => { accessToken: "access-token", refreshToken: "refresh-token", }); - vi.mocked(apiClient.isAuthenticated).mockReturnValue(true); vi.mocked(apiClient.getAuthHeader).mockReturnValue({ Authorization: "Bearer access-token", }); @@ -96,7 +100,7 @@ describe("App routing", () => { }); it("renders home page at /", () => { - renderWithRouter("/"); + renderWithRouter("/", { isAuthenticated: true }); expect(screen.getByRole("heading", { name: "Kioku" })).toBeDefined(); expect(screen.getByRole("heading", { name: "Your Decks" })).toBeDefined(); }); @@ -105,7 +109,6 @@ describe("App routing", () => { describe("when not authenticated", () => { beforeEach(() => { vi.mocked(apiClient.getTokens).mockReturnValue(null); - vi.mocked(apiClient.isAuthenticated).mockReturnValue(false); }); it("redirects to login when accessing / without authentication", () => { diff --git a/src/client/api/client.test.ts b/src/client/api/client.test.ts index 27c3a0a..e2d314f 100644 --- a/src/client/api/client.test.ts +++ b/src/client/api/client.test.ts @@ -119,27 +119,6 @@ describe("ApiClient", () => { }); }); - describe("isAuthenticated", () => { - it("returns true when tokens exist", () => { - const mockStorage = createMockTokenStorage(); - mockStorage.getTokens.mockReturnValue({ - accessToken: "token", - refreshToken: "refresh", - }); - const client = new ApiClient({ tokenStorage: mockStorage }); - - expect(client.isAuthenticated()).toBe(true); - }); - - it("returns false when no tokens", () => { - const mockStorage = createMockTokenStorage(); - mockStorage.getTokens.mockReturnValue(null); - const client = new ApiClient({ tokenStorage: mockStorage }); - - expect(client.isAuthenticated()).toBe(false); - }); - }); - describe("getAuthHeader", () => { it("returns auth header when tokens exist", () => { const mockStorage = createMockTokenStorage(); diff --git a/src/client/api/client.ts b/src/client/api/client.ts index fc718a2..539df8b 100644 --- a/src/client/api/client.ts +++ b/src/client/api/client.ts @@ -101,16 +101,22 @@ export class ApiClient { const response = await fetch(input, { ...init, headers }); - if (response.status === 401 && tokens?.refreshToken) { - // Try to refresh the token - const refreshed = await this.refreshToken(); - if (refreshed) { - // Retry with new token - const newTokens = this.tokenStorage.getTokens(); - if (newTokens?.accessToken) { - headers.set("Authorization", `Bearer ${newTokens.accessToken}`); + if (response.status === 401 && tokens?.accessToken) { + if (tokens.refreshToken) { + // Try to refresh the token + const refreshed = await this.refreshToken(); + if (refreshed) { + // Retry with new token + const newTokens = this.tokenStorage.getTokens(); + if (newTokens?.accessToken) { + headers.set("Authorization", `Bearer ${newTokens.accessToken}`); + } + return fetch(input, { ...init, headers }); } - return fetch(input, { ...init, headers }); + } else { + // No refresh token available — treat as session expiry + this.tokenStorage.clearTokens(); + this.sessionExpiredCallback?.(); } } @@ -205,10 +211,6 @@ export class ApiClient { this.tokenStorage.clearTokens(); } - isAuthenticated(): boolean { - return this.tokenStorage.getTokens() !== null; - } - getTokens(): Tokens | null { return this.tokenStorage.getTokens(); } diff --git a/src/client/atoms/auth.ts b/src/client/atoms/auth.ts index f618ccf..9ee69cf 100644 --- a/src/client/atoms/auth.ts +++ b/src/client/atoms/auth.ts @@ -1,17 +1,18 @@ import { atom, useSetAtom } from "jotai"; +import { atomWithStorage } from "jotai/utils"; import { useEffect } from "react"; +import { useLocation } from "wouter"; import { apiClient, type User } from "../api/client"; -// Primitive atoms -export const userAtom = atom(null); +// userAtom is the single source of truth for auth state. Persisted to +// localStorage so that the authenticated user survives page reloads alongside +// the tokens in apiClient's token storage. +export const userAtom = atomWithStorage("kioku_user", null); export const authLoadingAtom = atom(true); -// Derived atom - checks if user is authenticated via apiClient -export const isAuthenticatedAtom = atom((get) => { - // We need to trigger re-evaluation when user changes - get(userAtom); - return apiClient.isAuthenticated(); -}); +export const isAuthenticatedAtom = atom( + (get) => get(userAtom) !== null, +); // Action atom - login export const loginAtom = atom( @@ -36,22 +37,18 @@ export const logoutAtom = atom(null, (_get, set) => { export function useAuthInit() { const setAuthLoading = useSetAtom(authLoadingAtom); const setUser = useSetAtom(userAtom); + const [, navigate] = useLocation(); useEffect(() => { - // Check for existing auth on mount - const tokens = apiClient.getTokens(); - if (tokens) { - // We have tokens stored, but we don't have user info cached - // For now, just set authenticated state. User info will be fetched when needed. - } setAuthLoading(false); // Subscribe to session expired events from the API client const unsubscribe = apiClient.onSessionExpired(() => { apiClient.logout(); setUser(null); + navigate("/login", { replace: true }); }); return unsubscribe; - }, [setAuthLoading, setUser]); + }, [setAuthLoading, setUser, navigate]); } diff --git a/src/client/components/ProtectedRoute.test.tsx b/src/client/components/ProtectedRoute.test.tsx index 64a0678..0cf97e3 100644 --- a/src/client/components/ProtectedRoute.test.tsx +++ b/src/client/components/ProtectedRoute.test.tsx @@ -6,14 +6,13 @@ import { createStore, Provider } from "jotai"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { Router } from "wouter"; import { memoryLocation } from "wouter/memory-location"; -import { authLoadingAtom } from "../atoms"; +import { authLoadingAtom, userAtom } from "../atoms"; import { ProtectedRoute } from "./ProtectedRoute"; vi.mock("../api/client", () => ({ apiClient: { login: vi.fn(), logout: vi.fn(), - isAuthenticated: vi.fn(), getTokens: vi.fn(), onSessionExpired: vi.fn(() => vi.fn()), }, @@ -29,20 +28,17 @@ vi.mock("../api/client", () => ({ }, })); -import { apiClient } from "../api/client"; - function renderWithProvider( path: string, atomValues: { isAuthenticated: boolean; isLoading: boolean }, ) { - // Mock the apiClient.isAuthenticated to control isAuthenticatedAtom value - vi.mocked(apiClient.isAuthenticated).mockReturnValue( - atomValues.isAuthenticated, - ); - const { hook } = memoryLocation({ path }); const store = createStore(); store.set(authLoadingAtom, atomValues.isLoading); + store.set( + userAtom, + atomValues.isAuthenticated ? { id: "u1", username: "tester" } : null, + ); return render( diff --git a/src/client/pages/DeckCardsPage.test.tsx b/src/client/pages/DeckCardsPage.test.tsx index 6e8f444..c498056 100644 --- a/src/client/pages/DeckCardsPage.test.tsx +++ b/src/client/pages/DeckCardsPage.test.tsx @@ -22,7 +22,6 @@ vi.mock("../api/client", () => ({ apiClient: { login: vi.fn(), logout: vi.fn(), - isAuthenticated: vi.fn(), getTokens: vi.fn(), getAuthHeader: vi.fn(), onSessionExpired: vi.fn(() => vi.fn()), @@ -228,7 +227,6 @@ describe("DeckCardsPage", () => { accessToken: "access-token", refreshToken: "refresh-token", }); - vi.mocked(apiClient.isAuthenticated).mockReturnValue(true); vi.mocked(apiClient.getAuthHeader).mockReturnValue({ Authorization: "Bearer access-token", }); diff --git a/src/client/pages/DeckDetailPage.test.tsx b/src/client/pages/DeckDetailPage.test.tsx index c473275..a63d8a9 100644 --- a/src/client/pages/DeckDetailPage.test.tsx +++ b/src/client/pages/DeckDetailPage.test.tsx @@ -20,7 +20,6 @@ vi.mock("../api/client", () => ({ apiClient: { login: vi.fn(), logout: vi.fn(), - isAuthenticated: vi.fn(), getTokens: vi.fn(), getAuthHeader: vi.fn(), onSessionExpired: vi.fn(() => vi.fn()), @@ -165,7 +164,6 @@ describe("DeckDetailPage", () => { accessToken: "access-token", refreshToken: "refresh-token", }); - vi.mocked(apiClient.isAuthenticated).mockReturnValue(true); vi.mocked(apiClient.getAuthHeader).mockReturnValue({ Authorization: "Bearer access-token", }); diff --git a/src/client/pages/HomePage.test.tsx b/src/client/pages/HomePage.test.tsx index adcc651..a552c7f 100644 --- a/src/client/pages/HomePage.test.tsx +++ b/src/client/pages/HomePage.test.tsx @@ -22,7 +22,6 @@ vi.mock("../api/client", () => ({ apiClient: { login: vi.fn(), logout: vi.fn(), - isAuthenticated: vi.fn(), getTokens: vi.fn(), getAuthHeader: vi.fn(), onSessionExpired: vi.fn(() => vi.fn()), @@ -152,7 +151,6 @@ describe("HomePage", () => { accessToken: "access-token", refreshToken: "refresh-token", }); - vi.mocked(apiClient.isAuthenticated).mockReturnValue(true); vi.mocked(apiClient.getAuthHeader).mockReturnValue({ Authorization: "Bearer access-token", }); diff --git a/src/client/pages/LoginPage.test.tsx b/src/client/pages/LoginPage.test.tsx index 6ed4011..b559bff 100644 --- a/src/client/pages/LoginPage.test.tsx +++ b/src/client/pages/LoginPage.test.tsx @@ -7,14 +7,13 @@ import { createStore, Provider } from "jotai"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { Router } from "wouter"; import { memoryLocation } from "wouter/memory-location"; -import { authLoadingAtom } from "../atoms"; +import { authLoadingAtom, userAtom } from "../atoms"; import { LoginPage } from "./LoginPage"; vi.mock("../api/client", () => ({ apiClient: { login: vi.fn(), logout: vi.fn(), - isAuthenticated: vi.fn(), getTokens: vi.fn(), onSessionExpired: vi.fn(() => vi.fn()), }, @@ -49,7 +48,6 @@ describe("LoginPage", () => { beforeEach(() => { vi.clearAllMocks(); vi.mocked(apiClient.getTokens).mockReturnValue(null); - vi.mocked(apiClient.isAuthenticated).mockReturnValue(false); }); afterEach(() => { @@ -147,7 +145,6 @@ describe("LoginPage", () => { }); it("redirects when already authenticated", async () => { - vi.mocked(apiClient.isAuthenticated).mockReturnValue(true); vi.mocked(apiClient.getTokens).mockReturnValue({ accessToken: "access-token", refreshToken: "refresh-token", @@ -162,6 +159,7 @@ describe("LoginPage", () => { const store = createStore(); store.set(authLoadingAtom, false); + store.set(userAtom, { id: "u1", username: "tester" }); render( diff --git a/src/client/pages/NoteTypesPage.test.tsx b/src/client/pages/NoteTypesPage.test.tsx index ac68e35..612cf16 100644 --- a/src/client/pages/NoteTypesPage.test.tsx +++ b/src/client/pages/NoteTypesPage.test.tsx @@ -29,7 +29,6 @@ vi.mock("../api/client", () => ({ apiClient: { login: vi.fn(), logout: vi.fn(), - isAuthenticated: vi.fn(), getTokens: vi.fn(), getAuthHeader: vi.fn(), onSessionExpired: vi.fn(() => vi.fn()), @@ -126,7 +125,6 @@ describe("NoteTypesPage", () => { accessToken: "access-token", refreshToken: "refresh-token", }); - vi.mocked(apiClient.isAuthenticated).mockReturnValue(true); vi.mocked(apiClient.getAuthHeader).mockReturnValue({ Authorization: "Bearer access-token", }); diff --git a/src/client/pages/StudyPage.test.tsx b/src/client/pages/StudyPage.test.tsx index 860777a..d9e9d64 100644 --- a/src/client/pages/StudyPage.test.tsx +++ b/src/client/pages/StudyPage.test.tsx @@ -74,7 +74,6 @@ vi.mock("../api/client", () => ({ apiClient: { login: vi.fn(), logout: vi.fn(), - isAuthenticated: vi.fn(), getTokens: vi.fn(), getAuthHeader: vi.fn(), onSessionExpired: vi.fn(() => vi.fn()), @@ -212,7 +211,6 @@ describe("StudyPage", () => { accessToken: "access-token", refreshToken: "refresh-token", }); - vi.mocked(apiClient.isAuthenticated).mockReturnValue(true); vi.mocked(apiClient.getAuthHeader).mockReturnValue({ Authorization: "Bearer access-token", }); -- cgit v1.3-3-g829e