🛂 fix: Normalize Verification Flow Error Responses (#13558)

* fix: normalize verification flow responses

* fix: keep verification responses consistent

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);

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 = {

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));

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();