-
Notifications
You must be signed in to change notification settings - Fork 0
CSRF Update #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CSRF Update #24
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,9 +21,6 @@ import type { | |||||||
| } from '@validation/auth.schema.js'; | ||||||||
| import { sendSuccess } from '@shared/response.js'; | ||||||||
|
|
||||||||
| // ─── Cookie Configuration ─────────────────────────────────────────────────── | ||||||||
|
|
||||||||
| const CSRF_AGE = 15 * 24 * 60 * 60 * 1000; // 15 days | ||||||||
| const REFRESH_AGE = 15 * 24 * 60 * 60 * 1000; // 15 days | ||||||||
| const ACCESS_AGE = 30 * 60 * 1000; // 30 minutes | ||||||||
|
|
||||||||
|
|
@@ -33,21 +30,23 @@ const cookieOptions: CookieOptions = { | |||||||
| sameSite: process.env.NODE_ENV === 'production' ? 'none' : 'lax', | ||||||||
| domain: process.env.NODE_ENV === 'production' ? process.env.PARENT_DOMAIN : undefined, | ||||||||
| maxAge: ACCESS_AGE, | ||||||||
| path: '/', | ||||||||
| }; | ||||||||
|
|
||||||||
| /** Sets the three auth cookies (access / refresh / csrf) on the response. */ | ||||||||
| function setAuthCookies( | ||||||||
| res: Response, | ||||||||
| tokens: { accessToken: string; refreshToken: string; csrfToken: string } | ||||||||
| ) { | ||||||||
| res | ||||||||
| .cookie('access-token', tokens.accessToken, { ...cookieOptions, maxAge: ACCESS_AGE }) | ||||||||
| .cookie('refresh-token', tokens.refreshToken, { ...cookieOptions, maxAge: REFRESH_AGE }) | ||||||||
| .cookie('csrf-token', tokens.csrfToken, { ...cookieOptions, httpOnly: false, maxAge: CSRF_AGE }); | ||||||||
| .cookie('csrf_token', tokens.csrfToken, { | ||||||||
| ...cookieOptions, | ||||||||
| httpOnly: false, | ||||||||
| maxAge: REFRESH_AGE, | ||||||||
| }); | ||||||||
|
Comment on lines
41
to
+47
|
||||||||
| } | ||||||||
|
|
||||||||
| // ─── Controllers ──────────────────────────────────────────────────────────── | ||||||||
|
|
||||||||
| export const registerController = async (req: Request, res: Response) => { | ||||||||
| const body = req.validatedBody as RegisterInput; | ||||||||
| const userAgent = req.headers['user-agent'] || 'unknown'; | ||||||||
|
|
@@ -67,6 +66,7 @@ export const registerController = async (req: Request, res: Response) => { | |||||||
| avatarUrl: user.avatarUrl ?? null, | ||||||||
| }, | ||||||||
| }, | ||||||||
| csrfToken: tokens.csrfToken, | ||||||||
| }); | ||||||||
| }; | ||||||||
|
|
||||||||
|
|
@@ -90,34 +90,45 @@ export const loginController = async (req: Request, res: Response) => { | |||||||
| accountStatus: user.accountStatus, | ||||||||
| }, | ||||||||
| }, | ||||||||
| csrfToken: tokens.csrfToken, | ||||||||
| }); | ||||||||
| }; | ||||||||
|
|
||||||||
| export const forgotPasswordController = async (req: Request, res: Response) => { | ||||||||
| const { email } = req.validatedBody as ForgetPasswordInput; | ||||||||
| await createPasswordResetRequest(email); | ||||||||
| return sendSuccess(res, null, 'If an account with that email exists, a reset link has been sent.'); | ||||||||
| }; | ||||||||
|
|
||||||||
| export const resetPasswordController = async (req: Request, res: Response) => { | ||||||||
| const { token, password } = req.validatedBody as { token: string; password: string }; | ||||||||
| await resetPassword(token, password); | ||||||||
| return sendSuccess(res, null, 'Password updated. Please log in with your new password.'); | ||||||||
| }; | ||||||||
|
|
||||||||
| export const refreshTokenController = async (req: Request, res: Response) => { | ||||||||
| const reqRefreshToken = req.cookies['refresh-token']; | ||||||||
| const userAgent = req.headers['user-agent'] || 'unknown'; | ||||||||
| const ip = req.headers['x-forwarded-for'] || req.headers['x-real-ip'] || req.ip || req.socket.remoteAddress; | ||||||||
| const csrfToken = req.cookies?.['csrf_token']; | ||||||||
|
|
||||||||
| if (!reqRefreshToken) { | ||||||||
| throw new UnauthenticatedError('No refresh token, authentication failed'); | ||||||||
| } | ||||||||
|
|
||||||||
| if (!csrfToken) { | ||||||||
| throw new UnauthenticatedError('Missing CSRF token'); | ||||||||
| } | ||||||||
|
|
||||||||
| const tokens = await refreshTokens(reqRefreshToken, userAgent, ip); | ||||||||
| setAuthCookies(res, tokens); | ||||||||
| const allTokens = { ...tokens, csrfToken }; | ||||||||
| setAuthCookies(res, allTokens); | ||||||||
|
Comment on lines
97
to
+113
|
||||||||
|
|
||||||||
| return res.status(200).json({ | ||||||||
| message: 'Tokens refreshed successfully', | ||||||||
| data: null, | ||||||||
| csrfToken: csrfToken, | ||||||||
| }); | ||||||||
| }; | ||||||||
|
|
||||||||
| return sendSuccess(res, null, 'Tokens refreshed'); | ||||||||
| export const forgotPasswordController = async (req: Request, res: Response) => { | ||||||||
| const { email } = req.validatedBody as ForgetPasswordInput; | ||||||||
| await createPasswordResetRequest(email); | ||||||||
| return sendSuccess(res, null, 'If an account with that email exists, a reset link has been sent.'); | ||||||||
| }; | ||||||||
|
|
||||||||
| export const resetPasswordController = async (req: Request, res: Response) => { | ||||||||
| const { token, password } = req.validatedBody as { token: string; password: string }; | ||||||||
| await resetPassword(token, password); | ||||||||
| return sendSuccess(res, null, 'Password updated. Please log in with your new password.'); | ||||||||
| }; | ||||||||
|
|
||||||||
| export const emailVerificationController = async (req: Request, res: Response) => { | ||||||||
|
|
@@ -146,7 +157,7 @@ export const logoutController = async (req: Request, res: Response) => { | |||||||
| return res | ||||||||
| .clearCookie('access-token') | ||||||||
| .clearCookie('refresh-token') | ||||||||
| .clearCookie('csrf-token') | ||||||||
| .clearCookie('csrf_token') | ||||||||
|
||||||||
| .clearCookie('csrf_token') | |
| .clearCookie('csrf_token') | |
| .clearCookie('csrf-token') |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,12 +28,16 @@ import { | |
| updateProfile, | ||
| } from './user.service.js'; | ||
| import { sendSuccess } from '@shared/response.js'; | ||
| import { BadRequestError } from '@middleware/error/index.js'; | ||
| import { BadRequestError, UnauthorizedError } from '@middleware/error/index.js'; | ||
|
|
||
| export const getProfileController = async (req: Request, res: Response) => { | ||
| const userId = req.user!.id; | ||
| const user = await getProfile(userId); | ||
| return sendSuccess(res, user, 'Profile retrieved successfully'); | ||
|
|
||
| return res.status(200).json({ | ||
| message: 'Profile retrieved successfully', | ||
| data: user, | ||
| }); | ||
|
Comment on lines
+31
to
+40
|
||
| }; | ||
|
|
||
| export const updateProfileController = async (req: Request, res: Response) => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,14 @@ | ||
| import { Types } from "mongoose"; | ||
| import { generateAccessToken, generateCsrfToken, generateRefreshToken } from "./tokens.js"; | ||
| import { AccountStatus } from "@constants/userConsts.js"; | ||
| import { Types } from 'mongoose'; | ||
| import { generateAccessToken, generateRefreshToken } from './tokens.js'; | ||
| import { AccountStatus } from '@constants/userConsts.js'; | ||
|
|
||
| export const generateAllCookieTokens = (userId:Types.ObjectId, sessionId:string, role:string, accountStatus:AccountStatus) => { | ||
| const accessToken = generateAccessToken(userId, sessionId, role, accountStatus); | ||
| const refreshToken = generateRefreshToken(userId, sessionId); | ||
| const csrfToken = generateCsrfToken(); | ||
| return { accessToken, refreshToken, csrfToken }; | ||
| } | ||
| export const generateAllCookieTokens = ( | ||
| userId: Types.ObjectId, | ||
| sessionId: string, | ||
| role: string, | ||
| accountStatus: AccountStatus | ||
| ) => { | ||
| const accessToken = generateAccessToken(userId, sessionId, role, accountStatus); | ||
| const refreshToken = generateRefreshToken(userId, sessionId); | ||
| return { accessToken, refreshToken }; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,9 @@ const ACCESS_TOKEN_SECRET: string = process.env.ACCESS_TOKEN_SECRET!; | |
| const REFRESH_TOKEN_EXPIRES_IN: SignOptions['expiresIn'] = process.env | ||
| .REFRESH_TOKEN_EXPIRES_IN as SignOptions['expiresIn']; | ||
| const REFRESH_TOKEN_SECRET: string = process.env.REFRESH_TOKEN_SECRET!; | ||
| const CSRF_TOKEN_EXPIRES_IN: SignOptions['expiresIn'] = process.env | ||
| .CSRF_TOKEN_EXPIRES_IN as SignOptions['expiresIn']; | ||
| const CSRF_TOKEN_SECRET: string = process.env.CSRF_TOKEN_SECRET!; | ||
|
Comment on lines
+13
to
+15
|
||
|
|
||
| export function generateVerificationToken(bytes = 32) { | ||
| const raw = crypto.randomBytes(bytes).toString('hex'); | ||
|
|
@@ -60,13 +63,14 @@ export function generateAccessToken( | |
| expiresIn: ACCESS_TOKEN_EXPIRES_IN, | ||
| }); | ||
| } | ||
| export function generateRefreshToken(userId: Types.ObjectId, sessionId: string, reduceTime: boolean = false) { | ||
| export function generateRefreshToken(userId: Types.ObjectId, sessionId: string) { | ||
| return jwt.sign({ userId, sessionId }, REFRESH_TOKEN_SECRET, { | ||
| expiresIn: REFRESH_TOKEN_EXPIRES_IN, | ||
| }); | ||
| } | ||
|
|
||
| export function generateCsrfToken() { | ||
| const token = crypto.randomBytes(24).toString('hex'); | ||
| return token; | ||
| export function generateCsrfToken(userId: Types.ObjectId, sessionId: string) { | ||
| return jwt.sign({ userId, sessionId }, CSRF_TOKEN_SECRET, { | ||
| expiresIn: CSRF_TOKEN_EXPIRES_IN, | ||
| }); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new CSRF check does a raw
headerToken !== cookieTokencomparison without normalization/decoding. In practice, cookie values can be quoted or URL-encoded by clients/proxies, which the previousnormalizeTokenhandled; this change can cause valid requests to be rejected. Consider restoring normalization (decode + trim quotes) and using a timing-safe comparison to avoid regressions.