From b0aeb06f850d5b5e9730be7d8d72e35bedd3b1a0 Mon Sep 17 00:00:00 2001 From: Steven Date: Thu, 18 Dec 2025 22:14:30 +0800 Subject: [PATCH] refactor(web): improve auth flow and eliminate route duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract route paths to router/routes.ts as single source of truth - Refactor connect.ts auth interceptor with better structure and error handling - Add TokenRefreshManager class to prevent race conditions - Implement smart redirect logic for public/private routes - Support unauthenticated access to explore and user profile pages - Add proper error handling for missing access tokens - Extract magic strings to named constants - Maintain backward compatibility by aliasing Routes to ROUTES 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- web/src/connect.ts | 227 ++++++++++++++++++++++++++------------- web/src/router/index.tsx | 15 +-- web/src/router/routes.ts | 13 +++ 3 files changed, 168 insertions(+), 87 deletions(-) create mode 100644 web/src/router/routes.ts diff --git a/web/src/connect.ts b/web/src/connect.ts index 7f098ab89..e2ce276ec 100644 --- a/web/src/connect.ts +++ b/web/src/connect.ts @@ -2,6 +2,8 @@ import { timestampDate } from "@bufbuild/protobuf/wkt"; import { Code, ConnectError, createClient, type Interceptor } from "@connectrpc/connect"; import { createConnectTransport } from "@connectrpc/connect-web"; import { getAccessToken, setAccessToken } from "./auth-state"; +import { ROUTES } from "./router/routes"; +import { instanceStore } from "./store"; import { ActivityService } from "./types/proto/api/v1/activity_service_pb"; import { AttachmentService } from "./types/proto/api/v1/attachment_service_pb"; import { AuthService } from "./types/proto/api/v1/auth_service_pb"; @@ -11,18 +13,126 @@ import { MemoService } from "./types/proto/api/v1/memo_service_pb"; import { ShortcutService } from "./types/proto/api/v1/shortcut_service_pb"; import { UserService } from "./types/proto/api/v1/user_service_pb"; -let isRefreshing = false; -let refreshPromise: Promise | null = null; +// ============================================================================ +// Constants +// ============================================================================ + +const RETRY_HEADER = "X-Retry"; +const RETRY_HEADER_VALUE = "true"; + +const ROUTE_CONFIG = { + // Routes accessible without authentication (uses prefix matching) + public: [ + ROUTES.AUTH, // Authentication pages + ROUTES.EXPLORE, // Explore page + "/u/", // User profile pages (dynamic) + "/memos/", // Individual memo detail pages (dynamic) + ], + + // Routes that require authentication (uses exact matching) + private: [ROUTES.ROOT, ROUTES.ATTACHMENTS, ROUTES.INBOX, ROUTES.ARCHIVED, ROUTES.SETTING], +} as const; + +// ============================================================================ +// Token Refresh State Management +// ============================================================================ + +class TokenRefreshManager { + private isRefreshing = false; + private refreshPromise: Promise | null = null; + + async refresh(refreshFn: () => Promise): Promise { + if (this.isRefreshing && this.refreshPromise) { + return this.refreshPromise; + } + + this.isRefreshing = true; + this.refreshPromise = refreshFn().finally(() => { + this.isRefreshing = false; + this.refreshPromise = null; + }); + + return this.refreshPromise; + } + + isCurrentlyRefreshing(): boolean { + return this.isRefreshing; + } +} + +const tokenRefreshManager = new TokenRefreshManager(); + +// ============================================================================ +// Route Access Control +// ============================================================================ + +function isPublicRoute(path: string): boolean { + return ROUTE_CONFIG.public.some((route) => path.startsWith(route)); +} + +function isPrivateRoute(path: string): boolean { + return (ROUTE_CONFIG.private as readonly string[]).includes(path); +} + +function getAuthFailureRedirect(currentPath: string): string | null { + if (isPublicRoute(currentPath)) { + return null; + } + + if (instanceStore.state.memoRelatedSetting.disallowPublicVisibility) { + return ROUTES.AUTH; + } + + if (isPrivateRoute(currentPath)) { + return ROUTES.EXPLORE; + } + + return null; +} + +function performRedirect(redirectUrl: string | null): void { + if (redirectUrl) { + window.location.href = redirectUrl; + } +} + +// ============================================================================ +// Token Refresh +// ============================================================================ + +const fetchWithCredentials: typeof globalThis.fetch = (input, init) => { + return globalThis.fetch(input, { + ...init, + credentials: "include", + }); +}; + +// Separate transport without auth interceptor to prevent recursion +const refreshTransport = createConnectTransport({ + baseUrl: window.location.origin, + useBinaryFormat: true, + fetch: fetchWithCredentials, + interceptors: [], +}); + +const refreshAuthClient = createClient(AuthService, refreshTransport); + +async function refreshAccessToken(): Promise { + const response = await refreshAuthClient.refreshToken({}); + + if (!response.accessToken) { + throw new ConnectError("Refresh token response missing access token", Code.Internal); + } + + const expiresAt = response.expiresAt ? timestampDate(response.expiresAt) : undefined; + setAccessToken(response.accessToken, expiresAt); +} + +// ============================================================================ +// Authentication Interceptor +// ============================================================================ -/** - * Authentication interceptor that: - * 1. Attaches access token to outgoing requests - * 2. Handles 401 Unauthenticated errors by refreshing the token - * 3. Retries the original request with the new token - * 4. Redirects to login if refresh fails - */ const authInterceptor: Interceptor = (next) => async (req) => { - // Add access token to request if available const token = getAccessToken(); if (token) { req.header.set("Authorization", `Bearer ${token}`); @@ -31,78 +141,41 @@ const authInterceptor: Interceptor = (next) => async (req) => { try { return await next(req); } catch (error) { - // Only handle ConnectError with Unauthenticated code - if (error instanceof ConnectError && error.code === Code.Unauthenticated && !req.header.get("X-Retry")) { - // Prevent concurrent refresh attempts - if (!isRefreshing) { - isRefreshing = true; - refreshPromise = refreshAccessToken(); - } - - try { - await refreshPromise; - isRefreshing = false; - refreshPromise = null; - - // Retry with new token - const newToken = getAccessToken(); - if (newToken) { - req.header.set("Authorization", `Bearer ${newToken}`); - req.header.set("X-Retry", "true"); - return await next(req); - } - } catch (refreshError) { - isRefreshing = false; - refreshPromise = null; - // Refresh failed - redirect to login (only if not already there) - if (!window.location.pathname.startsWith("/auth")) { - window.location.href = "/auth"; - } - throw refreshError; - } + if (!(error instanceof ConnectError)) { + throw error; + } + + if (error.code !== Code.Unauthenticated) { + throw error; + } + + if (req.header.get(RETRY_HEADER) === RETRY_HEADER_VALUE) { + throw error; + } + + try { + await tokenRefreshManager.refresh(refreshAccessToken); + + const newToken = getAccessToken(); + if (!newToken) { + throw new ConnectError("Token refresh succeeded but no token available", Code.Internal); + } + + req.header.set("Authorization", `Bearer ${newToken}`); + req.header.set(RETRY_HEADER, RETRY_HEADER_VALUE); + return await next(req); + } catch (refreshError) { + const redirectUrl = getAuthFailureRedirect(window.location.pathname); + performRedirect(redirectUrl); + throw refreshError; } - throw error; } }; -/** - * Custom fetch that includes credentials for cookie handling. - * Required for HttpOnly refresh token cookie to be sent/received. - */ -const fetchWithCredentials: typeof globalThis.fetch = (input, init) => { - return globalThis.fetch(input, { - ...init, - credentials: "include", - }); -}; +// ============================================================================ +// Transport & Service Clients +// ============================================================================ -/** - * Separate transport for refresh token operations. - * Uses no auth interceptor to avoid circular dependency when the main - * interceptor triggers a refresh. - */ -const refreshTransport = createConnectTransport({ - baseUrl: window.location.origin, - useBinaryFormat: true, - fetch: fetchWithCredentials, - interceptors: [], // No interceptors to avoid recursion -}); - -// Dedicated auth client for refresh operations only -const refreshAuthClient = createClient(AuthService, refreshTransport); - -/** - * Refreshes the access token using the HttpOnly refresh token cookie. - * Called automatically by the auth interceptor when requests fail with 401. - */ -async function refreshAccessToken(): Promise { - const response = await refreshAuthClient.refreshToken({}); - setAccessToken(response.accessToken, response.expiresAt ? timestampDate(response.expiresAt) : undefined); -} - -/** - * Main transport for all API requests. - */ const transport = createConnectTransport({ baseUrl: window.location.origin, useBinaryFormat: true, diff --git a/web/src/router/index.tsx b/web/src/router/index.tsx index 017665877..25387d59f 100644 --- a/web/src/router/index.tsx +++ b/web/src/router/index.tsx @@ -22,16 +22,11 @@ const SignUp = lazy(() => import("@/pages/SignUp")); const UserProfile = lazy(() => import("@/pages/UserProfile")); const MemoDetailRedirect = lazy(() => import("./MemoDetailRedirect")); -export enum Routes { - ROOT = "/", - ATTACHMENTS = "/attachments", - CALENDAR = "/calendar", - INBOX = "/inbox", - ARCHIVED = "/archived", - SETTING = "/setting", - EXPLORE = "/explore", - AUTH = "/auth", -} +import { ROUTES } from "./routes"; + +// Backward compatibility alias +export const Routes = ROUTES; +export { ROUTES }; const router = createBrowserRouter([ { diff --git a/web/src/router/routes.ts b/web/src/router/routes.ts new file mode 100644 index 000000000..8f0058b57 --- /dev/null +++ b/web/src/router/routes.ts @@ -0,0 +1,13 @@ +export const ROUTES = { + ROOT: "/", + ATTACHMENTS: "/attachments", + CALENDAR: "/calendar", + INBOX: "/inbox", + ARCHIVED: "/archived", + SETTING: "/setting", + EXPLORE: "/explore", + AUTH: "/auth", +} as const; + +export type RouteKey = keyof typeof ROUTES; +export type RoutePath = (typeof ROUTES)[RouteKey];