From 29e7ccc333c49f29519af4e096146a9e8edd9ce4 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 2 Sep 2026 05:31:04 -0700 Subject: [PATCH] fix(auth): make email verification recoverable --- .../unit/controllers/authController.test.js | 50 ++++++++++ backend/controllers/authController.ts | 92 +++++++++++++------ backend/routes/auth.ts | 14 +++ frontend/src/i18n/locales/en.json | 7 ++ frontend/src/i18n/locales/zh-CN.json | 7 ++ frontend/src/v2/__tests__/V2Login.test.tsx | 23 ++++- .../v2/__tests__/v2-layout-invariants.test.ts | 10 ++ frontend/src/v2/components/V2Login.tsx | 50 +++++++++- frontend/src/v2/v2.css | 35 +++++++ 9 files changed, 255 insertions(+), 33 deletions(-) diff --git a/backend/__tests__/unit/controllers/authController.test.js b/backend/__tests__/unit/controllers/authController.test.js index 43fa67d00..40cf9c72d 100644 --- a/backend/__tests__/unit/controllers/authController.test.js +++ b/backend/__tests__/unit/controllers/authController.test.js @@ -7,7 +7,11 @@ const WaitlistRequest = require('../../../models/WaitlistRequest'); jest.mock('../../../services/communityPodService', () => ({ ensureUserInCommunityPod: jest.fn().mockResolvedValue(undefined), })); +jest.mock('../../../services/emailService', () => ({ + sendEmail: jest.fn().mockResolvedValue({ data: { succeeded: 1 } }), +})); const { ensureUserInCommunityPod } = require('../../../services/communityPodService'); +const { sendEmail } = require('../../../services/emailService'); const authController = require('../../../controllers/authController'); const { setupMongoDb, @@ -47,6 +51,9 @@ describe('Auth Controller Tests', () => { afterEach(async () => { await clearMongoDb(); jest.clearAllMocks(); + delete process.env.SMTP2GO_API_KEY; + delete process.env.SMTP2GO_FROM_EMAIL; + delete process.env.FRONTEND_URL; }); describe('register', () => { @@ -315,6 +322,7 @@ describe('Auth Controller Tests', () => { expect(res.json).toHaveBeenCalledWith( expect.objectContaining({ error: expect.stringContaining('Email not verified'), + code: 'EMAIL_UNVERIFIED', }), ); }); @@ -380,6 +388,48 @@ describe('Auth Controller Tests', () => { }); }); + describe('resendVerification', () => { + it('sends a fresh verification link for an unverified human account', async () => { + process.env.SMTP2GO_API_KEY = 'smtp-key'; + process.env.SMTP2GO_FROM_EMAIL = 'mail@example.com'; + process.env.FRONTEND_URL = 'https://commonly.example'; + const user = { + _id: 'unverified-user', + email: 'person@example.com', + verified: false, + }; + User.findOne = jest.fn().mockResolvedValueOnce(user); + const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; + + await authController.resendVerification({ body: { email: 'Person@Example.com' } }, res); + + expect(User.findOne).toHaveBeenCalledWith({ + email: 'person@example.com', + isBot: { $ne: true }, + }); + expect(sendEmail).toHaveBeenCalledWith(expect.objectContaining({ + to: 'person@example.com', + subject: 'Verify Your Email - Commonly', + textBody: expect.stringContaining('https://commonly.example/verify-email?token=test-jwt-token'), + })); + expect(res.json).toHaveBeenCalledWith({ + message: 'If that email has an unverified account, a verification link is on its way.', + }); + }); + + it('answers generically without sending for a verified or unknown address', async () => { + User.findOne = jest.fn().mockResolvedValueOnce(null); + const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; + + await authController.resendVerification({ body: { email: 'nobody@example.com' } }, res); + + expect(sendEmail).not.toHaveBeenCalled(); + expect(res.json).toHaveBeenCalledWith({ + message: 'If that email has an unverified account, a verification link is on its way.', + }); + }); + }); + describe('verifyEmail', () => { it('should verify a user email with valid token', async () => { // Create an unverified user object (not saved to DB) diff --git a/backend/controllers/authController.ts b/backend/controllers/authController.ts index 8e1bd0572..243a82d88 100644 --- a/backend/controllers/authController.ts +++ b/backend/controllers/authController.ts @@ -70,6 +70,26 @@ const isValidEmail = (email: any) => typeof email === 'string' && email.length <= 254 && /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email); +const hasEmailDeliveryConfig = () => Boolean(process.env.SMTP2GO_API_KEY) + && Boolean(process.env.SMTP2GO_FROM_EMAIL) + && Boolean(process.env.FRONTEND_URL); + +// Registration and resend must produce the same link. Keeping the delivery +// details here avoids a resend drifting to a different URL, TTL, or sender. +const sendVerificationEmail = async (user: any) => { + const token = jwt.sign({ id: user._id }, process.env.JWT_SECRET, { + expiresIn: '1d', + }); + const verifyUrl = `${process.env.FRONTEND_URL}/verify-email?token=${token}`; + + return sendEmail({ + to: user.email, + subject: 'Verify Your Email - Commonly', + textBody: `Click the link below to verify your email: ${verifyUrl}`, + htmlBody: `
Click the link below to verify your email:
Verify Email`, + }); +}; + // Give every new signup a default private workspace pod so the BYO // onboarding flow has a target to install/talk-to an agent in. Matches // the Mongo Pod shape created by podController.createPod (type 'chat', @@ -324,9 +344,7 @@ exports.register = async (req: any, res: any) => { }); } - const hasEmailConfig = Boolean(process.env.SMTP2GO_API_KEY) - && Boolean(process.env.SMTP2GO_FROM_EMAIL) - && Boolean(process.env.FRONTEND_URL); + const hasEmailConfig = hasEmailDeliveryConfig(); // When email is not configured there is no way to deliver a verification // link, so auto-verify regardless of NODE_ENV — otherwise a prod deploy // without SMTP would create verified=false accounts that login() permanently @@ -356,28 +374,13 @@ exports.register = async (req: any, res: any) => { if (shouldAutoVerify) joinCommunityPodBestEffort(user._id); if (hasEmailConfig) { - // Generate email verification token - const token = jwt.sign({ id: user._id }, process.env.JWT_SECRET, { - expiresIn: '1d', - }); - - const verifyUrl = `${process.env.FRONTEND_URL}/verify-email?token=${token}`; - const html = `Click the link below to verify your email:
- Verify Email`; - const text = `Click the link below to verify your email: ${verifyUrl}`; - try { console.log('SMTP2GO send attempt:', { - to: email, + to: user.email, sender: process.env.SMTP2GO_FROM_EMAIL, fromName: process.env.SMTP2GO_FROM_NAME, }); - const smtpRes = await sendEmail({ - to: email, - subject: 'Verify Your Email - Commonly', - textBody: text, - htmlBody: html, - }); + const smtpRes = await sendVerificationEmail(user); console.log('SMTP2GO send response:', smtpRes?.data); } catch (sendError: any) { console.error('SMTP2GO error during registration:', sendError?.response?.data || sendError.message); @@ -533,9 +536,7 @@ exports.forgotPassword = async (req: any, res: any) => { if (!email || !isValidEmail(email)) return res.json(genericResponse); const user = await User.findOne({ email, isBot: { $ne: true } }); - const hasEmailConfig = Boolean(process.env.SMTP2GO_API_KEY) - && Boolean(process.env.SMTP2GO_FROM_EMAIL) - && Boolean(process.env.FRONTEND_URL); + const hasEmailConfig = hasEmailDeliveryConfig(); if (!user || !hasEmailConfig) { if (user && !hasEmailConfig) { console.warn('[forgot-password] email delivery not configured; reset link not sent for', email); @@ -566,6 +567,34 @@ exports.forgotPassword = async (req: any, res: any) => { } }; +// 📌 Resend verification — always 200 so this public endpoint cannot reveal +// whether an address has an account. The login form already has a verified +// credential failure to act on; every other caller receives the same response. +exports.resendVerification = async (req: any, res: any) => { + const genericResponse = { + message: 'If that email has an unverified account, a verification link is on its way.', + }; + try { + const email = normalizeEmail(req.body?.email); + if (!email || !isValidEmail(email)) return res.json(genericResponse); + + const user = await User.findOne({ email, isBot: { $ne: true } }); + if (!user || user.verified || !hasEmailDeliveryConfig()) return res.json(genericResponse); + + try { + await sendVerificationEmail(user); + } catch (sendError: any) { + // Keep the response generic, but make delivery failure observable to the + // operator rather than presenting a false promise as success. + console.error('SMTP2GO error during verification resend:', sendError?.response?.data || sendError.message); + } + return res.json(genericResponse); + } catch (err: any) { + console.error('resend-verification failed:', err.message); + return res.json(genericResponse); + } +}; + // 📌 Reset password — consumes the emailed token exports.resetPassword = async (req: any, res: any) => { try { @@ -644,13 +673,6 @@ exports.login = async (req: any, res: any) => { return res.status(403).json({ error: 'This account has been suspended.' }); } - // Check if the email is verified - if (!user.verified) { - return res - .status(400) - .json({ error: 'Email not verified. Please check your inbox.' }); - } - // OAuth-only accounts have no password hash — send them to their provider // instead of letting bcrypt throw on an undefined hash. if (!user.password) { @@ -664,6 +686,16 @@ exports.login = async (req: any, res: any) => { const isMatch = await bcrypt.compare(password, user.password); if (!isMatch) return res.status(400).json({ error: 'Invalid credentials' }); + // Verify the password before exposing an account's verification state. + // A genuine signer gets an actionable code; an attacker gets the same + // invalid-credential result for every account state. + if (!user.verified) { + return res.status(400).json({ + error: 'Email not verified. Please check your inbox.', + code: 'EMAIL_UNVERIFIED', + }); + } + const token = jwt.sign({ id: user._id }, process.env.JWT_SECRET, { expiresIn: '7d', }); diff --git a/backend/routes/auth.ts b/backend/routes/auth.ts index 0921cc38e..d4967bf6d 100644 --- a/backend/routes/auth.ts +++ b/backend/routes/auth.ts @@ -20,6 +20,7 @@ const { requestWaitlist, redeemInvitation, forgotPassword, + resendVerification, resetPassword, } = require('../controllers/authController'); // eslint-disable-next-line global-require @@ -154,6 +155,18 @@ const forgotLimiter = rateLimit({ handler: rateLimitHandler('rate limit exceeded: 5 password-reset requests per hour'), }); +// Like password reset, a resend can amplify outbound mail. It has a separate +// bucket so asking for a verification link never starves password recovery. +const resendVerificationLimiter = rateLimit({ + windowMs: 60 * 60 * 1000, + max: 5, + standardHeaders: true, + legacyHeaders: false, + skip: () => process.env.NODE_ENV === 'test', + keyGenerator: cloudflareIpRateLimitKeyGenerator, + handler: rateLimitHandler('rate limit exceeded: 5 verification emails per hour'), +}); + // Social login shares the credential-stuffing posture of /login: each attempt // is one provider round-trip, so 30/15min/IP leaves room for retries without // letting a bot farm state rows. @@ -255,6 +268,7 @@ router.post('/redeem-invitation', loginLimiter, auth, redeemInvitation); // and always 200s, so it's an outbound-mail amplifier); reset consumes a // signed token so the login limiter's posture suffices. router.post('/forgot-password', forgotLimiter, forgotPassword); +router.post('/resend-verification', resendVerificationLimiter, resendVerification); router.post('/reset-password', loginLimiter, resetPassword); router.post('/refresh', auth, (req: AuthReq, res: Res) => { if (!requireBrowserJwt(req, res)) return; diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 2ed2dc42e..4473f667d 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -149,6 +149,13 @@ "forgotPassword": "Forgot password?", "newToCommonly": "New to Commonly?", "createAccount": "Create an account", + "verification": { + "pending": "{{email}} has not been verified yet. Check that inbox, or request a new link.", + "resend": "Resend verification email", + "sending": "Sending verification email…", + "sent": "Verification email sent to {{email}}.", + "failed": "Could not resend the verification email." + }, "errors": { "failed": "Login failed" }, diff --git a/frontend/src/i18n/locales/zh-CN.json b/frontend/src/i18n/locales/zh-CN.json index f9746a3b9..c7b56d410 100644 --- a/frontend/src/i18n/locales/zh-CN.json +++ b/frontend/src/i18n/locales/zh-CN.json @@ -149,6 +149,13 @@ "forgotPassword": "忘记密码?", "newToCommonly": "还没有 Commonly 账号?", "createAccount": "注册", + "verification": { + "pending": "{{email}} 尚未验证。请查看该邮箱,或重新请求验证链接。", + "resend": "重新发送验证邮件", + "sending": "正在发送验证邮件…", + "sent": "验证邮件已发送至 {{email}}。", + "failed": "无法重新发送验证邮件。" + }, "errors": { "failed": "登录失败" }, diff --git a/frontend/src/v2/__tests__/V2Login.test.tsx b/frontend/src/v2/__tests__/V2Login.test.tsx index 76893687f..fcd8984d2 100644 --- a/frontend/src/v2/__tests__/V2Login.test.tsx +++ b/frontend/src/v2/__tests__/V2Login.test.tsx @@ -1,6 +1,6 @@ // @ts-nocheck import React from 'react'; -import { act, render, screen } from '@testing-library/react'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; import { MemoryRouter, Routes, Route } from 'react-router-dom'; import axios from 'axios'; import { AuthContext } from '../../context/AuthContext'; @@ -76,6 +76,27 @@ describe('V2 routing', () => { expect(screen.getByLabelText(/password/i)).toBeInTheDocument(); }); + test('names an unverified address in an alert and resends its link', async () => { + const login = jest.fn().mockRejectedValue({ + response: { data: { error: 'Email not verified. Please check your inbox.', code: 'EMAIL_UNVERIFIED' } }, + }); + (axios.post as jest.Mock).mockResolvedValueOnce({ data: { message: 'sent' } }); + renderAt('/v2/login', { ...baseAuth, login }); + + fireEvent.change(screen.getByLabelText(/email/i), { target: { value: 'person@example.com' } }); + fireEvent.change(screen.getByLabelText(/password/i), { target: { value: 'Password123!' } }); + fireEvent.click(screen.getByRole('button', { name: /^sign in$/i })); + + expect(await screen.findByRole('alert')).toHaveTextContent('person@example.com has not been verified yet'); + fireEvent.click(screen.getByRole('button', { name: /resend verification email/i })); + + await waitFor(() => expect(axios.post).toHaveBeenCalledWith( + '/api/auth/resend-verification', + { email: 'person@example.com' }, + )); + expect(await screen.findByRole('status')).toHaveTextContent('Verification email sent to person@example.com.'); + }); + test('index route shows the public landing when not authenticated', async () => { renderAt('/v2'); // V2Home sends logged-out visitors to the canonical public front door, diff --git a/frontend/src/v2/__tests__/v2-layout-invariants.test.ts b/frontend/src/v2/__tests__/v2-layout-invariants.test.ts index 36b92ea8a..93d0109ed 100644 --- a/frontend/src/v2/__tests__/v2-layout-invariants.test.ts +++ b/frontend/src/v2/__tests__/v2-layout-invariants.test.ts @@ -113,6 +113,16 @@ describe('v2 layout invariants (CSS rule presence)', () => { expect(v2).toMatch(/@media \(hover: none\), \(pointer: coarse\) \{[\s\S]*?\.v2-root input,\n\s*\.v2-root textarea,\n\s*\.v2-root select \{\n\s*font-size: 16px !important;/); }); + test('signup controls meet the 44px / 16px phone floor', () => { + // Task-103: the full-register form was 39px tall with 14px text at 390px, + // a tap target and Safari zoom regression on the first real-user path. + const phoneStart = v2.indexOf('@media (max-width: 480px)'); + const phoneBlock = v2.slice(phoneStart, v2.indexOf('@media', phoneStart + 10)); + expect(phoneStart).toBeGreaterThan(-1); + expect(phoneBlock).toMatch(/\.v2-login__input \{[\s\S]*?min-height: 44px;[\s\S]*?font-size: 16px;/); + expect(phoneBlock).toMatch(/\.v2-root button\.v2-login__submit \{[\s\S]*?min-height: 44px;[\s\S]*?font-size: 16px;/); + }); + test('Activity queue actions stay in the row grammar and wrap on narrow screens', () => { // Day-zero onboarding and approval actions share the queue row. Keeping // their action cluster explicit prevents a later button refactor from diff --git a/frontend/src/v2/components/V2Login.tsx b/frontend/src/v2/components/V2Login.tsx index 98318e8fa..649f08799 100644 --- a/frontend/src/v2/components/V2Login.tsx +++ b/frontend/src/v2/components/V2Login.tsx @@ -4,6 +4,7 @@ import { useAuth } from '../../context/AuthContext'; import { useTranslation } from 'react-i18next'; import V2OAuthButtons from './V2OAuthButtons'; import V2AuthBrand from './V2AuthBrand'; +import axios from '../../utils/axiosConfig'; interface LocationState { from?: { pathname?: string }; @@ -31,10 +32,15 @@ const V2Login: React.FC = () => { const [password, setPassword] = useState(''); const [submitting, setSubmitting] = useState(false); const [localError, setLocalError] = useState