Indexing an array now yields `T | undefined`, which is what it always did — the compiler simply says so. Enabled in backend/tsconfig.json and frontend/tsconfig.json, and in tsconfig.sonar.json alongside it so the drift guard stays satisfied. The sequencing this issue insisted on was right, and the numbers show why. Before #159 typed the query results, `rows[0]` was `any` and `any` indexes to `any`: the flag would have found close to nothing and the count would have changed completely afterwards. With the rows typed it finds 57 — 50 in the backend, 4 in the frontend, 3 in the Playwright suite — and they fall into three kinds. Thirty are `rows[0]` after a `rows.length` guard. TypeScript cannot connect the two, and rewriting them as `const [row] = rows; if (!row) …` makes the guard and the use the same check, which is better code independently of the flag. Ten are rows a statement guarantees — `INSERT … RETURNING`, or a lookup for an id the session middleware has already matched. These get `requireRow(rows, what)`, a new helper in db.ts that throws naming the query. A thrown error rather than a non-null assertion: if the assumption is ever wrong, an assertion hands `undefined` to the next line and fails somewhere unrelated, whereas this fails at the query and says which. asyncRoute turns it into a 500, which is the right answer for "the database did not do what the statement says it does". It also states the assumption once instead of ten times. The rest is ordinary indexing the compiler cannot prove: a regex capture group that the pattern guarantees, `split('+')[0]`, a modulo kept in range, `hasOwnProperty` failing to narrow an index signature, and Express typing route params as an index signature so `req.params.itemId` is `string | undefined` on a route that cannot match without it. One correction to this issue's premise, which matters for what it was expected to find. The body says "in a handful it does not guard at all", and the unguarded-500 risk it describes was not found. Every `rows[0]` either sits behind a length check or behind a statement that guarantees a row. What the flag actually bought was the ten places where that guarantee was real but unstated, and those now say so. Two changes worth calling out because they are not mechanical. TAG_COLORS is typed `[string, ...string[]]` in both copies rather than `as const` — the first attempt used `as const`, which narrowed the elements to literals and broke adminTags, so the annotation keeps `string` while telling the compiler index 0 exists. And the filter drawer's slider falls back to the bounds it was given rather than to null, because null there reads as "no filter" and would widen the results rather than leave them unchanged. Test files needed changes too, since ts-jest compiles them against the same config: a regex destructure in the compose guard, and ten `mock.calls[0][0]` reads where the surrounding assertions already establish the call happened. Verified: tsc clean over backend, frontend src and the Playwright suite; unit 254/254; integration 238/238; frontend build clean; lint unchanged in both workspaces. Closes #101
151 lines
5.6 KiB
TypeScript
151 lines
5.6 KiB
TypeScript
import request from 'supertest';
|
|
import app from '../../src/app';
|
|
import { pool } from '../../src/db';
|
|
import { resetDb, closeDb } from './setup/testDb';
|
|
import { verificationResendStore } from '../../src/rateLimit';
|
|
|
|
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();
|
|
// resetDb truncates with RESTART IDENTITY, so every test's first customer is
|
|
// id 1 and the limiter — keyed on customer id, with a process-wide store —
|
|
// hands them all the same bucket. Without this, three tests that each send
|
|
// once leave the fourth starting at its limit.
|
|
await verificationResendStore.resetAll?.();
|
|
});
|
|
|
|
afterAll(async () => {
|
|
await pool.end();
|
|
await closeDb();
|
|
});
|
|
|
|
// Each test registers its own customer. That alone does NOT isolate the
|
|
// allowance, which is what the beforeEach above is for: RESTART IDENTITY hands
|
|
// every test the same customer id, so "a fresh customer" is a fresh row with a
|
|
// recycled identity. This is the same class of leakage #62 and #84 recorded,
|
|
// surviving a key that looked like it had solved it.
|
|
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 tokensFor = async (email: string) => {
|
|
const { rows } = await pool.query(
|
|
`SELECT t.token FROM customer_tokens t
|
|
JOIN customers c ON c.id = t.customer_id
|
|
WHERE c.email = $1 AND t.kind = 'verify_email'`,
|
|
[email]
|
|
);
|
|
return rows.map(r => r.token as string);
|
|
};
|
|
|
|
describe('resending your own verification email', () => {
|
|
it('refuses an unauthenticated caller', async () => {
|
|
const res = await request(app).post('/api/customers/resend-verification');
|
|
expect(res.status).toBe(401);
|
|
expect(sentMail).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('sends to the address on the account', async () => {
|
|
const email = 'resend1@example.com';
|
|
const agent = await register(email);
|
|
|
|
const res = await agent.post('/api/customers/resend-verification');
|
|
|
|
expect(res.status).toBe(204);
|
|
expect(sentMail).toHaveBeenCalledTimes(1);
|
|
expect(String(sentMail.mock.calls[0]?.[0])).toBe(email);
|
|
});
|
|
|
|
// The point of the whole thing. An un-superseded link means a message still
|
|
// sitting in the inbox goes on working, which is the case the supersede in
|
|
// issueVerificationEmail exists to prevent.
|
|
it('mints a new token and invalidates the previous one', async () => {
|
|
const email = 'resend2@example.com';
|
|
const agent = await register(email);
|
|
|
|
const [before] = await tokensFor(email);
|
|
expect(before).toBeDefined();
|
|
|
|
await agent.post('/api/customers/resend-verification');
|
|
|
|
const after = await tokensFor(email);
|
|
expect(after).toHaveLength(1);
|
|
expect(after[0]).not.toBe(before);
|
|
|
|
// And the old link is genuinely dead, asserted through the endpoint that
|
|
// would honour it rather than by counting rows.
|
|
const stale = await request(app).post('/api/customers/verify-email').send({ token: before });
|
|
expect(stale.status).toBe(400);
|
|
});
|
|
|
|
it('the new link verifies the address', async () => {
|
|
const email = 'resend3@example.com';
|
|
const agent = await register(email);
|
|
|
|
await agent.post('/api/customers/resend-verification');
|
|
const [token] = await tokensFor(email);
|
|
|
|
const res = await request(app).post('/api/customers/verify-email').send({ token });
|
|
|
|
expect(res.status).toBe(200);
|
|
const me = await agent.get('/api/customers/me');
|
|
expect(me.body.email_verified).toBe(true);
|
|
});
|
|
|
|
it('refuses once the address is already verified', async () => {
|
|
const email = 'resend4@example.com';
|
|
const agent = await register(email);
|
|
const [token] = await tokensFor(email);
|
|
await request(app).post('/api/customers/verify-email').send({ token });
|
|
sentMail.mockClear();
|
|
|
|
const res = await agent.post('/api/customers/resend-verification');
|
|
|
|
expect(res.status).toBe(400);
|
|
expect(res.body.error).toBe('your email address is already verified');
|
|
expect(sentMail).not.toHaveBeenCalled();
|
|
});
|
|
|
|
// Three per hour. The fourth is refused, and the message says what actually
|
|
// happened rather than only that a limit exists.
|
|
it('stops after the allowance, with a message worth reading', async () => {
|
|
const agent = await register('resend5@example.com');
|
|
|
|
for (let i = 0; i < 3; i++) {
|
|
expect((await agent.post('/api/customers/resend-verification')).status).toBe(204);
|
|
}
|
|
|
|
const fourth = await agent.post('/api/customers/resend-verification');
|
|
|
|
expect(fourth.status).toBe(429);
|
|
expect(String(fourth.body.error)).toContain('spam folder');
|
|
// Refused rather than merely reported: the fourth send must not have gone.
|
|
expect(sentMail).toHaveBeenCalledTimes(3);
|
|
});
|
|
|
|
// The allowance is per customer, not per address or per caller. Keying it any
|
|
// more coarsely would let one customer spend everybody else's.
|
|
it('one customer exhausting the allowance does not affect another', async () => {
|
|
const first = await register('resend6@example.com');
|
|
for (let i = 0; i < 3; i++) await first.post('/api/customers/resend-verification');
|
|
expect((await first.post('/api/customers/resend-verification')).status).toBe(429);
|
|
|
|
const second = await register('resend7@example.com');
|
|
expect((await second.post('/api/customers/resend-verification')).status).toBe(204);
|
|
});
|
|
});
|