From 8c71dbcb320cdbea2770317ef77dacce4bb068bf Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sat, 6 Jun 2026 15:08:43 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=82=20fix:=20Normalize=20Verification?= =?UTF-8?q?=20Flow=20Error=20Responses=20(#13558)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: normalize verification flow responses * fix: keep verification responses consistent --- api/server/controllers/UserController.js | 6 +- api/server/controllers/UserController.spec.js | 42 +++++++- api/server/services/AuthService.js | 48 +++++++-- api/server/services/AuthService.spec.js | 102 ++++++++++++++++++ 4 files changed, 184 insertions(+), 14 deletions(-) diff --git a/api/server/controllers/UserController.js b/api/server/controllers/UserController.js index 7884d0a611..7104910b39 100644 --- a/api/server/controllers/UserController.js +++ b/api/server/controllers/UserController.js @@ -388,7 +388,7 @@ const verifyEmailController = async (req, res) => { try { const verifyEmailService = await verifyEmail(req); if (verifyEmailService instanceof Error) { - return res.status(400).json(verifyEmailService); + return res.status(400).json({ message: verifyEmailService.message }); } else { return res.status(200).json(verifyEmailService); } @@ -402,9 +402,9 @@ const resendVerificationController = async (req, res) => { try { const result = await resendVerificationEmail(req); if (result instanceof Error) { - return res.status(400).json(result); + return res.status(400).json({ message: result.message }); } else { - return res.status(200).json(result); + return res.status(result.status ?? 200).json({ message: result.message }); } } catch (e) { logger.error('[verifyEmailController]', e); diff --git a/api/server/controllers/UserController.spec.js b/api/server/controllers/UserController.spec.js index 4dd2efaa7b..6a165fe718 100644 --- a/api/server/controllers/UserController.spec.js +++ b/api/server/controllers/UserController.spec.js @@ -108,9 +108,49 @@ afterEach(async () => { } }); -const { deleteUserController, getUserController } = require('./UserController'); +const { + deleteUserController, + getUserController, + resendVerificationController, + verifyEmailController, +} = require('./UserController'); const { Group } = require('~/db/models'); const { deleteConvos } = require('~/models'); +const { verifyEmail, resendVerificationEmail } = require('~/server/services/AuthService'); + +describe('verifyEmailController', () => { + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('returns the generic verification error message from service failures', async () => { + verifyEmail.mockResolvedValue(new Error('Invalid or expired email verification token')); + + await verifyEmailController( + { body: { email: 'user%40example.com', token: 'not-the-token' } }, + mockRes, + ); + + expect(mockRes.status).toHaveBeenCalledWith(400); + expect(mockRes.json).toHaveBeenCalledWith({ + message: 'Invalid or expired email verification token', + }); + }); + + it('uses the service status for resend verification responses', async () => { + resendVerificationEmail.mockResolvedValue({ status: 500, message: 'Something went wrong.' }); + + await resendVerificationController({ body: { email: 'user@example.com' } }, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(500); + expect(mockRes.json).toHaveBeenCalledWith({ message: 'Something went wrong.' }); + }); +}); describe('getUserController', () => { const mockRes = { diff --git a/api/server/services/AuthService.js b/api/server/services/AuthService.js index 0ffa8c6aee..8f6c281ee1 100644 --- a/api/server/services/AuthService.js +++ b/api/server/services/AuthService.js @@ -52,6 +52,7 @@ const AuthTokenTypes = Object.freeze({ const latestAuthTokenOptions = Object.freeze({ sort: { createdAt: -1 } }); const genericVerificationMessage = 'Please check your email to verify your email address.'; +const invalidEmailVerificationMessage = 'Invalid or expired email verification token'; const OPENID_SESSION_ID_TOKEN_EXPIRY_BUFFER_SECONDS = 30; const findPasswordResetToken = async (userId) => { @@ -251,25 +252,46 @@ const sendVerificationEmail = async (user) => { */ const verifyEmail = async (req) => { const { email, token } = req.body; - const decodedEmail = decodeURIComponent(email); + + if (typeof email !== 'string' || typeof token !== 'string' || !email || !token) { + logger.warn('[verifyEmail] [Invalid email verification request]'); + return new Error(invalidEmailVerificationMessage); + } + + let decodedEmail; + try { + decodedEmail = decodeURIComponent(email); + } catch { + logger.warn(`[verifyEmail] [Invalid email encoding] [Email: ${email}]`); + return new Error(invalidEmailVerificationMessage); + } const user = await findUser({ email: decodedEmail }, 'email _id emailVerified'); if (!user) { logger.warn(`[verifyEmail] [User not found] [Email: ${decodedEmail}]`); - return new Error('User not found'); - } - - if (user.emailVerified) { - logger.info(`[verifyEmail] Email already verified [Email: ${decodedEmail}]`); - return { message: 'Email already verified', status: 'success' }; + return new Error(invalidEmailVerificationMessage); } const emailVerificationData = await findEmailVerificationToken(user); if (!emailVerificationData) { logger.warn(`[verifyEmail] [No email verification data found] [Email: ${decodedEmail}]`); - return new Error('Invalid or expired email verification token'); + return new Error(invalidEmailVerificationMessage); + } + + if (!emailVerificationData.token) { + logger.warn( + `[verifyEmail] [Email verification token data is invalid] [Email: ${decodedEmail}]`, + ); + return new Error(invalidEmailVerificationMessage); + } + + const tokenUserId = emailVerificationData.userId?.toString(); + const userId = user._id?.toString(); + if (!tokenUserId || tokenUserId !== userId) { + logger.warn(`[verifyEmail] [Email verification token user mismatch] [Email: ${decodedEmail}]`); + return new Error(invalidEmailVerificationMessage); } const isValid = bcrypt.compareSync(token, emailVerificationData.token); @@ -278,14 +300,20 @@ const verifyEmail = async (req) => { logger.warn( `[verifyEmail] [Invalid or expired email verification token] [Email: ${decodedEmail}]`, ); - return new Error('Invalid or expired email verification token'); + return new Error(invalidEmailVerificationMessage); + } + + if (user.emailVerified) { + await deleteTokens(getEmailVerificationTokenDeleteQuery(emailVerificationData)); + logger.info(`[verifyEmail] Email already verified [Email: ${decodedEmail}]`); + return { message: 'Email verification was successful', status: 'success' }; } const updatedUser = await updateUser(emailVerificationData.userId, { emailVerified: true }); if (!updatedUser) { logger.warn(`[verifyEmail] [User update failed] [Email: ${decodedEmail}]`); - return new Error('Failed to update user verification status'); + return new Error(invalidEmailVerificationMessage); } await deleteTokens(getEmailVerificationTokenDeleteQuery(emailVerificationData)); diff --git a/api/server/services/AuthService.spec.js b/api/server/services/AuthService.spec.js index 84f758078a..03579e7278 100644 --- a/api/server/services/AuthService.spec.js +++ b/api/server/services/AuthService.spec.js @@ -473,6 +473,108 @@ describe('registerUser', () => { }); }); +describe('verifyEmail public response handling', () => { + const email = 'user@example.com'; + const encodedEmail = encodeURIComponent(email); + const invalidEmailVerificationMessage = 'Invalid or expired email verification token'; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('does not reveal that an account is already verified without a valid token', async () => { + findUser.mockResolvedValue({ _id: 'user-id', email, emailVerified: true }); + findToken.mockResolvedValue(null); + + const result = await verifyEmail({ body: { email: encodedEmail, token: 'not-the-token' } }); + + expect(result).toBeInstanceOf(Error); + expect(result.message).toBe(invalidEmailVerificationMessage); + expect(updateUser).not.toHaveBeenCalled(); + expect(deleteTokens).not.toHaveBeenCalled(); + }); + + it('returns the same generic error for missing users and invalid tokens', async () => { + findUser.mockResolvedValueOnce(null); + + const missingUserResult = await verifyEmail({ + body: { email: encodedEmail, token: 'not-the-token' }, + }); + + findUser.mockResolvedValueOnce({ _id: 'user-id', email, emailVerified: false }); + findToken.mockResolvedValueOnce({ + userId: 'user-id', + email, + token: bcrypt.hashSync('real-token', 10), + }); + + const invalidTokenResult = await verifyEmail({ + body: { email: encodedEmail, token: 'not-the-token' }, + }); + + expect(missingUserResult).toBeInstanceOf(Error); + expect(invalidTokenResult).toBeInstanceOf(Error); + expect(missingUserResult.message).toBe(invalidEmailVerificationMessage); + expect(invalidTokenResult.message).toBe(invalidEmailVerificationMessage); + }); + + it('verifies an unverified account when the token is valid', async () => { + const hashedToken = bcrypt.hashSync('real-token', 10); + findUser.mockResolvedValue({ _id: 'user-id', email, emailVerified: false }); + findToken.mockResolvedValue({ userId: 'user-id', email, token: hashedToken }); + updateUser.mockResolvedValue({ _id: 'user-id', emailVerified: true }); + + const result = await verifyEmail({ body: { email: encodedEmail, token: 'real-token' } }); + + expect(result).toEqual({ + message: 'Email verification was successful', + status: 'success', + }); + expect(updateUser).toHaveBeenCalledWith('user-id', { emailVerified: true }); + expect(deleteTokens).toHaveBeenCalledWith({ + token: hashedToken, + userId: 'user-id', + email, + identifier: null, + type: null, + }); + }); + + it('returns the generic error when a valid verification update fails', async () => { + const hashedToken = bcrypt.hashSync('real-token', 10); + findUser.mockResolvedValue({ _id: 'user-id', email, emailVerified: false }); + findToken.mockResolvedValue({ userId: 'user-id', email, token: hashedToken }); + updateUser.mockResolvedValue(null); + + const result = await verifyEmail({ body: { email: encodedEmail, token: 'real-token' } }); + + expect(result).toBeInstanceOf(Error); + expect(result.message).toBe(invalidEmailVerificationMessage); + expect(deleteTokens).not.toHaveBeenCalled(); + }); + + it('allows idempotent success only when an already verified account presents a valid token', async () => { + const hashedToken = bcrypt.hashSync('real-token', 10); + findUser.mockResolvedValue({ _id: 'user-id', email, emailVerified: true }); + findToken.mockResolvedValue({ userId: 'user-id', email, token: hashedToken }); + + const result = await verifyEmail({ body: { email: encodedEmail, token: 'real-token' } }); + + expect(result).toEqual({ + message: 'Email verification was successful', + status: 'success', + }); + expect(updateUser).not.toHaveBeenCalled(); + expect(deleteTokens).toHaveBeenCalledWith({ + token: hashedToken, + userId: 'user-id', + email, + identifier: null, + type: null, + }); + }); +}); + describe('requestPasswordReset', () => { beforeEach(() => { jest.clearAllMocks();