📧 feat: add email address change - #14597
Conversation
ed8df3d to
eeb7a57
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a full “change registered email” flow for local accounts (Settings → Account), including current-password confirmation, email verification to the new address, security notifications to both old/new addresses, and E2E coverage for enabled/disabled modes. It spans backend token/storage changes, new API endpoints, client UI/UX, and Playwright-based E2E infrastructure with an in-process SMTP mailbox.
Changes:
- Introduces an email-change service and API endpoints to request + confirm email changes (with password confirmation, token verification, and notification emails).
- Adds token “scope” support plus an atomic
upsertTokenpath to ensure only one pending email-change token exists per user. - Adds client Settings UI + verify page handling for email-change verification links, plus dedicated E2E test profile (including a local SMTP mailbox server) and CI workflow steps.
Reviewed changes
Copilot reviewed 47 out of 47 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/data-schemas/src/types/token.ts | Adds optional scope to token typings for scoped token operations. |
| packages/data-schemas/src/schema/token.ts | Adds scope field and a unique sparse index to support scoped token uniqueness. |
| packages/data-schemas/src/methods/user.ts | Extends updateUser to support conditional updates via expectedState and invalidates auth cache. |
| packages/data-schemas/src/methods/user.methods.spec.ts | Tests conditional updateUser behavior with expected-state matching. |
| packages/data-schemas/src/methods/token.ts | Adds upsertToken and supports querying/deleting by scope. |
| packages/data-schemas/src/methods/token.spec.ts | Adds concurrency coverage for upsertToken in the email-change scope. |
| packages/data-provider/src/types.ts | Adds request/confirm payloads and error-code typing for email change responses. |
| packages/data-provider/src/data-service.ts | Adds requestEmailChange and confirmEmailChange API calls. |
| packages/data-provider/src/config.ts | Adds allowEmailChange to startup config typing for client feature gating. |
| packages/data-provider/src/api-endpoints.ts | Adds API endpoints for requesting and confirming email changes. |
| packages/api/src/auth/index.ts | Re-exports the new email-change auth module. |
| packages/api/src/auth/email.ts | Implements the email-change service: request flow, confirm flow, token handling, and notifications. |
| packages/api/src/auth/email.spec.ts | Unit tests for email-change request/confirm behavior and edge cases. |
| package.json | Adds e2e:email-change and e2e:email-change:disabled scripts. |
| e2e/specs/email/email-change.spec.ts | E2E test for changing email via settings + verifying via delivered link + notifications. |
| e2e/specs/email/email-change-disabled.spec.ts | E2E test ensuring UI is hidden and endpoints reject when disabled (no mail sent). |
| e2e/setup/users.email.ts | Test user + target email helpers for email-change E2E profile. |
| e2e/setup/mailbox.ts | Mailbox HTTP client utilities to query/clear/wait for messages and extract verification URLs. |
| e2e/setup/mailbox-server.js | In-process SMTP + HTTP mailbox server used by email E2E profile. |
| e2e/setup/global-teardown.email.ts | Cleans up E2E users/emails after the email-change profile run. |
| e2e/setup/global-setup.email.ts | Registers and verifies the E2E account via delivered email, then saves storage state. |
| e2e/setup/authenticate.ts | Refactors registration helper (submitRegistration) and exports login/appURL helpers used by email setup. |
| e2e/README.md | Documents the dedicated email-delivery E2E profile and disabled-mode coverage. |
| e2e/playwright.config.email.ts | Adds an email-specific Playwright config that starts the mailbox server and sets email env vars. |
| client/src/utils/errors.ts | Adds getResponseErrorCode helper for typed error-code extraction from Axios responses. |
| client/src/locales/en/translation.json | Adds UI/auth strings for email-change flow and error messaging. |
| client/src/data-provider/Auth/mutations.ts | Adds React Query mutations for request/confirm email change, invalidating user query on confirm. |
| client/src/components/Nav/SettingsTabs/Account/ChangeEmail.tsx | Adds Settings → Account UI dialog to request email change with current-password confirmation. |
| client/src/components/Nav/Settings/types.ts | Extends settings context type with emailEnabled and allowEmailChange. |
| client/src/components/Nav/Settings/registry.tsx | Registers the Change Email setting entry and gates visibility by provider + email + allow flag. |
| client/src/components/Nav/Settings/context.tsx | Derives emailEnabled and allowEmailChange from startup config for UI gating. |
| client/src/components/Nav/Settings/tests/Sidebar.spec.tsx | Updates settings test context to include new flags. |
| client/src/components/Nav/Settings/tests/registry.spec.ts | Adds visibility tests for the change-email entry under different gating conditions. |
| client/src/components/Auth/VerifyEmail.tsx | Extends /verify page to handle type=email-change confirmations (calls confirm mutation). |
| client/src/components/Auth/tests/LoginForm.spec.tsx | Updates startup config mock to include allowEmailChange. |
| api/server/utils/emails/verifyEmailChange.handlebars | Adds template for “verify your new email address” message. |
| api/server/utils/emails/emailChanged.handlebars | Adds template notifying both addresses that the email was changed (includes IP/time). |
| api/server/utils/emails/emailChangeAttempt.handlebars | Adds template notifying the old address that an email change was requested (includes IP/time). |
| api/server/services/AuthService.spec.js | Updates password-reset tests for typed tokens and cleanup of pending email changes. |
| api/server/services/AuthService.js | Binds password-reset tokens to the current email and cleans up pending email-change tokens on reset. |
| api/server/routes/user.verify-ratelimit.test.js | Ensures confirmation route is rate-limited before token checks. |
| api/server/routes/user.js | Adds /api/user/email/change and /api/user/email/verify routes. |
| api/server/routes/config.js | Switches to checkEmailConfig() and adds allowEmailChange to post-login config. |
| api/server/routes/tests/config.spec.js | Adds tests for allowEmailChange inclusion/behavior. |
| api/server/controllers/UserController.js | Wires controllers for request/confirm email change using tenant-aware execution and shared service. |
| .github/workflows/playwright-mock.yml | Runs the new email-change E2E profile(s) in CI on the mock workflow. |
| .env.example | Documents ALLOW_EMAIL_CHANGE env var (default enabled when omitted). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb987d4e39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| findUserByEmail: (email, tenantId) => | ||
| withTenant(tenantId, () => db.findUser({ email }, 'email _id tenantId')), |
There was a problem hiding this comment.
Scope legacy-user email conflicts to the default tenant
When an account has no tenantId, withTenant switches findUserByEmail to runAsSystem, so the availability check searches every tenant. Because the user index permits the same email in different tenants, a legacy/default-tenant user trying to adopt an address used only in an unrelated tenant receives email_in_use even though the database would allow the update; confirmation has the same problem. Keep the lookup constrained to tenant-less users rather than using the cross-tenant system context.
Useful? React with 👍 / 👎.
| (passwordResetToken.type === AuthTokenTypes.PASSWORD_RESET && | ||
| (!passwordResetToken.email || | ||
| passwordResetToken.email.toLowerCase() !== currentUser.email.toLowerCase())) |
There was a problem hiding this comment.
Preserve reset tokens issued before email binding
During a rolling deployment, an old server can still issue a typed password_reset token without an email, while a new server handling the submitted link rejects it here solely because the field is absent. The parent version issues exactly that token shape, so otherwise valid 15-minute reset links fail nondeterministically during and shortly after rollout. Distinguish pre-binding tokens from tokens explicitly bound to another email, or otherwise provide rollout compatibility.
Useful? React with 👍 / 👎.
| logger.warn( | ||
| `[emailChange] Incorrect password [User ID: ${userId}] [New Email: ${newEmail}] [IP: ${ip}]`, | ||
| ); | ||
| return result(401, 'Current password is incorrect', 'current_password_invalid'); |
There was a problem hiding this comment.
Avoid using 401 for a wrong current password
When the authenticated user enters an incorrect current password, this 401 is treated by the global Axios response interceptor in packages/data-provider/src/request.ts as an expired bearer token: it refreshes authentication and automatically repeats the email-change request. The retry sends a second change-attempt notification and, with the default VERIFY_EMAIL_MAX=2, consumes the entire request limit so the user's next corrected attempt is rejected with 429 for two minutes. Return a non-authentication status such as 400/403, or explicitly exempt this endpoint from auth recovery.
Useful? React with 👍 / 👎.
| setHeaderText(localize(messageKey) + ' 😢'); | ||
| setShowResendLink(false); | ||
| setVerificationStatus(true); |
There was a problem hiding this comment.
Stop the redirect countdown on failed email changes
When an email-change link is expired, invalid, or conflicts with another account, this handler marks verification complete but leaves countdown at its initial value of 3 and never starts countdownRedirect. The completed-state renderer therefore displays “Redirecting in 3 seconds” indefinitely beneath the failure message, with no resend action or actual navigation. Set the countdown to zero for this failure state or start a real redirect.
Useful? React with 👍 / 👎.
Summary
ALLOW_EMAIL_CHANGEwith permanent enabled/disabled E2E coverage; closes Enhancement: Change Registered Email #5109.Change Type
Testing
npx jest src/auth/email.spec.tsinpackages/apinpx jest server/services/AuthService.spec.jsinapipackages/data-schemasnpm run e2e:email-changenpm run e2e:email-change:disabledTest Configuration:
ALLOW_EMAIL_CHANGE=trueandALLOW_EMAIL_CHANGE=falseChecklist