fix(email): preserve dots + subaddresses across all normalization sites (#574)

Closes #574.

Reporter (@blazmaric) identified the root cause cleanly:
express-validator's `.normalizeEmail()` applies provider-specific
canonicalization by default — Gmail dot-stripping, +tag stripping,
googlemail → gmail folding, etc. That's wrong for identity: PicPeak
uses email as a login identifier, so `john.doe@gmail.com` getting
silently stored as `johndoe@gmail.com` means the user can't log in
with the address they were invited with.

The bug existed at 17 call sites across the codebase (auth, admin user
create/update, customer create/update, event create/update on three
different routes, customer login, feedback submission). All of them
are identity-bearing — none had a legitimate reason to strip dots
for deduplication.

Fix: introduce one shared options object in `utils/emailNormalization`
disabling every provider-specific normalization
(gmail_remove_dots, gmail_remove_subaddress,
gmail_convert_googlemaildotcom, outlookdotcom_remove_subaddress,
yahoo_remove_subaddress, icloud_remove_subaddress). The only default
left enabled is `all_lowercase`, which is safe — local-parts are
case-insensitive in practice on every major provider, and lowercasing
keeps login lookup consistent.

Every call site updated to pass the shared options. 7 unit tests pin
the preserved-dots, preserved-subaddress, preserved-googlemail-domain,
and still-lowercase behaviours so a future refactor can't silently
regress.

## Migration note

Existing accounts whose emails were already stripped before this fix
remain with the stripped form in the DB. The fix takes effect for new
invitations going forward. If an admin re-invites an existing user
with the un-stripped address, that would create a duplicate account —
out of scope here; if it becomes a real problem we can add a
backward-compat login fallback (try lookup with dot-stripped form too)
as a separate change.
This commit is contained in:
Paul Nothaft
2026-05-29 11:42:25 +02:00
parent 3ceeccd85a
commit 075b45f020
10 changed files with 118 additions and 17 deletions
@@ -0,0 +1,57 @@
/**
* Regression coverage for the identity-preserving email normalization
* options (#574).
*
* express-validator's `.normalizeEmail()` applies provider-specific
* canonicalization by default — Gmail dot-stripping, +tag stripping,
* googlemail → gmail folding, etc. That breaks identity because login
* lookups expect the address as the user was invited with, not the
* canonicalized form.
*
* The tests below run validator.js's `normalizeEmail` (the same
* implementation express-validator delegates to) through the
* `IDENTITY_PRESERVING_NORMALIZE_EMAIL` options object and pin the
* behaviour we depend on:
* - dots preserved on Gmail
* - +tags preserved on Gmail / Outlook / Yahoo / iCloud
* - googlemail.com domain preserved (not folded to gmail.com)
* - local-part lowercased (still the default — safe and consistent)
*/
const validator = require('validator');
const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('../../src/utils/emailNormalization');
const norm = (email) => validator.normalizeEmail(email, IDENTITY_PRESERVING_NORMALIZE_EMAIL);
describe('IDENTITY_PRESERVING_NORMALIZE_EMAIL', () => {
it('preserves dots in the Gmail local-part (the #574 root cause)', () => {
expect(norm('john.doe@gmail.com')).toBe('john.doe@gmail.com');
expect(norm('j.o.h.n@gmail.com')).toBe('j.o.h.n@gmail.com');
});
it('preserves Gmail +tags (subaddresses)', () => {
expect(norm('john.doe+invoices@gmail.com')).toBe('john.doe+invoices@gmail.com');
});
it('does not fold googlemail.com to gmail.com', () => {
expect(norm('john.doe@googlemail.com')).toBe('john.doe@googlemail.com');
});
it('preserves Outlook +tags', () => {
expect(norm('jane+work@outlook.com')).toBe('jane+work@outlook.com');
});
it('preserves Yahoo -tags', () => {
expect(norm('jane-work@yahoo.com')).toBe('jane-work@yahoo.com');
});
it('preserves iCloud +tags', () => {
expect(norm('jane+receipts@icloud.com')).toBe('jane+receipts@icloud.com');
});
it('lowercases the local-part (default behaviour we keep)', () => {
// all_lowercase defaults true in validator.js. Local-parts are
// case-insensitive in practice on every major provider, and
// lowercasing keeps login lookup consistent.
expect(norm('John.Doe@Gmail.com')).toBe('john.doe@gmail.com');
});
});
+2 -1
View File
@@ -9,6 +9,7 @@ const { validatePasswordStrength } = require('../utils/passwordGenerator');
const { handleAsync, validateRequest, successResponse } = require('../utils/routeHelpers');
const { NotFoundError, ConflictError, ValidationError } = require('../utils/errors');
const { setAdminAuthCookie } = require('../utils/tokenUtils');
const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('../utils/emailNormalization');
const router = express.Router();
// Get admin profile
@@ -36,7 +37,7 @@ router.put('/profile', [
.trim()
.isEmail()
.withMessage('A valid email address is required')
.normalizeEmail()
.normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL)
], handleAsync(async (req, res) => {
validateRequest(req);
+3 -2
View File
@@ -12,6 +12,7 @@ const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
const { handleAsync, validateRequest, successResponse } = require('../utils/routeHelpers');
const customerAccountsService = require('../services/customerAccountsService');
const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('../utils/emailNormalization');
const router = express.Router();
@@ -121,7 +122,7 @@ router.get('/invitations', [
router.post('/invite', [
adminAuth,
requirePermission('customers.create'),
body('email').isEmail().normalizeEmail().withMessage('Valid email is required'),
body('email').isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL).withMessage('Valid email is required'),
// Optional prefill — admin can stash any subset of customer profile fields
// on the invitation. The customer sees them pre-populated on the accept
// form and can edit before submitting. Validators are deliberately lax:
@@ -199,7 +200,7 @@ router.put('/:id', [
adminAuth,
requirePermission('customers.create'),
param('id').isInt({ min: 1 }),
body('email').optional().isEmail().normalizeEmail(),
body('email').optional().isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL),
body('salutation').optional().isString().isLength({ max: 32 }),
body('first_name').optional().isString().isLength({ max: 80 }),
body('last_name').optional().isString().isLength({ max: 80 }),
+3 -2
View File
@@ -4,6 +4,7 @@
const { validatePasswordInContext, getBcryptRounds } = require('../utils/passwordValidation');
const { buildShareLinkVariants } = require('../services/shareLinkService');
const { requirePermission } = require('../middleware/permissions');
const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('../utils/emailNormalization');
// Enhanced event creation with password validation
// Note: This is a partial/reference file - dynamic event type validation should be implemented
@@ -12,8 +13,8 @@ router.post('/', adminAuth, requirePermission('events.create'), [
body('event_type').notEmpty().trim(), // Dynamic validation via eventTypeService
body('event_name').notEmpty().trim(),
body('event_date').isDate(),
body('customer_email').isEmail().normalizeEmail(),
body('admin_email').isEmail().normalizeEmail(),
body('customer_email').isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL),
body('admin_email').isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL),
body('password').notEmpty(), // Remove the weak isLength validation
body('expiration_days').isInt({ min: 1, max: 365 }).optional(),
body('welcome_message').optional().trim(),
+4 -3
View File
@@ -5,6 +5,7 @@ const { formatBoolean } = require('../utils/dbCompat');
const { slugify } = require('../utils/slug');
const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('../utils/emailNormalization');
const router = express.Router();
const bcrypt = require('bcrypt');
const crypto = require('crypto');
@@ -335,11 +336,11 @@ router.post('/', adminAuth, requirePermission('events.create'), [
body('event_name').notEmpty().trim(),
body('event_date').optional({ values: 'falsy' }).isDate(),
body('customer_name').optional().trim(),
body('customer_email').optional({ values: 'falsy' }).isEmail().normalizeEmail(),
body('customer_email').optional({ values: 'falsy' }).isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL),
body('customer_phone').optional({ nullable: true, checkFalsy: true })
.isString().trim()
.isLength({ max: 32 }).withMessage('Phone number must be at most 32 characters'),
body('admin_email').optional({ values: 'falsy' }).isEmail().normalizeEmail(),
body('admin_email').optional({ values: 'falsy' }).isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL),
body('require_password').optional().isBoolean(),
body('password').optional().isString().custom((value, { req }) => {
const input = req.body.require_password;
@@ -1109,7 +1110,7 @@ router.put('/:id', adminAuth, requirePermission('events.edit'), requireEventOwne
body('color_theme').optional({ nullable: true }),
body('allow_user_uploads').optional().isBoolean(),
body('customer_name').optional({ nullable: true, checkFalsy: true }).trim(),
body('customer_email').optional().isEmail().normalizeEmail(),
body('customer_email').optional().isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL),
body('customer_phone').optional({ nullable: true, checkFalsy: true })
.isString().trim()
.isLength({ max: 32 }).withMessage('Phone number must be at most 32 characters'),
+3 -2
View File
@@ -9,6 +9,7 @@ const { adminAuth } = require('../middleware/auth');
const { requirePermission, requireSuperAdmin, getUserPermissions } = require('../middleware/permissions');
const { handleAsync, validateRequest, successResponse } = require('../utils/routeHelpers');
const userManagementService = require('../services/userManagementService');
const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('../utils/emailNormalization');
const router = express.Router();
/**
@@ -137,7 +138,7 @@ router.get('/invitations', adminAuth, requirePermission('users.view'), handleAsy
router.post('/invite', [
adminAuth,
requirePermission('users.create'),
body('email').isEmail().normalizeEmail().withMessage('Valid email is required'),
body('email').isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL).withMessage('Valid email is required'),
body('role_id').isInt({ min: 1 }).withMessage('Role ID is required')
], handleAsync(async (req, res) => {
validateRequest(req);
@@ -197,7 +198,7 @@ router.put('/:id', [
requirePermission('users.edit'),
param('id').isInt({ min: 1 }).withMessage('Valid user ID is required'),
body('username').optional().trim().isLength({ min: 3, max: 50 }).withMessage('Username must be 3-50 characters'),
body('email').optional().isEmail().normalizeEmail().withMessage('Valid email is required'),
body('email').optional().isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL).withMessage('Valid email is required'),
body('role_id').optional().isInt({ min: 1 }).withMessage('Valid role ID is required'),
body('is_active').optional().isBoolean().withMessage('is_active must be boolean')
], handleAsync(async (req, res) => {
+2 -1
View File
@@ -41,6 +41,7 @@ const { getClientIp } = require('../utils/requestIp');
// at least one digit. No special-character or breach-list requirement.
const customerAccountsService = require('../services/customerAccountsService');
const { customerAuth } = require('../middleware/customerAuth');
const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('../utils/emailNormalization');
const router = express.Router();
@@ -63,7 +64,7 @@ const TOKEN_TTL_SECONDS = 24 * 60 * 60; // mirrors admin tokens
// rows, which verifyGalleryAccess re-checks on customer-minted
// gallery JWTs (instant per-gallery revocation).
router.post('/login', [
body('email').isEmail().normalizeEmail().withMessage('Valid email is required'),
body('email').isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL).withMessage('Valid email is required'),
body('password').isString().notEmpty(),
], async (req, res) => {
try {
+3 -2
View File
@@ -13,6 +13,7 @@ const router = express.Router();
const { buildShareLinkVariants } = require('../services/shareLinkService');
const { parseBooleanInput, parseStringInput } = require('../utils/parsers');
const eventTypeService = require('../services/eventTypeService');
const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('../utils/emailNormalization');
// Use parseStringInput from shared parsers for customer data extraction
const getCustomerNameFromPayload = (payload = {}) => parseStringInput(payload.customer_name);
@@ -67,7 +68,7 @@ router.post('/', adminAuth, [
body('event_name').notEmpty(),
body('event_date').isDate(),
body('customer_name').notEmpty().trim(),
body('customer_email').isEmail().normalizeEmail(),
body('customer_email').isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL),
body('admin_email').isEmail(),
body('require_password').optional().isBoolean(),
body('password').optional().isString().custom((value, { req }) => {
@@ -257,7 +258,7 @@ router.get('/', adminAuth, async (req, res) => {
// Update event
router.put('/:id', adminAuth, [
body('customer_name').optional().trim().notEmpty(),
body('customer_email').optional().isEmail().normalizeEmail(),
body('customer_email').optional().isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL),
body('require_password').optional().isBoolean()
], async (req, res) => {
try {
+36
View File
@@ -0,0 +1,36 @@
/**
* Identity-preserving email normalization options for express-validator.
*
* express-validator's `.normalizeEmail()` defaults to provider-specific
* "canonicalization" — Gmail dot-stripping, +tag stripping, googlemail
* → gmail domain folding, etc. That's appropriate for *deduplication*
* (treating the same mailbox as the same identity for, say, a free-tier
* abuse check) but it's wrong for *identity* (the user expects to log
* in with the exact address they were invited with).
*
* PicPeak uses email as a login identifier across admin users, customer
* accounts, and customer-portal invitations. Stripping dots/+tags
* silently means an invitation sent to `john.doe@gmail.com` is stored
* as `johndoe@gmail.com`, and the user then can't log in with the
* address they were given — see #574.
*
* Use this options object on every `.normalizeEmail()` call. The only
* default left enabled is `all_lowercase` (true by default), which is
* safe — local-parts are case-insensitive in practice on every major
* provider, and lowercasing keeps login lookup consistent.
*/
// NOT Object.frozen — validator.js's `merge()` mutates the options
// object to add its own defaults (notably `all_lowercase: true`).
// Freezing would crash at the first call site. The mutation is
// idempotent (validator only adds keys it has defaults for, not ones
// we already set), so subsequent calls reuse the same enriched object.
const IDENTITY_PRESERVING_NORMALIZE_EMAIL = {
gmail_remove_dots: false,
gmail_remove_subaddress: false,
gmail_convert_googlemaildotcom: false,
outlookdotcom_remove_subaddress: false,
yahoo_remove_subaddress: false,
icloud_remove_subaddress: false,
};
module.exports = { IDENTITY_PRESERVING_NORMALIZE_EMAIL };
+5 -4
View File
@@ -1,5 +1,6 @@
const { body, param, validationResult } = require('express-validator');
const validator = require('validator');
const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('./emailNormalization');
/**
* Validation rules for feedback submission
@@ -19,7 +20,7 @@ const feedbackValidationRules = {
.optional()
.trim()
.isEmail()
.normalizeEmail()
.normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL)
.withMessage('Invalid email address')
],
@@ -33,7 +34,7 @@ const feedbackValidationRules = {
.optional()
.trim()
.isEmail()
.normalizeEmail()
.normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL)
],
favorite: [
@@ -46,7 +47,7 @@ const feedbackValidationRules = {
.optional()
.trim()
.isEmail()
.normalizeEmail()
.normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL)
],
comment: [
@@ -67,7 +68,7 @@ const feedbackValidationRules = {
.optional()
.trim()
.isEmail()
.normalizeEmail()
.normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL)
.withMessage('Invalid email address')
]
};