clean routes
This commit is contained in:
441
plans/2026-01-02_website-auth-route-protection-rethink.md
Normal file
441
plans/2026-01-02_website-auth-route-protection-rethink.md
Normal file
@@ -0,0 +1,441 @@
|
||||
# Website auth + route protection rethink (class-based single concept)
|
||||
|
||||
Goal: replace the current mixed system of Next middleware + client guards + demo cookies + alpha mode branches with **one coherent, predictable system** implemented via a small set of **clean, solid classes**.
|
||||
|
||||
Non-negotiables:
|
||||
|
||||
- **Server-side is canonical** for access control and redirects.
|
||||
- **Client-side is UX only** (show/hide UI, session-aware components) and never a source of truth.
|
||||
- “Demo” is just **a predefined user account**; no special routing/auth logic.
|
||||
- “Alpha mode” is removed; **feature flags** decide what UI/features are visible.
|
||||
|
||||
This plan is designed to keep existing integration coverage in [`tests/integration/website/auth-flow.test.ts`](../tests/integration/website/auth-flow.test.ts:1) passing, adjusting tests only when the old behavior was accidental.
|
||||
|
||||
---
|
||||
|
||||
## 1) Current state (what exists today)
|
||||
|
||||
### 1.1 Server-side (Edge middleware)
|
||||
|
||||
[`apps/website/middleware.ts`](../apps/website/middleware.ts:1) currently:
|
||||
|
||||
- Treats presence of cookie `gp_session` as “authenticated”.
|
||||
- Uses a hardcoded `publicRoutes` array derived from [`routes`](../apps/website/lib/routing/RouteConfig.ts:114).
|
||||
- Redirects unauthenticated users to `/auth/login?returnTo=...`.
|
||||
- Redirects authenticated users away from `/auth/*` pages based on cookie `gridpilot_demo_mode` (special-case sponsor).
|
||||
|
||||
Problems:
|
||||
|
||||
- Cookie presence ≠ valid session (session drift tests exist).
|
||||
- Authorization decisions are made without server-side session validation.
|
||||
- Demo cookies influence routing decisions (non-canonical).
|
||||
|
||||
### 1.2 Client-side (guards + AuthContext)
|
||||
|
||||
- [`apps/website/lib/auth/AuthContext.tsx`](../apps/website/lib/auth/AuthContext.tsx:1) fetches session via `sessionService.getSession()` on mount.
|
||||
- Client-only route wrappers:
|
||||
- [`apps/website/lib/guards/AuthGuard.tsx`](../apps/website/lib/guards/AuthGuard.tsx:1)
|
||||
- [`apps/website/lib/guards/RoleGuard.tsx`](../apps/website/lib/guards/RoleGuard.tsx:1)
|
||||
|
||||
Problems:
|
||||
|
||||
- Double guarding: middleware may redirect, and guards may redirect again after hydration (flicker).
|
||||
- Guards treat “wrong role” like “unauthenticated” (this is fine and matches chosen UX), but enforcement is inconsistent.
|
||||
|
||||
### 1.3 “Alpha mode” and demo exceptions
|
||||
|
||||
- [`apps/website/app/layout.tsx`](../apps/website/app/layout.tsx:1) branches on `mode === 'alpha'` and renders a different shell.
|
||||
- Demo logic leaks into routing via `gridpilot_demo_mode` in middleware (and various components).
|
||||
- Tests currently set cookies like `gridpilot_demo_mode`, sponsor id/name, plus drift cookies (see [`tests/integration/website/websiteAuth.ts`](../tests/integration/website/websiteAuth.ts:1)).
|
||||
|
||||
We will remove all of this:
|
||||
|
||||
- **No alpha mode**: replaced with feature flags.
|
||||
- **No demo routing exceptions**: demo is a user, not a mode.
|
||||
|
||||
---
|
||||
|
||||
## 2) Target concept (one clean concept expressed as classes)
|
||||
|
||||
### 2.1 Definitions
|
||||
|
||||
**Authentication**
|
||||
|
||||
- A request is “authenticated” iff API `/auth/session` (or `/api/auth/session`) returns a valid session object.
|
||||
- The `gp_session` cookie is an opaque session identifier; presence alone is never trusted.
|
||||
|
||||
**Authorization**
|
||||
|
||||
- A request is “authorized” for a route iff the session exists and session role satisfies the route requirement.
|
||||
|
||||
**Canonical redirect behavior (approved)**
|
||||
|
||||
- If route is protected and user is unauthenticated OR unauthorized (wrong role):
|
||||
- redirect to `/auth/login?returnTo=<current path>`.
|
||||
|
||||
This is intentionally strict and matches the existing integration expectations for role checks.
|
||||
|
||||
### 2.2 Where things live (server vs client)
|
||||
|
||||
**Server-side (canonical)**
|
||||
|
||||
- Route protection + redirects, implemented in Next App Router **server layouts**.
|
||||
- Route access matrix is defined once and reused.
|
||||
|
||||
**Client-side (UX only)**
|
||||
|
||||
- `AuthProvider` holds `session` to render navigation, user pill, etc.
|
||||
- Client may refresh session on demand (after login/logout), but not on every navigation.
|
||||
|
||||
---
|
||||
|
||||
## 3) Proposed architecture (clean classes)
|
||||
|
||||
The core idea: build a tiny “auth kernel” for the website that provides:
|
||||
|
||||
- route access decisions (pure)
|
||||
- server session retrieval (gateway)
|
||||
- redirect URL construction (pure + safe)
|
||||
- route enforcement (guards)
|
||||
|
||||
These are classes so responsibilities are explicit, testable, and deletions are easy.
|
||||
|
||||
### 3.1 Class inventory (what we will build)
|
||||
|
||||
This section also addresses the hard requirement:
|
||||
|
||||
- avoid hardcoded route pathnames so we can extend later (e.g. i18n)
|
||||
|
||||
That means:
|
||||
|
||||
- internal logic talks in **route IDs / route patterns**, not raw string paths
|
||||
- redirects are built via **route builders** (locale-aware)
|
||||
- policy checks run on a **normalized logical pathname** (locale stripped)
|
||||
|
||||
#### 3.1.1 `RouteAccessPolicy`
|
||||
|
||||
**Responsibility:** answer “what does this path require?”
|
||||
|
||||
Inputs:
|
||||
|
||||
- `logicalPathname` (normalized path, locale removed; see `PathnameInterpreter`)
|
||||
|
||||
Outputs:
|
||||
|
||||
- `isPublic(pathname): boolean`
|
||||
- `isAuthPage(pathname): boolean` (e.g. `/auth/*`)
|
||||
- `requiredRoles(pathname): string[] | null`
|
||||
- `roleHome(role): string`
|
||||
|
||||
Source of truth for route set:
|
||||
|
||||
- The existing inventory in [`tests/integration/website/websiteRouteInventory.ts`](../tests/integration/website/websiteRouteInventory.ts:1) must remain consistent with runtime rules.
|
||||
- Canonical route constants remain in [`apps/website/lib/routing/RouteConfig.ts`](../apps/website/lib/routing/RouteConfig.ts:114).
|
||||
|
||||
Why a class?
|
||||
|
||||
- Centralizes route matrix and prevents divergence between middleware/guards/layouts.
|
||||
|
||||
Avoiding hardcoded paths:
|
||||
|
||||
- `RouteAccessPolicy` should not hardcode strings like `/auth/login`.
|
||||
- It should instead rely on a `RouteCatalog` (below) that exposes route IDs + patterns derived from [`apps/website/lib/routing/RouteConfig.ts`](../apps/website/lib/routing/RouteConfig.ts:114).
|
||||
|
||||
#### 3.1.2 `ReturnToSanitizer`
|
||||
|
||||
**Responsibility:** make `returnTo` safe and predictable.
|
||||
|
||||
- `sanitizeReturnTo(input: string | null, fallbackPathname: string): string`
|
||||
|
||||
Rules:
|
||||
|
||||
- Only allow relative paths starting with `/`.
|
||||
- Strip protocol/host if someone passes an absolute URL.
|
||||
- Optionally disallow `/api/*` and static assets.
|
||||
|
||||
Why a class?
|
||||
|
||||
- Open redirects become impossible by construction.
|
||||
|
||||
#### 3.1.3 `SessionGateway` (server-only)
|
||||
|
||||
**Responsibility:** fetch the canonical session for the current request.
|
||||
|
||||
- `getSession(): Promise<AuthSessionDTO | null>`
|
||||
|
||||
Implementation details:
|
||||
|
||||
- Use server-side `cookies()` to read the incoming cookies.
|
||||
- Call same-origin `/api/auth/session` so Next rewrites (see [`apps/website/next.config.mjs`](../apps/website/next.config.mjs:52)) forward to the API.
|
||||
- Forward cookies via the `cookie` header.
|
||||
- Treat any non-OK response as `null` (never throw for auth checks).
|
||||
|
||||
Why a class?
|
||||
|
||||
- Encapsulates the “server fetch with forwarded cookies” complexity.
|
||||
|
||||
#### 3.1.4 `AuthRedirectBuilder`
|
||||
|
||||
**Responsibility:** construct redirect targets consistently (and locale-aware).
|
||||
|
||||
- `toLogin({ current }): string` → `<login route>?returnTo=<sanitized current>`
|
||||
- `awayFromAuthPage({ session }): string` → role home (driver/sponsor/admin)
|
||||
|
||||
Internally uses:
|
||||
|
||||
- `RouteAccessPolicy` for roleHome decision
|
||||
- `ReturnToSanitizer` for returnTo
|
||||
- `RoutePathBuilder` (below) so we do not hardcode `/auth/login` or `/dashboard`
|
||||
|
||||
Why a class?
|
||||
|
||||
- Eliminates copy/paste `URLSearchParams` and subtle mismatches.
|
||||
|
||||
#### 3.1.5 `RouteGuard` (server-only)
|
||||
|
||||
**Responsibility:** enforce the policy by redirecting.
|
||||
|
||||
- `enforce({ pathname }): Promise<void>`
|
||||
|
||||
Logic:
|
||||
|
||||
1. If `isPublic(pathname)` and not an auth page: allow.
|
||||
2. If `isAuthPage(pathname)`:
|
||||
- if session exists: redirect to role home
|
||||
- else: allow
|
||||
3. If protected:
|
||||
- if no session: redirect to login
|
||||
- if `requiredRoles(pathname)` and role not included: redirect to login (approved UX)
|
||||
- else: allow
|
||||
|
||||
Why a class?
|
||||
|
||||
- Moves all enforcement into one place.
|
||||
|
||||
#### 3.1.6 `FeatureFlagService` (server + client)
|
||||
|
||||
**Responsibility:** replace “alpha mode” with flags.
|
||||
|
||||
- `isEnabled(flag): boolean`
|
||||
|
||||
Rules:
|
||||
|
||||
- Flags can hide UI or disable pages, but **must not** bypass auth.
|
||||
|
||||
Note: implementation depends on your existing flag system; the plan assumes it exists and becomes the only mechanism.
|
||||
|
||||
### 3.1.7 `PathnameInterpreter` (i18n-ready, server-only)
|
||||
|
||||
**Responsibility:** turn an incoming Next.js `pathname` into a stable “logical” pathname plus locale.
|
||||
|
||||
- `interpret(pathname: string): { locale: string | null; logicalPathname: string }`
|
||||
|
||||
Rules:
|
||||
|
||||
- If later you add i18n where URLs look like `/<locale>/...`, this class strips the locale prefix.
|
||||
- If you add Next `basePath`, this class can also strip it.
|
||||
|
||||
This allows the rest of the auth system to remain stable even if the URL structure changes.
|
||||
|
||||
### 3.1.8 `RouteCatalog` + `RoutePathBuilder` (no hardcoded strings)
|
||||
|
||||
**Responsibility:** remove stringly-typed routes from the auth system.
|
||||
|
||||
`RouteCatalog` exposes:
|
||||
|
||||
- route IDs (e.g. `auth.login`, `protected.dashboard`, `sponsor.dashboard`, `admin.root`)
|
||||
- route patterns (for matching): sourced from [`apps/website/lib/routing/RouteConfig.ts`](../apps/website/lib/routing/RouteConfig.ts:114)
|
||||
- helpers built on existing matching tools like `routeMatchers` in [`apps/website/lib/routing/RouteConfig.ts`](../apps/website/lib/routing/RouteConfig.ts:193)
|
||||
|
||||
`RoutePathBuilder` builds locale-aware URLs:
|
||||
|
||||
- `build(routeId, params?, { locale? }): string`
|
||||
|
||||
Implementation direction:
|
||||
|
||||
- Use the existing `routes` object + `buildPath()` in [`apps/website/lib/routing/RouteConfig.ts`](../apps/website/lib/routing/RouteConfig.ts:307) as the underlying canonical mapping.
|
||||
- Add an optional locale prefix when i18n is introduced.
|
||||
|
||||
With this, auth code never writes literals like `/auth/login`, `/dashboard`, `/sponsor/dashboard`.
|
||||
|
||||
### 3.2 How the classes are used (App Router)
|
||||
|
||||
Route enforcement happens in **server layouts**:
|
||||
|
||||
- [`apps/website/app/dashboard/layout.tsx`](../apps/website/app/dashboard/layout.tsx:1)
|
||||
- [`apps/website/app/admin/layout.tsx`](../apps/website/app/admin/layout.tsx:1)
|
||||
- [`apps/website/app/sponsor/layout.tsx`](../apps/website/app/sponsor/layout.tsx:1)
|
||||
- [`apps/website/app/profile/layout.tsx`](../apps/website/app/profile/layout.tsx:1)
|
||||
- [`apps/website/app/onboarding/layout.tsx`](../apps/website/app/onboarding/layout.tsx:1)
|
||||
|
||||
Each layout becomes a small server component wrapper:
|
||||
|
||||
1. Instantiate `RouteGuard` with its collaborators.
|
||||
2. `PathnameInterpreter` produces `{ locale, logicalPathname }`.
|
||||
3. `await guard.enforce({ logicalPathname, locale })`.
|
||||
3. Render children.
|
||||
|
||||
### 3.3 How matching works without hardcoded paths
|
||||
|
||||
When `RouteGuard` needs to answer questions like “is this an auth page?” or “does this require sponsor role?”, it should:
|
||||
|
||||
- Match `logicalPathname` against patterns from `RouteCatalog`.
|
||||
- Prefer the existing matcher logic in `routeMatchers` (see [`apps/website/lib/routing/RouteConfig.ts`](../apps/website/lib/routing/RouteConfig.ts:193)) so dynamic routes like `/leagues/[id]/settings` continue to work.
|
||||
|
||||
This keeps auth rules stable even if later:
|
||||
|
||||
- `/auth/login` becomes `/de/auth/login`
|
||||
- or `/anmelden` in German via a localized route mapping
|
||||
|
||||
because the matching happens against route IDs/patterns, not by string prefix checks.
|
||||
|
||||
### 3.4 Middleware becomes minimal (or removed)
|
||||
|
||||
After server layouts exist, middleware should either be:
|
||||
|
||||
- **Removed entirely**, or
|
||||
- Reduced to only performance/edge cases (static assets bypass, maybe public route list).
|
||||
|
||||
Important: middleware cannot reliably call backend session endpoint in all environments without complexity/cost; server layouts can.
|
||||
|
||||
### 3.5 Replace alpha mode with feature flags
|
||||
|
||||
Alpha mode branch currently in [`apps/website/app/layout.tsx`](../apps/website/app/layout.tsx:1) should be removed.
|
||||
|
||||
Target:
|
||||
|
||||
- Introduce a feature flags source (existing system in repo) and a small provider.
|
||||
- Feature flags decide:
|
||||
- which navigation items are shown
|
||||
- which pages/features are enabled
|
||||
- which UI shell is used (if we need an “alpha shell”, it’s just a flag)
|
||||
|
||||
Rules:
|
||||
|
||||
- Feature flags must not bypass auth/authorization.
|
||||
- Feature flags must be evaluated server-side for initial render, and optionally rehydrated client-side.
|
||||
|
||||
### 3.6 Demo user without logic exceptions
|
||||
|
||||
Replace “demo mode cookies” with:
|
||||
|
||||
- A standard login flow that returns a normal `gp_session` cookie.
|
||||
- Demo login endpoint remains acceptable in non-production, but it should:
|
||||
- authenticate as a *predefined seeded user*
|
||||
- return a normal session payload
|
||||
- set only `gp_session`
|
||||
- not set or depend on `gridpilot_demo_mode`, sponsor id/name cookies
|
||||
|
||||
Update all UI that reads `gridpilot_demo_mode` to read session role instead.
|
||||
|
||||
---
|
||||
|
||||
## 4) Migration plan (implementation sequence, class-driven)
|
||||
|
||||
This is ordered to keep tests green most of the time and reduce churn.
|
||||
|
||||
### Step 0 — Document and freeze behavior
|
||||
|
||||
- Confirm redirect semantics match integration tests:
|
||||
- unauthenticated protected → `/auth/login?returnTo=...`
|
||||
- wrong-role protected → same redirect
|
||||
- authenticated hitting `/auth/login` → redirect to role home (tests currently assert `/dashboard` or `/sponsor/dashboard`)
|
||||
|
||||
### Step 1 — Introduce the classes (incl. i18n-ready routing)
|
||||
|
||||
- Implement `RouteCatalog` + `RoutePathBuilder` first (removes hardcoded strings, enables i18n later).
|
||||
- Implement `PathnameInterpreter` (normalize pathnames).
|
||||
- Implement `RouteAccessPolicy` + `ReturnToSanitizer` next (pure logic, easy unit tests).
|
||||
- Implement `SessionGateway` (server-only).
|
||||
- Implement `AuthRedirectBuilder` (pure + uses sanitizer/policy).
|
||||
- Implement `RouteGuard` (composition).
|
||||
|
||||
### Step 2 — Convert protected layouts to server enforcement using `RouteGuard`
|
||||
|
||||
### Step 3 — Fix auth routes and redirects (server-first)
|
||||
|
||||
### Step 4 — Remove alpha mode branches and replace with `FeatureFlagService`
|
||||
|
||||
### Step 5 — Remove demo cookies and demo logic exceptions
|
||||
|
||||
### Step 6 — Simplify or delete middleware
|
||||
|
||||
- Remove all `gridpilot_demo_mode`, sponsor id/name cookies usage.
|
||||
- Ensure sponsor role is derived from session.
|
||||
|
||||
### Step 7 — Update integration tests
|
||||
|
||||
- If server layouts cover all protected routes, middleware can be deleted.
|
||||
- If kept, it should only do cheap routing (no role logic, no demo logic).
|
||||
|
||||
### Step 8 — Delete obsolete code + tighten tests
|
||||
|
||||
- Update cookie setup in [`tests/integration/website/websiteAuth.ts`](../tests/integration/website/websiteAuth.ts:1):
|
||||
- stop setting demo cookies
|
||||
- keep drift cookies if still supported by API
|
||||
- rely solely on `gp_session` from demo-login
|
||||
|
||||
- Update expectations in [`tests/integration/website/auth-flow.test.ts`](../tests/integration/website/auth-flow.test.ts:1) only if necessary.
|
||||
|
||||
### Step 9 — Run repo verifications
|
||||
|
||||
- `eslint`
|
||||
- `tsc`
|
||||
- integration tests including [`tests/integration/website/auth-flow.test.ts`](../tests/integration/website/auth-flow.test.ts:1)
|
||||
|
||||
---
|
||||
|
||||
## 5) Files to remove (expected deletions)
|
||||
|
||||
These are the primary candidates to delete because they become redundant or incorrect under the new concept.
|
||||
|
||||
### 5.1 Website auth/route-protection code to delete
|
||||
|
||||
- [`apps/website/lib/guards/AuthGuard.tsx`](../apps/website/lib/guards/AuthGuard.tsx:1)
|
||||
- [`apps/website/lib/guards/RoleGuard.tsx`](../apps/website/lib/guards/RoleGuard.tsx:1)
|
||||
- [`apps/website/lib/guards/AuthGuard.test.tsx`](../apps/website/lib/guards/AuthGuard.test.tsx:1)
|
||||
- [`apps/website/lib/guards/RoleGuard.test.tsx`](../apps/website/lib/guards/RoleGuard.test.tsx:1)
|
||||
|
||||
Rationale: client-side guards are replaced by server-side enforcement in layouts.
|
||||
|
||||
### 5.2 Website Next route handlers that conflict with the canonical API auth flow
|
||||
|
||||
- [`apps/website/app/auth/iracing/start/route.ts`](../apps/website/app/auth/iracing/start/route.ts:1)
|
||||
- [`apps/website/app/auth/iracing/callback/route.ts`](../apps/website/app/auth/iracing/callback/route.ts:1)
|
||||
|
||||
Rationale: these are placeholder/mocks and should be replaced with a single canonical auth flow via the API.
|
||||
|
||||
### 5.3 Website logout route handler (currently incorrect)
|
||||
|
||||
- [`apps/website/app/auth/logout/route.ts`](../apps/website/app/auth/logout/route.ts:1)
|
||||
|
||||
Rationale: deletes `gp_demo_session` instead of `gp_session` and duplicates API logout.
|
||||
|
||||
### 5.4 Demo-cookie driven UI (to remove or refactor)
|
||||
|
||||
These files likely contain `gridpilot_demo_mode` logic and must be refactored to session-based logic; if purely demo-only, delete.
|
||||
|
||||
- [`apps/website/components/dev/DevToolbar.tsx`](../apps/website/components/dev/DevToolbar.tsx:1) (refactor: use session, not demo cookies)
|
||||
- [`apps/website/components/profile/UserPill.tsx`](../apps/website/components/profile/UserPill.tsx:1) (refactor)
|
||||
- [`apps/website/components/sponsors/SponsorInsightsCard.tsx`](../apps/website/components/sponsors/SponsorInsightsCard.tsx:1) (refactor)
|
||||
|
||||
Note: these are not guaranteed deletions, but demo-cookie logic in them must be removed.
|
||||
|
||||
### 5.5 Alpha mode (to remove)
|
||||
|
||||
- “Alpha mode” branching in [`apps/website/app/layout.tsx`](../apps/website/app/layout.tsx:1) should be removed.
|
||||
|
||||
Whether any specific “alpha-only” files are deleted depends on feature flag mapping; the hard requirement is: no `mode === 'alpha'` routing/auth exceptions remain.
|
||||
|
||||
---
|
||||
|
||||
## 6) Acceptance criteria
|
||||
|
||||
- There is exactly one canonical place where access is enforced: server layouts.
|
||||
- Middleware contains no auth/role/demo logic (or is deleted).
|
||||
- Auth logic has zero hardcoded pathname strings; it relies on route IDs + builders and is i18n-ready.
|
||||
- No code uses `gridpilot_demo_mode` or sponsor-id/name cookies to drive auth/redirect logic.
|
||||
- Demo login returns a normal session; “demo user” behaves like any other user.
|
||||
- Alpha mode is removed; feature flags are used instead.
|
||||
- Integration tests under [`tests/integration/website`](../tests/integration/website/auth-flow.test.ts:1) pass.
|
||||
- Repo checks pass: eslint + tsc + tests.
|
||||
Reference in New Issue
Block a user