diff --git a/.planning/phases/24-public-access-infrastructure/24-RESEARCH.md b/.planning/phases/24-public-access-infrastructure/24-RESEARCH.md new file mode 100644 index 0000000..c9f4817 --- /dev/null +++ b/.planning/phases/24-public-access-infrastructure/24-RESEARCH.md @@ -0,0 +1,429 @@ +# Phase 24: Public Access & Infrastructure - Research + +**Researched:** 2026-04-10 +**Domain:** Auth middleware bypass, client-side routing for anonymous users, rate limiting tiers +**Confidence:** HIGH + +## Summary + +Phase 24 removes the login wall from all read-only public routes and adds tiered rate limiting to protect public endpoints. The server-side public allowlist in `src/server/index.ts` (lines 121-140) already includes the four public GET endpoints needed (`/api/global-items`, `/api/tags`, `/api/users/:id/profile`, `/api/setups/:id/public`). The client-side root layout (`__root.tsx`) currently has two blocking problems: it shows a full-page spinner while auth resolves, and it hard-redirects any unauthenticated visitor who isn't on `/users/*` or `/login` directly to `/login` via `window.location.href`. Both must change. + +The `TotalsBar` component already conditionally renders a "Sign in" link for anonymous visitors vs. the `UserMenu` for authenticated users — this part is already done. The primary client-side work is: (1) expand `isPublicRoute` to include `/global-items/*`, `/setups/*` (public view context), and `/`; (2) remove the blocking spinner and the hard redirect; (3) add an inline auth-prompt modal for write action interception. The rate limiter currently has a single hardcoded 5 req/15 min tier that is only appropriate for OAuth endpoints — it needs a factory function producing configurable tiers for browse (higher) and sensitive (low) use. + +**Primary recommendation:** Make `__root.tsx` render-first by removing the `authLoading` spinner gate and the `window.location.href` redirect; widen the public route list; add a `createRateLimit(max, windowMs)` factory to `rateLimit.ts`; apply appropriate tiers to public GET endpoints in `index.ts`. + +--- + + +## User Constraints (from CONTEXT.md) + +### Locked Decisions + +- **D-01:** Keep `requireAuth` as default middleware on `/api/*`. Expand the existing allowlist of public GET routes that skip auth (current pattern: regex checks in `src/server/index.ts`). +- **D-02:** Categories stay auth-gated — they are user-scoped organizational data. Public browsing uses tags (already public via GET `/api/tags`). +- **D-03:** Public setup views include the owner's category names as read-only display context (data already returned by GET `/api/setups/:id/public`). +- **D-04:** Expand the `isPublicRoute` check in `__root.tsx` to include catalog routes (`/global-items/*`), public setup views, and the root `/`. Keep login redirect only for truly private routes (`/collection`, `/settings`, `/threads`). +- **D-05:** No changes needed for TotalsBar — it's already only shown in collection views, not in the global header. (**Note from research: TotalsBar IS the global header; it already handles the Sign-in vs UserMenu conditional — no changes needed.**) +- **D-06:** When an anonymous user attempts a write action (add to collection, create thread, etc.), show an inline popup/modal saying "To manage your own collection, sign in or sign up" with links to both. Do NOT hard-redirect to `/login`. +- **D-07:** Apply rate limiting to all public GET endpoints. Current rate limiter needs new tiers — the existing 5 req/15 min is only appropriate for OAuth. +- **D-08:** Same rate limits for authenticated and anonymous users — no exemptions. +- **D-09:** Fire-and-forget auth check — render the page immediately, check `/api/auth/me` in the background. Anonymous users see content right away with no spinner or redirect. +- **D-10:** Show a "Sign in" button in the top-right corner on all public pages for anonymous visitors. When authenticated, replace with the existing user menu. + +### Claude's Discretion + +- Rate limit numbers: Claude picks appropriate limits per endpoint type (browse, search, detail). Start with reasonable defaults, expect tuning later. + +### Deferred Ideas (OUT OF SCOPE) + +None — discussion stayed within phase scope. + + +--- + + +## Phase Requirements + +| ID | Description | Research Support | +|----|-------------|------------------| +| PUBL-01 | User can browse the global item catalog without logging in | Server allowlist already includes GET `/api/global-items/*`; client `isPublicRoute` must add `/global-items/*` | +| PUBL-02 | User can view public setups without logging in | Server allowlist already includes GET `/api/setups/:id/public`; client must add `/setups/*` to public routes and render a public-safe view for anonymous visitors | +| PUBL-03 | User can view user profiles without logging in | Server allowlist already includes GET `/api/users/:id/profile`; client already treats `/users/*` as public | +| PUBL-04 | Anonymous visitors see the landing page without auth spinner or redirect | Root layout has `authLoading` spinner gate and `window.location.href` hard redirect — both must be removed | +| PUBL-05 | Login is only required when user attempts to create/edit/delete their own data | Write action buttons across the app need to check `isAuthenticated` and show the auth prompt modal instead | +| INFR-01 | Public API endpoints are rate-limited to prevent abuse | `rateLimit.ts` needs configurable tiers; new limits applied to public GET routes in `index.ts` | + + +--- + +## Standard Stack + +### Core (all already in project — no new installs) + +| Library | Version | Purpose | Why Standard | +|---------|---------|---------|--------------| +| Hono | ^4.12.8 | Server middleware and routing | Already in use; `app.use()` middleware chains support per-path rate limit application | +| TanStack Router | ^1.167.0 | Client-side routing | Already in use; `useLocation` and `useMatchRoute` are the tools for `isPublicRoute` logic | +| TanStack React Query | ^5.90.21 | Auth state management | `useAuth()` hook returns `{ data: auth, isLoading }` — the `isLoading` gating in `__root.tsx` is what to remove | +| React 19 | ^19.2.4 | UI | Already in use | + +### No New Dependencies Required + +All required functionality is achievable with existing libraries. The rate limiter refactor is purely internal to `rateLimit.ts`. The auth popup modal follows the existing pattern of inline dialogs already present in `__root.tsx` (CandidateDeleteDialog, ResolveDialog). + +**Installation:** None required. + +--- + +## Architecture Patterns + +### Recommended Approach: Rate Limit Factory + +The existing `rateLimit` middleware is a single closure with hardcoded limits. Convert it to a factory function: + +```typescript +// src/server/middleware/rateLimit.ts +export function createRateLimit(maxAttempts: number, windowMs: number) { + return async function rateLimit(c: Context, next: Next) { + cleanup(); + const ip = getClientIp(c); + const key = `${ip}:${c.req.path}`; + // ... same logic, uses maxAttempts and windowMs + }; +} + +// Keep the original export for backward compatibility (OAuth usage) +export async function rateLimit(c: Context, next: Next) { + return createRateLimit(MAX_ATTEMPTS, WINDOW_MS)(c, next); +} +``` + +Tiers to create (Claude's discretion — reasonable defaults): + +| Tier | Max | Window | Applied to | +|------|-----|--------|------------| +| `browseTier` | 120 req | 1 min | GET `/api/global-items`, GET `/api/tags` | +| `detailTier` | 60 req | 1 min | GET `/api/global-items/:id`, GET `/api/setups/:id/public`, GET `/api/users/:id/profile` | +| `sensitivesTier` | 5 req | 15 min | `/login`, `/api/auth/setup`, OAuth endpoints (existing behavior, unchanged) | + +Rationale: A catalog browse page may make 1 list + N detail requests in a session. 120/min for list endpoints and 60/min for detail endpoints allows normal browsing while still blocking automated scraping. These are generous defaults — D-08 says no auth exemptions so they apply equally to all callers. + +### Server-Side: Applying Rate Limits in `index.ts` + +Apply rate limits as middleware before the `requireAuth` block, scoped to the public GET paths: + +```typescript +// After db injection, before requireAuth block +const browseTier = createRateLimit(120, 60_000); +const detailTier = createRateLimit(60, 60_000); + +app.use("/api/global-items", async (c, next) => { + if (c.req.method === "GET") return browseTier(c, next); + return next(); +}); +app.use("/api/global-items/:id", async (c, next) => { + if (c.req.method === "GET") return detailTier(c, next); + return next(); +}); +app.use("/api/tags", async (c, next) => { + if (c.req.method === "GET") return browseTier(c, next); + return next(); +}); +// etc. +``` + +### Client-Side: `__root.tsx` Restructure + +**Current flow (broken for PUBL-04):** +1. Render spinner while `authLoading === true` +2. After auth resolves: if unauthenticated AND not public route → `window.location.href = "/login"` + +**Required flow (D-09):** +1. Render immediately — no `authLoading` gate +2. Auth resolves in background; UI updates silently (FAB appears, write buttons enable) +3. If unauthenticated AND route is private (`/collection`, `/settings`, `/threads`) → redirect to login + +**Key change in `isPublicRoute`:** +```typescript +// Current +const isPublicRoute = + location.pathname.startsWith("/users/") || location.pathname === "/login"; + +// Required +const isPublicRoute = + location.pathname === "/" || + location.pathname.startsWith("/users/") || + location.pathname.startsWith("/global-items") || + location.pathname.startsWith("/setups/") || // public setup detail view + location.pathname === "/login"; +``` + +**Note on `/setups/$setupId.tsx`:** The current setup detail page is fully authenticated — it includes Add Items, Delete Setup, and Public toggle buttons. For anonymous visitors hitting a `/setups/:id` URL, the page must render only read-only content. The approach: check `isAuthenticated` before rendering write action buttons (same pattern as `showFab`). The underlying data comes from the private `GET /api/setups/:id` endpoint which IS auth-gated. Either: (a) anonymous visitors get the public endpoint response instead (`/api/setups/:id/public`), or (b) a separate route `setups/$setupId.public.tsx` for unauthenticated views. Option (a) is simpler — the public endpoint already exists and is in the server allowlist. + +**Recommended:** Add `/setups/$setupId` to `isPublicRoute` and have the setup detail page detect auth state to decide which API endpoint to call (`useSetup` vs `usePublicSetup`). Since `useSetup` will return 401 for anonymous users anyway, the cleanest approach is to always call the public endpoint for unauthenticated visitors and the private endpoint when authenticated. + +### Auth Prompt Modal Pattern + +New component `AuthPromptModal` (or inline dialog in root): follows the same pattern as `CandidateDeleteDialog` and `ResolveDialog` in `__root.tsx` — a fixed overlay with a centered card. State managed via Zustand `uiStore`. + +```typescript +// In uiStore.ts — add: +showAuthPrompt: boolean; +openAuthPrompt: () => void; +closeAuthPrompt: () => void; +``` + +```typescript +// Modal content: +

Sign in to manage your collection

+

To manage your own collection, sign in or sign up.

+Sign in +Create account {/* Logto handles signup at same endpoint */} +``` + +Note: Logto handles both sign-in and sign-up at the `/login` OIDC redirect. The two links can both go to `/login` — Logto's UI presents options. The UX distinction is in the button labels ("Sign in" vs "Create account"), not different URLs. + +### Write Action Interception + +All write action buttons that anonymous visitors could encounter on public pages need this guard: + +```typescript +// Pattern: check isAuthenticated before write action +function handleAddToCollection() { + if (!isAuthenticated) { + openAuthPrompt(); // from uiStore + return; + } + // existing add logic +} +``` + +Pages with write actions reachable by anonymous visitors: +- `/global-items/$globalItemId` — "Add to Collection" button, "Add to Thread" button +- `/setups/$setupId` (public view) — no write actions needed in read-only view +- `/users/$userId` — read-only, no write actions + +The catalog pages are the primary concern. + +### Anti-Patterns to Avoid + +- **Hard redirecting in `__root.tsx`:** `window.location.href = "/login"` causes a full page reload and is hostile to visitors who haven't signed up. Replace with `navigate({ to: "/login" })` for private-only routes, and only after auth resolves — not during loading. +- **Per-path rate limit keys without normalization:** The current store key is `${ip}:${c.req.path}`. Dynamic segments like `/api/global-items/123` vs `/api/global-items/456` create separate buckets. For detail endpoints, this is fine (per-item limits). For list endpoints, the path is always `/api/global-items` so no issue. +- **Blocking render on auth:** The existing `authLoading` return early renders a spinner. This violates D-09 — remove it entirely. +- **Memory leak in rate limit store:** The `cleanup()` function is called on every request — this is fine for low-traffic but grows with unique IPs. Not a concern for this phase; document as future optimization. + +--- + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | +|---------|-------------|-------------| +| IP extraction from proxy headers | Custom header parsing | Existing `getClientIp()` in `rateLimit.ts` already handles `x-forwarded-for` | +| Auth state in components | Local `useState` for auth | Existing `useAuth()` hook — already cached by React Query | +| Modal overlay | Custom CSS backdrop | Follow existing `CandidateDeleteDialog` pattern in `__root.tsx` — `fixed inset-0 z-50` with `bg-black/30` backdrop | +| Zustand store additions | New store | Extend existing `uiStore.ts` with `showAuthPrompt` boolean | + +--- + +## Common Pitfalls + +### Pitfall 1: `/setups/$setupId` is an Authenticated Route +**What goes wrong:** Adding `/setups/` to `isPublicRoute` lets the anonymous user past the redirect, but `useSetup(id)` calls `GET /api/setups/:id` which requires auth. The request returns 401, the component shows "Setup not found." +**Why it happens:** The private setup endpoint is auth-gated (not in the allowlist). The public variant is `/api/setups/:id/public`. +**How to avoid:** In the setup detail page, detect `isAuthenticated`. If unauthenticated, call `usePublicSetup(id)` (new hook wrapping GET `/api/setups/:id/public`). If authenticated, call `useSetup(id)` as before. Render only read-only content when using the public hook. +**Warning signs:** 404 or empty page for anonymous visitors on `/setups/:id`. + +### Pitfall 2: Onboarding Loading Gate Still Blocks +**What goes wrong:** After removing the `authLoading` spinner, the `onboardingLoading` spinner (lines 147-154 in `__root.tsx`) still blocks render for unauthenticated users. +**Why it happens:** `useOnboardingComplete()` calls an auth-required endpoint. For unauthenticated users, it returns an error, and `onboardingLoading` may be `true` briefly. +**How to avoid:** The `showWizard` check already guards on `isAuthenticated` — `useOnboardingComplete` should be disabled/skipped when not authenticated. Use `{ enabled: isAuthenticated }` in the query options. +**Warning signs:** Anonymous visitors see a spinner on first render even after removing the auth spinner. + +### Pitfall 3: Rate Limiter `_resetForTesting` Not Exported for New Tiers +**What goes wrong:** If `createRateLimit` uses a shared store or the test reset only clears the original store, new-tier tests bleed state between tests. +**Why it happens:** The store `Map` is module-level. Multiple tier instances share it. +**How to avoid:** Either (a) pass a store instance per tier (cleanest), or (b) keep the single module-level store and export `_resetForTesting` (it clears the whole store — fine for tests). Option (b) is simpler given existing test pattern. + +### Pitfall 4: TotalsBar "Sign in" vs D-10 Conflict +**What goes wrong:** D-10 says "Show a 'Sign in' button in the top-right corner on all public pages." The TotalsBar already does this (verified in `TotalsBar.tsx` lines 39-47). But D-05 was misread in the context as "no changes needed for TotalsBar." TotalsBar IS the global header. +**Why it happens:** Context note says "TotalsBar only shown in collection views" — this is inaccurate based on code review. It renders on every page (it's in `__root.tsx` line 159 as ``). +**How to avoid:** No changes to TotalsBar are needed — it already correctly shows "Sign in" for anonymous users. This is already done. The D-10 requirement is satisfied by existing code. + +### Pitfall 5: `window.location.href` vs Router Navigate +**What goes wrong:** Removing the hard redirect to `/login` but forgetting to add a soft redirect for genuinely private routes (`/collection`, `/settings`, `/threads`) means those pages render for anonymous users. +**Why it happens:** The redirect removal is necessary for public routes, but private routes still need protection. +**How to avoid:** Replace `window.location.href = "/login"` with TanStack Router `navigate({ to: "/login" })` scoped only to private routes, and only invoked after `authLoading` is false (not during the loading state). + +--- + +## Code Examples + +### Rate Limit Factory +```typescript +// src/server/middleware/rateLimit.ts +// Source: based on existing implementation in this file + +const store = new Map(); + +export function createRateLimit(maxAttempts: number, windowMs: number) { + return async function(c: Context, next: Next) { + cleanup(); + const ip = getClientIp(c); + const key = `${ip}:${c.req.path}`; + const now = Date.now(); + const entry = store.get(key); + + if (!entry || now >= entry.resetAt) { + store.set(key, { count: 1, resetAt: now + windowMs }); + return next(); + } + if (entry.count >= maxAttempts) { + const retryAfter = Math.ceil((entry.resetAt - now) / 1000); + c.header("Retry-After", String(retryAfter)); + return c.json({ error: "Too many requests. Try again later." }, 429); + } + entry.count++; + return next(); + }; +} + +// Backward-compatible export for existing OAuth usage +export async function rateLimit(c: Context, next: Next) { + return createRateLimit(5, 15 * 60 * 1000)(c, next); +} + +export function _resetForTesting() { + store.clear(); +} +``` + +### Root Layout Auth Fix (key diff) +```typescript +// src/client/routes/__root.tsx — key changes + +// REMOVE: full-page spinner on authLoading +// REMOVE: window.location.href = "/login" + +// REPLACE isPublicRoute with: +const isPublicRoute = + location.pathname === "/" || + location.pathname.startsWith("/users/") || + location.pathname.startsWith("/global-items") || + location.pathname.startsWith("/setups/") || + location.pathname === "/login"; + +// REPLACE hard redirect with soft redirect for private routes only: +if (!isAuthenticated && !isPublicRoute && !authLoading) { + navigate({ to: "/login" }); + return null; +} + +// REMOVE: onboarding spinner gate for unauthenticated users +// (showWizard already guards on isAuthenticated — just remove the separate loading gate) +``` + +### usePublicSetup Hook (new) +```typescript +// src/client/hooks/useSetups.ts — add: +export function usePublicSetup(id: number) { + return useQuery({ + queryKey: ["setups", id, "public"], + queryFn: () => apiGet(`/api/setups/${id}/public`), + }); +} +``` + +--- + +## State of the Art + +| Old Approach | Current Approach | Impact | +|--------------|------------------|--------| +| Auth spinner then redirect (current) | Render immediately, soft redirect only for private routes | PUBL-04 requirement | +| Hard `window.location.href` redirect | TanStack Router `navigate()` | No full page reload, better UX | +| Single rate limit tier (5/15min) | Factory with configurable tiers | Enables appropriate limits per endpoint type | + +--- + +## Open Questions + +1. **`/setups/$setupId` anonymous route — separate page or conditional rendering?** + - What we know: The current setup detail page (`setups/$setupId.tsx`) has heavy write actions (Add Items, Delete, Public toggle) that make no sense for anonymous visitors. + - What's unclear: Whether to build a completely separate read-only public setup page at a new route (e.g., `/setups/$setupId/view`) or gate the existing page's write actions on `isAuthenticated`. + - Recommendation: Keep the single route `/setups/$setupId`. Detect `isAuthenticated`, call the correct API endpoint, and conditionally render write action sections. This is lower scope and matches D-04's intent of "expand public routes" rather than "add new routes." + +2. **Rate limit tier for tag browse endpoint?** + - What we know: GET `/api/tags` is already public; used for filtering in the catalog. + - Recommendation: Apply `browseTier` (120 req/min). Tags are lightweight and unlikely to be abused separately from global items. + +--- + +## Environment Availability + +Step 2.6: SKIPPED — Phase 24 is code-only changes. No external services or CLI tools beyond the existing Bun/Node runtime are introduced. Rate limiting uses the existing in-memory Map. No new database migrations required. + +--- + +## Validation Architecture + +### Test Framework + +| Property | Value | +|----------|-------| +| Framework | Bun test (built-in) | +| Config file | `bunfig.toml` (if present) or none — `bun test` discovers `tests/**/*.test.ts` | +| Quick run command | `bun test tests/middleware/rateLimit.test.ts tests/routes/global-items.test.ts tests/routes/profiles.test.ts` | +| Full suite command | `bun test` | + +### Phase Requirements → Test Map + +| Req ID | Behavior | Test Type | Automated Command | File Exists? | +|--------|----------|-----------|-------------------|-------------| +| PUBL-01 | GET `/api/global-items` returns 200 without auth | unit | `bun test tests/routes/global-items.test.ts` | YES | +| PUBL-02 | GET `/api/setups/:id/public` returns 200 without auth | unit | `bun test tests/routes/profiles.test.ts` (contains public setup tests) | YES | +| PUBL-03 | GET `/api/users/:id/profile` returns 200 without auth | unit | `bun test tests/routes/profiles.test.ts` | YES | +| PUBL-04 | Root layout renders without spinner for anonymous visitor | e2e / manual | `bun run test:e2e` + manual visual check | Wave 0 — e2e test needed | +| PUBL-05 | Write actions intercepted for anonymous users | e2e / manual | `bun run test:e2e` — visit catalog, click "Add to Collection" unauthed | Wave 0 — e2e test needed | +| INFR-01 | Rate limit returns 429 after limit exceeded on public endpoints | unit | `bun test tests/middleware/rateLimit.test.ts` | Partially YES — existing tests cover old API; new tests needed for factory tiers | + +### Sampling Rate + +- **Per task commit:** `bun test tests/middleware/rateLimit.test.ts tests/routes/global-items.test.ts tests/routes/profiles.test.ts` +- **Per wave merge:** `bun test` +- **Phase gate:** Full suite green before `/gsd:verify-work` + +### Wave 0 Gaps + +- [ ] `tests/middleware/rateLimit.test.ts` — extend with `createRateLimit` factory tests (configurable max/window) +- [ ] `e2e/public-access.spec.ts` — covers PUBL-04 (no spinner on load) and PUBL-05 (auth prompt on write action) + +*(Existing test infrastructure covers PUBL-01 through PUBL-03 and INFR-01 base cases.)* + +--- + +## Sources + +### Primary (HIGH confidence) +- Direct code inspection: `src/server/index.ts` — verified existing public route allowlist (lines 121-140) +- Direct code inspection: `src/client/routes/__root.tsx` — verified `authLoading` spinner gate and `window.location.href` hard redirect +- Direct code inspection: `src/server/middleware/rateLimit.ts` — verified current single-tier implementation +- Direct code inspection: `src/client/components/TotalsBar.tsx` — verified "Sign in" link already present for anonymous users +- Direct code inspection: `src/client/hooks/useAuth.ts` — verified React Query `useQuery` with `retry: false` +- Direct code inspection: `tests/middleware/rateLimit.test.ts`, `tests/routes/profiles.test.ts`, `tests/routes/global-items.test.ts` — verified existing test coverage +- Direct code inspection: `package.json` — verified TanStack Router ^1.167.0, React Query ^5.90.21, Hono ^4.12.8 + +### Secondary (MEDIUM confidence) +- None required — all findings are from direct source code inspection + +--- + +## Metadata + +**Confidence breakdown:** +- Standard stack: HIGH — all libraries already in project, verified versions +- Architecture patterns: HIGH — based on direct code reading of files to be modified +- Pitfalls: HIGH — each pitfall identified from actual code behavior verified in source files +- Rate limit tier values: MEDIUM — reasonable defaults per D-08 discretion; expect tuning + +**Research date:** 2026-04-10 +**Valid until:** 2026-05-10 (stable stack, no fast-moving dependencies)