diff --git a/backend/__tests__/utils/emailNormalization.test.js b/backend/__tests__/utils/emailNormalization.test.js new file mode 100644 index 00000000..2d02103d --- /dev/null +++ b/backend/__tests__/utils/emailNormalization.test.js @@ -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'); + }); +}); diff --git a/backend/src/routes/adminAuth.js b/backend/src/routes/adminAuth.js index d14860cb..b2224825 100644 --- a/backend/src/routes/adminAuth.js +++ b/backend/src/routes/adminAuth.js @@ -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); diff --git a/backend/src/routes/adminCustomers.js b/backend/src/routes/adminCustomers.js index 54d7a8ae..0039f919 100644 --- a/backend/src/routes/adminCustomers.js +++ b/backend/src/routes/adminCustomers.js @@ -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 }), diff --git a/backend/src/routes/adminEvents-enhanced.js b/backend/src/routes/adminEvents-enhanced.js index 8d97018d..b08575a2 100644 --- a/backend/src/routes/adminEvents-enhanced.js +++ b/backend/src/routes/adminEvents-enhanced.js @@ -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(), diff --git a/backend/src/routes/adminEvents.js b/backend/src/routes/adminEvents.js index 4ce28d14..bd281762 100644 --- a/backend/src/routes/adminEvents.js +++ b/backend/src/routes/adminEvents.js @@ -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'), diff --git a/backend/src/routes/adminUsers.js b/backend/src/routes/adminUsers.js index 1852f903..b54bbaeb 100644 --- a/backend/src/routes/adminUsers.js +++ b/backend/src/routes/adminUsers.js @@ -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) => { diff --git a/backend/src/routes/customerAuth.js b/backend/src/routes/customerAuth.js index 8de968d8..3abe4642 100644 --- a/backend/src/routes/customerAuth.js +++ b/backend/src/routes/customerAuth.js @@ -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 { diff --git a/backend/src/routes/events.js b/backend/src/routes/events.js index 4122bd5d..209dfb68 100644 --- a/backend/src/routes/events.js +++ b/backend/src/routes/events.js @@ -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 { diff --git a/backend/src/utils/emailNormalization.js b/backend/src/utils/emailNormalization.js new file mode 100644 index 00000000..3e7c39db --- /dev/null +++ b/backend/src/utils/emailNormalization.js @@ -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 }; diff --git a/backend/src/utils/feedbackValidation.js b/backend/src/utils/feedbackValidation.js index 84d004de..ba619fcc 100644 --- a/backend/src/utils/feedbackValidation.js +++ b/backend/src/utils/feedbackValidation.js @@ -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') ] };