Fix the redirect after sign-in (#3)

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
+<Route path="/Login" element={authCtx.isLoggedIn ? <LoginRedirect /> : <Login />} />
+```
+
+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=<tampered>
+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 `<form>` has no
+  `onSubmit` and `Button` renders an untyped `<button>`, so "Send OTP" submits
+  natively and the page navigates to `?email=…` before the 1.5s handler can run.
+  Reproduced identically in the original — inherited, not a port defect. Left
+  alone because fixing it changes behaviour rather than restoring it.
+- **`/profile/members` and `/profile/BlogForm` are not access-gated in the UI.**
+  App.jsx only registered those routes for `ADMIN` (and `SENIOR_EXECUTIVE_CREATIVE`
+  for the blog form), so a non-admin hitting the URL fell through to the error
+  page; here they render for any signed-in user. Every mutation behind them is
+  still enforced server-side — `createBlog` checks `canManageBlogs`, `addMember`
+  and `editDetails` return 403 — so the exposure is the admin screen itself, not
+  the ability to use it. The data it lists comes from `fetchTeam`, which is
+  public either way (see below).
 - `/api/user/fetchTeam` returns members' email addresses to anonymous callers.
   Preserved deliberately: trimming the projection changes the response bytes and
   the Team page's sort order. Worth fixing, but it is a behaviour change, not a

app/(auth)/layout.jsx

@@ -2,6 +2,16 @@
  * Auth layout — the `AuthLayout` component from App.jsx.
  * No navbar, no footer, just the `.authpage` wrapper.
  *
+ * App.jsx also guarded these five routes with
+ * `authCtx.isLoggedIn ? <Navigate /> : <Page />`. That is deliberately *not*
+ * reproduced as a wrapper here. Two of the pages sign the user in and then
+ * navigate themselves (SignUp and CompleteProfile both go to "/"), and a
+ * layout-level guard reacting to `isLoggedIn` cancels their in-flight
+ * `router.push` before it commits — measured: the push never reached
+ * `history`. The redirect therefore lives in each component that needs it,
+ * using the `shouldNavigate` state they already carried, and `proxy.ts` covers
+ * a signed-in visitor arriving at one of these URLs.
+ *
  * No Suspense boundary here: wrapping children made prerendered pages emit
  * their markup twice (once inside the streamed boundary, once outside). Pages
  * that read search params declare their own boundary.

src/authentication/Login/GoogleLogin.jsx

@@ -11,6 +11,7 @@ import google from "../../assets/images/google.png";
 import { Alert, MicroLoading } from "../../microInteraction";
 import { api } from "../../services";
 import { useRouter } from "next/navigation";
+import postAuthRedirect from "../../utils/postAuthRedirect";
 
 export default function GoogleLogin() {
   const [alert, setAlert] = useState(null);
@@ -40,9 +41,10 @@ export default function GoogleLogin() {
     }
   }, [alert]);
 
+  // `replace`, not `push`: App.jsx redirected with <Navigate replace />.
   useEffect(() => {
     if (shouldNavigate) {
-      router.push(navigatePath);
+      router.replace(navigatePath);
       setShouldNavigate(false);
     }
   }, [shouldNavigate, navigatePath, router]);
@@ -67,7 +69,7 @@ export default function GoogleLogin() {
             duration: 2800,
           });
 
-          setNavigatePath(sessionStorage.getItem("prevPage") || "/");
+          setNavigatePath(postAuthRedirect());
 
           setTimeout(() => {
             localStorage.setItem("token", response.data.token);
@@ -90,6 +92,10 @@ export default function GoogleLogin() {
               response.data.token,
               9600000
             );
+            // App.jsx re-rendered /Login as <LoginRedirect /> once isLoggedIn
+            // flipped; nothing watches that flag under the App Router, so the
+            // navigation this component was already wired for is triggered here.
+            setShouldNavigate(true);
           }, 800);
 
         } else {

src/authentication/Login/Login.jsx

@@ -17,6 +17,7 @@ import ArrowBackIcon from "@mui/icons-material/ArrowBack";
 import { Alert, MicroLoading } from "../../microInteraction";
 import Link from "next/link";
 import { useRouter } from "next/navigation";
+import postAuthRedirect from "../../utils/postAuthRedirect";
 
 const Login = () => {
   const router = useRouter();
@@ -40,9 +41,11 @@ const Login = () => {
     }
   }, [alert]);
 
+  // `replace`, not `push`: App.jsx redirected with <Navigate replace />, so the
+  // login page must not sit in the history stack behind the destination.
   useEffect(() => {
     if (shouldNavigate) {
-      router.push(navigatePath);
+      router.replace(navigatePath);
       setShouldNavigate(false);
     }
   }, [shouldNavigate, navigatePath, router]);
@@ -81,7 +84,7 @@ const Login = () => {
           duration: 2800,
         });
 
-        setNavigatePath(sessionStorage.getItem("prevPage") || "/");
+        setNavigatePath(postAuthRedirect());
 
         setTimeout(() => {
           localStorage.setItem("token", response.data.token);
@@ -104,6 +107,11 @@ const Login = () => {
             response.data.token,
             9600000
           );
+          // App.jsx re-rendered /Login as <LoginRedirect /> the moment
+          // isLoggedIn flipped. App Router routes are files and nothing watches
+          // that flag, so the navigation this component was already wired for
+          // has to be triggered explicitly.
+          setShouldNavigate(true);
         }, 800);
         // console.log(authCtx);
 

src/authentication/SignUp/GoogleSignup.jsx

@@ -12,6 +12,7 @@ import users from "../../data/user.json";
 import { Alert, MicroLoading } from "../../microInteraction";
 import { api } from "../../services";
 import { useRouter } from "next/navigation";
+import postAuthRedirect from "../../utils/postAuthRedirect";
 
 export default function GoogleSignup({ setAlert }) {
   // const [alert, setAlert] = useState(null);
@@ -35,9 +36,10 @@ export default function GoogleSignup({ setAlert }) {
 
 
 
+  // `replace`, not `push`: App.jsx redirected with <Navigate replace />.
   useEffect(() => {
     if (shouldNavigate) {
-      router.push(navigatePath);
+      router.replace(navigatePath);
       setShouldNavigate(false); // Reset state after navigation
     }
   }, [shouldNavigate, navigatePath, router]);
@@ -64,8 +66,10 @@ export default function GoogleSignup({ setAlert }) {
             position: "bottom-right",
             duration: 3000,
           });
-          setNavigatePath("/");
           sessionStorage.removeItem("prevPage"); // Clean up
+          // Order matters: App.jsx cleared prevPage here and only then rendered
+          // <LoginRedirect />, which therefore fell through to /profile.
+          setNavigatePath(postAuthRedirect());
 
           setTimeout(() => {
             localStorage.setItem("token",response.data.token);
@@ -88,6 +92,10 @@ export default function GoogleSignup({ setAlert }) {
               response.data.token,
               9600000
             );
+            // App.jsx re-rendered /SignUp as <LoginRedirect /> once isLoggedIn
+            // flipped; nothing watches that flag under the App Router, so the
+            // navigation this component was already wired for is triggered here.
+            setShouldNavigate(true);
           }, 800);
         } else {
           // Handle unexpected response status

src/utils/postAuthRedirect.js

@@ -0,0 +1,34 @@
+/**
+ * Where to send someone straight after they sign in.
+ *
+ * Reproduces `LoginRedirect` from FED-Frontend/src/App.jsx:
+ *
+ *   const redirectTo = sessionStorage.getItem("prevPage") || "/profile";
+ *   sessionStorage.removeItem("prevPage");
+ *   return <Navigate to={redirectTo} replace />;
+ *
+ * `?next=` is new: `proxy.ts` appends it when it turns an anonymous request for
+ * a protected route away, which is the server-side equivalent of the
+ * `prevPage` that the original's `ProtectedRoute` stashed. Both mean the same
+ * thing — the page the visitor was actually trying to reach — so either is
+ * accepted, the proxy's first because it is the more recent intent.
+ *
+ * The return path is read from the URL, so unlike the original it is
+ * attacker-supplied: anything that is not a plain internal path is discarded,
+ * the same rule `proxy.ts` applies to the value on its way out.
+ */
+
+const isSafeInternalPath = (path) =>
+  typeof path === "string" && path.startsWith("/") && !path.startsWith("//");
+
+export default function postAuthRedirect() {
+  if (typeof window === "undefined") return "/profile";
+
+  const next = new URLSearchParams(window.location.search).get("next");
+  const prevPage = sessionStorage.getItem("prevPage");
+  sessionStorage.removeItem("prevPage");
+
+  if (isSafeInternalPath(next)) return next;
+  if (isSafeInternalPath(prevPage)) return prevPage;
+  return "/profile";
+}