diff --git a/MIGRATION.md b/MIGRATION.md index 4af73cd..697b516 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -74,6 +74,58 @@ These redirects live in `proxy.ts`, not `next.config.ts`: Next matches a redirect `source` case-insensitively, so a rule from `/Events` to `/events` also matches `/events` and loops forever. +## Route guards — the redirect after sign-in + +The original never navigated from inside `Login.jsx`. It called +`authCtx.login(...)` and let the **route table** react: + +```jsx + : } /> +``` + +App Router routes are files, so nothing observes `isLoggedIn`, and `proxy.ts` +only runs on a server request — which a client-side sign-in never makes. The +first cut of this port dropped the behaviour: a correct login showed "Login +successful" and then sat on `/Login` forever. + +**The redirect belongs in the components, not in a layout wrapper.** A guard in +`app/(auth)/layout.jsx` reacting to `isLoggedIn` was tried first and is wrong: +`SignUP.jsx` and `CompleteProfile.jsx` sign the user in and then navigate +themselves to `/`, and a layout guard cancels that in-flight `router.push` +before it commits. Measured on the signup flow — the push never reached +`history` at all, and a new account landed on `/profile` instead of `/`: + +``` +20494ms resolve /api/auth/register +21025ms history.replaceState(/Login?next=%2Fprofile) <- guard won + (no history.pushState(/) — SignUp's own push was discarded) +``` + +No delay fixes that reliably, because the push only commits once its RSC +payload arrives. So each component owns its own navigation, which is how the +ported source was already written: `Login.jsx`, `GoogleLogin.jsx` and +`GoogleSignup.jsx` all carry `shouldNavigate` / `navigatePath` state and an +effect that acts on it — dead code in the original precisely *because* the route +table did the job. Setting `setShouldNavigate(true)` after `authCtx.login(...)` +brings it to life. `SendOtp.jsx` already did exactly this and needed no change. + +`src/utils/postAuthRedirect.js` resolves the destination the way `LoginRedirect` +did, plus the `?next=` the proxy appends. Because that value now comes off the +query string it is attacker-supplied, so anything that is not a plain internal +path is discarded — `//evil.com` included. + +Verified in the browser by driving the real forms with the API stubbed at the +XHR layer: + +| Flow | Start | Lands on | +|---|---|---| +| Login | `/Login` | **`/profile`** | +| Login | `/Login?next=/Events` | **`/Events`** | +| Login | `/Login?next=//example.com/phish` | **`/profile`** — origin preserved | +| Login | blocked page → login | **back to the blocked page**, `prevPage` cleared | +| Signup | `/SignUp` | **`/`** — matches the original | +| Login | stale localStorage, no cookie | **login form, one bounce, no loop** | + --- ## What was ported @@ -265,8 +317,47 @@ Two further problems surfaced while verifying, both inherited from the original length. The Express controller checked only for presence, so `email: "bad"` was accepted and wrote unreplyable rows into `contactus`. +## Auth routes — verified + +Every auth route was exercised against the running server, signed out and +signed in. `proxy.ts` is the gate; the numbers below are what it returned. + +| Route | Signed out | Signed in | +|---|---|---| +| `/Login` `/SignUp` `/ForgotPassword` `/completeProfile` `/otp` | 200 | **307 → `/profile`** | +| `/profile` and all six sub-pages | **307 → `/Login?next=…`** | 200 | +| `/login` `/signup` `/forgotpassword` `/completeprofile` | 308 → canonical casing | — | + +A forged or expired token is treated as no token, and the bad cookie is cleared +on the way out: + +``` +GET /profile Cookie: token= +307 → /Login?next=%2Fprofile +set-cookie: token=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT +``` + +The seven auth endpoints reject malformed input rather than failing open — +`login`, `register`, `verifyEmail`, `forgotPassword` and `googleAuth` all +answer 400 on an empty body, and `logout` is idempotent. `changePassword` is +the reset step and is gated on a single-use OTP, rate limited, and returns the +same message whether or not the account exists. + ## Known issues +- **`/ForgotPassword` reloads instead of submitting.** Its `
` has no + `onSubmit` and `Button` renders an untyped `