SonarQube Analysis / sonarqube (pull_request) Failing after 12m11s
Two of the three already existed on the backend and had no caller. PUT /api/customers/me updated the name; POST /api/customers/change-password already demanded the current password and enforced the eight-character minimum. Neither was reachable from the frontend, which is why the gap was easy to miss — the API looked finished. The name endpoint accepted empty values and wrote nulls, letting a customer clear fields registration refuses to let them skip. That is the same rule disagreeing with itself, so it now refuses each by name exactly as registration does. Changing a password now ends other sessions and keeps the one making the change. Reset already deleted every session for the customer, on the reasoning that a password is changed precisely when the old one may be known to someone else — change reached the opposite conclusion for no recorded reason, and a session opened with a leaked password outlived the change meant to lock it out. The current session is spared so the change does not eject the person making it. Changing the email address is new. It asks for the current password, because swapping the address a password reset goes to is how an account is taken over and a live session alone is not enough; that also matches what change-password already required. The address is normalised and validated, an address another account holds is refused with the same 409 as registration, and on success the row is marked unverified and any outstanding verification token superseded — one already sitting in the old inbox must not be able to verify the new address. Two emails then go out, to different places. Verification to the new address, and a notice to the old one naming what the address was changed to. The notice is the only thing that tells a real owner their account was taken, and one that does not say where the address went is nearly useless to someone checking whether it was them. Both sends happen after the row is written, never before, so a change that failed cannot produce mail saying it succeeded. That notice is a sixth template in #92's system, which cost a definition and a default body. The unit tests iterate every template, so its defaults were checked against its own required placeholder without writing a new test. Verified: 199 unit and 208 integration passing. The session test signs in on a second agent, changes the password on the first, and asserts the second is refused while the first still works — the property being claimed rather than the code path being executed. One of my own assertions was wrong on the way: /me answers an unauthenticated caller with 401 and an error body, not an empty one, and the frontend is what turns that into null. Refs #111 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
200 lines
7.1 KiB
TypeScript
200 lines
7.1 KiB
TypeScript
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);
|
|
});
|
|
});
|