Description
Found via code review on #907 (tryAuthenticate(), be#903): the JWT cookie-verification path checks that the token is validly signed and, via @fastify/jwt's built-in exp handling, unexpired — but never checks the token's type claim.
authenticate()'s cookie branch (src/server/plugins/jwt.ts, around the request.jwtVerify() call) does:
try {
await request.jwtVerify();
} catch {
throw new UnauthenticatedError("Authorization failed.");
}
const userId = request.user?.id;
user = await fastify.db.userRepository.findOne({ where: { id: userId } });
There's no check that request.user.type === "access". Every other token type (verify, reset, refresh) is signed with the same secret and — for sendEmailVerification specifically — with no expiresIn at all:
// src/services/notify/events/email-verification.ts
const token = jwt.sign({ id: user.id, email: user.email, type: "verify" as TokenType });
If a verify token ever leaked (forwarded email, browser history, a log line, etc.) and were set as the access cookie, authenticate() would accept it — permanently, since it never expires — and authenticate as that user on every route with no role restriction.
Ask
Add a type === "access" check to authenticate()'s cookie-verification branch, mirroring the fix already applied narrowly to tryAuthenticate() in #907. This touches every authenticated route in the app, so it deserves its own PR with full regression testing rather than being bundled into that one.
Note
sendEmailVerification's missing expiresIn is arguably worth fixing too (verification links shouldn't be forever-valid credentials), but is a separate, smaller decision from the type check itself — flagging here rather than conflating the two.
Description
Found via code review on #907 (
tryAuthenticate(), be#903): the JWT cookie-verification path checks that the token is validly signed and, via@fastify/jwt's built-inexphandling, unexpired — but never checks the token'stypeclaim.authenticate()'s cookie branch (src/server/plugins/jwt.ts, around therequest.jwtVerify()call) does:There's no check that
request.user.type === "access". Every other token type (verify,reset,refresh) is signed with the same secret and — forsendEmailVerificationspecifically — with noexpiresInat all:If a
verifytoken ever leaked (forwarded email, browser history, a log line, etc.) and were set as theaccesscookie,authenticate()would accept it — permanently, since it never expires — and authenticate as that user on every route with no role restriction.Ask
Add a
type === "access"check toauthenticate()'s cookie-verification branch, mirroring the fix already applied narrowly totryAuthenticate()in #907. This touches every authenticated route in the app, so it deserves its own PR with full regression testing rather than being bundled into that one.Note
sendEmailVerification's missingexpiresInis arguably worth fixing too (verification links shouldn't be forever-valid credentials), but is a separate, smaller decision from thetypecheck itself — flagging here rather than conflating the two.