backend/src/emailTemplates.ts

@@ -16,7 +16,8 @@ export type TemplateKey =
   | 'passwordReset'
   | 'favoriteSold'
   | 'favoriteWithdrawn'
-  | 'cartReminder';
+  | 'cartReminder'
+  | 'emailChanged';
 
 export interface TemplateDefinition {
   /** Shown in the admin so a card is identifiable without reading its body. */
@@ -93,6 +94,23 @@ export const TEMPLATES: Record<TemplateKey, TemplateDefinition> = {
     footer: FAVORITE_CONSENT_FOOTER
   },
 
+  emailChanged: {
+    label: 'Email address changed',
+    // Naming the new address is the point: a notice that does not say what
+    // the address was changed *to* is nearly useless to someone checking
+    // whether it was them. This is the mail that catches an account
+    // takeover, so it goes to the address being replaced.
+    required: ['newEmail'],
+    available: ['greeting', 'newEmail'],
+    defaultSubject: 'Your Redefined Designs email address was changed',
+    defaultBody:
+      '{{greeting}}\n\n' +
+      'The email address on your account was changed to **{{newEmail}}**.\n\n' +
+      'If you made this change, nothing more is needed. This message is only a\n' +
+      'record of it.\n\n' +
+      'If you did not, contact us straight away: whoever made the change can now\n' +
+      'receive password reset links for your account.'
+  },
   cartReminder: {
     label: 'Cart reminder',
     required: ['itemList', 'cartUrl'],

backend/src/routes/customers.ts

@@ -307,9 +307,20 @@ router.get('/me', requireCustomer, asyncRoute(async (req: Request, res: Response
 // surface without a caller rather than a path in use.
 router.put('/me', requireCustomer, asyncRoute(async (req: Request, res: Response) => {
   const { firstName, lastName } = req.body;
+  // Registration demands both and refuses each by name. Accepting empty values
+  // here would let a customer clear fields they could not have skipped when
+  // signing up, which is the same rule disagreeing with itself.
+  const first = String(firstName ?? '').trim();
+  const last = String(lastName ?? '').trim();
+  if (!first) {
+    return res.status(400).json({ error: 'first name is required' });
+  }
+  if (!last) {
+    return res.status(400).json({ error: 'last name is required' });
+  }
   const { rows } = await pool.query(
     `UPDATE customers SET first_name = $1, last_name = $2 WHERE id = $3 RETURNING *`,
-    [String(firstName ?? '').trim() || null, String(lastName ?? '').trim() || null, req.customerId]
+    [first, last, req.customerId]
   );
   res.json(publicCustomer(rows[0]));
 }));
@@ -326,9 +337,89 @@ router.post('/change-password', requireCustomer, asyncRoute(async (req: Request,
   }
   const newHash = await bcrypt.hash(newPassword, 12);
   await pool.query(`UPDATE customers SET password_hash = $1 WHERE id = $2`, [newHash, req.customerId]);
+
+  // Password reset already ends every session, on the reasoning that a password
+  // is changed precisely when the old one may be known to someone else. A
+  // change left the other sessions alive, which is the same reasoning reaching
+  // the opposite conclusion for no recorded reason. The current session is
+  // spared so the change does not eject the person making it.
+  await pool.query(
+    `DELETE FROM customer_sessions WHERE customer_id = $1 AND token <> $2`,
+    [req.customerId, req.cookies?.rd_session ?? '']
+  );
+
   res.status(204).end();
 }));
 
+// Changing the address a password reset goes to is how an account is taken
+// over, so this asks for the current password exactly as change-password does.
+// A live session alone is not enough.
+router.put('/me/email', requireCustomer, asyncRoute(async (req: Request, res: Response) => {
+  const { currentPassword, email } = req.body;
+
+  const normalized = String(email ?? '').toLowerCase().trim();
+  if (!normalized || !isValidEmail(normalized)) {
+    return res.status(400).json({ error: 'a valid email is required' });
+  }
+
+  const { rows } = await pool.query(`SELECT * FROM customers WHERE id = $1`, [req.customerId]);
+  const customer = rows[0];
+
+  if (!(await bcrypt.compare(String(currentPassword ?? ''), customer.password_hash))) {
+    return res.status(401).json({ error: 'current password is incorrect' });
+  }
+
+  if (normalized === customer.email) {
+    return res.status(400).json({ error: 'that is already your email address' });
+  }
+
+  const { rows: taken } = await pool.query(`SELECT id FROM customers WHERE email = $1`, [normalized]);
+  if (taken.length) {
+    return res.status(409).json({ error: 'an account with this email already exists' });
+  }
+
+  // Captured before the update, because it is where the notice has to go.
+  const previousEmail = customer.email;
+
+  await pool.query(
+    `UPDATE customers SET email = $1, email_verified = false WHERE id = $2`,
+    [normalized, req.customerId]
+  );
+
+  // Supersede any outstanding link, so one already sitting in the old inbox
+  // cannot be used to verify the new address.
+  await pool.query(
+    `DELETE FROM customer_tokens WHERE customer_id = $1 AND kind = 'verify_email'`,
+    [req.customerId]
+  );
+  const verifyToken = crypto.randomBytes(24).toString('hex');
+  await pool.query(
+    `INSERT INTO customer_tokens (token, customer_id, kind, expires_at) VALUES ($1, $2, 'verify_email', $3)`,
+    [verifyToken, req.customerId, new Date(Date.now() + 24 * 60 * 60 * 1000)]
+  );
+
+  // Both sends happen after the row is written, never before — the same rule
+  // favoriteAlerts follows, so a change that failed cannot produce mail saying
+  // it succeeded.
+  const verifyUrl = process.env.PUBLIC_URL + '/verify-email?token=' + verifyToken;
+  const verify = renderTemplate('verification', await loadStoredTemplate('verification'), {
+    greeting: greeting(customer.first_name),
+    verifyUrl
+  });
+  sendMail(normalized, verify.subject, verify.html)
+    .catch(err => console.error('verify email send failed', err));
+
+  const notice = renderTemplate('emailChanged', await loadStoredTemplate('emailChanged'), {
+    greeting: greeting(customer.first_name),
+    newEmail: normalized
+  });
+  sendMail(previousEmail, notice.subject, notice.html)
+    .catch(err => console.error('email change notice send failed', err));
+
+  const { rows: updated } = await pool.query(`SELECT * FROM customers WHERE id = $1`, [req.customerId]);
+  res.json(publicCustomer(updated[0]));
+}));
+
 router.post('/me/consent', requireCustomer, asyncRoute(async (req: Request, res: Response) => {
   const consent = !!req.body.marketingConsent;
   await pool.query(

backend/tests/integration/accountDetails.integration.test.ts

@@ -0,0 +1,199 @@
+import request from 'supertest';
+import app from '../../src/app';
+import { pool } from '../../src/db';
+import { resetDb, closeDb } from './setup/testDb';
+
+jest.mock('../../src/mailer', () => ({
+  sendMail: jest.fn().mockResolvedValue(undefined)
+}));
+import { sendMail } from '../../src/mailer';
+const sentMail = sendMail as jest.MockedFunction<typeof sendMail>;
+
+const PASSWORD = 'supersecret123';
+
+beforeEach(async () => {
+  await resetDb();
+  sentMail.mockClear();
+});
+
+afterAll(async () => {
+  await pool.end();
+  await closeDb();
+});
+
+async function register(email: string) {
+  const agent = request.agent(app);
+  const res = await agent
+    .post('/api/customers/register')
+    .send({ email, password: PASSWORD, firstName: 'Thom', lastName: 'Lamb' });
+  expect(res.status).toBe(200);
+  sentMail.mockClear();
+  return agent;
+}
+
+const recipients = () => sentMail.mock.calls.map(call => String(call[0]));
+
+describe('editing your own name', () => {
+  it('stores both parts', async () => {
+    const agent = await register('namer@example.com');
+
+    const res = await agent.put('/api/customers/me').send({ firstName: ' Ada ', lastName: ' Lovelace ' });
+
+    expect(res.status).toBe(200);
+    expect(res.body.first_name).toBe('Ada');
+    expect(res.body.last_name).toBe('Lovelace');
+  });
+
+  // Registration refuses these individually. Accepting them here would let a
+  // customer clear fields they could not have skipped when signing up.
+  it.each([
+    [{ firstName: '', lastName: 'Lamb' }, 'first name is required'],
+    [{ firstName: 'Thom', lastName: '  ' }, 'last name is required']
+  ])('refuses %p', async (body, expected) => {
+    const agent = await register(`blank${Math.random().toString(36).slice(2, 8)}@example.com`);
+
+    const res = await agent.put('/api/customers/me').send(body);
+
+    expect(res.status).toBe(400);
+    expect(res.body.error).toBe(expected);
+  });
+
+  it('refuses an unauthenticated caller', async () => {
+    const res = await request(app).put('/api/customers/me').send({ firstName: 'A', lastName: 'B' });
+    expect(res.status).toBe(401);
+  });
+});
+
+describe('changing your own password', () => {
+  it('refuses without the current password', async () => {
+    const agent = await register('pw1@example.com');
+
+    const res = await agent
+      .post('/api/customers/change-password')
+      .send({ currentPassword: 'wrong-password', newPassword: 'brandnewpass1' });
+
+    expect(res.status).toBe(401);
+  });
+
+  // The reason for ending sessions at all: a password is changed precisely when
+  // the old one may be known to someone else, and a session opened with it
+  // would otherwise outlive the change.
+  it('ends other sessions but keeps the one making the change', async () => {
+    const email = 'pw2@example.com';
+    const here = await register(email);
+
+    // A second signed-in device.
+    const elsewhere = request.agent(app);
+    expect((await elsewhere.post('/api/customers/login').send({ email, password: PASSWORD })).status).toBe(200);
+    expect((await elsewhere.get('/api/customers/me')).status).toBe(200);
+
+    const res = await here
+      .post('/api/customers/change-password')
+      .send({ currentPassword: PASSWORD, newPassword: 'brandnewpass1' });
+    expect(res.status).toBe(204);
+
+    // The other device is signed out; this one is not. Asserted on the status,
+    // because /me answers an unauthenticated caller with 401 and an error body
+    // rather than an empty one — the frontend is what turns that into null.
+    expect((await elsewhere.get('/api/customers/me')).status).toBe(401);
+    const stillHere = await here.get('/api/customers/me');
+    expect(stillHere.status).toBe(200);
+    expect(stillHere.body.email).toBe(email);
+  });
+
+  it('leaves the new password working and the old one not', async () => {
+    const email = 'pw3@example.com';
+    const agent = await register(email);
+    await agent.post('/api/customers/change-password').send({
+      currentPassword: PASSWORD,
+      newPassword: 'brandnewpass1'
+    });
+
+    const stale = request.agent(app);
+    expect((await stale.post('/api/customers/login').send({ email, password: PASSWORD })).status).toBe(401);
+    expect((await stale.post('/api/customers/login').send({ email, password: 'brandnewpass1' })).status).toBe(200);
+  });
+});
+
+describe('changing your own email address', () => {
+  it('requires the current password, because this is how an account is taken over', async () => {
+    const agent = await register('mail1@example.com');
+
+    const res = await agent
+      .put('/api/customers/me/email')
+      .send({ currentPassword: 'not-it', email: 'attacker@example.com' });
+
+    expect(res.status).toBe(401);
+    const { rows } = await pool.query(`SELECT email FROM customers WHERE email = $1`, ['mail1@example.com']);
+    expect(rows).toHaveLength(1);
+    expect(sentMail).not.toHaveBeenCalled();
+  });
+
+  it('refuses an address that is not valid', async () => {
+    const agent = await register('mail2@example.com');
+
+    const res = await agent
+      .put('/api/customers/me/email')
+      .send({ currentPassword: PASSWORD, email: 'not-an-address' });
+
+    expect(res.status).toBe(400);
+  });
+
+  it('refuses an address another account already holds', async () => {
+    await register('taken@example.com');
+    const agent = await register('mail3@example.com');
+
+    const res = await agent
+      .put('/api/customers/me/email')
+      .send({ currentPassword: PASSWORD, email: 'taken@example.com' });
+
+    expect(res.status).toBe(409);
+  });
+
+  it('changes the address, marks it unverified, and mints a fresh token', async () => {
+    const agent = await register('mail4@example.com');
+
+    const res = await agent
+      .put('/api/customers/me/email')
+      .send({ currentPassword: PASSWORD, email: '  NewAddress@Example.com ' });
+
+    expect(res.status).toBe(200);
+    expect(res.body.email).toBe('newaddress@example.com');
+    expect(res.body.email_verified).toBe(false);
+
+    const { rows } = await pool.query(
+      `SELECT COUNT(*)::int AS n FROM customer_tokens t
+       JOIN customers c ON c.id = t.customer_id
+       WHERE c.email = $1 AND t.kind = 'verify_email'`,
+      ['newaddress@example.com']
+    );
+    expect(rows[0].n).toBe(1);
+  });
+
+  // Both messages matter, and they go to different places: verification to the
+  // new address, and the warning to the one being replaced — which is the only
+  // thing that tells a real owner their account was taken.
+  it('mails the new address to verify it and the old one to warn it', async () => {
+    const agent = await register('old@example.com');
+
+    await agent.put('/api/customers/me/email').send({
+      currentPassword: PASSWORD,
+      email: 'new@example.com'
+    });
+
+    expect(recipients().sort()).toEqual(['new@example.com', 'old@example.com']);
+
+    const notice = sentMail.mock.calls.find(call => String(call[0]) === 'old@example.com');
+    expect(String(notice?.[2])).toContain('new@example.com');
+  });
+
+  it('refuses changing to the address already held', async () => {
+    const agent = await register('same@example.com');
+
+    const res = await agent
+      .put('/api/customers/me/email')
+      .send({ currentPassword: PASSWORD, email: 'same@example.com' });
+
+    expect(res.status).toBe(400);
+  });
+});

backend/tests/integration/emailTemplates.integration.test.ts

@@ -25,6 +25,7 @@ describe('GET /api/admin/email-templates', () => {
     expect(res.status).toBe(200);
     expect(res.body.map((t: { key: string }) => t.key).sort()).toEqual([
       'cartReminder',
+      'emailChanged',
       'favoriteSold',
       'favoriteWithdrawn',
       'passwordReset',

backend/tests/unit/emailTemplates.test.ts

@@ -10,7 +10,8 @@ const KEYS: TemplateKey[] = [
   'passwordReset',
   'favoriteSold',
   'favoriteWithdrawn',
-  'cartReminder'
+  'cartReminder',
+  'emailChanged'
 ];
 
 describe('the built-in templates', () => {