Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds JWT token propagation to the frontend for Socket.IO authentication by including the token in backend auth/session responses and persisting it client-side.
Changes:
- Backend: include
tokeninsendTokenResponseJSON payload. - Backend: include
tokenin/api/auth/sessionresponse when authenticated. - Frontend: store/clear
tokeninlocalStorageduring session check, login/register, and logout.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| frontend/src/context/AuthContext.jsx | Persists response.token to localStorage on auth events and clears it on logout/errors. |
| backend/src/utils/tokenUtils.js | Adds the JWT token field to the JSON body returned alongside the auth cookie. |
| backend/src/controllers/authController.js | Adds token to the authenticated /auth/session response. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .json({ success: true, authenticated: false, user: null }); | ||
|
|
||
| res.status(200).json({ success: true, authenticated: true, user }); | ||
| res.status(200).json({ success: true, authenticated: true, user, token }); |
There was a problem hiding this comment.
getSession is echoing the cookie JWT back in the response body, which exposes the primary auth token to client-side JavaScript and undermines the httpOnly cookie model. If Socket.IO needs auth, prefer cookie-based socket auth (parse cookie server-side) or issue a short-lived secondary token rather than returning the session JWT here.
| res.status(200).json({ success: true, authenticated: true, user, token }); | |
| // Issue a short-lived secondary token instead of exposing the primary cookie JWT | |
| const secondaryToken = jwt.sign( | |
| { id: user.id, purpose: "session" }, | |
| process.env.JWT_SECRET, | |
| { expiresIn: "5m" }, | |
| ); | |
| res | |
| .status(200) | |
| .json({ success: true, authenticated: true, user, token: secondaryToken }); |
| // Store token in localStorage if authenticated for Socket.io connection | ||
| if (response.authenticated && response.token) { | ||
| localStorage.setItem("token", response.token); | ||
| } else { | ||
| localStorage.removeItem("token"); | ||
| } |
There was a problem hiding this comment.
Storing the auth JWT in localStorage makes it retrievable by any XSS payload and increases the blast radius compared to an httpOnly cookie. If the goal is Socket.IO auth, consider authenticating the socket using the existing cookie (send credentials in the handshake and validate cookies server-side) or keep the token only in memory for the current tab/session instead of persistent storage.
| // Store token in localStorage for Socket.io and other client-side usage | ||
| if (response.token) { | ||
| localStorage.setItem("token", response.token); | ||
| } |
There was a problem hiding this comment.
On successful login, the token is only written when response.token is truthy; if the backend ever omits token (or a proxy strips it), an older token already in localStorage would remain and could be used for Socket.IO auth. Consider explicitly removing/overwriting the stored token when the login succeeds but no token is present.
| setIsAuthenticated(true); | ||
| // Store token in localStorage for Socket.io and other client-side usage | ||
| if (response.token) { | ||
| localStorage.setItem("token", response.token); |
There was a problem hiding this comment.
On successful registration, the token is only written when response.token is truthy; if no token is returned, any previous localStorage token would remain and could be used by Socket.IO. Consider explicitly clearing/overwriting the stored token when registration succeeds but no token is present.
| localStorage.setItem("token", response.token); | |
| localStorage.setItem("token", response.token); | |
| } else { | |
| localStorage.removeItem("token"); |
| .json({ success: true, authenticated: false, user: null }); | ||
|
|
||
| res.status(200).json({ success: true, authenticated: true, user }); | ||
| res.status(200).json({ success: true, authenticated: true, user, token }); |
There was a problem hiding this comment.
This change adds a new token field to the /auth/session response but there are no integration tests covering that endpoint. Add/update tests to validate the session response shape (authenticated true/false) and ensure the token behavior matches the intended security model.
| res.status(200).json({ success: true, authenticated: true, user, token }); | |
| res.status(200).json({ success: true, authenticated: true, user }); |
| token, // Include token for Socket.io and client-side storage | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Returning the raw JWT in the JSON response negates the protection provided by an httpOnly cookie and makes the token available to any injected script (XSS), enabling easy token exfiltration/replay. Prefer keeping the JWT only in an httpOnly cookie and authenticating Socket.IO via cookies on the handshake, or mint a separate short-lived, socket-scoped token instead of exposing the primary auth JWT.
| token, // Include token for Socket.io and client-side storage | |
| }); | |
| }; | |
| }); | |
| }; | |
| }; |
… authentication