Merge pull request #579 from the-luap/fix/email-normalization-574

fix(email): preserve dots + subaddresses across all normalization sites
This commit is contained in:
Paul Nothaft
2026-05-29 22:02:40 +02:00
committed by GitHub
10 changed files with 119 additions and 18 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);
+4 -3
View File
@@ -14,6 +14,7 @@ const { handleAsync, validateRequest, successResponse } = require('../utils/rout
const customerAccountsService = require('../services/customerAccountsService');
const customerHoursService = require('../services/customerHoursService');
const invoiceService = require('../services/invoiceService');
const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('../utils/emailNormalization');
const router = express.Router();
@@ -141,7 +142,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:
@@ -227,7 +228,7 @@ router.delete('/invitations/:id', [
router.post('/', [
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'),
body('prefill').optional().isObject(),
body('prefill.salutation').optional({ nullable: true }).isString().isLength({ max: 32 }),
body('prefill.first_name').optional({ nullable: true }).isString().isLength({ max: 80 }),
@@ -343,7 +344,7 @@ router.put('/:id', [
// customers.edit on upgrade so behavior is preserved.
requirePermission('customers.edit'),
param('id').isInt({ min: 1 }),
body('email').optional().isEmail().normalizeEmail(),
body('email').optional().isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL),
// `{ nullable: true }` so a passive customer who has no salutation /
// phone / company in their record can still save the page — the
// form sends `null` for those empty fields, and plain `.optional()`
+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');
@@ -343,11 +344,11 @@ router.post('/', adminAuth, requirePermission('events.create'), [
.withMessage('event_time_end must be HH:MM 24h'),
body('is_full_day').optional().isBoolean().toBoolean(),
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;
@@ -1152,7 +1153,7 @@ router.put('/:id', adminAuth, requirePermission('events.edit'), requireEventOwne
body('event_reminder_body_override').optional({ nullable: true, checkFalsy: true })
.isString().isLength({ max: 10_000 }),
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')
]
};